Skip to content

Approved Tickets - #57

Merged
datajohnson merged 43 commits into
ytgov:mainfrom
icefoganalytics:main
Aug 18, 2026
Merged

datajohnson merged 43 commits into
ytgov:mainfrom
icefoganalytics:main

Conversation

@datajohnson

Copy link
Copy Markdown
Collaborator

This pull request introduces several important backend improvements, including a new AttachmentsController for managing attachments, enhancements to user and notification handling, bug fixes for file downloads, and updates to documentation and development tooling. The changes improve API consistency, add new notification capabilities, and streamline the local development experience.

API and Controller Enhancements:

  • Added a new AttachmentsController to handle listing, viewing, and deleting attachments with proper authorization and error handling. This controller also scopes queries and integrates with policies and services for clean separation of concerns. [1] [2]
  • Updated UsersController to include related externalOrganization data in user queries and user loading, and improved the user creation policy check to properly branch based on internal/external user status. [1] [2] [3]
  • Enhanced InformationSharingAgreementArchiveItemsController to include access grants from both the agreement and the archive item, ensuring more accurate policy checks.

Notification and Mailer Improvements:

  • Added a new migration to allow notifications to reference their source records, with a unique index to prevent duplicate notifications from concurrent jobs.
  • Introduced a mailer (NotifyOfDestroyedAttachmentMailer) that notifies relevant users when an attachment is deleted from an agreement, and exported it for use in the application. [1] [2]

File Download and API Bug Fixes:

  • Fixed a bug in the confidentiality acknowledgement/receipt download endpoints: now, the file is fully generated before sending any bytes to avoid corrupt downloads, and file names are sanitized to prevent header issues. [1] [2] [3] [4] [5] [6]

Documentation and Development Environment Updates:

  • Updated the README.md to clarify usage of the shared gateway, service URLs, mail service access, and Auth0 configuration for local development. Also improved instructions for starting/stopping services and running tests. [1] [2] [3] [4]
  • Updated .vscode settings and extension recommendations for better editor support and consistency across the team. [1] [2]
  • Added new development dependencies to the Gemfile to support local Ruby development and language server features.

Needed so `dev up` can persist `.dev-ports.env` without showing it as noise.
Why? `dev up` needs to coexist with other local stacks without manual port picking.

What? Write `.dev-ports.env` on startup and reuse it until the file is removed.
Why? `dev up` must coexist with WRAP without changing production selection semantics.

What? Map the development Compose host ports from `.dev-ports.env`, keep the web config on localhost-only dev selection, and point archiver at its real listener on 3001 while leaving production Compose untouched.
Why? `dev up` now chooses ports automatically, so the setup instructions need to explain the generated port file and the direct Compose fallback.

What? Document the new `dev up` flow, the `.dev-ports.env` file, and the existing seed-skip reminder.
Why? Auth0 wildcard callbacks need local hostnames rather than changing random browser ports.
Why? Every generated development stack needs a loopback-only browser origin that Auth0 can authorize once.

What? Persist a generated .tk.localhost host and start the shared gateway when browser-facing services are requested.
Why? The browser must select development configuration and Vite HMR through generated .tk.localhost origins.

What? Treat generated loopback hosts as development origins and allow them through Vite.
Why? Developers need the generated loopback hostname and one-time Auth0 wildcard configuration.

What? Document the shared gateway lifecycle and direct Compose prerequisite.
Why? The local gateway must not force direct Vite clients to reconnect through port 80.

What? Let Vite derive its HMR port from the browser origin.
Why? Direct startup must target the same gateway project as dev up.

What? Document the explicit local-gateway project name.
Why? Randomized ports need stable loopback hostnames for shared local development while a missing gateway must retain the original ports.

What? Consume the external Local Development Gateway, use descriptive generated values, and fall back to the original localhost ports when it is unavailable.
Why? Shortened environment names obscure which service each generated port belongs to.

What? Use backend and mail-delivery names throughout the wrapper, Compose file, and setup guidance.
Keep persisted ports when a gateway stack is partially running, but regenerate them when a stopped stack has an occupied service port. Add a focused Minitest check for generation, reuse, collision recovery, and Compose env-file propagation.
Use names that distinguish the generated ports file from its environment variables and keep the focused lifecycle test aligned.
Keep the main development wrapper focused on command dispatch while the development ports plugin owns persistence, gateway detection, and Compose arguments. Remove the standalone dev-helper test in favor of the real wrapper smoke path.
…ult-dev-ports

Support Non-Default Dev Ports for Docker Services
datajohnson and others added 13 commits August 14, 2026 09:29
…Knowledge.

The dashboard led with Knowledge Items, which invited users to add Traditional
Knowledge before an agreement existed to govern it. It now shows Step 1 (create an
agreement) then Step 2 (add Traditional Knowledge), and the standalone create-item
shortcut in the app bar is gone.

Also renames "Yukon's Traditional Knowledge Archive" to "Traditional Knowledge Vault"
and replaces the remaining user-facing "archive" wording with "the Vault". Code
identifiers such as archive-items are deliberately left alone.

Also adds the sign-in disclaimer covering auditing and the expectations of working
with Traditional Knowledge. It sits before the sign-in button because the wording
says "by logging in, you indicate you are aware".

See https://yg-hpw.atlassian.net/browse/TK-60
See https://yg-hpw.atlassian.net/browse/TK-73
See https://yg-hpw.atlassian.net/browse/TK-83

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…lay Name field.

"Yukon First Nation" now reads "Yukon First Nation or Indigenous Government" so the
label covers everyone it applies to.

Display Name is removed from the external user form because the back end already
derives it from first and last name when it is blank, so asking for it added a field
without adding information.

See https://yg-hpw.atlassian.net/browse/TK-59
See https://yg-hpw.atlassian.net/browse/TK-57

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Reports to Matomo site 115, including page views for client-side navigation, which a
plain tracker snippet would miss in a single page app.

NOTE: tracking stays off until VITE_MATOMO_HOST is set, so development and test builds
never contact a tracker. The host still needs configuring for production.

See https://yg-hpw.atlassian.net/browse/TK-34

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
What? Downloading the agreement for a draft with an unset contact produced a 4-byte
file that Word refused to open, and the real error only appeared in the server log.

Why? The serializer guarded its associations with isUndefined, but Sequelize sets an
eagerly-included association to null when the foreign key is null, so a draft missing
any contact threw. By then the response had already been given the ZIP magic number,
so the browser saved those four bytes as a .docx and the controller swallowed the
error because the headers were sent.

How? Missing contacts now render as "Not specified" instead of throwing, since a draft
is incomplete by definition. Both download endpoints also build the document before
sending any bytes, so a failure returns a real 400 rather than a truncated file. That
left quick-start-buffer-stream unused, and its four-byte head start was not worth the
risk of shipping a corrupt document.

Also sanitizes the agreement title used in the Content-Disposition filename.

Also extends the same serializer with the optional agreement sections, which belongs
with the form that collects them a few commits later. The two changes sit in the same
file and were not worth splitting apart.

See https://yg-hpw.atlassian.net/browse/TK-39
See https://yg-hpw.atlassian.net/browse/TK-44

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
What? Server messages are written for developers, so a failed request could surface
something like a raw Sequelize error. Each status now carries its own explanation,
covering 400, 401, 403, 404, 408, 410, 422, 429 and the 5xx family, plus offline
detection and a maintenance reading for 502, 503 and 504.

How? ApiError exposes the friendly copy as its message, because the existing call
sites interpolate the error directly and would otherwise keep showing the developer
text. The raw server message is kept on serverMessage for logging. Validation statuses
are the exception: 400 and 422 explain which field is wrong, so their server message
is worth showing.

Also adds Forbidden, Server Error and Maintenance pages alongside the existing 401 and
404 pages, and gives them a shared layout whose Back button returns to the previous
page instead of hard-redirecting to the home page.

Also adds the wording for roles and the optional agreement sections, used by later
commits.

See https://yg-hpw.atlassian.net/browse/TK-35

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Agreement pages all said "Information Sharing Agreement", so the breadcrumbs could not
tell one open agreement from another. They now lead with the agreement number, in the
same #ISA-42 form already used on the agreement page and in the generated document.

Also fixes breadcrumbs that pointed at the wrong place: the main Sharing Agreements
list claimed to sit under the Administration Dashboard, the Administration Dashboard
rendered no breadcrumbs at all, and a group's user list was titled "Groups".

See https://yg-hpw.atlassian.net/browse/TK-64

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…ent.

What? Choosing the external contact for an agreement showed a name and little else, so
there was no way to confirm you had picked someone from the right First Nation or
Indigenous Government. Their organization now appears alongside their email while
selecting, on the agreement itself, and in the user popup.

Why? The affiliation was already recorded against the user but was never sent to the
browser: the serializers exposed only externalOrganizationId, with no name to show.

NOTE: users-controller.ts also gains an isExternal-aware policy record for create,
which the role work in a later commit depends on.

See https://yg-hpw.atlassian.net/browse/TK-69

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Naming someone as the Manager on an agreement silently made them an admin of its
internal group, which granted them access to the Traditional Knowledge shared under
it. That field records who is accountable on the contract, not who may read what.

Signing an agreement no longer adds the Manager to the group, and changing the Manager
now only revokes the previous holder rather than admitting the new one. The revoke
skips anyone who is also the primary contact, since they are a group admin in their
own right.

NOTE: sourcing the Manager from the Active Directory, the other half of this ticket,
is deferred because the directory's manager field is not reliable enough to depend on.

See https://yg-hpw.atlassian.net/browse/TK-66

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…dmin roles.

What? Managing users was all-or-nothing: only a System Admin could do it, for everyone.
Admin now covers internal Yukon Government users and External Admin covers external
users, leaving System Admin as the only role that spans both.

How? The matrix lives in one place, User#canManageUser, so the policy, the services and
the interface cannot drift apart. Only a System Admin may grant System Admin, enforced
in the update service rather than by hiding the option, since the interface is not a
security boundary.

NOTE: the roles column is a comma-joined string, so the role scope matches on the
delimiters. A plain LIKE '%admin%' would also match system_admin and external_admin.

Also fixes two problems found here. The roles validator compared against the joined
string, so saving any user with more than one role failed even though the role picker
is a multi-select. And both create services discarded whatever roles were passed
because the default was applied after the spread.

Also renames the buttons to "Add YG User" and "Add External User", and removes the
Knowledge Items entry from the sidebar so items are reached through their agreement.

See https://yg-hpw.atlassian.net/browse/TK-36
See https://yg-hpw.atlassian.net/browse/TK-57
See https://yg-hpw.atlassian.net/browse/TK-73

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…the knowledge.

What? An internal employee could only see agreements whose group they belonged to, so
there was no way to find out an agreement already existed. Every non-draft agreement is
now readable by any internal Yukon Government employee, alongside their own drafts.

NOTE: this widens the agreement record only. The Traditional Knowledge shared under it
stays restricted to those holding an access grant.

How? Keeping that boundary took an accompanying fix. The link between an agreement and
its Knowledge Items was scoped by the agreement alone, so widening agreements would
have revealed which items hang off each one. The scope now intersects both, and show
requires the caller to be able to read both halves. A test asserts an internal employee
who can see an agreement but holds no grant sees none of its items.

The archive item scope also stopped passing its user id through replacements, which are
not honoured once a scope is spread into an include.

Also narrows the user directory: an external user could list every user in the system,
including people from other First Nations. Non-admins now see only active users, and an
external user sees internal staff plus their own organization.

See https://yg-hpw.atlassian.net/browse/TK-24

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…omplete document.

What? Sections 2, 3, 7, 8, 9 and 10 of the agreement template had nowhere to be filled
in, so a downloaded draft always showed them blank. An "Additional Details (optional)"
step now collects level of detail, formats, credit, expiration notifications, breach
actions and compelled disclosure.

Why? The columns and the permitted attributes already existed; only the form and the
document were missing. The selections are stored comma-joined, matching the columns, so
a small checkbox group maps between the two.

TODO: the acknowledgement template does not reference the new placeholders yet, so
those sections still print empty checkboxes. Adding them means editing the .docx in
Word: its placeholders are split across several runs, and a text-level edit produces a
file Word refuses to open. See the new README beside the templates for the full
placeholder list and the pattern to follow.

See https://yg-hpw.atlassian.net/browse/TK-44

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…cted.

Lets a notification point back at the record that caused it, which the agreement expiry
scan needs to know whether it has already told someone.

NOTE: the filtered unique index is the backstop. Without it two schedulers running at
once would each pass the "already sent?" check and both send.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…ents near expiry.

What? Notifications and email existed but only covered group membership. They now also
cover the three cases the ticket asks for: a new user is created, an attachment is
removed, and an agreement is approaching expiry at 30, 7 and 0 days.

Why the expiry reminder matters: expiry permanently removes the Traditional Knowledge
from the Vault, and that cannot be undone, so the designated contacts need warning
while there is still time to amend or extend.

NOTE: two things had to be repaired before any of this could run. The scheduler called
process.exit(0) immediately after registering its jobs, which killed them, and it was
wired into no compose file, so no scheduled job could ever have fired. It now stays
alive and runs as its own service.

Also adds the attachment delete endpoint the notification hangs off, which did not
exist: there was no policy, service, controller or route, only a bulk destroy buried in
reverting an agreement to draft. That path now deletes through the service so those
removals notify too.

Notification failures are caught and logged outside the surrounding transaction, so a
mail problem cannot roll back a new user or resurrect a deleted attachment.

See https://yg-hpw.atlassian.net/browse/TK-6

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@datajohnson
datajohnson merged commit 1bd7f9c into ytgov:main Aug 18, 2026
1 check passed
datajohnson pushed a commit that referenced this pull request Aug 28, 2026
…owledge-items-from-isa

TK-82: Unable to create Knowledge items from ISA
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants