Skip to content

Add patient resources plugin - #447

Open
admir-vicert wants to merge 22 commits into
Medical-Software-Foundation:mainfrom
vicert-healthcare:vicert/patient-resources
Open

Add patient resources plugin#447
admir-vicert wants to merge 22 commits into
Medical-Software-Foundation:mainfrom
vicert-healthcare:vicert/patient-resources

Conversation

@admir-vicert

@admir-vicert admir-vicert commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

PLUGIN-57 Patient resources

Patient Resources: an admin-curated library staff can share to a patient's portal

Adds a new plugin, extensions/patient-resources. Administrators curate a list of resource links; any staff member can open a patient's chart, search that list, pick one or several, adjust the note for the person in
front of them, and send. The patient finds them in the portal under My Resources, with the note and the date they were shared.

Ships with an empty library — nothing here is specific to any clinic.

Why

Most practices hand out the same handful of educational links repeatedly. Sent ad hoc, they get pasted into messages with typos and stale URLs, and there's no record of who was given what. Curating once means the
link a patient receives is the one the practice reviewed.

What ships

Three surfaces appear after install:

  • Patient Resources — the library as a full page, in the provider menu (top group). Every staff member can open it; only administrators can change it.
  • Resources — a button in the patient chart header that opens the picker for the open chart. All staff.
  • My Resources — the patient's own list, in the portal menu, carrying a badge that counts resources they haven't opened yet.

Eight components: two Applications, one ActionButton, and five SimpleAPI route classes split so that curation needs no patient access (LibraryAPI reads Staff/StaffRole; ShareAPI reads Staff/Patient). No event
handlers and no scheduled work — everything happens in response to a click.

Two tables in the custom_data__patient_resources namespace: PatientResource (the catalog) and PatientResourceShare (one row per resource given to one patient).

All configuration is optional; the plugin works unconfigured.

Design decisions worth a reviewer's attention

Title and note behave in deliberately opposite ways. The portal reads the resource's current title, so fixing a typo fixes it for everyone who already received it. The note is the opposite — it's written for one
patient and stored on their share, so editing the library default changes only what the next send starts from. A title describes the resource; a note describes what this patient should do with it.

That asymmetry is only safe because a shared resource's link is frozen. Once anyone has received it, an edit is refused with a prompt to add a replacement and archive the original. The link is the identity of what a
patient was given, so with it immutable a title edit can only redescribe the same resource — never quietly swap it for a different one.

Delete vs. Withdraw is one slot, and which control appears is information. A resource nobody ever received offers Delete (archiving is the wrong record to leave for a typo or test data). Anything with share history
offers Withdraw — disabled with the reason on hover when every share is already withdrawn. The server refuses that case with a 409 too, so a direct request can't succeed at doing nothing. Hard delete is refused for
anything a patient received, including withdrawn shares: the foreign keys carry no cascade, so removing the catalog row would orphan the share records.

Withdrawing is louder than archiving. It marks every patient's copy withdrawn and archives the resource, and the portal says the item was withdrawn by their care team, with the date, rather than the row silently
vanishing from a list they'd already read. Requires typed confirmation. Restoring makes a resource offerable again but does not un-withdraw — hence the library marks such rows Withdrawn rather than Archived.

Blank config means "unset", not "off". A manifest variable with no value reaches the plugin as an empty string, indistinguishable from a missing key. An earlier version read blank as "switched off", which left every
fresh install with no administrator and no way to add a first resource. Blank now falls back to ADM; NONE switches curation off explicitly; an unrecognized value denies everyone and logs why, rather than being
quietly replaced with the default.

Both staff lists page at 25. The library adds a 25/50/100 per-page control, since a curator scanning a large library wants more rows than someone picking two in a modal. Selection in the picker survives a page
change, and anything selected but off-screen is named in the footer so a resource picked on page one can't go out unnoticed. One send carries at most 25; the API enforces it and the picker holds the same limit so
feedback arrives before the click.

The picker confirms, then closes itself. Sharing is the only thing that window does, so a clean send ends the task. The host protocol carries no notification of its own, so the notice lives on the page and the close
waits for it. The summary dialog stays only for a send that needs explaining — something already in the patient's list, or archived since the page was drawn. Sharing twice is not an error: the response distinguishes
newly sent, already shared, and since archived.

Security

  • A patient can only read their own list. The portal endpoint takes no identifier of any kind — not a patient key, not a share id — and scopes from the session header alone. There is nothing in the request to tamper
    with.
  • The admin app icon can't be hidden from non-admins — Canvas has no role scoping for application visibility. It renders read-only with an explanation, and every write endpoint re-checks permission server-side.
  • Links validated on write and again on render. Only absolute http:///https://; javascript:, data:, protocol-relative and relative links refused. Re-checked at serialization so a row stored before a validation change
    can't render as a live link.
  • rel="noopener noreferrer" plus a no-referrer policy on the portal page. This is a privacy control: without it a third-party health site receives the portal URL and learns a patient viewed a resource about a
    particular condition.
  • No outbound HTTP. The plugin never fetches a resource; the browser opens the link.

admir-vicert and others added 22 commits August 20, 2026 11:01
PLUGIN-57. A configurable library of patient-facing resource links that staff
can share to a patient's portal. Scope as locked on 4 Aug: links only, portal
only, admin-managed, searchable.

Three surfaces. A global-scope Application for administrators to curate the
library; a chart-header ActionButton opening a searchable multi-select picker;
and a portal_menu_item Application giving the patient a "My Resources" page,
badged with the count they have not looked at yet. The badge is how a patient
notices new resources -- no message is sent, because the locked scope asks for
neither a channel nor wording this plugin would be inventing on the practice's
behalf.

Two plugin-owned tables in custom_data__patient_resources. A share copies the
resource's title, link and label at the moment it is sent, so a later edit to
the catalog cannot rewrite what a patient was told they received; the portal
query still joins the catalog's status, so archiving a wrong link pulls it from
every patient at once. Editing a shared resource's link is refused outright --
the link is the identity of the thing the patient was given -- and withdrawal
shows in the portal as a dated notice rather than a row quietly disappearing.

Curation is gated on StaffRole.domain, defaulting to ADM, with an optional
staff-id allowlist that replaces rather than extends it. Closed in every branch
while still working on a fresh install, which avoids the fail-open default two
other plugins in this repo settled for. The admin app icon cannot be hidden from
non-administrators -- Canvas has no role scoping for application visibility -- so
the page renders read-only with an explanation and every write re-checks
server-side.

The portal endpoint accepts no identifier of any kind, so one patient cannot
name another patient's row. Links are validated on write and again at serialize
time, and are opened with rel="noopener noreferrer" under a no-referrer policy
so a third-party site cannot learn which condition a patient read about.

299 tests, 100% branch coverage, mypy clean, and all 8 handlers load under
`canvas validate`. That last gate caught a urlparse import the whole suite was
happy with and which would have 404'd three of the eight routes on the instance.
tests/test_plugin_contract.py pins the sandbox and DDL traps that a green
CPython suite cannot see.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Two bugs found while testing 0.1.0 on vicert-testing.

The archived-resources toggle showed to a user who had not been granted
curation rights. The templates hide the Add button, the read-only notice and
that toggle with the `hidden` attribute and reveal them from JavaScript, but
`[hidden] { display: none }` lives in the user-agent stylesheet while
`.pr-toggle { display: flex }` is an author rule -- so the author rule won and
the attribute did nothing. Both stylesheets now restate the rule at author
level, marked important because the two are then at the same level.

The staff pages reported every failure as "That did not work." That turned out
to be actively misleading: a plugin-runner restart, a 404 mid-reload and a
permissions problem all rendered as the same red line, so the page looked
broken with no way to tell which. Failures now surface the HTTP status and say
whether retrying is the right move; a rejected fetch, which never reached
Canvas and so has no status, is reported the same way as a 5xx. The patient
page keeps its plain sentence -- a status code is noise to a patient.

Tests for both, including the CSS guard and the template-to-stylesheet link, so
neither can regress silently. Also strips comments before the static front-end
checks: the files document the rules they follow, and the first version of these
assertions matched the prose instead of the code.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Every LibraryAPI and ShareAPI route returned an empty-bodied HTTP 500 on
vicert-testing while every StaffPagesAPI route returned 200. The runner was
healthy throughout -- CRON responded every minute, all 8 handlers were loaded --
and the failing requests produced no plugin-runner activity at all, so they were
failing in front of the plugin rather than inside it.

The difference between the working and failing requests is who issued them. The
page, stylesheet and script are fetched by the browser and carry no
Content-Type. The data calls come from this plugin's own fetch helper, which set
`Content-Type: application/json` on every request including bodyless GETs. That
header describes a payload, and advertising a JSON body while sending nothing
invites whatever parses request bodies ahead of the plugin to read an empty
string as JSON.

Now the header is sent only when there is a body. portal.js never set it.

Provisional: the diagnosis fits every observation but is not yet confirmed
end-to-end, because the layer returning the 500 is not visible in `canvas logs`.
The discriminating test is requesting the same route from the address bar, where
the browser sends no Content-Type. Correct either way -- the old behaviour was
wrong on its own terms.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Two bugs, both found by instrumenting the running instance rather than reading
the code, and both invisible to the test suite.

The 500 on every LibraryAPI and ShareAPI route was a sandbox restriction:

    AttributeError: "patient_resources.services.catalog.list_resources"
    is an invalid attribute name (not in ALLOWED_MODULES)

`from patient_resources.services import catalog` followed by
`catalog.list_resources()` imports cleanly, passes `canvas validate`, and then
raises when the line executes. Reaching a function as an attribute of a plugin
submodule is not allowed; importing the symbol is. The three route classes that
imported symbols directly worked throughout, and the two that went through a
module object failed on every call -- which is what finally isolated it after
five other hypotheses had been falsified. Same mechanism that breaks
`arrow.Arrow` in another plugin on this instance.

The second bug was hiding behind the first. A manifest variable that has never
been given a value arrives as an empty string, not as a missing key -- the
opposite of what installation.py's shape suggested. So the "blank means an
operator deliberately switched curation off" branch fired on every fresh
install, leaving the library with no administrator and no way to add a first
resource. That would have surfaced as the missing Add button the moment the 500
was fixed. Blank now means unconfigured and defaults to ADM-domain roles;
switching curation off is an explicit NONE, because blank can no longer carry
that meaning.

Tests: patches now target the importing module's namespace, which is what the
direct imports require and what this repo's convention said all along. A new
contract test walks the AST of every module and fails on any
`from patient_resources.<pkg> import <submodule>`, so this cannot come back.
The config tests now pin blank-means-default and NONE-means-nobody.

Diagnostics under routes/diagnostics.py are temporary and still deployed; they
are what produced the evidence above and come out once the UI is confirmed.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The library and picker opened with no way out. The host modal for this target
draws no chrome of its own, so the page has to supply the control and dismiss
itself: the platform hands the iframe a MessagePort in an INIT_CHANNEL message
and listens for CLOSE_MODAL on it, the contract documented in
provider_note_vitals_companion and used by scheduling-with-rooms, sticky_note
and health-risk-assessment. The listener checks event.origin first, following
sticky_note rather than the companion plugin, which does not.

Also fixes the title running into the URL, visible on the picker as
"Testhttps://...". The picker renders .pr-item-title on a <label> so the whole
title is a click target for its checkbox, and a label is inline by default. The
class now sets display: block explicitly.

Inlined the fifteen lines of port plumbing in each staff bundle rather than
adding a shared asset: a fourth route and cache-busting surface costs more than
the duplication, and it keeps the patient bundle free of anything that speaks to
the staff host. A test asserts the patient page has none of it.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The picker opened at the host's default and left most of a full-height window
empty below a one-row list. The platform accepts a RESIZE message on the same
port that carries CLOSE_MODAL, used by chart-collision-detector and sticky_note,
both of which post fixed dimensions.

This measures instead: the page asks for the height its rendered content needs
and re-measures whenever the list changes, so the window tracks a search that
narrows three rows to one. Clamped at both ends -- a large library scrolls inside
the modal rather than requesting a window taller than the viewport -- and a
request is skipped when it would move the height by less than eight pixels, so
typing in the search box does not post a message per keystroke.

The library gets more width than the picker because each of its rows carries
Edit, Archive and Withdraw controls.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Editing a resource meant scrolling to reach Save. The resize measured
#pr-app.scrollHeight, but a <dialog> is an overlay laid out on top of the page
rather than inside it, so the form contributed nothing to the measurement and
the window stayed sized to the list behind it.

The measurement now takes the taller of the page content and any open dialog,
and re-runs on the dialog's `close` event so the window shrinks back afterwards.
Listening for `close` rather than wiring each dismissal covers Esc, the cancel
buttons and a programmatic close with one handler.

Raised both ceilings for headroom, so a dialog whose field errors wrap onto
extra lines still fits without scrolling: library 700 to 780, picker 620 to 660.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Two mockups annotated "CONFIRMED WITH JESS" arrived after the UI was built, so
this brings the staff surfaces in line with them. Visual work, plus one field
addition the design requires.

The library becomes a table -- TITLE / TYPE / LABEL with uppercase headers and
row rules -- instead of a card list. Search moves inline with the heading, the
label filter and archived toggle drop to a quieter row beneath, "+ Add resource"
becomes the filled primary, and the accent moves from teal to the design's indigo
through the :root tokens rather than a sweep through rule bodies. The title cell
is an anchor styled as body text: the design shows no blue link, but an admin
checking where a resource points needs to be able to click it, and a stored value
that fails validation still renders as inert text.

The picker gains the patient card the design shows -- name with DOB and MRN --
which needed two fields get_patient did not return. Both exist on the SDK model.
The date is formatted server-side as MM/DD/YYYY on purpose: toLocaleDateString
follows the reader's locale, and this browser's en-GB session renders 1979-04-12
as "12/04/1979", the same digits reading as a different day on a clinical record.
A missing birth date returns empty so the card drops the separator instead of
printing "DOB None". Rows become checkbox cards with a right-aligned type badge,
and the button reads "Send to patient portal".

Kept against the drawings, both confirmed: Archive / Restore / Withdraw stay
beside Edit, because withdrawal is the only way to pull a bad link back out of
patients' portals; and the label filter and archived toggle stay, since Jess's
note says categories are in scope. The visible selection counter goes, but its
live region stays visually hidden so the count is still announced. The close
button stays too -- the host modal draws no chrome, so removing it would make the
window undismissable.

The type badge is a constant. Every resource is a link until PDF support lands,
so it is a placeholder for a field that does not exist yet; a test pins that so
the next reader replaces it deliberately rather than inventing logic for it.

Also removes the diagnostic scaffolding built during the HTTP 500 investigation,
now that the portal surface is confirmed working on the instance: the
key-authenticated diagnostics module and its manifest entry, the probe routes on
the real classes, the simpleapi-api-key variable, and the exemption that kept the
submodule-import test from checking that module. Coverage returns to 100% branch
and mypy passes again -- the untested probes had pulled both down.

README corrected on one point it had wrong since the config fix: blank means
unconfigured and falls back to ADM, and NONE is the explicit off switch.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Adding a resource put the buttons below the fold: the three fields filled the
dialog and the actions row had to be scrolled to.

Two causes. The dialog laid out as one block, so a tall form pushed the actions
past the bottom edge. And the resize measured `dialog.offsetHeight` -- but a
<dialog> is capped by the user agent's own max-height, so once clamped that
property reports the clamped value. Asking the host for exactly the height it
already has can never grow the window out of the clamp, which is why the form
stayed scrolled no matter how much room was available.

The dialog is now a column: only the body scrolls, and the actions row is pinned
with a divider above it, so Save and Cancel are reachable at any height. The
measurement sums the body's scrollHeight with the pinned actions, which is the
height the content actually wants rather than the height it was allowed.

The `display: flex` is scoped to `dialog[open]`. Unscoped it would outrank the
user agent's `dialog:not([open]) { display: none }` and leave every closed dialog
rendered on the page -- the same trap as styling `display` over the `[hidden]`
attribute, which already shipped once in this plugin.

Also reclaimed the space that caused it: each field reserved a blank line for an
error it did not have, which added about 48px of nothing to a three-field form.
The error row now appears only when it has something to say.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The search box in the library header rendered noticeably thinner than the
controls around it. The input styling was scoped to `.pr-controls input`, but
that search lives in the header rather than in the filter row below it, so the
rule never matched and the box fell through to user-agent styling.

Text inputs and selects are now styled by element, which covers any control
wherever a template puts it. Slightly taller than before at 10px vertical
padding and 15px text, with buttons grown to match so the header line up. The
dialog's own field rule shrinks to just the width stretch, since the shared rule
carries everything else.

A test pins the selector shape: the rule has to name the elements rather than a
container, and cover every control type the templates actually use.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Dropped the "All labels" filter from the picker, leaving a single full-width
search. A provider with one patient's chart open is looking for a resource by
name rather than browsing categories, and the approved design shows only a
search box on that surface.

The library keeps its filter. That is where a growing list actually needs
narrowing, and Jess's note on the library design says categories are in scope.

Also removed what the control left behind: the picker no longer fetches
/library/labels for a vocabulary it cannot display, so opening it is one request
lighter. The search input already carried `flex: 1 1 240px`, so it fills the row
with no styling change.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Adding a resource by mistake had no clean remedy. Archive hid the row but left it
in the library forever, and "we used to offer this" is the wrong thing to record
about something that should never have existed. On a trial instance, where the
whole point is that people experiment, there was no way to clear up afterwards.

Delete is offered only for a resource that has never been shared with anyone.
That case is provably safe: no share rows reference it, so nothing is orphaned
and no patient's list changes. Anything a patient ever received is refused with a
409 naming Withdraw and Archive instead -- including shares that were later
withdrawn, since a withdrawn share is still a record that somebody received
something, and the foreign keys carry no cascade.

The row keeps three controls rather than growing a fourth: the destructive slot
holds Withdraw when a patient ever had it and Delete when nobody did. Which
control appears is itself the signal about that resource's history, and the two
can never both render, so the UI never offers an action the server would reject.
Confirmed but not typed -- Withdraw demands a typed word because it changes what
patients hold, and this reaches nobody, so the same ceremony would be theatre.

Choosing between them needs a has-shares flag per row, computed as one set lookup
for the page rather than a check per row, and only for a curator, since nobody
else sees those controls.

One limitation, stated rather than papered over: the check and the delete are
separate statements, so a provider sharing that exact resource in between loses
the share. Closing it needs row locking the DDL pipeline does not offer, and the
window is one statement wide.

Outside the scope locked with Jess, but admin-side only with no patient-visible
effect, so it is documented in the README rather than held for a product
decision.

Revert point for this feature: 111daf3.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The Delete row's buttons sat out of line with the Withdraw rows above it. The
right edges matched, but the left ones did not: "Delete" is a shorter word than
"Withdraw", so that row's group was narrower, and because the group is
right-aligned the whole set shifted sideways.

The three controls now share a minimum width, sized for the longest label so a
longer one later widens rather than clips. Same effect on Restore against
Archive, which was off by a few pixels for the same reason.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Withdrawing a resource archives it as part of taking it back from patients, so it
turned up under "Show archived" looking identical to something that had simply
been retired. Only one of those two changed what patients already held, and the
library gave no way to tell which.

Inactive rows now read Withdrawn or Archived, with a tooltip spelling out the
difference: taken back from patients who had it, against no longer offered and
nobody affected.

Deciding that needs a second page-wide lookup alongside the existing
has-ever-been-shared one. Two queries rather than deriving both from a single
one, because one query returning every share row would drag back a row per
patient; each of these returns at most one row per resource on the page.

Which exposed a scale bug in the lookup added with Delete: it had no distinct(),
so a resource given to five hundred patients fetched five hundred rows to build a
set of one. Both lookups now ask for distinct rows, and a test pins it.

Also documents what Restore does not do. It makes a withdrawn resource offerable
again, but patients who had it taken back do not get it returned -- they can be
sent it afresh, since the unique constraint on live shares leaves a revoked row
out of the way.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
An already-withdrawn resource still showed Withdraw. Clicking it demanded a typed
confirmation and then did nothing: revoke_resource_shares filters on live shares,
so it updated zero rows, and set_status re-archived an archived row. That breaks
the rule the rest of this UI follows -- never offer an action the server would
reject.

The rule for the destructive slot was too coarse. It asked whether a patient had
ever received the resource, when what matters is whether one holds it now. Three
states, and the control follows what is possible: Withdraw when a share is live,
Delete when none ever existed, and nothing when every share was already taken
back -- the Withdrawn marker beside the title being the explanation. Note it is
not "withdrawn once, never again": a resource withdrawn, restored and re-sent has
live shares, and Withdraw returns.

So the page lookup changes question, from has-ever-been-shared to
has-live-shares, and the payload carries two facts the row can act on rather than
one it cannot. Still two queries per page, each bounded to one row per resource.

The server now refuses the same case with a 409 rather than relying on the UI to
hide the control, so a direct request cannot report success for an action that
changed nothing.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A row with two controls slid both of them rightwards under the three-control rows
above. Flex collapses the space of a button that is not there, and the group is
right-aligned, so the row hugged the edge and lost its columns.

Two controls is a legitimate state, not an edge case: an already-withdrawn
resource has nothing left to withdraw and cannot be deleted either, so its slot
is genuinely empty.

The actions cell is now a three-column inline grid with fixed widths, which keeps
the slot whether or not it is filled. Fixed is the point -- lining up across rows
needs deterministic widths, so a longer label later means raising the constant
rather than letting one row size itself and drift out of line again. This
supersedes the per-button minimum width from 225cc4f, which fixed the
short-label case but could not fix the missing-button one.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ke back

Restoring a withdrawn resource left the row with two controls and no explanation.
Omitting Withdraw was defensible while the row still carried a "Withdrawn" marker
beside the title, because the marker accounted for the gap. Restoring clears that
marker, so the row simply looked arbitrarily different from its neighbours.

Withdraw is now always present for a resource with share history, and disabled
when no patient currently holds it, with the reason on hover. A disabled control
explains itself; an absent one leaves the reader guessing. It also still cannot
be used, which was the point of the previous change -- and the server keeps its
409 for the same case, so hiding or disabling remains a convenience rather than
the enforcement.

Delete is unchanged: offered only where no patient ever received the resource.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Editing a shared resource's title left every patient who already had it looking
at the old one, with no way to fix it. The portal read the snapshot columns
captured at share time, so a typo followed those patients forever.

The snapshot was there to stop an admin repurposing a row -- renaming "Diabetes
basics" to "Post-op care" and having earlier recipients retroactively appear to
have received something else. But the link is already frozen once a resource has
been shared, and with the URL immutable a title edit can only redescribe the same
resource. The two protections overlapped, and the snapshot was the one that
stopped protecting anything and just preserved mistakes.

The live list now reads title and label through the catalog row, with the
snapshot as the fallback for a share whose resource is missing -- that foreign
key is nullable. The URL still comes from the snapshot: it cannot have changed,
so the two agree, and the snapshot survives a missing row. Withdrawn notices stay
on the snapshot entirely, since a withdrawn resource may since have been edited
and the patient cannot open it anyway.

The list gains select_related on the resource it now reads, so this costs no
extra queries.

The edit dialog already told admins that title and label changes would show in
the patient's portal. That was false when it was written and is now true, so it
stays as it is. The README said the opposite and has been corrected.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Jess's three adjustments after reviewing the plugin.

Labels are internal. They were shown to patients in the portal alongside
the title; they are staff taxonomy for filing and filtering a growing
library, not something written for a patient to read. Dropped from both
patient-facing projections. The picker's staff payload keeps them.

A note that patients see. Set as a default on the resource in the library,
pre-filled in the picker, and editable there for the patient in front of
you before sending. Stored on the share row, never on the catalog: the
note was written about one person, so editing the library's default must
not reach back and rewrite it. That is deliberately the opposite of the
title, which is read live so one correction reaches everyone -- a title
describes the resource, a note describes what this patient should do
with it.

Membership decides the fallback, not truthiness. The picker pre-fills the
default, so a box the sender cleared means "send no note"; only a resource
the caller said nothing about inherits the default.

The chart button reads "Resources". The chart header gives each plugin
button a narrow fixed slot and clipped "Share resources"; the modal it
opens still says it in full.

Two new columns, both append-only: PatientResource.default_note and
PatientResourceShare.note.

410 passed, 100% branch coverage, mypy clean, 8/8 handlers load.
…e the picker on send

Three things from review, none of which needed a schema or API change.

Paging. The listing endpoint has taken limit/offset and returned a total from
the start; neither front end sent them, so both silently showed the server's
default first 50 rows. The library at least printed "Showing 50 of 137" with no
control to go further; the picker printed nothing at all, so past fifty
resources a provider could not reach the rest and was not told. Both now send a
page and offer a range with Previous/Next -- 50 rows in the library, 25 in the
picker, where a full page is exactly one sendable batch. Narrowing a list returns
to page one, because the result set changes underneath the offset, and a page
that comes back empty while rows remain steps back rather than showing an empty
table under a total that disagrees with it.

Selection in the picker now outlives what is on screen, so anything selected on
another page is named in the footer -- the design drops the visible counter,
which was fine while everything selected was also rendered. The 25-resource send
cap is held in the front end too, on the send path rather than only in a disabled
attribute, so the feedback arrives before the click instead of as a 400 after it.

Curation moves from the app drawer to the provider menu. It is configuration, not
something a user opens against the patient in front of them, and every other
admin surface behind that menu in this repo declares the same scope. It opens as
a page, because a menu entry has no modal host; the modal plumbing stays and the
page reveals its close control only when a host actually offers a port to close
through.

The picker closes itself after a clean send. Sharing is the only thing that
window does, and the summary dialog cost an extra click on every share. The
dialog stays for a send that needs explaining -- something already in the
patient's list, or something archived since the page was drawn.
Closing on a clean send took the confirmation with it: the summary dialog was
the only thing that said the send had worked, so the window now just vanished.

The picker shows a brief notice in the top-right corner instead -- "Shared 2
resources.", the same wording the summary already used -- and defers the close
until it has been up long enough to read. Send is disabled while it shows, so a
second click cannot fire another request into a window that is going away.

Shown on the page rather than by the host because there is nothing to ask: the
modal protocol carries INIT_CHANNEL, CLOSE_MODAL and RESIZE and nothing else,
and no SDK effect raises a transient notice. Fixed rather than in flow, so it
sits in the corner of the modal viewport and changes neither the list nor the
height this page asks the host for. On the existing indigo accent, because the
palette has one accent and a success colour would be the start of a second.

A partial or failed send is unchanged: it still keeps the dialog that explains
what happened, and claims no success.
50 was this plugin taking the API's fallback limit as though it were a design
decision. The two other paginated tables in this repo -- encounter_list and
custom-observation-management -- both show 25 and let the reader raise it, and
they are right: a library row carries three action buttons, so fifty of them is
a scroll long enough that the pager underneath the table leaves the screen.

DEFAULT_PAGE_SIZE is now 25, which the picker already used, so both staff lists
step by the same amount. The library also gets the per-page control those two
tables have, offering 25/50/100 -- a curator scanning a large library wants more
rows than somebody picking two resources in a modal, and no single number is
right for both. Every option stays at or under MAX_PAGE_SIZE, pinned by a test,
because a value above the cap would be clamped and the control would lie.

Changing the size restarts from the first page: a given offset names different
rows once the size changes. The pager row is shown for a library larger than the
*smallest* size on offer rather than the current one -- at 100 per page with 30
resources there is nothing to page through, and the naive rule would hide the
only control that could put it back to 25.
@admir-vicert
admir-vicert marked this pull request as ready for review September 4, 2026 08:51
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.

1 participant