Add scheduling waitlist plugin - #446
Open
admir-vicert wants to merge 30 commits into
Open
Conversation
Shared, priority-ordered waitlist of patients waiting to be scheduled, with a task raised to the scheduling team whenever a booked slot frees up. Components: - WaitlistApp — global Application serving the roster page - WaitlistAppAPI, WaitlistAPI — SimpleAPI routes for the page and entry CRUD - AddToWaitlistChartButton, AddToWaitlistAppointmentButton — ActionButtons - SlotFreedHandler — APPOINTMENT_CANCELED, raises one task naming matches - AppointmentBookedHandler — APPOINTMENT_CREATED, marks entries scheduled - WaitlistMaintenanceCron — nightly aging and metrics Entries persist in the plugin's custom_data namespace. Filtering, search, and sorting run server-side so waitlisted patients' names and dates of birth are not shipped to the browser regardless of the active filter. 477 tests pass; mypy reports no issues once SDK imports resolve. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
… chart Aligns with scheduling feedback: the waitlist is practice-wide, so it is managed from the roster rather than from a patient's chart. What the chart needs is the answer to "is this person already waiting?", not an add button. Remove both ActionButtons. The chart-header button was explicitly not wanted, and the button on a cancelled appointment goes with it -- neither surface should be a way onto a practice-wide list. Removing them left no way to add anyone: the roster page had no create call, so POST /entries was reachable only from the buttons' modal form. Build the flow the spec always described but never had -- a staff-authenticated GET /waitlist/patients name search and an add dialog in the roster that reuses the existing edit-dialog field builders and the existing POST /entries. Add a keyed chart banner naming what a patient is waiting for, linking back to the roster. Emitted from all five write paths rather than from apply_transition, which writes directly and has no channel for an effect. The nightly sweep caps its refreshes, since clearing a banner needs a query per patient. Also fold services/form.py away: build_form_context and add_to_waitlist.html had no callers left, and live_entries_for_patient moved to services/entries.py beside the other entry queries. 496 tests pass (98% coverage, banner and patients at 100%); mypy clean; manifest validates. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…lot events Three sandbox incompatibilities meant the plugin passed its tests and then failed to load on the instance. Found with `canvas validate`, which sandbox- loads every handler: - `@dataclass` under `from __future__ import annotations` is unloadable. The future import stringifies annotations, and dataclasses resolves those via sys.modules[cls.__module__], which the sandbox's synthetic module scope does not register. Dropped the future import from services/config.py and services/slot.py and quoted their self-referencing return types. This alone broke 4 of 6 handlers. - setattr() is blocked. update_entry now assigns field by field, with a test pinning the assignments against EDITABLE_FIELDS so they cannot drift. - Augmented assignment to a dict item is rejected. metrics.py reassigns explicitly. Alongside: SlotFreedHandler also responds to APPOINTMENT_RESCHEDULED and the two PATIENT_PORTAL__APPOINTMENT_* events, a chart-header button opens the roster primed for the chart's patient, patient_by_id backs that prefill, and the manifest moves from `secrets` to `variables`. Adds mypy.ini and uv.lock. 573 tests pass; mypy clean; all 7 handlers load cleanly in the sandbox. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The page rendered but refused to run: "The waitlist could not start: its
configuration is missing."
Django's template engine autoescapes by default, so `{{ config_json }}` emitted
the JSON with " in place of every string delimiter. JSON.parse threw,
roster.js fell back to an empty config, found no apiBase, and stopped at its
opening guard. Marking the value |safe is what services/html.py:safe_json is
for -- it has already escaped <, > and & to \u00xx so the payload cannot close
the script tag early.
Every existing test asserted on the context dict going into the template, which
is why this shipped: the JSON was valid at that point and was mangled after.
Added tests that read the rendered template instead, including one that fails
for any *_json interpolation missing |safe rather than only this tag.
Two things found on the way:
- The HTML response set no Cache-Control, though the CSS and JS it references
both do. A redeploy could leave a browser rendering the previous shell
against the new API. Now no-cache like the assets.
- The HTMLResponse/JSONResponse test doubles did not accept `headers`, which the
real effects do, so adding the header failed only under test. Widened both and
pinned them in test_stub_contract.
579 tests pass; mypy clean; all 7 handlers load cleanly in the sandbox.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
With no configured services every submission is refused, but the Service dropdown is populated from the instance regardless -- so the dialog offered choices that could never be saved and answered with "Some details need fixing". The header button already checked is_configured before opening. The arrive-from-a-chart path did not, so the chart button opened exactly that dead form. Moved the guard inside openAddDialog, where both entry points and any future caller pass through it, and dropped the now-duplicated check from the button handler. Bumps CACHE_BUST for the changed script. 579 tests pass; all 7 handlers load cleanly in the sandbox. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Django's {# #} comment is single-line only. The escaping note I added last
commit spanned five lines, so only the first was treated as a comment and the
remaining four rendered as visible text in the middle of the roster.
Rewritten as a comment tag. The note names the tags in prose rather than
quoting them, because the tag form does not nest.
Pinned both shapes: every hash comment must close on its own line, and the
comment/endcomment tags must balance. The balance check is what caught the
nested-tag version of this fix.
581 tests pass; a real Django render of the template now leaks nothing and its
config payload parses; all 7 handlers load cleanly in the sandbox.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Three defects, found by reading cnv_scheduling_waitlist rather than guessing. Appointment types were a precondition, not a filter. options.py already documented the intent -- "the secret narrows that list; it does not define it" -- and returned every bookable type when nothing was configured, but validation.py rejected all of them in that case. So the dropdown offered services the validator refused, and the plugin was unusable until a variable was set. The reference does the opposite: unconfigured means everything bookable, a list matching nothing falls back to everything with a logged error, and validation only checks the type is on offer. Matched that, and made options.py the single authority so the two cannot diverge again -- validation now checks submissions against list_appointment_types instead of re-deriving the rule. is_configured becomes can_add, which is about the instance having bookable types rather than about our variables. The chart button opened the whole roster. The reference serves a small dedicated page; ours reused the roster with a query parameter, so clicking "Add to waitlist" on a chart put a full-width table on screen to collect six fields. Adds templates/add_patient.html and GET /app/add, and drops the roster's now-unused arrive-from-a-chart path. Nothing spoke the host modal's protocol. Canvas hands an embedded page a MessagePort; RESIZE is what makes a modal dialog-sized and CLOSE_MODAL is what lets a saved form dismiss itself. Nine plugins in this repo use it and we used it nowhere, which is the actual reason the modal filled the screen. Both pages now handshake -- the form asks for 560x640, the roster for 1200x800. Also drops menu_position: "top" from the application entry to match the reference and the other working global apps here. Unverified: whether that is why the app is missing from the drawer. 592 tests pass; mypy clean; all 7 handlers load in the sandbox; both templates render under real Django with their config payloads parsing and no leaked comment text, and the form's inline script passes node --check. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The button reads "Add to waitlist" or "On waitlist" from live data, but opened the compact add form either way. So "On waitlist" promised status and management and delivered an add form -- which refuses the obvious resubmission with a 409 from DuplicateEntryError and manages nothing. Now the label and the action agree. Not listed keeps the compact form. Already listed opens the roster filtered to that patient, where editing, marking scheduled and removing already live, rather than growing a second management surface for them. Filtered rather than the whole list: the unfiltered roster means hunting for the patient whose chart is already open. Filtered by key rather than by name, because the search box matches names and a shared surname would put someone else's waitlist entry on this patient's chart. Roster rather than an add-or-edit form as the reference does. That works there because it assumes about one entry per patient; our schema allows one live entry per patient per service, so "edit the entry" is ambiguous and the roster is the only surface that already renders several. The roster says when it is showing one person and offers "Show everyone", because a silently filtered list reads as a practice-wide one that has lost nearly everybody. Reset already clears it too. The chart banner now links to the same filtered view for the same reason. 603 tests pass; mypy clean; all 7 handlers load in the sandbox; the roster renders under real Django with its new config key parsing. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The last piece of v1 in the ticket: "an 'Add to waitlist' ActionButton from the patient header and from a cancelled/declined appointment". Only the first existed. Canvas has no button surface on the calendar grid or an appointment card, but every appointment has a note and notes do, so the button lives on the note header and shows itself only while that appointment is cancelled or no-showed -- inviting a waitlist entry for a visit somebody is about to attend would be clutter. The state comes from the appointment's own status field rather than the note's state history: one field, and the same one the freed-slot handler reacts to. It opens the compact form pre-filled with the freed slot's service, provider and location, which is the whole reason to offer it here rather than making the scheduler re-enter what the cancellation already said. Keys travel in the URL, never names, and the form matches them against dropdowns it fetches itself -- so a service that is no longer bookable or a provider since gone inactive leaves the dropdown on its default instead of inventing an option nobody can book. A slot with no provider sends no provider key at all, since a blank one would be indistinguishable from a deliberate "any provider". 627 tests pass; mypy clean; all 8 handlers load in the sandbox; the form renders under real Django with its prefill parsing and its inline script passing node --check. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The note-header button did not appear after marking an appointment no-show.
Its visibility test read Appointment.status alone. Marking no-show in the UI is a
note state transition -- a NoteStateChangeEvent surfaced by the
CurrentNoteStateEvent view -- and whether the platform also writes
Appointment.status is server behaviour a plugin cannot see. When it does not, the
button never renders.
The implementation this replaced read the note state. I changed it to the status
field on the reasoning that it was "one field, and the same one the freed-slot
handler reacts to"; that was tidiness, not evidence, and it was the regression.
Now either record is enough: status in {cancelled, noshowed}, or the note's
current state in {CLD, NSW}. The note state is select_related so it costs no
extra query on a path that runs for every note header, and the traversal is
defensive -- an appointment may carry no note, and a note may have no state
history yet.
Tests cover the two signals disagreeing in both directions, which is the whole
point, plus a missing note and a missing state history. Re-adds the NoteStates
stub with the stored codes rather than member names, since the handler compares
strings, and pins that in test_stub_contract.
636 tests pass; mypy clean; all 8 handlers load in the sandbox.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…t" too Its title was a constant, so adding the patient changed nothing on the note -- the chart-header button flipped to "On waitlist" and this one did not. Scoped to the freed slot's service rather than to the waitlist as a whole, which is the narrower and more useful question here: somebody waiting for a physical is not waiting for the follow-up that just opened, and "On waitlist" there would talk a scheduler out of adding the thing they should. A slot with no service cannot answer it and reads as not waiting -- the form it opens has no service to pre-fill either. Clicking follows the label, as on the chart: "On waitlist" opens the roster filtered to that patient, since offering an add form for a service they already want would only earn a 409 from the duplicate guard. Adds services/entries.py:has_live_entry_for_service, a yes/no beside the existing has_live_entry rather than reusing find_live_entry, which selects five relations to build a model that would be discarded. This runs on every note header render. 647 tests pass; mypy clean; all 8 handlers load in the sandbox. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The roster opened filtered to one patient when reached from a chart. That was my decision, not a requirement: the ticket's v1 acceptance lists the filters as service, provider and location plus keyword search, and says nothing about a patient-scoped view. The original suggestion was to open the waitlist with all patients in it, and I argued it down. It also read badly. The count line said "1 patient waiting. Showing one patient." -- reporting the filtered total as though it were the practice-wide one, so the list appeared to hold one person. So the chart button's "On waitlist", the freed-appointment button's "On waitlist" and the chart banner all open the practice-wide roster again. Removes the patient_id filter from build_queryset, the patient parameter from GET /entries, focusPatientId from the roster page, roster_for_patient_url, the state filter, the "Show everyone" control and its inline-button styling. Unaffected: the compact add form still takes a patient key -- it is a form about one person, which is a different thing from a list narrowed to them. 638 tests pass; mypy clean; all 8 handlers load in the sandbox. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ion filters The two dropdowns each carried both "All providers" (no filter) and "Any provider" (only entries whose patient will see anyone). A real distinction, but two options a word apart read as a duplicate, and the second matches nothing on a list where every patient named someone -- so it looked broken rather than selective. Service and Priority carry one each; these now match. Keeps the "Any ..." wording as the no-filter placeholder. Only the filter bar changed. The add and edit forms still offer "Any provider" and "Any location" as choices, which is a stored preference on the entry, not a filter, and is what makes an entry match every slot. build_queryset still honours PREFERENCE_ANY for callers that pass it; the roster no longer does, and the branch says so rather than looking like an oversight. 638 tests pass; mypy clean; all 8 handlers load in the sandbox. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Cancelling a future appointment raised no task. The handler cleared the slot
guards, matched waitlisted patients, then threw in resolve_team_id:
team = Team.objects.filter(id__in=candidates).first()
The candidate set was built as {value, value.replace("-", "")} with the try/except
guarding only the *addition* of the canonical UUID form, so the raw value was in
there regardless. A team name therefore reached a UUID column, which raises rather
than missing, and the name lookup two lines below was never reached. The
docstring's "or the team's exact name" had never worked.
Candidates are now built by parsing: a value that is not an identifier
contributes none, the identifier query is skipped entirely, and the name lookup
runs. Both the dashed and bare 32-hex forms still normalise to the same key.
Also resolves the team before claiming the slot. The claim was happening first, so
an instance with no usable team spent each slot's fingerprint on the way past and
could never announce it even after the configuration was fixed. The claim still
precedes task creation, so the duplicate-task guard is unchanged.
The old test passed because the mocked Team.objects.filter returned a MagicMock
and never raised -- the third stub-more-permissive-than-reality bug on this
branch, after HTMLResponse(headers=...) and reading Appointment.status alone. The
double now rejects non-identifiers the way the column does, and there is a test
asserting the double itself still does. Reintroducing the old code fails five of
them.
644 tests pass; mypy clean; all 8 handlers load in the sandbox.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
… instruction The task was a description of a query result. It now tells a scheduler what to do and gets out of the way. Shape borrowed from the reference plugin's task, which is markedly tighter than what we had, with two things kept that it leaves out. Fixed on the way: - "Slot freed (4)" was an internal protobuf enum integer in staff-facing text. slot_freed read event.type where it wanted event.name. The cause is now words -- "Cancelled by the patient.", "Marked no-show." -- and an unrecognised event says nothing rather than guessing. The fingerprint excludes the event, so the duplicate-task guard is untouched. - "Waiting 0 days" reads like a bug; a same-day entry says "added today". Tightened: - The title was 100+ characters and wrapped to eight lines in the queue column, burying the count. Now "Slot opened Thu 13 Aug, 4:00 PM · Office visit · 2 to call" at 58, with provider, location, year, zone and duration moved to the comment where there is room. "to call", not "match": the next action is a phone call. - The slot is stated once instead of in both title and comment. - Each patient is one line plus their note. The per-patient "Wants" restated this slot's own service, provider and location, which a matching patient necessarily accepts -- so it said nothing and buried what differs. - Weekday runs collapse: "Mon-Fri 08:00-12:00", not "Mon, Tue, Wed, Thu, Fri". - "Any time" is omitted rather than printed; it is nothing to act on. - Two paragraphs of boilerplate become one sentence, and the preferences caveat appears only when a preference was shown and window enforcement is actually off -- otherwise it was noise or a lie. The test double set event.type to the event *name*, so the suite saw "APPOINTMENT_CANCELED" where the instance produced "4" -- the same stub-more-permissive-than-reality family as the last three bugs here. It now mirrors the real Event's two fields, and reverting the handler fails two tests. Also sets WAITLIST_DISPLAY_TIMEZONE to America/Los_Angeles on vicert-testing so slot times read in the clinic's own clock rather than UTC. PracticeLocation has no timezone field, so this can only be one instance-wide setting. 663 tests pass; mypy clean; all 8 handlers load in the sandbox. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Ran the CPA wrap-up checklist by hand (CPA_PLUGIN_DIR is unset — this plugin lives in canvas-msf-fresh rather than a CPA workspace). Report and a UAT test-case list saved under .cpa-workflow-artifacts/. Verdict: ready for UAT, no blockers. 663 tests, 98% branch coverage, mypy clean over 70 files, 8/8 handlers load in the sandbox. Security passes. Both SimpleAPI classes use the SDK's StaffSessionAuthMixin rather than a hand-written authenticate(), every route adds a second guard resolving the session to a real Staff row, and both authorisation helpers fail closed — including for an entry with a NULL creator. No outbound HTTP, no API keys, no patient-session endpoints, and page documents carry only wiring rather than names. Three README statements were stale and are corrected: - WAITLIST_APPOINTMENT_TYPES was documented as required. It has been optional since 392ab27 — unset offers every bookable type, and a list matching nothing falls back to all of them with a logged error. The old text told an adopter the plugin could not start without it. - The note about the manifest using `secrets` predates the move to `variables`. - The configuration table header said "Secret". Three performance findings recorded, none blocking and none an N+1 on a request path: waitlist_cron._report loads the whole table unbounded where every other sweep is capped; _refresh_banners costs one query per patient (capped at 200, batchable to one); and live_entries_for_patient selects five relations for a caller that reads one. Also recorded, because it is a trap for the next reader: permissions.py staff_id_candidates uses the same unparsed-value-into-id__in shape that crashed resolve_team_id, and is safe only because Staff.id is a CharField while Team.id is a real UUIDField. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…shed tasks Three things found in UAT. The remove confirmation was window.confirm. The roster is embedded in a host modal, so the browser drew that dialog at the very top of the window, detached from the page that asked and styled by Chrome rather than by us. Replaced with an in-page <dialog> matching the add and edit ones, with the confirming button filled red because it is the thing being asked about. Keeps window.confirm as a fallback only if the element is missing, so a destructive action is never taken silently. A note-header button's label stayed stale until the page was reloaded while the chart header updated immediately. Not a platform limit: the SDK has two separate effects and we only emitted ReloadPatientActionButtonsEffect. Now emits ReloadNoteActionButtonsEffect too, narrowed to the changed entry's own service -- the note button asks about *this slot's* service, so a Follow-up entry cannot alter the label on a cancelled Physical -- and capped, so one write on a patient with a long cancellation history cannot fan out without limit. Slot-opened tasks accumulated with nothing ever closing them. A freed slot is only fillable until it starts, so afterwards the task is dead work; the queue grew by one per cancellation and the live call-lists would be lost among finished ones. The nightly job now completes tasks whose slot has passed, stamping SlotNotification.task_closed_at so the same tasks are not closed again every night forever. Deliberately placed before the WAITLIST_TTL_DAYS guard: an instance with no shelf life configured is still a working instance and must not accumulate dead tasks. Not closed on rebooking yet -- that needs matching a new appointment back to a freed slot, and is called out in the README as unimplemented. The UpdateTask stub requires an id like the real effect, and the cron test double grew exclude(); both would otherwise have accepted code the instance rejects. 684 tests pass; mypy clean over 70 files; all 8 handlers load in the sandbox. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…edup key Reported from UAT: freeing a slot with an empty waitlist correctly raised no task, but adding a patient and freeing that slot again still raised nothing. _claim runs before the match, so the first freeing wrote a SlotNotification with task_id="" -- an announcement of nothing -- and every later freeing of the same slot hit "already announced" and could never raise a task. The guard exists because one cancellation can reach the plugin several times within seconds, each delivery otherwise raising its own task. It was never meant to mean "this slot may never be announced". Now only a row carrying a real task id closes the door. A row that announced nothing is claimable again, and re-announcing records the freeing that actually counted rather than leaving trigger_event and notified_at pointing at the one that matched nobody. Both endings of compute() are now logged. Neither was: a slot matching nobody returned silently, which is the commonest answer to "why was there no task" and could only be inferred from the absence of the other lines. Diagnosing this report meant reasoning from missing log output. The old dedup tests passed either way because a MagicMock's auto-attribute is truthy, so the ledger always looked like it carried a task. The context now sets task_id explicitly; reverting the fix fails four of the new tests. Checked and cleared while investigating: the previous commit did not break task creation. The instance logs show SlotFreedHandler completing normally with no exception, so the task_closed_at column added there was created correctly. 692 tests pass; mypy clean over 70 files; all 8 handlers load in the sandbox. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The task is still not appearing, and the log now shows why it is not the dedup bug fixed in a31fb64: the handler runs, clears the team and claim guards, and reports "matched nobody". That much was progress, but "matched nobody" is unactionable -- an empty list, an incompatible list, and a list whose only candidate is the patient who gave the slot up all read identically from outside. explain_no_match now names the slot's own shape (service / provider / location) and then the actual reason: nobody on the list, nobody who asked for that combination, or -- the trap that costs testers the most time -- that the only compatible entries belong to the patient who cancelled, who is deliberately never offered their own slot back. Anything else points at the two remaining filters, being already marked scheduled against this appointment or falling outside a preferred window while enforcement is on. Counted only on the no-match path, so a cancellation that raises a task pays nothing for it. 698 tests pass; mypy clean over 70 files; all 8 handlers load in the sandbox. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The no-match diagnostic quoted what the slot was but not what the entries asked for, so a mismatch showed only that one existed -- not which of the three dimensions caused it. Identifiers are printed beside the labels because two NoteType rows can share a name, which a names-only log cannot show at all.
The service field was the one matched field with no any option, so it defaulted to whichever bookable type sorted first -- on a test instance, Generic event. An entry created that way is well-formed, shows correctly on the roster, and matches no slot the practice ever books. note_type is already nullable for exactly this state, and the serializer and banner already render it; only the three forms could not produce it. Any now leads all three selects, so it is also the default: matching too widely is visible and correctable, matching nothing is silent.
…ton surface The README promised the appointment button on a cancelled or no-showed note. Only the no-show gets it: cancelling tombstones the note to a timeline strip with a Restore link, and all four note button locations need an open note. The SDK defines no appointment, calendar or timeline location, so no plugin can draw there. visible() already admits both states and is unchanged -- the condition was never the problem. Records why, so it is not re-litigated, and names the workflow it costs: the task excludes the patient who cancelled, so only the button puts them on the list.
Marking no-show left 'Add to waitlist' absent until the page was reloaded: an ActionButton decides visibility as the note renders and nothing redraws it. A new handler subscribes to NOTE_STATE_CHANGE_EVENT_CREATED and emits ReloadNoteActionButtonsEffect for the affected note. Subscribed to the note state rather than APPOINTMENT_NO_SHOWED because a UI no-show does not emit that event -- on vicert-testing the note moved to NSW while SlotFreedHandler never ran. Deliberately unfiltered by state, since leaving cancelled/no-showed has to hide the button again and that list goes stale silently; gated on the note having an appointment instead.
Roster filters excluded the entries most likely to fill a slot. Choosing a provider filtered on desired_provider_id, so every patient who said they would see anybody was hidden -- while the freed-slot matcher named those same patients. Same for service and location. The per-field predicates now live in services/preferences.py and are shared by both, so the roster and the matcher cannot answer the question differently again. Calendar blocks were offered as services. Canvas marks schedule events scheduleable because staff schedule time with them; there is no patient, so an entry for one can never be filled. Excluded by category, with a logged fallback so an oddly-categorised instance still gets a usable form. The roster had no way out of its own modal -- it only ever sent RESIZE, never CLOSE_MODAL. Added a Close button, hidden until the host hands over the message port, since that port is the only thing that can close it.
Four changes asked for after UAT, plus one carried over. Colour the button label by state. ActionButton exposes BUTTON_TEXT_COLOR and BUTTON_BACKGROUND_COLOR, read at the same moment as BUTTON_TITLE, so both waitlist buttons now fill in the listed state and keep the platform's own styling otherwise. The two labels do different jobs -- "Add to waitlist" is an action, "On waitlist" is a statement of fact -- and drawn identically the second read as an action too, which is what made the note-header button confusing. Colouring only the exception means a plain button always means "there is something to do here". Add a one-click chart-header button. Every field on the form already defaults to its broadest setting, so the modal and the second click were confirming answers that were correct on arrival. "Waitlist: any" writes the entry outright and hides itself once the patient has one, since a click that writes immediately has nowhere to report a refusal. It goes through the same validate_entry the forms post to rather than assembling model fields of its own. A button click carries no request and so no session header, so the actor comes off the event -- a CanvasUser dbid, resolved through Staff.user. When that fails the click opens the ordinary form instead of writing: an entry attributed to nobody can be edited only by a configured manager, never by the person who added it, so degrading to two clicks is the cheaper failure. Show each waiting patient's next appointment. AppointmentBookedHandler closes an entry only when a booking satisfies what it asked for, which is right -- someone waiting for Dr Chen who gets booked elsewhere still wants Dr Chen -- but it leaves the entry open with nothing to show anything happened. The column supplies that signal and changes nothing: a row for a patient already seen is tinted, and the judgement stays with the reader. Filtered to real patient visits by the same exclusion the appointment-type dropdown uses, and to genuinely attended statuses; one query per page. Move the roster to the provider menu. Scope decides where the icon lives, not how it opens, so this is a manifest change with no Python behind it. Also drop the standing "this plugin never books anyone into the slot" footer from task bodies. A line on every task is read once and skipped from then on. 852 tests pass, mypy clean over 79 files, 10/10 handlers load in the sandbox.
The second button was a mistake. A chart header truncates labels at roughly twelve characters, so "Add to waitlist" and "Waitlist: any" both rendered as an ellipsis and became impossible to tell apart -- two controls that looked identical and did different things, which is a worse version of the confusion the colour change was meant to fix. So the one-click behaviour moves onto the button that was already there. "Waitlist" adds them on the broadest terms with no modal; "On waitlist" opens the roster as before. QuickAddToWaitlistButton is deleted, and services/quick_add.py stays -- it is the write path now, and still goes through the same validate_entry the forms post to. Only the action label is shortened. "On waitlist" is eleven characters and always fitted, and reviewers named it as the thing they liked, so it is unchanged; the pair now also differs from its first character rather than its last, which is the part truncation takes. The chart loses the ability to state a specific want -- this service, that provider. It is the right surface to lose it on: the roster's form still does it by patient search, the freed-appointment button still does it pre-filled from the slot, and the chart is the one surface with no slot to copy and so had the least to gain from a form. Also drops has_live_general_entry, which only the deleted button needed. 839 tests pass, mypy clean over 77 files, 9/9 handlers load in the sandbox.
The quick add commits the broadest possible entry -- any service, any provider,
any location -- which is right for the common case and was what reviewers asked
for. What it left unanswered was the next question: "she'll only see Dr Chen."
Answering it meant opening the practice-wide roster, searching for the patient
whose chart was already on screen, and finding their row.
So the second click on the same button now opens that entry's own form. First
click puts them on the list, second click narrows it. The order is deliberate:
the patient is listed before anything can go wrong, and the detail is optional.
**The form is the roster's own dialog, not a lookalike.** It links roster.css and
uses the dialog's classes -- wl-dialog-body, wl-form-grid, wl-field,
wl-dialog-actions -- so the two cannot diverge. The inline copy it used to carry
had its own spacing, sentence-case labels and no action bar, which made the form
opened from a chart and the form opened from the roster visibly two different
dialogs. Same six fields in the same two-column order, same footer; the only
thing left out is the patient picker, because the chart already knows who this is
about and an edit cannot reassign the patient.
Two CSS bugs surfaced while doing that, both fixed for the roster too:
- `[hidden]` lost to any author `display`, so `.wl-pager { display: flex }` had
been keeping the pager on screen with the attribute set, and the form page
would have flashed before its options loaded.
- The 720px breakpoint assumed the grid sits inside a wide page. The form *is*
520px wide, so it collapsed to one column at exactly the width the roster's
dialog shows two. It now keeps both columns until the viewport is narrower
than the dialog itself.
**A patient with several live entries opens the roster searched for their name.**
There is no single entry to edit, and guessing one would edit the wrong want --
but "open the roster" otherwise means searching a table that can run to thousands
for the patient whose chart is already open. The URL carries ?q=<name>, which
lands in the search box the roster already has.
That is not the patient-scoped roster reverted in cf94418, and the difference is
what that revert asked for: no filter the UI does not show, no patient_id key, no
"Show everyone" control. It is the ticket's own keyword search, pre-typed --
visible in the box, cleared by Reset, and counted honestly: the status line now
says "2 matching patients." when anything is filtered, instead of reporting a
filtered total as the practice-wide one, which is the misreading that sank the
earlier attempt.
For that to work the search had to be fixed: build_queryset now filters once per
word rather than testing the whole term against each name column, so a full name
typed into a box whose placeholder says "Patient name" finds someone. It used to
find nobody. Capped at SEARCH_TERM_WORD_LIMIT words.
One consequence worth naming: a patient's display name travels in that query
string, so it lands in browser history where a dbid would not. The page it opens
lists that name and every other waiting patient's anyway, and the URL is
same-origin and staff-authenticated. If that trade stops being acceptable, the
button should pass a key and the roster route resolve the name -- which means
WaitlistAppAPI starts reading patients and its manifest data_access stops being
empty.
Also: the freed-appointment button's "On waitlist" opens the entry for that
slot's service, the one visible() already checked to choose the label. Two
buttons wearing one label had to behave one way. And GET /waitlist/entries/<dbid>
loads the edit form, gated by the write check rather than read access -- refusing
someone after they have retyped an entry is worse than refusing them now.
886 tests pass; mypy clean; all 9 handlers load in the sandbox. Both modes of the
form and the pre-filtered roster rendered and checked in a browser.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Two things that read as Vicert-specific in a repo that is not Vicert's: - The instance name. Two tracked places said the note-state asymmetry was observed on `vicert-testing`; the observation is what matters, not whose instance it was, so both now say "a test instance". - The README's section names. Medical-Software-Foundation/canvas CONTRIBUTING.md asks every plugin for "Problem it solves", "Who it's for", "How to install" and "Configuration options", and every plugin already in extensions/ has them. Adds the two missing sections and renames the two headings. Problem it solves and Who it's for are placed before the long "What it does" walkthrough rather than after it, so a reviewer meets the why before the tour. 886 tests pass; mypy clean; 9/9 handlers load.
admir-vicert
marked this pull request as ready for review
September 4, 2026 08:51
Both described the buttons as they behaved before fbe441c: - The chart button was documented as opening "the roster once they are listed". It opens that entry's own form now, and the roster only when the patient has several live entries -- in which case it opens searched for their name. - The freed-appointment button's row said nothing about its listed state, which opens that slot's existing entry rather than the roster. The prose sections were already right; only the summary table had drifted, which is the usual way a table drifts -- nothing reads it on the way past. Found by the CPA wrap-up checklist. That run also reported: structure pass, 9 routes with 9 authorisation guards, no outbound HTTP, mypy clean across 77 files, 886 tests at 98% branch coverage, no debug logs or PHI in the 31 log lines, no dead code or orphan assets, valid 48x48 icon, MIT licence matching the other MSF plugins, and every declared variable read. Verdict: ready to ship.
A reviewer on the test instance reported the modal was not responsive to screen
size. It was not: both surfaces asked the host for a fixed pixel size over the
MessagePort, and the roster's 1200x800 is wider than a narrowed browser window.
The host honoured it anyway, so the modal was centred and clipped about 150px on
each side -- the row action buttons went off one edge and the roster's own
header, Close button included, off the top. Nothing inside could scroll to
recover, because the clipping happens outside the iframe.
**The roster now asks for nothing.** With no RESIZE the host opens its default
full size, which follows the window by construction, and a wide table wants that
anyway -- `.wl-table-wrap` already scrolls horizontally so the page never does.
Clamping the request was tried first and does not work, which is worth recording
so it is not tried again. A cross-origin iframe cannot measure the host window:
`window.parent.innerWidth` throws, `window.innerWidth` is only the iframe's own
box, and `screen.avail*` is the physical display, which does not change when a
window is resized. Clamping against `screen.avail*` helped small *displays* and
did nothing for small *windows*, which was the actual complaint. It also looked
correct under test, because headless Chrome reports
`screen.availWidth === window.innerWidth`.
**The form still asks for 520x640, and dropping that was a mistake.** 520 is
narrower than any window anyone works in, so naming it is safe in a way the
roster's 1200 was not. Without it the host gave full size and the 520px card
floated in a screen-sized empty modal -- what the chart's "On waitlist" button
showed for a patient with a single entry. The card fills that modal edge to edge
in plain white, and still centres at `max-width: 520px` so a larger surface
degrades quietly instead of stretching form fields across a screen.
Two height bugs fixed alongside, both of which put a save button out of reach:
- `.wl-dialog` was width-constrained but not height-constrained, so on a short
viewport the edit dialog grew past the bottom edge and took "Save changes" with
it. Capped at 88vh with only `.wl-dialog-body` scrolling and the actions bar
outside that scroll area.
- `.wl-modal-page` is bounded with `max-height: 100vh`. `max-height: 100%` was
tried and measured failing: on a 560x420 window it left the card 577px tall
with the actions off the bottom, because the parent's height is driven by the
card's own content, so 100% of it constrains nothing.
The dialog flex layout is scoped to `.wl-dialog[open]`, and that scoping is
load-bearing. A closed <dialog> is hidden by the browser's own
`dialog:not([open]) { display: none }`, which any author `display` outranks --
so declaring `display: flex` on `.wl-dialog` left all three dialogs permanently
on screen in normal flow rather than the top layer: no backdrop, the table's
sticky header drawing over them, and Cancel appearing to do nothing because
`dialog.close()` removes `open` while the author rule kept them displayed. Same
trap as the `[hidden]` guard a few rules above.
Verified on a host+iframe rig that reproduces how Canvas actually embeds these,
across nine window sizes from 1920x1080 down to 480x700: the roster's header and
Close stay reachable, the table scrolls instead of the page, the form fits both
axes, and Cancel/Save stay on screen. The dialog's open/close cycle was checked
by clicking Cancel, asserting `:modal` while open and `display: none` after. The
`[open]` regression test was confirmed to fail when the bug is reintroduced.
898 tests pass at 98% branch coverage; mypy clean across 77 files; manifest valid
and all 9 handlers load in the sandbox.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
PLUGIN-354 Scheduling waitlist
What it does
A shared, priority-ordered list of patients waiting to be scheduled — and a task to the scheduling team the moment a booked slot frees up.
time window.
The plugin never books anyone. It recommends; staff schedule from the task.
Problem it solves
A cancellation is a slot already paid for in staff time and about to go to waste. Someone almost always wants it — but who was waiting lives in a spreadsheet, a sticky note, or one scheduler's memory, and nobody
reconstructs that list under time pressure. So the slot goes unfilled while the patient who wanted it waits another three weeks.
Three things have to be true at once for a waitlist to actually get used, and the plugin is built around them:
Who it's for
Providers need not interact with it at all, beyond seeing a banner on the chart of a patient who is already waiting.