diff --git a/data/outstanding-issues-snapshot.json b/data/outstanding-issues-snapshot.json index fb9ed7a162..ad71089ac1 100644 --- a/data/outstanding-issues-snapshot.json +++ b/data/outstanding-issues-snapshot.json @@ -10,7 +10,7 @@ "p2": 40, "p3": 33, "queued": 10, - "pending": 138, + "pending": 139, "resolved": 376 }, "queue": [ @@ -1205,6 +1205,12 @@ "summary": "Cancel request 608150d5-7aa4-4b98-b497-1a9f1feaf4b5: Outcome stored a local account identifier under a personal home path; replace with repository-relative worktree logs, transcript filenames, and redacted home placeholders while keeping timestamps, hashes, and Claude Code 2.1.237 provenance.", "created_at": "2026-08-23" }, + { + "request_id": "801d50ab-adbb-48e7-90ab-f8e6ed7ca588", + "action": "add", + "summary": "Caring Contacts: nothing enforces that a new workspace screen joins the browser spec's WORKSPACE_SCREENS", + "created_at": "2026-08-24" + }, { "request_id": "82838fb1-7ed2-4f37-bd50-191a603ecd0f", "action": "add", diff --git a/data/repo-awareness-snapshot.json b/data/repo-awareness-snapshot.json index 8f30030902..87cf2ba50d 100644 --- a/data/repo-awareness-snapshot.json +++ b/data/repo-awareness-snapshot.json @@ -1,8 +1,8 @@ { "version": "repo-awareness-snapshot-v1", "captured_revision": { - "sha": "c93dd40b99e84e0a445bb16fc36c82d3089f5674", - "committed_at": "2026-08-25T05:43:27+08:00" + "sha": "01fe261f36dd001734cfd52a0556193edfcbac15", + "committed_at": "2026-08-25T06:16:41+08:00" }, "routes": { "modes": [ @@ -113,6 +113,11 @@ "file": "src/app/caring-contacts/page.tsx", "area": "product" }, + { + "path": "/caring-contacts/patients", + "file": "src/app/caring-contacts/patients/page.tsx", + "area": "product" + }, { "path": "/dictionary/[slug]", "file": "src/app/(search-app)/dictionary/[slug]/page.tsx", @@ -1339,8 +1344,8 @@ ], "counts": { "modes": 15, - "pages": 185, - "product_pages": 65, + "pages": 186, + "product_pages": 66, "mockup_pages": 120, "redirects": 17, "api": 56 @@ -1958,6 +1963,26 @@ "section": "caring-contacts", "catalogued": false }, + { + "path": "docs/caring-contacts/phase-2b-sdd-archive/task-5-brief.md", + "section": "caring-contacts", + "catalogued": false + }, + { + "path": "docs/caring-contacts/phase-2b-sdd-archive/task-5-report.md", + "section": "caring-contacts", + "catalogued": false + }, + { + "path": "docs/caring-contacts/phase-2b-sdd-archive/task-5b-brief.md", + "section": "caring-contacts", + "catalogued": false + }, + { + "path": "docs/caring-contacts/phase-2b-sdd-archive/task-5b-report.md", + "section": "caring-contacts", + "catalogued": false + }, { "path": "docs/caring-contacts/phase-2b-sdd-archive/task-c-brief.md", "section": "caring-contacts", @@ -3497,8 +3522,8 @@ }, { "name": "caring-contacts", - "documents": 62, - "uncatalogued": 62 + "documents": 66, + "uncatalogued": 66 }, { "name": "codex", @@ -3567,9 +3592,9 @@ } ], "counts": { - "documents": 425, + "documents": 429, "catalogued": 101, - "uncatalogued": 324, + "uncatalogued": 328, "sections": 18 } }, @@ -3582,6 +3607,14 @@ }, "review_state": { "records": [ + { + "date": "2026-08-24", + "ref": "2354", + "head": "97be623d5cd0d878130ff1eb6aa8ef9851d92d97", + "scope": "PR #2354 current changed scope", + "outcome": "P1 privacy URL leak and P2 inventory/data-boundary defects fixed; final staged re-review clean", + "checks": "diff check; sitemap; docs links/index; issue snapshot pass; focused DOM blocked by repository Playwright lease" + }, { "date": "2026-08-24", "ref": "2360", @@ -24448,8 +24481,8 @@ } ], "counts": { - "records": 2608, - "refs": 1590 + "records": 2609, + "refs": 1591 } } } diff --git a/docs/branch-review-records/4fd3c57dfcaeeca4519af86125867531ecd26b357b492519598c72a8dc49b0ff.record.md b/docs/branch-review-records/4fd3c57dfcaeeca4519af86125867531ecd26b357b492519598c72a8dc49b0ff.record.md new file mode 100644 index 0000000000..c0af206241 --- /dev/null +++ b/docs/branch-review-records/4fd3c57dfcaeeca4519af86125867531ecd26b357b492519598c72a8dc49b0ff.record.md @@ -0,0 +1 @@ +| 2026-08-24 | 2354 | 97be623d5cd0d878130ff1eb6aa8ef9851d92d97 | PR #2354 current changed scope | P1 privacy URL leak and P2 inventory/data-boundary defects fixed; final staged re-review clean | diff check; sitemap; docs links/index; issue snapshot pass; focused DOM blocked by repository Playwright lease | diff --git a/docs/caring-contacts/phase-2b-build-record.md b/docs/caring-contacts/phase-2b-build-record.md index 8d377171a2..99e86c301b 100644 --- a/docs/caring-contacts/phase-2b-build-record.md +++ b/docs/caring-contacts/phase-2b-build-record.md @@ -723,3 +723,318 @@ deferred).** Browser gate green at the post-fix head: `32 passed`, exit 0. **Group 0 is finished.** Task 2 was cut (Ruling 84), Task 4 merged forward (Ruling 89), so the shared scaffolding is: `ListEmptyState`, the overlay trigger and its commit contract. Group 1 begins. + +## Task 5 — the Patients directory, and the question it was right to stop on + +Returned complete at `d27030405`. `Tests 2 failed | 9851 passed | 74 skipped (9927)` — the two known +`gate-receipts` file-mode failures only. Typecheck and lint fresh passes, not reused receipts. Twelve +mutations, each red on its covering test, presence proved with `;` rather than `&&` — the previous +task's non-run lesson applied without being restated. + +**Browser gate green at this head: `32 passed`, exit 0.** The implementer flagged that changing +Patients from an unavailable button to a link shifts unavailable-control counts (its own DOM +equivalents went 16 → 14) and warned the Playwright spec might carry the same counts. It does not. +Measured rather than assumed, and the warning was the right one to give. + +**Ruling [91] — a names-only projection is built, as its own task. The OWNER decided this, not me.** +The brief forbade `getEpisode` on a list, and instructed the implementer to stop and report if the +approved design needed a name rather than decide for itself. It did exactly that: the design shows +`row.name`, `PersonAvatar initials` and a "Search name or synthetic ID" box, and it built rows headed +by the synthetic identifier instead. + +That was the correct stop. The tension is real in both directions: `getEpisode` is the only read that +releases patient name **together with** mobile number, identifiers and cultural identity, so using it +for a list would pull all four into a page that shows one — and yet **a caseload a clinician cannot +recognise their own patients in is barely a caseload.** Put to the owner, who chose the narrow read: +a projection returning the name alone, permission-checked in its own right, never widening +`getEpisode`. + +— Cost if wrong: it changes `repository.ts`'s interface, so both stores and the shared contract suite +move together; that is the price of the storage contract being the thing that holds them equal, and it +is the reason this is **its own task rather than a fix round on Task 5**. A fix round would have +balloonded into a domain change reviewed as a screen change. + +**The implementer's own recommendation was the one the owner picked**, arrived at independently. Worth +recording: an implementer told to stop and report rather than decide produced a better-reasoned option +than the brief anticipated, because it had read the design and the read contract side by side. + +**Concern 2 is a false-evidence claim and must be fixed, not deferred.** The adoption generator +refuses an undeclared production page route, so the new route had to join the +`caring-contacts-workspace` surface — whose proof is declared `passed` with +`tests/ui-caring-contacts-workspace.spec.ts` as its evidence. **That spec has never visited +`/caring-contacts/patients`.** So the design-system contract now asserts browser proof for a route +nothing has proved. Same family as Ruling 88: a governance artifact making a claim about coverage that +is not true. The remedy is a visit in the spec, not a quieter claim. + +**Two concerns carried to the review rather than settled here:** the role-restricted empty state uses +`ListEmptyState`'s `"filtered"` kind for something that is not a filter (an auditor cannot view plans, +so `listPlans` returns `[]` and "No patients yet" would be a lie) — a third `"not-permitted"` kind may +be right, but it touches a Group 0 component; and the prohibited vocabulary now bites ordinary English +(`\bleads?\b` matches "team lead", `\bsafe\b` is banned outright), which the Team screen will hit +immediately. + +### Task 5 review — spec ✅, quality NOT approved: 1 Critical, 4 Important, 5 Minor + +The twelve mutation proofs were all traced to the assertion reading the mutated value and judged real, +including a self-corrected one (M4 → M4b) recorded rather than quietly re-run — the opposite of the +two misreported mutations earlier in this programme. Reads are clean: `getEpisode` is absent and +pinned by a spy, and `PlanRecord` excludes `patientDetail` structurally, with the in-memory store +projecting through `toPlanRecord` rather than returning the stored object, so nothing rides along. +Filtering is genuinely server-side — both filters are navigations, no new client boundary. + +**C-1 is worse than the implementer reported, and it found it itself.** The design-system contract +declares the `caring-contacts-workspace` surface with **all five** proof categories — dark, +forced-colours, compact-320, print, browser — as `passed`, evidenced solely by +`tests/ui-caring-contacts-workspace.spec.ts`. That spec pins `WORKSPACE_ROUTE = "/caring-contacts"` +and every `page.goto` in 952 lines uses it. It has never loaded `/caring-contacts/patients`. + +Three things make this Critical rather than untidy: + +- **The spec's own header states the rule this breaks**: "a proof pointer at a suite that never visits + this route is a red gate that has been silenced." The change made that sentence untrue about the + file it is written in. +- **No honest weaker declaration exists.** The generator fails any v2 surface whose proof category is + not `passed`, so "declare it more quietly" is unavailable, and a separate surface would be forced to + `passed` too. The generator also cannot detect the problem — evidence paths are checked for being + tracked files, never for relevance to the route. +- **The remedy is bigger than a `goto`.** Four of the five are accessibility-mode claims, so honesty + requires the dark / forced-colours / 320px / print coverage to run against the new route as well. + +**Which family:** unambiguously Ruling 88's — a **false** attribution — not the true-attribution case +kept in Task 3. Fix, do not keep. Credit where due: the implementer found it, described it accurately, +and refused to edit a spec it could not run. + +**Ruling [92] — `ListEmptyState` gains a third kind, `"not-permitted"`.** — Why: an auditor cannot view +plans, so `listPlans` returns `[]`; `"no-data"` would say "No patients yet", which is a lie, so the +implementer used `"filtered"` — whose own documentation says records exist, and whose icon selection +renders a struck-through magnifying glass **on a screen where no search was performed**, in a component +whose comment says the icon "is part of what states the difference wordlessly". The screen's words are +honest; the type and the icon are not. `ListEmptyState` has exactly ONE consumer today, so this is the +cheapest it will ever be, and Schedule, Templates and Team will each meet the same role case and copy +whatever Patients did. — Cost if wrong: a fourth kind later, and a union that is one member wider than +strictly needed. Against that: leaving it reintroduces at the type level exactly the blur the component +was built to refuse. + +**Ruling [93] — the role-restricted remedy must say what is true, and there is no role switcher.** +The screen currently says "What changes it: The role switcher changes which role you are acting in." +I verified it myself: `CARING_CONTACTS_ROLE_COOKIE` appears exactly once in `src/`, its own +declaration in `session.ts`, and nothing writes it — `resolveDemoActor` silently defaults to +coordinator. Spec §4.4 requires a **reachable** remedy, and **naming a control that does not exist is +worse than naming none**, because a clinician will hunt for it. The covering test asserts the +_presence_ of "What changes it:" and never that its content is real, so this whole class is invisible +to the gate. — Cost if wrong: if a switcher is built later the wording needs revisiting, which is a +one-line edit at the moment someone is already in the file. + +**The lesson C-1 and Ruling 93 share.** Both are false statements that passed every gate, because the +gates check **shape** rather than **truth**: the generator checks an evidence path is a tracked file, +never that the suite visits the route; the empty-state test checks a remedy is present, never that it +exists. **A gate that checks a claim is well-formed will certify a well-formed lie.** + +### Task 5 fix round 1 — all ten, and the browser gate went 32 to 38 + +`Tests 2 failed | 9860 passed | 74 skipped (9936)` (the two known `gate-receipts` file-mode +failures), typecheck and lint fresh passes, and the Playwright gate **`38 passed (58.3s)`, exit 0** — +six new Patients tests, so C-1's proof claim is now backed by coverage rather than by a pointer. + +**Ruling 93's PREMISE WAS WRONG, and the error was mine.** I wrote that +`CARING_CONTACTS_ROLE_COOKIE` "appears exactly once in `src/`, its own declaration, and nothing writes +it". The implementer corrected it and I verified the correction myself: the constant appears **four** +times and `src/app/api/caring-contacts/session/route.ts:53` **does** write the cookie. + +**How I got it wrong is the part worth keeping.** I grepped `"caring-contacts-demo-role"` — the +cookie's string _literal_ — which appears exactly once, at the `const` that names it. Every actual use +goes through the constant. So my grep returned a true answer to a question I was not asking, and I +reported it as though it answered the one I was. The reviewer made the identical mistake independently, +which is what made it feel confirmed. + +This is the same family as the five "checks that cannot fail" already recorded, and it is the variant +that catches careful people: **not a check that cannot fail, but a check that answers a neighbouring +question.** When grepping for whether something is used, grep the SYMBOL, not the value it holds — and +when two people agree, check whether they ran the same flawed command rather than two different ones. + +**The conclusion survives:** there is still no role-switching control in the interface, so naming one +was false and the new wording is right. Only the stated fact was wrong. Recorded rather than quietly +amended, because the server half of a switcher existing matters to whoever builds the UI. + +**Three findings the mutations PRODUCED rather than confirmed:** + +1. **`expect(status).toBe(200)` cannot catch a `notFound()` on this route.** With `notFound()` added + the route still answered **200**: it is dynamic and streams under a Suspense boundary, so headers + flush before the render reaches the refusal and the 404 arrives as content. Only the content + assertions failed. The status assertion is kept — it still catches a refusal made before the stream + opens — but the test now names which assertions are load-bearing. **The "gates check shape, not + truth" lesson landed on a test written in the same round it was issued.** +2. **I-4's branch had no covering test at all.** Restoring `?? []` left all 56 tests green, because + the branch is unreachable through the real stores — which is precisely why it was wrong and why + nobody noticed. Now pinned by spying `listPlans` to `null`. +3. **M-9's assertion covered less than it looked.** It checked the filter chips but not the empty + state's own remedy link; stripping the attribute from that one link left the file green. + +**A process failure that was MINE.** Partway through this round I switched this worktree to +`claude/caring-contacts-foundation` to fix the PR's CI — **while an implementer was working in it**. +Its source files vanished from disk and two of its edits landed on the wrong branch. It recovered +cleanly: saved the stray diff, restored that tree exactly as found, switched back, re-applied. Nothing +was lost, and the recovery was better than the incident deserved. **Never switch a worktree's branch +while a subagent is working in it** — one worktree, one branch, for the duration of a dispatch. If a +second branch needs work, it needs a second worktree. + +**Still open and captured:** nothing enforces that a new screen joins `WORKSPACE_SCREENS`, so a Task 6 +screen added to the adoption surface but not to the spec recreates C-1 exactly. The implementer +identified this and did not build the closure. + +**PR #2350 (the foundation: Task C, Task 1, Task 3) MERGED to `main` at 2026-08-24T15:49:29Z**, by the +owner's armed auto-merge once the checks went green. Two CI failures were fixed on the way, both mine: +a stale outstanding-issues snapshot, and five broken path references — two from Ruling 88's rename that +my own briefs still pointed at, one a quoted `tsc` diagnostic whose `path(line,col)` the link checker +read as a path, and a Task 5 brief describing work that PR did not contain. + +### Task 5 scoped re-review — all ten ADDRESSED, three new Minors, one repeat + +C-1's remedy was judged real rather than cosmetic: `openWorkspace()` is parameterised by screen so the +route-specific heading travels with it, and three of the four modes earn their category on evidence +only the new route can supply — a 48px tap-target measurement at 320px, a `borderTopWidth` read off an +element that exists only on Patients, and the heading under print media. Dark is the thinnest: its +load-bearing comparisons come from shell chrome identical on both routes, so it is substantively +Today's proof re-run on a second URL. Not a re-creation of C-1 — the route genuinely loads in dark — +but named and sent back. + +**Ruling [94] — drop the client-component count; keep the conclusion. The replacement was wrong the +same way the original was.** "The workspace ships five client components in total" misses +`src/app/caring-contacts/error.tsx`, which is a Client Component because Next requires it, sits inside +the workspace by the very paragraph doing the correcting, and pulls a sixth client module +(`route-error-boundary.tsx`) into the route's bundle. The implementer had scoped its count to +`workspace/**`, which is defensible, but no site said so and "in total" forecloses it. + +— Why drop rather than re-count: **this is the second time in two rounds that this one paragraph has +carried a false number**, and it is now replicated across three files exactly as "one" was. A count in +prose is a claim that decays whenever anyone adds a file, and nothing checks it. What actually carries +Ruling 13 is the module boundary — the dashboard cannot reach this workspace's chunks — which is true +independent of how many client components exist. — Cost if wrong: a reader loses a number that was +never reliable anyway. + +**The generalisable shape: a fact that must be restated to stay true will eventually be false.** +Prefer the invariant over the tally. + +**Three new Minors, each a different flavour of the same week's lesson:** + +- **N-1** — the new null guard tests `=== null` while `auditedRead` treats **null or undefined** as + denied. `released` is typed `T | null`, so the compiler cannot see the gap and the pinning test mocks + `null` specifically. Fails closed either way, but with a `TypeError` rather than the stated message, + and the branch the item is about would not be the one that fired. +- **N-2** — an absorbed contact and a transition-suppressed contact in the same plan disagree: the + count subtracts all suppressed contacts, the explanation covers only absorption. Exactly the blur + M-8 fixed, one case further along. +- **N-3** — `scheduled` changed its definition this round from absorbed-only to all-suppressed, and + **nothing asserts that clinician-facing number**. Same shape as the implementer's own I-4 finding, + and the same shape as the Ward Flow lesson where green tests missed a wrong value on every screen. + +**The mutation ledger does not close, and that is worth more than the two missing lines.** Thirteen +claimed, eleven verifiable; two Vitest mutations are counted and never named. Every mutation the report +_describes_ was traced to an assertion reading the mutated value — the descriptions are sound. But +**a total that outruns its itemisation is the weakest form of the misdescription problem** this +programme has already met twice. Sent back to be named or corrected to eleven. Also flagged: M-6's +mutation raises the test's own threshold, so it proves the assertion executes rather than that the +floor detects anything — accurate as described, but not product proof. + +**The reviewer answered the open enforcement question, and its caution is the valuable half.** A static +check comparing the adoption surface's `routes` against the spec's screen list is the right closure — +it converts the generator's _shape_ check into a _truth_ check, which is precisely C-1's lesson — and +is ~30 lines offline if scoped to this surface. **But it must not be generalised**: `ward-management` +declares 12 routes evidenced by generic contract suites that enumerate no routes at all, so a repo-wide +rule goes red immediately on surfaces nobody asked to remediate, and the pressure would then be to +weaken the rule rather than fix them. Opt-in marker, joined deliberately. + +Task 5: fix round 2/5 dispatched — Ruling 94, the mutation ledger, N-1, N-2, N-3, and the dark-mode +assertion. + +### Task 5 fix round 2 re-review — all six ADDRESSED, nothing new + +Ruling 94's fix verified at all three sites, and the reviewer checked the replacement claim is +actually true today rather than taking it on faith — it grepped for imports of the workspace from +outside its own route segment and found none. It also grepped all three files plus `error.tsx` for a +reintroduced number and found none. One file says "a handful", which is deliberately vague rather than +a tally that can go stale; read as consistent with the ruling rather than a loophole. + +**The ledger closed by dropping the total rather than forcing a reconciliation**, which the reviewer +read — correctly — as the same move Ruling 94 makes: every attempt is now a row, including the skipped +anchor-mismatch and the two that did NOT go red, and no aggregate is claimed. A reader who wants a +number adds it up themselves. That is the right way round: the itemisation is the evidence, the total +was only ever a summary of it. + +**One of my instructions was overstated and the reviewer said so.** I asked for N-3's two mutations +"from opposite directions". Both — subtract-only-absorbed, and subtract-nothing — produce a count that +is too HIGH, so they are not opposite; they are two plausible wrong implementations of the same +definition. The practice is sound mutation method and the fix is right; my geometry was wrong. Recorded +because a controller's framing gets copied into later briefs if nobody corrects it. + +The reviewer also traced the store invariant the N-2 fix relies on — that absorbed contacts always land +terminal-`suppressed` — back to `createPlan` in the in-memory store rather than accepting it as an +assumed invariant. That is the standard this programme keeps asking for and rarely has to ask twice. + +**Task 5: COMPLETE (commits `efb84c556`..`6df257b35`, review clean after 2 fix rounds, 4 minors +deferred).** Playwright `38 passed`, up from 32 — the six new tests are the Critical finding's remedy. + +**Ruling [95] — Task 5b's names-only read is its own repository method with its own capability check, +and that check reuses the existing `viewPatientRecord` rather than minting a new capability.** +— Why: the owner approved "a narrow read that returns just the name, permission-checked separately". +The substance he was buying is **data narrowing** — a caseload that no longer pulls mobile number, +identifiers and cultural identity for every row to show one field — and a separate projection delivers +that completely. A new capability would only buy something if some role should see names but not +records, and no such role exists: `viewPatientRecord` is granted at five sites covering the human +roles that can list plans at all. Minting `viewPatientName` would invent a permission tier nobody has +asked for, and every role's grant would have to be decided to satisfy the exhaustiveness guard. +— Cost if wrong: if a see-names-but-not-records role ever appears, the capability splits at that +point. The split is mechanical precisely because the read is already its own method — which is the +part that matters and is being built now. + +**Stated plainly because it is a narrowing of my own words to the owner:** I said "permission-checked +separately", and separately means its own method and its own check, not a new capability. If he meant +a new permission tier, this is the sentence that will let him say so. + +## Task 5b — the names-only projection, and the implementer improved on my ruling + +Returned complete at `c6cd1ede8`. Full suite **`Tests 9985 passed | 74 skipped (10059)`, zero +failures**; Postgres suite `Tests 182 passed (182)`; typecheck and lint recorded passes. Twelve +mutation attempts itemised, eleven red, one (M7) not red — fixed and re-run red as M7b, with the +failed attempt kept in the table. No aggregate total claimed, per Ruling 94's shape. + +**Ruling [95] is REFINED, and the refinement is the implementer's.** I ruled the read reuse the +existing `viewPatientRecord` rather than mint a capability. It honoured that — no new capability +exists — and then found the hole my ruling left open: + +`READ_ACTIONS.plan` is `"viewReferral"`, and that is what gates listing plans at all. The **auditor** +holds `viewPatientRecord` but NOT `viewReferral`. So on `viewPatientRecord` alone, `listPatientNames` +would have handed the auditor **an enumeration of every patient name in the team** — obtainable by no +route that exists today. **A change whose entire purpose is narrowing would have widened auditor +access.** + +`PATIENT_NAME_READ_ACTIONS` is therefore an ALL-of list, `[READ_ACTIONS.plan, +READ_ACTIONS.patientName]`, and the reasoning is not arbitrary: the projection **enumerates** the +team's plans, so it must release a name only for a plan the actor could already see. The in-memory +implementation filters by exactly `listPlans`' predicate and then by the name capability on top, so +the result is always a subset of the plans that actor can already list. It also builds the returned +objects rather than deriving them from the stored plan, "so no widening can ride along by accident: +there is no spread of `patientDetail` to forget to narrow." + +**What I take from this about ruling.** My ruling answered the question I had asked myself — "does +this need a new capability?" — and was correct on it. It did not ask the adjacent question: "what +does the capability I am reusing already grant, to whom?" A ruling scoped to the question that +prompted it can be right and still leave a hole, and the implementer nearest the code is the one +positioned to see it. **Rulings should be written to be improvable, and an implementer that says "you +may want to overrule this" should be read as doing its job rather than hedging.** Upheld as built. + +**A STALE STANDING INSTRUCTION, corrected — and it was mine.** I have been telling every subagent to +"expect exactly 2 failures in `tests/gate-receipts.test.ts`". Those failures are **gone**: the merge +from origin brought `cbde6ecbb` "Make the gate-receipt tests environment-explicit so they pass under +CI", and the file now runs `Tests 34 passed (34)`. This implementer noticed the prediction did not +match reality and said so. + +The hazard is not the wasted sentence, it is the direction it points: **an instruction to expect a +named failure tells a reader to look past a file that can once again fail for real.** A standing +"known noise" note is a licence to ignore, and it must expire the moment the noise does. Removed from +the briefs and corrected in durable memory. + +**Two concerns carried to review rather than settled here:** a row cannot distinguish "de-identified" +from "your role may not see names" — it states the kind of thing the heading is and claims nothing +more, which is conservative but may not be enough; and `patientDirectory` now names two different +reads in the access trail, distinguishable by `objectId` but not by action name. diff --git a/docs/caring-contacts/phase-2b-sdd-archive/task-5-brief.md b/docs/caring-contacts/phase-2b-sdd-archive/task-5-brief.md new file mode 100644 index 0000000000..98e791d4e6 --- /dev/null +++ b/docs/caring-contacts/phase-2b-sdd-archive/task-5-brief.md @@ -0,0 +1,137 @@ +# Task 5 brief — the Patients directory (absorbing Task 4) + +**Plan:** `docs/superpowers/plans/2026-08-24-caring-contact-phase-2b-screens.md`, Group 1, Task 5. +**These are your requirements.** Read Rulings 85 and 89 in `docs/caring-contacts/phase-2b-build-record.md` +first — they are why this task is shaped the way it is. + +Caring Contacts is a suicide-prevention prototype: patients discharged from hospital receive a fixed +schedule of brief, non-demanding messages. Every patient is fictional and **nothing is ever sent to any +number**. This is the **first real screen of Phase 2B** and the owner's first priority. + +## What you are building + +`src/app/caring-contacts/patients/page.tsx` — the team's caseload, listing the plans this actor's team +may see, plus the navigation and documentation that make it a real destination. + +**Task 4 was merged into this one (Ruling 89).** The plan originally lit up the navigation link with an +empty placeholder page first. That page would have said _"No patients yet"_ whether or not patients +existed — a false statement on a clinical caseload screen, and precisely the defect the component you +are about to use was built to prevent. So the link and the real screen land together, and the screen is +never reachable in a state where it can lie. + +## Do NOT build a data source — it exists (Ruling 85) + +`GET /api/caring-contacts/plans` already lists the team's plans through `readHandler`. More directly, +**your page is a Server Component and should read the store the same way the Today page does**, not +over HTTP. + +**Read `src/app/caring-contacts/page.tsx` before writing anything.** It is heavily commented and is the +established pattern: `isCaringContactsDemoEnabled()` then `notFound()`; `resolveDemoActor()`; +`caringContactsStore()`; the read wrapped in `auditedRead`; fail closed on every bad outcome; then +render ``. + +Your page needs **two** audited reads — the service state (so the safety banner can render, which is a +required prop on every screen) and the plans list. Use the same access identity the API route already +records for each, so the access trail does not grow a second vocabulary for the same read: + +- service state — `{ kind: "administrative", objectType: "serviceState", objectId: "service" }` +- plans — `{ kind: "search", objectType: "plan", objectId: "all" }` and `store.listPlans({ actor })` + +**Do NOT call `getEpisode`.** It is the only read that releases `patientDetail` — name, mobile number, +identifiers, cultural identity — and a directory does not need them. If the approved design appears to +show a patient's name in the list, **stop and report it** rather than reaching for `getEpisode`; that +is a question for me, not a decision for the implementer. Every other read returns `PlanRecord`, which +excludes patient detail by construction. + +## The empty-list contract test — the one thing that survived the cut of Task 2 + +`auditedRead` maps a `null` or `undefined` release to `denied`, which `readHandler` turns into +`not-found`. **An empty array is neither**, and an empty caseload must never present as a missing +resource. + +Write a test pinning that **an empty list renders the empty state on a 200-shaped success path, never a +404 / `notFound()`**. This is not obvious from reading the code, which is exactly why it needs a test +rather than a comment. The factory's own note says the trail cannot distinguish "you may not see these" +from "there are none" for a list — so the HTTP and render shapes must be pinned deliberately. + +## Use `ListEmptyState`, and use both of its kinds honestly + +`src/components/caring-contacts/workspace/list-empty-state.tsx` (Task 1). It has two kinds and they are +not interchangeable: + +- `"no-data"` — the team genuinely has no plans. +- `"filtered"` — plans exist but the current filter or search hides all of them. **Requires** a + `because` and a `changedBy`. + +A filtered-empty caseload that renders `"no-data"` tells a clinician their caseload is empty when it is +not. That is the defect this whole component exists to prevent — get it right here, because this is the +first screen to use it and every later screen will copy this one. + +## Filtering + +Provide at least one filter or search. Keep it **server-side** if you can do so without a client +boundary — Ruling 13 holds this workspace's client payload to a rounding error, and a URL-parameter +filter read by the Server Component costs none. If you conclude a client boundary is unavoidable, +**say so in your report with your reasoning** rather than adding one quietly. + +## What makes it a real destination (the absorbed Task 4) + +All four, or the route is an orphan and the build fails: + +1. `href: CARING_CONTACTS_ROUTES.patients` added to `PRIMARY_DESTINATIONS` in + `src/components/caring-contacts/workspace/shell.tsx`. The file's own comment says this is the whole + of the change. Note `PHONE_DESTINATIONS` derives from it, so the phone dock follows automatically. +2. `npm run sitemap:update`. +3. An entry in `docs/codebase-index.md`. +4. A reachability assertion — see `tests/route-reachability.test.ts`. + +Build hrefs from `src/lib/caring-contacts-routes.ts`, never from string literals. It already exports +`patientRoute(patientId)` and `planRoute(planId)` for the rows. + +## Constraints + +- **Internal navigation** uses `` / `router.push` / server `redirect()` — never a raw + ``. +- Every ` + )} + + + {visible.length > 0 ? ( +

+ Showing {plural(visible.length, "plan", "plans")} of {plural(totalPlanCount, "plan", "plans")} this team + holds. +

+ ) : null} + + ) : null} + + {visible.length > 0 ? ( + + ) : ( +
+ setRawQuery("")} + /> +
+ )} + + ); +} + +function DirectoryEmptyState({ + totalPlanCount, + filter, + query, + filtering, + mayViewPlans, + clearSearch, +}: { + totalPlanCount: number; + filter: PatientsDirectoryFilter; + query: string; + filtering: boolean; + mayViewPlans: boolean; + clearSearch: () => void; +}) { + if (!mayViewPlans) { + return ( + + ); + } + + if (totalPlanCount === 0) { + return ( + + ); + } + + if (filtering) { + return ( + + Show every plan + + ) : ( + + Show every plan + + ) + } + /> + ); + } + + // Unreachable in practice -- `visible` can only be shorter than `rows` when something is + // filtering -- but stated rather than left to fall through to nothing at all. + return ( + + ); +} + +/** Plain words for exactly which of the two filters is hiding the list, and how much it is hiding. */ +function hiddenBecause(total: number, filter: PatientsDirectoryFilter, query: string): string { + const held = `the ${plural(total, "plan", "plans")} this team holds`; + if (filter.state !== "all" && query !== "") { + return `The state filter is set to ${PLAN_STATE_LABELS[filter.state]} and the search is "${query}". Nothing among ${held} satisfies both.`; + } + if (filter.state !== "all") { + return `The state filter is set to ${PLAN_STATE_LABELS[filter.state]}, and none of ${held} is in that state.`; + } + return `The search for "${query}" matches no name or identifier among ${held}.`; +} + +/** + * One plan's row. + * + * `patientName` is null when the names read held nothing for this plan -- a de-identified episode, or a + * role without `viewPatientRecord`. The heading then falls back to the synthetic identifier and the + * label above it says which of the two the heading is, so the row never presents an identifier as a + * name or leaves a reader guessing. It deliberately does not try to say WHICH cause applies: the + * screen is not told, and guessing between "this episode was de-identified" and "your role may not + * see names" would be a claim nothing here can support. + */ +function PatientRow({ row }: { row: PatientsDirectoryRow }) { + const suppressed = row.absorbedContactCount + row.otherSuppressedContactCount; + const explanation = suppressionExplanation(row.absorbedContactCount, row.otherSuppressedContactCount); + + return ( +
  • +
    +
    +

    + {row.patientName === null ? "Synthetic patient identifier" : "Patient"} +

    +

    + {row.patientName ?? row.patientId} +

    + {/* + Kept beside the name, not replaced by it. Two patients can share a name, the row's + detail control is named by the identifier, and it is the identifier a clinician quotes + when asking about a record. + */} + {row.patientName === null ? null : ( +

    + Synthetic identifier: {row.patientId} +

    + )} +

    + Plan state: + {PLAN_STATE_LABELS[row.state]} +

    +

    + Discharged: + {row.dischargeDay} (AWST) +

    +

    + Contacts: + {plural(row.scheduledContactCount, "message in the schedule", "messages in the schedule")} +

    +
    + {/* + `label` is a destination NOUN, not an instruction. `UnavailableDestination` builds its + screen-reader note as "
    + + {/* + Spec 4.4: a state the system reached on its own must state, in place, why and what would + change it. A suppressed contact is one the system decided not to send, and the row would + otherwise show a smaller message count with no reachable reason for it. + + Two causes exist, they have different remedies, and a plan can carry BOTH — so the reason + covers whichever are actually present rather than picking one. The schedule absorbing Week 1 + into the first contact is reversible by the coordinator; any other suppression is terminal + (`suppressed` is in the contact model's terminal set) and this row does not hold what caused + it, so it says that rather than inventing a remedy or naming a screen that does not exist. + */} + {suppressed > 0 ? ( +
    + +
    + ) : null} +
    • + ); +} diff --git a/src/components/caring-contacts/workspace/patients-directory-row.ts b/src/components/caring-contacts/workspace/patients-directory-row.ts new file mode 100644 index 0000000000..4eae8c361d --- /dev/null +++ b/src/components/caring-contacts/workspace/patients-directory-row.ts @@ -0,0 +1,15 @@ +import type { PatientId, PlanId, ReferralId } from "@/lib/caring-contacts/ids"; +import type { PlanState } from "@/lib/caring-contacts/model"; + +/** The complete, deliberately narrow value allowed across the Patients client boundary. */ +export type PatientsDirectoryRow = Readonly<{ + planId: PlanId; + patientId: PatientId; + referralId: ReferralId; + state: PlanState; + patientName: string | null; + dischargeDay: string; + scheduledContactCount: number; + absorbedContactCount: number; + otherSuppressedContactCount: number; +}>; diff --git a/src/components/caring-contacts/workspace/patients-directory.tsx b/src/components/caring-contacts/workspace/patients-directory.tsx new file mode 100644 index 0000000000..0af886771b --- /dev/null +++ b/src/components/caring-contacts/workspace/patients-directory.tsx @@ -0,0 +1,54 @@ +import { awstCalendarDay } from "@/lib/caring-contacts/clock"; +import type { PatientsDirectoryFilter } from "@/lib/caring-contacts/patients-directory-filter"; +import type { PatientNameProjection, PlanRecord } from "@/lib/caring-contacts/repository"; + +import { PatientsDirectoryClient } from "./patients-directory-client"; +import type { PatientsDirectoryRow } from "./patients-directory-row"; + +export type PatientsDirectoryProps = { + /** Every plan the server-side read released, before the non-identifying state filter is applied. */ + records: readonly PlanRecord[]; + /** The separate, names-only projection released by its own permission-checked read. */ + patientNames: readonly PatientNameProjection[]; + filter: PatientsDirectoryFilter; + /** False when the acting role does not include viewing plans. */ + mayViewPlans: boolean; +}; + +/** + * Server-owned data-minimisation boundary for the Patients directory. + * + * The interactive search needs a small client island so patient names never enter a GET URL. Raw + * `PlanRecord` objects must not cross with it: they also hold team, pathway, outcome, version and + * every planned contact. This wrapper derives only the values the directory renders or searches and + * passes that explicit DTO to the client component. + */ +export function PatientsDirectory({ records, patientNames, filter, mayViewPlans }: PatientsDirectoryProps) { + const nameByPlan = new Map( + patientNames.filter((entry) => entry.patientName !== "").map((entry) => [entry.planId, entry.patientName]), + ); + const filteredRecords = + filter.state === "all" ? records : records.filter((record) => record.plan.state === filter.state); + const rows: PatientsDirectoryRow[] = filteredRecords.map((record) => { + const suppressedContactCount = record.contacts.filter((stored) => stored.contact.state === "suppressed").length; + const absorbedContactCount = record.contacts.filter( + (stored) => stored.planned.suppressed?.reason === "absorbedByFirstContact", + ).length; + + return { + planId: record.plan.id, + patientId: record.patientId, + referralId: record.referralId, + state: record.plan.state, + patientName: nameByPlan.get(record.plan.id) ?? null, + dischargeDay: awstCalendarDay(record.dischargeAt), + scheduledContactCount: record.contacts.length - suppressedContactCount, + absorbedContactCount, + otherSuppressedContactCount: suppressedContactCount - absorbedContactCount, + }; + }); + + return ( + + ); +} diff --git a/src/components/caring-contacts/workspace/shell.tsx b/src/components/caring-contacts/workspace/shell.tsx index e48a598efd..ec869663ca 100644 --- a/src/components/caring-contacts/workspace/shell.tsx +++ b/src/components/caring-contacts/workspace/shell.tsx @@ -33,10 +33,16 @@ type WorkspaceDestination = { /** * The four primary destinations, frozen by the approved route identities. * - * Only `Today` has a page in Phase 2A, so only `Today` carries an `href`. The - * other three render as unavailable controls that state their reason rather - * than as links into a not-found page (Ruling 52). Plan 2B gives them pages, - * and adding an `href` here is the whole of that change. + * A destination carries an `href` once — and only once — it has a page. The + * rest render as unavailable controls that state their reason rather than as + * links into a not-found page (Ruling 52). Plan 2B gives them pages, and adding + * an `href` here is the whole of that change; `Today` (Phase 2A) and `Patients` + * (Phase 2B Task 5) have theirs. + * + * Ruling 89: the link and the real screen land together. A navigation entry lit + * up ahead of its screen would point at a page that says "No patients yet" + * whether or not patients exist, which is a false statement on a clinical + * caseload screen — so the `href` is added in the same change as the page. */ const PRIMARY_DESTINATIONS: readonly WorkspaceDestination[] = [ { @@ -50,6 +56,7 @@ const PRIMARY_DESTINATIONS: readonly WorkspaceDestination[] = [ id: "patients", label: "Patients", icon: Users, + href: CARING_CONTACTS_ROUTES.patients, reason: "Every patient with a caring-contact plan, and where each plan has got to.", }, { id: "schedule", label: "Schedule", icon: CalendarDays, reason: "Contacts due, day by day." }, diff --git a/src/components/caring-contacts/workspace/unavailable-destination.tsx b/src/components/caring-contacts/workspace/unavailable-destination.tsx index a5570a182e..ea2bc9d054 100644 --- a/src/components/caring-contacts/workspace/unavailable-destination.tsx +++ b/src/components/caring-contacts/workspace/unavailable-destination.tsx @@ -29,8 +29,23 @@ export type UnavailableDestinationProps = { * reached by keyboard. The two attributes are never used together; * `eslint-rules/require-button-wiring.mjs` fails on the pair. * - * This is the only client component the production workspace ships, which is - * what keeps the route's client payload to a rounding error (Ruling 13). + * A client component, and one of a handful this workspace ships. + * + * Ruling 94: no count is stated here, and none should be added. This comment + * once said "the only client component the production workspace ships", which + * was true when written, stopped being true as the overlay half landed, and was + * copied verbatim into three later files before anyone recounted. Its + * replacement — "five" — was wrong within the same round, because it silently + * scoped itself to this directory and omitted the route's own `error.tsx` and + * the module that one pulls in. A tally in prose is a claim that decays every + * time someone adds a file. + * + * What actually holds Ruling 13 is the MODULE BOUNDARY, which does not decay: + * nothing outside the `/caring-contacts` route segment imports this workspace — + * the tools catalogue names it by href, never by import — so the Clinical KB + * dashboard references no chunk exclusive to it, whatever this directory grows + * to. A screen that adds no client component of its own therefore adds no client + * payload, and that is the property worth checking, not the number of files. */ export function UnavailableDestination({ id, label, reason, className, children }: UnavailableDestinationProps) { const noteId = `caring-contacts-unavailable-${id}`; diff --git a/src/lib/caring-contacts/db/postgres-repository.ts b/src/lib/caring-contacts/db/postgres-repository.ts index 53c577f0d5..cc2edfbb81 100644 --- a/src/lib/caring-contacts/db/postgres-repository.ts +++ b/src/lib/caring-contacts/db/postgres-repository.ts @@ -94,6 +94,7 @@ import { emptyTrainingRecord, recordCompetency, type TrainingCompetency, type Tr import { CLEARED_PATIENT_DETAIL, PATHWAY_VERSION_READ_ACTIONS, + PATIENT_NAME_READ_ACTIONS, READ_ACTIONS, REPOSITORY_REFUSALS, SERVICE_STATE_UNSET_TEAM, @@ -455,6 +456,11 @@ export function createPostgresRepository( return mayRead(context.actor, action, context.actor.teamId); } + /** True only if the actor holds EVERY one of the given read capabilities for their own team. */ + function mayReadAllOwnTeam(context: ReadContext, actions: readonly CaringContactAction[]): boolean { + return actions.every((action) => mayReadOwnTeam(context, action)); + } + /** True if the actor holds ANY of the given read capabilities for their own team. */ function mayReadAnyOwnTeam(context: ReadContext, actions: readonly CaringContactAction[]): boolean { return actions.some((action) => mayReadOwnTeam(context, action)); @@ -2121,6 +2127,29 @@ export function createPostgresRepository( }); }, + /** + * The names projection (Ruling 91). + * + * `runRead` is what makes this team-scoped AT ALL: it opens the transaction that emits + * `set_config('caring_contacts.team_id', ...)` and `set local role caring_contacts_app`, so + * row-level security applies and the migration role's policy bypass does not. A query issued + * outside it would read EVERY team's names and fail no test that is not looking for it. + * + * The select names two columns rather than reusing `PLAN_COLUMNS`, so the mobile number and the + * identifier list are never fetched into this process in the first place -- the narrowing is in + * the query, not only in the mapping afterwards. + */ + async listPatientNames(context: ReadContext) { + if (!mayReadAllOwnTeam(context, PATIENT_NAME_READ_ACTIONS)) return []; + return runRead(context, async (connection) => { + const result = await connection.query("select id, patient_name from caring_contacts.plans order by id"); + return result.rows.map((row) => ({ + planId: textOf(row.id) as PlanId, + patientName: textOf(row.patient_name), + })); + }); + }, + async listContacts(planId: PlanId, context: ReadContext) { if (!mayReadOwnTeam(context, READ_ACTIONS.contacts)) return []; return runRead(context, async (connection) => { diff --git a/src/lib/caring-contacts/in-memory-repository.ts b/src/lib/caring-contacts/in-memory-repository.ts index 18a67529d4..6a8a5bf70f 100644 --- a/src/lib/caring-contacts/in-memory-repository.ts +++ b/src/lib/caring-contacts/in-memory-repository.ts @@ -52,6 +52,7 @@ import { emptyTrainingRecord, recordCompetency, type TrainingCompetency, type Tr import { CLEARED_PATIENT_DETAIL, PATHWAY_VERSION_READ_ACTIONS, + PATIENT_NAME_READ_ACTIONS, READ_ACTIONS, REPOSITORY_REFUSALS, SERVICE_STATE_UNSET_TEAM, @@ -264,6 +265,15 @@ export function createInMemoryRepository(clock: Clock, options: RepositoryOption return canPerformCaringContactAction(actor, action, { teamId: resourceTeamId }).allowed; } + /** True only if the actor holds EVERY one of the given read capabilities for the resource's team. */ + function mayReadAll( + actor: CaringContactActor, + actions: readonly CaringContactAction[], + resourceTeamId: TeamId, + ): boolean { + return actions.every((action) => mayRead(actor, action, resourceTeamId)); + } + /** True if the actor holds ANY of the given read capabilities for the resource's team. */ function mayReadAny( actor: CaringContactActor, @@ -1286,6 +1296,20 @@ export function createInMemoryRepository(clock: Clock, options: RepositoryOption .map(toPlanRecord); }, + /** + * The names projection (Ruling 91). Filtered by exactly the predicate `listPlans` uses, and + * then by `READ_ACTIONS.patientName` on top -- so this list is always a subset of the plans the + * same actor can already see, and holds a name for every one of them. + * + * The returned objects are built here rather than derived from the stored plan, so no widening + * can ride along by accident: there is no spread of `patientDetail` to forget to narrow. + */ + async listPatientNames(context: ReadContext) { + return [...plans.values()] + .filter((stored) => mayReadAll(context.actor, PATIENT_NAME_READ_ACTIONS, stored.plan.teamId)) + .map((stored) => ({ planId: stored.plan.id, patientName: stored.patientDetail.patientName })); + }, + async listContacts(planId: PlanId, context: ReadContext) { const stored = visiblePlan(planId, context, READ_ACTIONS.contacts); return stored ? stored.contacts.map(cloneStoredContact) : []; diff --git a/src/lib/caring-contacts/patients-directory-filter.ts b/src/lib/caring-contacts/patients-directory-filter.ts new file mode 100644 index 0000000000..6c4a4eeea6 --- /dev/null +++ b/src/lib/caring-contacts/patients-directory-filter.ts @@ -0,0 +1,31 @@ +import type { PlanState } from "./model"; + +/** Every plan state, in lifecycle order, as the directory filter offers them. */ +export const PATIENTS_DIRECTORY_STATE_ORDER: readonly PlanState[] = Object.freeze([ + "draft", + "active", + "paused", + "completed", + "withdrawn", + "cancelled", +]); + +export type PatientsDirectoryStateFilter = PlanState | "all"; + +export type PatientsDirectoryFilter = { + /** The plan state the URL asks for, already validated; "all" when absent or unrecognised. */ + state: PatientsDirectoryStateFilter; +}; + +/** Parse only non-identifying state from the URL; patient-name search stays in browser memory. */ +export function parsePatientsDirectoryFilter( + searchParams: Readonly>, +): PatientsDirectoryFilter { + const rawState = searchParams.state; + const state: PatientsDirectoryStateFilter = + typeof rawState === "string" && (PATIENTS_DIRECTORY_STATE_ORDER as readonly string[]).includes(rawState) + ? (rawState as PlanState) + : "all"; + + return { state }; +} diff --git a/src/lib/caring-contacts/repository.ts b/src/lib/caring-contacts/repository.ts index e5dea40777..82292fb746 100644 --- a/src/lib/caring-contacts/repository.ts +++ b/src/lib/caring-contacts/repository.ts @@ -22,7 +22,9 @@ // bounded by the return types declared below, and those already exclude patient-identifying detail: // every write returns a `PlanRecord`, `StoredContact`, `Referral`, `PathwayVersion`, // `PlanAssignment`, `DispatchRecord`, `ServiceState`, `NotificationPreferences`, `TrainingRecord` -// or `void`, and `StoredPlan`'s `patientDetail` is released through `getEpisode` alone. A new write +// or `void`, and no write returns any of `StoredPlan`'s `patientDetail`. That detail is released by +// READS alone, and only by two of them: `getEpisode`, which releases all four fields together, and +// `listPatientNames`, which releases the name and structurally cannot release the rest. A new write // that would put anything else identifying in a result needs a NARROWER RETURN TYPE, not a filter on // the way into storage: the stored result is the answer a replay must return, so filtering it makes // a genuine retry receive less than the first call did. @@ -179,6 +181,13 @@ export const READ_ACTIONS = Object.freeze({ auditTrail: "viewAccessTrail", episode: "generateClinicalRecordSummary", referral: "viewReferral", + /** + * Ruling 95: the names-only projection is checked in its own right, against the EXISTING + * `viewPatientRecord`. No new capability is minted -- a see-names-but-not-records tier would have + * to be decided for every role to satisfy `permissions.ts`'s exhaustiveness guard, and no role + * wants one. + */ + patientName: "viewPatientRecord", } as const satisfies Record); /** Either governance action reading a pathway version's content is granted by. Same rule, same reason. */ @@ -187,6 +196,26 @@ export const PATHWAY_VERSION_READ_ACTIONS: readonly CaringContactAction[] = Obje "approvePathwayVersion", ]); +/** + * EVERY action `listPatientNames` requires -- an ALL-of list, unlike `PATHWAY_VERSION_READ_ACTIONS` + * above, which is an any-of. + * + * `READ_ACTIONS.patientName` is the capability Ruling 95 names, and it is not the whole rule. The + * projection ENUMERATES the team's plans, so it must release a name only for a plan the actor could + * have listed for themselves -- the same "scoped through the plan" rule `listContacts` follows. + * Without `READ_ACTIONS.plan` alongside it the read would be a WIDENING rather than a narrowing: + * the auditor role holds `viewPatientRecord` but not `viewReferral`, so it answers `[]` to + * `listPlans` and `null` to `getEpisode` and can obtain no patient's name by any route today. A + * names read gated on `viewPatientRecord` alone would hand that role every name the team holds. + * + * This decides the SCOPE of the read, not its capability, so it re-opens nothing: no action is + * minted, and the name still travels on `viewPatientRecord` exactly as ruled. + */ +export const PATIENT_NAME_READ_ACTIONS: readonly CaringContactAction[] = Object.freeze([ + READ_ACTIONS.plan, + READ_ACTIONS.patientName, +]); + /** * Whether a plan has ended, and what it ended as. * @@ -241,9 +270,47 @@ export type PlanRecord = { contacts: readonly StoredContact[]; }; -/** What the datastore holds. The patient detail is released only through `getEpisode`. */ +/** What the datastore holds. Full detail is released only by `getEpisode`; `listPatientNames` releases the name alone. */ export type StoredPlan = PlanRecord & { patientDetail: EpisodePatientDetail }; +/** + * One plan's patient NAME, and the plan it belongs to. The whole of what `listPatientNames` + * releases (Ruling 91). + * + * WHY THIS SHAPE, AND NOT A NARROWED RECORD. It is declared here as its own two-field type rather + * than as a `Pick` of `Episode` or a `PlanRecord` with the other fields blanked, because a shape + * that COULD hold a mobile number, an identifier list or a cultural identity is one edit away from + * doing so -- and it would still typecheck, still pass every existing test, and still be described + * by its own name as a names projection. Two fields is the guarantee; empty fields are a promise. + * The assertion below turns that from a convention into a compile error. + * + * WHY KEYED BY PLAN, NOT BY PATIENT. The patient detail is held PER PLAN, and + * `markRetentionCleared` clears it per plan -- so one patient's two episodes can honestly differ, + * one cleared and one not. A patient-keyed map would have to invent a rule for which of those wins. + * Plan-keyed joins 1:1 onto `listPlans`, which is what a caseload renders, and needs no such rule. + * + * WHY A LIST, NOT A LOOKUP PER PLAN. A caseload costs ONE round trip rather than one per row, and + * a list cannot be used as an existence oracle: a per-plan lookup would have to answer for a plan + * id the caller supplied, and `getPlan` deliberately gives the same answer for "no such plan" and + * "another team's plan" so a cross-team actor cannot tell them apart. This read never takes a plan + * id at all, so there is nothing for it to be asked about. + * + * `patientName` is `""` for a plan whose detail a retention clearance has already removed -- + * `CLEARED_PATIENT_DETAIL` above is what the stores write, and an emptied field IS the cleared + * value. A caller must therefore treat blank as "no name held", never as a name. + */ +export type PatientNameProjection = { planId: PlanId; patientName: string }; + +/** + * Pins the projection's fields to exactly those two. Adding `patientMobileNumber`, + * `patientIdentifiers` or `culturalIdentity` -- or anything else -- stops this line compiling, so + * the read cannot be widened quietly by someone who has not read the paragraph above. + */ +export const PATIENT_NAME_PROJECTION_RELEASES_ONLY_THE_NAME: SameUnion< + keyof PatientNameProjection, + "planId" | "patientName" +> = true; + export type CreatePlanInput = { planId: PlanId; referralId: ReferralId; @@ -446,6 +513,18 @@ export interface CaringContactRepository { /** Null for a plan that does not exist AND for one belonging to another team. */ getPlan(planId: PlanId, context: ReadContext): Promise; listPlans(context: ReadContext): Promise; + /** + * The patient NAME for each plan this actor could list, and nothing else about them (Ruling 91). + * + * Its own method with its own capability check, so a caseload can name the people on it without + * `getEpisode` -- the only other read that releases a name, and one that releases the mobile + * number, the identifiers and the cultural identity alongside it. + * + * Empty, never a refusal, for an actor whose role does not cover it and for one outside the team + * -- the same answer `listPlans` gives, for the same reason: a refusal would confirm that records + * exist. Team scoping is `listPlans`'s exactly, so a plan invisible there has no name here. + */ + listPatientNames(context: ReadContext): Promise; listContacts(planId: PlanId, context: ReadContext): Promise; /** The contacts that may actually go out. Keyed off contact state, never off `sendAt`. */ listSendableContacts(planId: PlanId, context: ReadContext): Promise; diff --git a/tests/caring-contacts-api-handler.test.ts b/tests/caring-contacts-api-handler.test.ts index 90f6ec7f1c..5ef5847a44 100644 --- a/tests/caring-contacts-api-handler.test.ts +++ b/tests/caring-contacts-api-handler.test.ts @@ -215,6 +215,32 @@ describe("caring-contacts API boundary", () => { expect(recorded()).toContainEqual(expect.objectContaining({ outcome: "denied" })); }); + // Ruling 84's HTTP half. `auditedRead` maps a null/undefined release to "denied", and this + // handler turns "denied" into 404 -- see the denied-read test above. AN EMPTY ARRAY IS NEITHER, + // and a list read that comes back empty must reach the caller as a 200 carrying `[]`. Nothing + // pinned it: the render half is pinned in tests/caring-contacts-patients-page.dom.test.tsx, and + // an edit that "simplified" the outcome check to a falsy test would have turned every empty + // caseload in the workspace into a missing resource with no test to notice. + it("releases an EMPTY LIST as a 200, never as the 404 a denied read produces", async () => { + // The auditor holds no `viewReferral`, so `listPlans` filters the seeded plan out and returns + // `[]` -- the exact case this module's own note calls out as the one the trail cannot tell + // apart from "there are none". Both must leave here as 200, because both released a list. + const { recorded } = await inMemoryStoreWithSpy({ actorRole: "auditor" }); + const handler = readHandler({ + access: { kind: "search", objectType: "plan", objectId: () => "all" }, + read: async (repository, actor) => repository.listPlans({ actor }), + }); + + const response = await handler(get("/api/caring-contacts/plans")); + + expect(response.status).toBe(200); + await expect(response.json()).resolves.toEqual([]); + // And it is recorded as ALLOWED: an empty list IS what was released. + expect(recorded()).toContainEqual( + expect.objectContaining({ kind: "search", objectType: "plan", objectId: "all", outcome: "allowed" }), + ); + }); + it("names the acting actor and team on the access event, so the trail can be read back", async () => { const { recorded } = await inMemoryStoreWithSpy({ actorRole: "teamLead" }); const handler = readHandler({ diff --git a/tests/caring-contacts-empty-state.dom.test.tsx b/tests/caring-contacts-empty-state.dom.test.tsx index 27e184459a..cf98da4f03 100644 --- a/tests/caring-contacts-empty-state.dom.test.tsx +++ b/tests/caring-contacts-empty-state.dom.test.tsx @@ -163,3 +163,73 @@ describe("ListEmptyState — rendering", () => { } }); }); + +describe("ListEmptyState — not-permitted (Ruling 92)", () => { + it("states a reason and a remedy, the same shape as filtered", () => { + const { container } = render( + , + ); + expect(screen.getByRole("group", { name: "Plans are not visible in this role" })).toBeInTheDocument(); + expect(container.textContent ?? "").toContain("Why:"); + expect(container.textContent ?? "").toContain("What changes it:"); + }); + + it("never claims records exist, and never claims none do", () => { + // The whole reason this kind exists: an actor without the capability and a team holding + // nothing are answered identically by the stores, so this state may assert neither. + const { container } = render( + , + ); + // `"no-data"`'s wording shape must not leak in — it would state a caseload of zero. + expect(container.textContent ?? "").not.toMatch(/\byet\b/); + }); + + it("does NOT reuse the search icon, which would report a search nobody ran", () => { + // The icon is part of what states the difference wordlessly (this component's own rule), so + // an icon shared with `"filtered"` is a false statement made without words. + const filtered = render( + , + ); + const filteredIcon = filtered.container.querySelector("svg")?.getAttribute("class") ?? ""; + filtered.unmount(); + + const restricted = render( + , + ); + const restrictedIcon = restricted.container.querySelector("svg")?.getAttribute("class") ?? ""; + + expect(filteredIcon).not.toBe(""); + expect(restrictedIcon).not.toBe(""); + expect(restrictedIcon, "not-permitted reuses filtered's icon").not.toBe(filteredIcon); + }); + + it("keeps the icon decorative and the group named, like every other kind", () => { + const { container } = render( + , + ); + const icons = container.querySelectorAll("svg"); + expect(icons).toHaveLength(1); + expect(icons[0].getAttribute("aria-hidden")).toBe("true"); + expect(container.firstElementChild!.className).toContain("forced-colors:border-[CanvasText]"); + }); +}); diff --git a/tests/caring-contacts-explained-automation.dom.test.tsx b/tests/caring-contacts-explained-automation.dom.test.tsx index 6827a5c068..bf218ca2c0 100644 --- a/tests/caring-contacts-explained-automation.dom.test.tsx +++ b/tests/caring-contacts-explained-automation.dom.test.tsx @@ -2,6 +2,7 @@ import { existsSync, readdirSync, readFileSync, statSync } from "node:fs"; import path from "node:path"; import { render, screen } from "@testing-library/react"; +import ts from "typescript"; import { describe, expect, it } from "vitest"; import { AutomatedState } from "@/components/caring-contacts/workspace/automated-state"; @@ -265,6 +266,12 @@ const ALLOWED_CLIENT_COMPONENTS = [ // that module or type; and it toggles one attribute on an element the server rendered from // the note-free facts type, so the bar's wording never enters the client module graph. "service-stop-scroll-watcher.tsx", + // Patient-name filtering must remain out of GET URLs and request logs, so this directory island + // owns only the local search input. Its server wrapper converts plans and names into an explicit + // scalar DTO first; the exact-key regression test proves no raw plan, contact schedule or service + // state crosses the boundary. The companion graph check below proves this module and everything + // it imports never name the service-state module or type. + "patients-directory-client.tsx", ]; /** @@ -396,7 +403,54 @@ function guardedModuleGraph(entry: string): string[] { return [...seen]; } +function serviceStateReferences(source: string, fileName: string) { + const sourceFile = ts.createSourceFile( + fileName, + source, + ts.ScriptTarget.Latest, + /* setParentNodes */ false, + fileName.endsWith(".tsx") ? ts.ScriptKind.TSX : ts.ScriptKind.TS, + ); + let moduleReference = false; + let identifier = false; + + const visit = (node: ts.Node): void => { + if (ts.isIdentifier(node) && node.text === "ServiceState") identifier = true; + if ( + (ts.isImportDeclaration(node) || ts.isExportDeclaration(node)) && + node.moduleSpecifier && + ts.isStringLiteral(node.moduleSpecifier) && + node.moduleSpecifier.text.includes("service-state") + ) { + moduleReference = true; + } + if ( + ts.isCallExpression(node) && + node.expression.kind === ts.SyntaxKind.ImportKeyword && + node.arguments.some((argument) => ts.isStringLiteral(argument) && argument.text.includes("service-state")) + ) { + moduleReference = true; + } + ts.forEachChild(node, visit); + }; + visit(sourceFile); + return { module: moduleReference, identifier }; +} + describe("the service-state path stays on the server", () => { + it("distinguishes architecture prose from executable service-state references", () => { + expect(serviceStateReferences("// ServiceState from the service-state module", "comment-only.ts")).toEqual({ + module: false, + identifier: false, + }); + expect( + serviceStateReferences( + 'import type { ServiceState } from "@/lib/caring-contacts/service-state";', + "real-reference.ts", + ), + ).toEqual({ module: true, identifier: true }); + }); + it("keeps every workspace component but the allowlisted client controls a Server Component", () => { const clientComponents = workspaceSourceFiles() .filter(({ source }) => USE_CLIENT_DIRECTIVE.test(source)) @@ -438,10 +492,11 @@ describe("the service-state path stays on the server", () => { for (const file of guardedModuleGraph(entry)) { const label = path.relative(process.cwd(), file).split(path.sep).join("/"); const moduleSource = readFileSync(file, "utf8"); - expect(moduleSource, `${label} (reached from ${name}) references the service-state module`).not.toMatch( - /service-state/, - ); - expect(moduleSource, `${label} (reached from ${name}) names ServiceState`).not.toMatch(/ServiceState/); + // Parse executable syntax so architecture comments may describe the boundary without + // masquerading as an import or type reference inside the client graph. + const references = serviceStateReferences(moduleSource, file); + expect(references.module, `${label} (reached from ${name}) references the service-state module`).toBe(false); + expect(references.identifier, `${label} (reached from ${name}) names ServiceState`).toBe(false); } } }); diff --git a/tests/caring-contacts-patients-directory.dom.test.tsx b/tests/caring-contacts-patients-directory.dom.test.tsx new file mode 100644 index 0000000000..33da5baee3 --- /dev/null +++ b/tests/caring-contacts-patients-directory.dom.test.tsx @@ -0,0 +1,559 @@ +// tests/caring-contacts-patients-directory.dom.test.tsx +// +// The Patients directory body (Task 5), handed the narrowed records the page already read and +// deciding only what to show. Its client boundary keeps patient-name search out of URLs. +// +// The assertions that matter are about honesty rather than layout: +// * an empty caseload and a caseload hidden by a filter are DIFFERENT facts, and the two +// `ListEmptyState` kinds are not interchangeable (Task 1's whole reason for existing); +// * a role that may not view plans at all must not be told the team has no patients; +// * the directory receives the separate names-only projection and never the wider `Episode`; +// * a row's detail control is an UNAVAILABLE control with a stated reason, not a link into a +// route that does not exist yet (Ruling 52). +import { fireEvent, render, screen, within } from "@testing-library/react"; +import type { ReactElement } from "react"; +import { describe, expect, it } from "vitest"; + +import { patientsDirectoryHref } from "@/components/caring-contacts/workspace/patients-directory-client"; +import type { PatientsDirectoryRow } from "@/components/caring-contacts/workspace/patients-directory-row"; +import { PatientsDirectory } from "@/components/caring-contacts/workspace/patients-directory"; +import { CARING_CONTACTS_ROUTES } from "@/lib/caring-contacts-routes"; +import { contactId, pathwayVersionId, patientId, planId, referralId, teamId } from "@/lib/caring-contacts/ids"; +import type { PlanState } from "@/lib/caring-contacts/model"; +import { parsePatientsDirectoryFilter } from "@/lib/caring-contacts/patients-directory-filter"; +import type { PatientNameProjection, PlanRecord, StoredContact } from "@/lib/caring-contacts/repository"; + +const TEAM = teamId("demo-team"); + +function contact(sequence: number, options: { absorbed?: boolean; suppressed?: boolean } = {}): StoredContact { + return { + contact: { + id: contactId(`contact-${sequence}`), + planId: planId("plan-1"), + // Both stores write an absorbed contact straight into the terminal `suppressed` state so it + // can never be dispatched (in-memory-repository.ts, `createPlan`). The fixture matches the + // stores rather than the schedule, because the screen counts what the store holds. + state: options.absorbed || options.suppressed ? "suppressed" : "scheduled", + version: 1, + }, + planned: { + sequence, + cadenceLabel: sequence === 1 ? "Day 1" : `Month ${sequence}`, + calendarDay: "2026-03-02", + sendAt: new Date("2026-03-02T02:00:00.000Z"), + messageType: sequence === 1 ? "first" : "standard", + ...(options.absorbed ? { suppressed: { reason: "absorbedByFirstContact" as const } } : {}), + }, + }; +} + +function planRecord(options: { id: string; state: PlanState; contacts?: readonly StoredContact[] }): PlanRecord { + return { + plan: { id: planId(options.id), teamId: TEAM, state: options.state, version: 1 }, + patientId: patientId(`patient-${options.id}`), + referralId: referralId(`referral-${options.id}`), + pathwayVersionId: pathwayVersionId("pathway-1"), + dischargeAt: new Date("2026-03-01T02:00:00.000Z"), + completedAt: null, + outcome: "inProgress", + contacts: options.contacts ?? [contact(1), contact(2)], + }; +} + +const ALL = parsePatientsDirectoryFilter({}); + +/** + * What `listPatientNames` releases when it releases nothing for these plans -- a de-identified + * episode, or a role that may list plans without holding `viewPatientRecord`. Most tests below use + * it because they are about something other than the name, and it keeps the row on its fallback: + * headed by the synthetic identifier, exactly as Task 5 shipped it. + */ +const NO_NAMES: readonly PatientNameProjection[] = []; + +/** The names read's answer for one plan. Two fields, which is all the type has. */ +function name(planIdText: string, patientName: string): PatientNameProjection { + return { planId: planId(planIdText), patientName }; +} + +describe("Patients directory - the two empty states are not interchangeable", () => { + it("an empty caseload renders the no-data kind, which states how a first patient arrives", () => { + const { container } = render(); + + const empty = screen.getByRole("group", { name: /no patients yet/i }); + expect(empty).toBeInTheDocument(); + // The "filtered" wording shape must never appear on a genuinely empty caseload: it would + // tell a clinician that patients exist and something is hiding them. + expect(container.textContent ?? "").not.toContain("What changes it:"); + expect(empty.textContent ?? "").toMatch(/referral/i); + }); + + it("a caseload hidden by the state filter renders the filtered kind, naming the filter and the remedy", () => { + const records = [planRecord({ id: "plan-1", state: "active" }), planRecord({ id: "plan-2", state: "active" })]; + const filter = parsePatientsDirectoryFilter({ state: "paused" }); + + const { container } = render( + , + ); + + const empty = screen.getByRole("group", { name: /no patients match/i }); + expect(empty).toBeInTheDocument(); + expect(within(empty).getByText(/Why:/)).toBeInTheDocument(); + expect(within(empty).getByText(/What changes it:/)).toBeInTheDocument(); + // The reason must name the filter that is actually set, and the count it is hiding. + expect(empty.textContent ?? "").toContain("Paused"); + expect(empty.textContent ?? "").toContain("2"); + // Never the no-data claim. + expect(container.textContent ?? "").not.toContain("No patients yet"); + // The remedy is reachable, not merely described... + const remedy = screen.getByRole("link", { name: /show every plan/i }); + expect(remedy).toHaveAttribute("href", CARING_CONTACTS_ROUTES.patients); + // ...and reaches it as a ``, not a raw anchor. The row test below makes this assertion + // for the filter chips; without it here, the empty state's own action was the one link on this + // screen that nothing checked -- measured, not assumed: removing the attribute from this link + // alone left the whole file green. + expect(remedy).toHaveAttribute("data-internal-link", "true"); + }); + + it("a caseload hidden by the identifier search names the search text as the reason", () => { + const records = [planRecord({ id: "plan-1", state: "active" })]; + + render(); + fireEvent.change(screen.getByRole("searchbox"), { target: { value: "nothing-matches-this" } }); + + const empty = screen.getByRole("group", { name: /no patients match/i }); + expect(empty.textContent ?? "").toContain("nothing-matches-this"); + }); + + it("a role that may not view plans is never told the team has no patients", () => { + const { container } = render( + , + ); + + expect(container.textContent ?? "").not.toContain("No patients yet"); + const empty = screen.getByRole("group", { name: /not visible in this role/i }); + expect(within(empty).getByText(/Why:/)).toBeInTheDocument(); + expect(within(empty).getByText(/What changes it:/)).toBeInTheDocument(); + // The reason must not claim anything about how many plans exist: a read this role may not make + // and a team holding none are indistinguishable by design, and saying otherwise would leak the + // very thing the store withholds. + expect(empty.textContent ?? "").toMatch(/says nothing about how many/i); + }); + + // Ruling 93. Asserting that "What changes it:" is PRESENT is a shape check, and a shape check + // certifies a well-formed lie: the first version of this screen said "The role switcher changes + // which role you are acting in", and no role switcher exists anywhere in this workspace's + // interface. Spec 4.4 requires a remedy that can be REACHED, so the content is pinned here. + it("states a remedy that exists, and never names a control the workspace does not have", () => { + render(); + const empty = screen.getByRole("group", { name: /not visible in this role/i }); + const text = empty.textContent ?? ""; + + expect(text).toMatch(/nothing on this screen changes it/i); + expect(text).toMatch(/no control for it anywhere in this workspace/i); + // The exact false claim this test was written to keep out. + expect(text).not.toMatch(/role switcher/i); + }); + + it("uses the not-permitted kind, so the icon does not report a search nobody ran", () => { + // Ruling 92: the words were honest under `"filtered"`, the TYPE and the ICON were not. + // `"filtered"` selects `SearchX`; this case must not. + const restricted = render( + , + ); + const restrictedIcon = restricted.container.querySelector("[role='group'] svg")?.getAttribute("class") ?? ""; + restricted.unmount(); + + const filtered = render( + , + ); + const filteredIcon = filtered.container.querySelector("[role='group'] svg")?.getAttribute("class") ?? ""; + + expect(restrictedIcon).not.toBe(""); + expect(filteredIcon).not.toBe(""); + expect(restrictedIcon, "the role-restricted empty state reuses the search icon").not.toBe(filteredIcon); + }); + + it("hides the filter controls entirely for a role that may not view plans", () => { + render(); + expect(screen.queryByRole("link", { name: "Active" })).toBeNull(); + expect(screen.queryByRole("searchbox")).toBeNull(); + }); +}); + +describe("Patients directory - rows", () => { + it("lists one row per plan the team may see, keyed by the synthetic patient identifier", () => { + const records = [ + planRecord({ id: "plan-1", state: "active" }), + planRecord({ id: "plan-2", state: "paused" }), + planRecord({ id: "plan-3", state: "completed" }), + ]; + + render(); + + expect(screen.getAllByRole("listitem")).toHaveLength(3); + expect(screen.getByRole("heading", { name: "patient-plan-1" })).toBeInTheDocument(); + expect(screen.getByRole("heading", { name: "patient-plan-2" })).toBeInTheDocument(); + }); + + it("states each plan's state in words, not by colour alone", () => { + render( + , + ); + const row = screen.getAllByRole("listitem")[0]; + expect(row.textContent ?? "").toContain("Paused"); + }); + + it("explains a contact the SYSTEM suppressed, in place, with why and what would change it", () => { + const withAbsorbed = planRecord({ + id: "plan-1", + state: "active", + contacts: [contact(1), contact(2, { absorbed: true })], + }); + + render(); + + const automated = screen.getByRole("group", { name: "Suppressed" }); + expect(within(automated).getByText(/Why:/)).toBeInTheDocument(); + expect(within(automated).getByText(/What changes it:/)).toBeInTheDocument(); + // Reachable as text, never only as a hover title. + expect(automated.querySelector("[title]")).toBeNull(); + }); + + it("explains a contact suppressed by the transition, which carries no schedule marker", () => { + // M-8: `planned.suppressed` marks only the schedule's own absorption. A contact moved to + // `suppressed` by `applyContactTransition` has no such marker, and counting the plan rather + // than the outcome left it with no explanation at all. + const record = planRecord({ + id: "plan-1", + state: "active", + contacts: [contact(1), contact(2, { suppressed: true })], + }); + + render(); + + const automated = screen.getByRole("group", { name: "Suppressed" }); + expect(within(automated).getByText(/Why:/)).toBeInTheDocument(); + expect(within(automated).getByText(/What changes it:/)).toBeInTheDocument(); + // Not the Week 1 wording -- that reason is false for this contact. + expect(automated.textContent ?? "").not.toMatch(/Week 1/); + // And the remedy is honest about being final rather than inventing one. + expect(automated.textContent ?? "").toMatch(/final/i); + }); + + // N-3. `scheduled` changed definition this round -- it counts `contacts.length` minus the + // contacts whose OWN state is suppressed, where it used to subtract only the schedule's absorbed + // ones. Nothing asserted the rendered number, so the change could have been silently wrong on + // every row. This is a clinician-facing count of how many messages a patient will receive. + it("states how many messages remain in the schedule, and subtracts every suppressed one", () => { + const plain = planRecord({ id: "plan-1", state: "active", contacts: [contact(1), contact(2)] }); + const withSuppressed = planRecord({ + id: "plan-2", + state: "active", + contacts: [contact(1), contact(2, { suppressed: true }), contact(3, { absorbed: true })], + }); + + render(); + + const [first, second] = screen.getAllByRole("listitem"); + expect(first.textContent ?? "").toContain("2 messages in the schedule"); + // Three contacts, two of them suppressed by two different causes, so one message remains. + expect(second.textContent ?? "").toContain("1 message in the schedule"); + }); + + // N-2. The count subtracts EVERY suppressed contact, so the reason beside it has to account for + // every one of them. The first shape branched on whether an absorbed contact existed at all, so + // this plan showed a count short by two beside a reason explaining one. + it("accounts for both causes when a plan carries an absorbed AND a transition-suppressed contact", () => { + const record = planRecord({ + id: "plan-1", + state: "active", + contacts: [contact(1), contact(2, { absorbed: true }), contact(3, { suppressed: true })], + }); + + render(); + + const automated = screen.getByRole("group", { name: "Suppressed" }); + const text = automated.textContent ?? ""; + // The absorbed cause, with its own remedy... + expect(text).toMatch(/Week 1/); + expect(text).toMatch(/first-contact date/i); + // ...and the other one, with its own, which is a different remedy entirely. + expect(text).toMatch(/does not hold what caused that/i); + expect(text).toMatch(/final/i); + }); + + it("says nothing about suppression when the system suppressed nothing", () => { + render( + , + ); + expect(screen.queryByRole("group", { name: "Suppressed" })).toBeNull(); + }); + + it("offers the row's detail control as an unavailable control, never a link into a route with no page", () => { + render( + , + ); + + const control = screen.getByRole("button", { name: /patient-plan-1/i }); + expect(control).toHaveAttribute("aria-disabled", "true"); + expect(control).toHaveAttribute("type", "button"); + expect(control).toHaveAttribute("title", expect.stringContaining("coming soon")); + // Native `disabled` would remove the tab stop, so the stated reason could never be reached. + expect(control).not.toHaveAttribute("disabled"); + // And nothing on this screen links into the not-yet-built detail routes... + for (const link of screen.getAllByRole("link")) { + expect(link.getAttribute("href") ?? "").not.toMatch(/\/caring-contacts\/(patients\/[^?]|plans\/)/); + // ...nor reaches an internal route by a raw anchor. `data-internal-link` is the marker the + // shell test uses to tell a `` from an `    `, which render identically. + expect(link.getAttribute("data-internal-link"), `${link.getAttribute("href")} is not a `).toBe("true"); + } + }); + + it("releases the name and nothing else - a directory never calls getEpisode", () => { + const records = [planRecord({ id: "plan-1", state: "active" })]; + const { container } = render( + , + ); + + // The name is rendered WITH the names read in place, so the absences below are the screen + // holding nothing else rather than the screen holding nothing at all. + expect(screen.getByRole("heading", { name: "Jordan Nguyen" })).toBeInTheDocument(); + // `PlanRecord` carries no identifying detail by construction and `PatientNameProjection` has + // two fields, so the other three fields `getEpisode` releases have nowhere to come from. This + // pins that the screen does not invent a place to put one either. + expect(container.textContent ?? "").not.toMatch(/mobile|cultural/i); + }); +}); + +describe("Patients directory - only non-identifying state enters the URL", () => { + it("parses a known plan state and ignores an unknown one rather than failing the render", () => { + expect(parsePatientsDirectoryFilter({ state: "paused" }).state).toBe("paused"); + expect(parsePatientsDirectoryFilter({ state: "not-a-state" }).state).toBe("all"); + expect(parsePatientsDirectoryFilter({ state: ["active", "paused"] }).state).toBe("all"); + }); + + it("ignores a legacy query parameter rather than passing patient text into the directory", () => { + expect(parsePatientsDirectoryFilter({ q: "Jordan Nguyen", state: "active" })).toEqual({ state: "active" }); + }); + + it("builds every filter href from the route module with state and no patient search text", () => { + expect(patientsDirectoryHref({ state: "all" })).toBe(CARING_CONTACTS_ROUTES.patients); + expect(patientsDirectoryHref({ state: "active" })).toBe(`${CARING_CONTACTS_ROUTES.patients}?state=active`); + }); + + it("filters by plan state from the validated URL state", () => { + const records = [planRecord({ id: "plan-1", state: "active" }), planRecord({ id: "plan-2", state: "paused" })]; + + render( + , + ); + expect(screen.getAllByRole("listitem")).toHaveLength(1); + expect(screen.getByRole("heading", { name: "patient-plan-2" })).toBeInTheDocument(); + }); + + it("matches the identifier search case-insensitively against the patient and plan identifiers", () => { + // Unchanged by the names projection: an identifier search still works with no name in play. + const records = [planRecord({ id: "plan-1", state: "active" }), planRecord({ id: "plan-2", state: "active" })]; + render(); + fireEvent.change(screen.getByRole("searchbox"), { target: { value: "PLAN-2" } }); + expect(screen.getAllByRole("listitem")).toHaveLength(1); + expect(screen.getByRole("heading", { name: "patient-plan-2" })).toBeInTheDocument(); + }); + + it("never exposes the patient-name search as a GET form or URL parameter", () => { + const { container } = render( + , + ); + fireEvent.change(screen.getByRole("searchbox"), { target: { value: "Jordan Nguyen" } }); + expect(container.querySelector("form")).toBeNull(); + expect(screen.getByRole("searchbox")).not.toHaveAttribute("name"); + for (const link of screen.getAllByRole("link")) expect(link.getAttribute("href")).not.toContain("Jordan"); + }); + + it("marks the current state filter, so the screen and the URL cannot disagree", () => { + render( + , + ); + expect(screen.getByRole("link", { name: "Paused" })).toHaveAttribute("aria-current", "true"); + expect(screen.getByRole("link", { name: "Active" })).not.toHaveAttribute("aria-current"); + }); +}); + +describe("Patients directory - the names-only projection (Ruling 91)", () => { + it("filters identifying rows before the client boundary", () => { + const element = PatientsDirectory({ + patientNames: [name("plan-1", "Jordan Nguyen"), name("plan-2", "Alex Whitlock")], + records: [planRecord({ id: "plan-1", state: "active" }), planRecord({ id: "plan-2", state: "paused" })], + filter: parsePatientsDirectoryFilter({ state: "paused" }), + mayViewPlans: true, + }) as ReactElement<{ rows: readonly PatientsDirectoryRow[]; totalPlanCount: number }>; + + expect(element.props.totalPlanCount).toBe(2); + expect(element.props.rows.map((row) => row.patientName)).toEqual(["Alex Whitlock"]); + }); + + it("crosses the client boundary with only the rendered, searched or pre-derived row fields", () => { + const element = PatientsDirectory({ + patientNames: [name("plan-1", "Jordan Nguyen")], + records: [planRecord({ id: "plan-1", state: "active" })], + filter: ALL, + mayViewPlans: true, + }) as ReactElement<{ rows: readonly Record[] }>; + + expect(Object.keys(element.props.rows[0] ?? {}).sort()).toEqual( + [ + "absorbedContactCount", + "dischargeDay", + "otherSuppressedContactCount", + "patientId", + "patientName", + "planId", + "referralId", + "scheduledContactCount", + "state", + ].sort(), + ); + }); + + it("heads the row with the patient's name, and keeps the synthetic identifier beside it", () => { + const records = [planRecord({ id: "plan-1", state: "active" })]; + + render( + , + ); + + expect(screen.getByRole("heading", { name: "Jordan Nguyen" })).toBeInTheDocument(); + // Still present in the row's own body, because two patients can share a name. + // + // M7 in the mutation ledger: this assertion was first written as "the row's text contains the + // identifier", which the DETAIL CONTROL satisfies on its own -- so deleting the identifier line + // entirely left the file green. It reads the line itself now. + const row = screen.getAllByRole("listitem")[0]; + expect(within(row).getByText(/Synthetic identifier: patient-plan-1/)).toBeInTheDocument(); + // And the control is still named by the identifier, which is what distinguishes one row's + // control from the next to a screen reader. + expect(screen.getByRole("button", { name: /patient-plan-1/i })).toBeInTheDocument(); + }); + + it("falls back to the synthetic identifier, and labels it as one, when no name came back", () => { + // The case a role without `viewPatientRecord` produces: `listPatientNames` answers `[]`, exactly + // as `listPlans` answers an actor who may not read it, and the screen must not present an + // identifier as though it were a name. + render( + , + ); + + expect(screen.getByRole("heading", { name: "patient-plan-1" })).toBeInTheDocument(); + expect(screen.getByText("Synthetic patient identifier")).toBeInTheDocument(); + }); + + it("treats a de-identified plan's empty name as no name held, never as a blank heading", () => { + // `markRetentionCleared` writes the empty string for a removed name in both stores, so the + // projection carries an entry whose name is "". Rendering it verbatim would give the row an + // empty heading and no identifier at all -- a row naming nobody. + render( + , + ); + + expect(screen.getByRole("heading", { name: "patient-plan-1" })).toBeInTheDocument(); + expect(screen.getByText("Synthetic patient identifier")).toBeInTheDocument(); + }); + + it("matches the trimmed local search against the name as well as the identifiers", () => { + const records = [planRecord({ id: "plan-1", state: "active" }), planRecord({ id: "plan-2", state: "active" })]; + const names = [name("plan-1", "Jordan Nguyen"), name("plan-2", "Alex Whitlock")]; + + render(); + fireEvent.change(screen.getByRole("searchbox"), { target: { value: " nguyen " } }); + + expect(screen.getAllByRole("listitem")).toHaveLength(1); + expect(screen.getByRole("heading", { name: "Jordan Nguyen" })).toBeInTheDocument(); + }); + + it("treats whitespace-only local search as no search", () => { + render( + , + ); + fireEvent.change(screen.getByRole("searchbox"), { target: { value: " " } }); + + expect(screen.getAllByRole("listitem")).toHaveLength(2); + expect(screen.queryByRole("button", { name: /clear search/i })).toBeNull(); + }); + + it("finds no row by a name it does not hold, when the names read released nothing", () => { + // The empty haystack segment must not turn into a wildcard: a role that may not read names + // searching for one must find nothing, not everything. + render( + , + ); + fireEvent.change(screen.getByRole("searchbox"), { target: { value: "Jordan" } }); + + expect(screen.queryAllByRole("listitem")).toHaveLength(0); + expect(screen.getByRole("group", { name: /no patients match/i })).toBeInTheDocument(); + }); +}); diff --git a/tests/caring-contacts-patients-page.dom.test.tsx b/tests/caring-contacts-patients-page.dom.test.tsx new file mode 100644 index 0000000000..def4784d00 --- /dev/null +++ b/tests/caring-contacts-patients-page.dom.test.tsx @@ -0,0 +1,316 @@ +// tests/caring-contacts-patients-page.dom.test.tsx +// +// `/caring-contacts/patients` (`src/app/caring-contacts/patients/page.tsx`) -- the team's caseload. +// +// THE CONTRACT THIS FILE EXISTS FOR, and the one thing that survived the cut of Task 2: +// +// `auditedRead` maps a `null`/`undefined` release to `denied`, and `readHandler` turns `denied` +// into `not-found`. AN EMPTY ARRAY IS NEITHER. A team with no plans yet has a real, readable, +// permitted, EMPTY caseload -- and an empty caseload must never present as a missing resource. +// +// That is not obvious from reading the page: `listPlans` returning `[]` and `getPlan` returning +// `null` look alike at a glance, and `handler.ts`'s own note says the access trail cannot tell +// "you may not see these" from "there are none" for a list. So the render shape is pinned here +// deliberately rather than left to a comment: empty renders the empty STATE on the success path, +// and `notFound()` is never called. +// +// Built on the same helper shape as `caring-contacts-page-access-audit.test.ts`, which pins the +// equivalent contract for the Today page's service-state read. +import { render, screen } from "@testing-library/react"; +import type { ReactElement } from "react"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; + +const mocks = vi.hoisted(() => ({ + store: { current: null as unknown }, + notFound: vi.fn(() => { + throw new Error("NEXT_NOT_FOUND"); + }), +})); + +vi.mock("next/headers", () => ({ + cookies: vi.fn(async () => ({ get: (name: string) => mockCookies[name] })), +})); + +vi.mock("next/navigation", async (importOriginal) => ({ + ...(await importOriginal()), + notFound: mocks.notFound, +})); + +vi.mock("@/lib/caring-contacts-server/store", () => ({ + caringContactsStore: async () => mocks.store.current, +})); + +import { CARING_CONTACTS_ROLE_COOKIE, demoActorForRole } from "@/lib/caring-contacts-server/session"; +import type { AccessRecord } from "@/lib/caring-contacts/access-audit"; +import { fixedClock } from "@/lib/caring-contacts/clock"; +import { pathwayVersionId, patientId, planId, referralId } from "@/lib/caring-contacts/ids"; +import { createInMemoryRepository } from "@/lib/caring-contacts/in-memory-repository"; +import type { CaringContactRepository, PatientNameProjection, PlanRecord } from "@/lib/caring-contacts/repository"; + +let mockCookies: Record = {}; + +const NOW = "2026-03-02T03:00:00.000Z"; + +function inMemoryStoreWithSpy(role = "coordinator"): { + store: CaringContactRepository; + recorded: () => AccessRecord[]; +} { + mockCookies = { [CARING_CONTACTS_ROLE_COOKIE]: { value: role } }; + + const repository = createInMemoryRepository(fixedClock(NOW)); + const records: AccessRecord[] = []; + const store: CaringContactRepository = { + ...repository, + async recordAccess(record: AccessRecord) { + await repository.recordAccess(record); + records.push(record); + }, + }; + + mocks.store.current = store; + return { store, recorded: () => records }; +} + +/** One synthetic plan record, carrying no patient-identifying detail -- `PlanRecord`, never `Episode`. */ +function planRecord(id: string): PlanRecord { + return { + plan: { id: planId(id), teamId: demoActorForRole("coordinator").teamId, state: "active", version: 1 }, + patientId: patientId(`patient-${id}`), + referralId: referralId(`referral-${id}`), + pathwayVersionId: pathwayVersionId("pathway-1"), + dischargeAt: new Date("2026-03-01T02:00:00.000Z"), + completedAt: null, + outcome: "inProgress", + contacts: [], + }; +} + +async function renderPage(searchParams: Record = {}) { + const { default: PatientsPage } = await import("@/app/caring-contacts/patients/page"); + const element = await PatientsPage({ searchParams: Promise.resolve(searchParams) }); + // The shell is the page's root; the directory it wraps is what this file inspects. Rendering + // the shell itself would drag `next/dynamic` and the whole workspace chrome into a test about + // one screen's body. + render((element as ReactElement<{ children: ReactElement }>).props.children); + return element; +} + +beforeEach(() => { + mockCookies = {}; + mocks.store.current = null; + mocks.notFound.mockClear(); +}); + +afterEach(() => { + vi.unstubAllEnvs(); + vi.restoreAllMocks(); +}); + +describe("the /caring-contacts/patients page - an empty caseload is not a missing resource", () => { + it("renders the empty STATE on the success path when the team has no plans, and never calls notFound()", async () => { + inMemoryStoreWithSpy(); + + const element = await renderPage(); + + expect(element).toBeTruthy(); + expect(mocks.notFound).not.toHaveBeenCalled(); + expect(screen.getByRole("group", { name: /no patients yet/i })).toBeInTheDocument(); + }); + + it("records the empty list read as ALLOWED, the outcome an empty array actually earns", async () => { + const { recorded } = inMemoryStoreWithSpy(); + + await renderPage(); + + // `auditedRead` maps null/undefined to "denied"; `[]` is neither, so this must be "allowed". + // A "denied" here would be the same defect wearing a different hat. + expect(recorded()).toContainEqual( + expect.objectContaining({ + kind: "search", + objectType: "plan", + objectId: "all", + outcome: "allowed", + actorId: demoActorForRole("coordinator").id, + }), + ); + }); +}); + +describe("the /caring-contacts/patients page - reads", () => { + it("uses the same access identity the plans API route already records", async () => { + const { store, recorded } = inMemoryStoreWithSpy(); + vi.spyOn(store, "listPlans").mockResolvedValue([planRecord("plan-1")]); + + await renderPage(); + + expect(recorded()).toContainEqual( + expect.objectContaining({ kind: "search", objectType: "plan", objectId: "all", outcome: "allowed" }), + ); + // And the service state, so the safety banner on this screen is a state that was READ. + expect(recorded()).toContainEqual( + expect.objectContaining({ kind: "administrative", objectType: "serviceState", outcome: "allowed" }), + ); + }); + + it("never reads the episode - the one read that releases a name, a mobile number or an identifier", async () => { + const { store } = inMemoryStoreWithSpy(); + const getEpisode = vi.spyOn(store, "getEpisode"); + vi.spyOn(store, "listPlans").mockResolvedValue([planRecord("plan-1")]); + + await renderPage(); + + expect(getEpisode).not.toHaveBeenCalled(); + }); + + it("lists a plan the team holds, keyed by its synthetic patient identifier", async () => { + const { store } = inMemoryStoreWithSpy(); + vi.spyOn(store, "listPlans").mockResolvedValue([planRecord("plan-1"), planRecord("plan-2")]); + + await renderPage(); + + expect(screen.getAllByRole("listitem")).toHaveLength(2); + expect(screen.getByRole("heading", { name: "patient-plan-1" })).toBeInTheDocument(); + }); + + it("applies the URL state filter server-side, and says so rather than showing an empty caseload", async () => { + const { store } = inMemoryStoreWithSpy(); + vi.spyOn(store, "listPlans").mockResolvedValue([planRecord("plan-1")]); + + await renderPage({ state: "paused" }); + + expect(mocks.notFound).not.toHaveBeenCalled(); + expect(screen.getByRole("group", { name: /no patients match/i })).toBeInTheDocument(); + expect(screen.queryByText("No patients yet")).toBeNull(); + }); +}); + +describe("the /caring-contacts/patients page - fails closed", () => { + it("throws rather than rendering when the access trail cannot take the event", async () => { + const { store } = inMemoryStoreWithSpy(); + vi.spyOn(store, "recordAccess").mockRejectedValue(new Error("access trail unavailable")); + + const { default: PatientsPage } = await import("@/app/caring-contacts/patients/page"); + + await expect(PatientsPage({ searchParams: Promise.resolve({}) })).rejects.toThrow(/access trail is unavailable/i); + }); + + it("throws rather than rendering when the plans read itself fails, and still records the attempt", async () => { + const { store, recorded } = inMemoryStoreWithSpy(); + vi.spyOn(store, "listPlans").mockRejectedValue(new Error("store unreachable")); + + const { default: PatientsPage } = await import("@/app/caring-contacts/patients/page"); + + await expect(PatientsPage({ searchParams: Promise.resolve({}) })).rejects.toThrow("store unreachable"); + expect(recorded()).toContainEqual(expect.objectContaining({ objectType: "plan", outcome: "failed" })); + }); + + // I-4. `listPlans` returns an array for every actor by contract, so this branch is unreachable + // through the real stores -- which is exactly why it was wrong and why nothing caught it. The + // page originally defaulted a null release to `[]`, and the ONE path this branch exists for + // would then have rendered "No patients yet" from an answer that was never given: a false + // statement about a caseload, from a store that had just broken its own contract. A branch that + // cannot run is still read, and is still copied by the next screen. + it("throws rather than inventing an empty caseload when the store breaks its list contract", async () => { + const { store } = inMemoryStoreWithSpy(); + vi.spyOn(store, "listPlans").mockResolvedValue(null as unknown as PlanRecord[]); + + const { default: PatientsPage } = await import("@/app/caring-contacts/patients/page"); + + await expect(PatientsPage({ searchParams: Promise.resolve({}) })).rejects.toThrow(/returned no list/i); + }); + + // N-1. `auditedRead` calls a read denied on null OR undefined, but `AuditedReadResult` types + // `released` as `T | null`, so the compiler cannot see this case and the test above could not + // either -- it mocked `null` specifically. A `=== null` guard let `undefined` through to die on + // `records.length` with a TypeError: still closed, still no false caseload, but the wrong branch. + it("throws the same way when the store answers undefined rather than null", async () => { + const { store } = inMemoryStoreWithSpy(); + vi.spyOn(store, "listPlans").mockResolvedValue(undefined as unknown as PlanRecord[]); + + const { default: PatientsPage } = await import("@/app/caring-contacts/patients/page"); + + await expect(PatientsPage({ searchParams: Promise.resolve({}) })).rejects.toThrow(/returned no list/i); + }); + + it("throws rather than rendering a caseload beside a service state it could not read", async () => { + const { store } = inMemoryStoreWithSpy(); + vi.spyOn(store, "getServiceState").mockRejectedValue(new Error("service state unreachable")); + + const { default: PatientsPage } = await import("@/app/caring-contacts/patients/page"); + + await expect(PatientsPage({ searchParams: Promise.resolve({}) })).rejects.toThrow("service state unreachable"); + }); +}); + +describe("the /caring-contacts/patients page - roles", () => { + it("tells an auditor its role cannot view plans, rather than that the team has none", async () => { + inMemoryStoreWithSpy("auditor"); + + await renderPage(); + + expect(mocks.notFound).not.toHaveBeenCalled(); + expect(screen.getByRole("group", { name: /not visible in this role/i })).toBeInTheDocument(); + expect(screen.queryByText("No patients yet")).toBeNull(); + }); +}); + +describe("the /caring-contacts/patients page - the names-only projection (Ruling 91)", () => { + it("reads the names through their OWN access identity, not folded into the plans read", async () => { + const { store, recorded } = inMemoryStoreWithSpy(); + vi.spyOn(store, "listPlans").mockResolvedValue([planRecord("plan-1")]); + + await renderPage(); + + // Its own row. This is the one read on this page that releases patient identity, and a trail + // that recorded it as part of a plan search could not answer "who read patients' names". + expect(recorded()).toContainEqual( + expect.objectContaining({ + kind: "search", + objectType: "patientDirectory", + objectId: "names", + outcome: "allowed", + actorId: demoActorForRole("coordinator").id, + }), + ); + // And the plans read is still recorded separately, so the two are distinguishable. + expect(recorded()).toContainEqual( + expect.objectContaining({ kind: "search", objectType: "plan", objectId: "all", outcome: "allowed" }), + ); + }); + + it("names the row from the projection, and STILL never reads the episode", async () => { + const { store } = inMemoryStoreWithSpy(); + const getEpisode = vi.spyOn(store, "getEpisode"); + vi.spyOn(store, "listPlans").mockResolvedValue([planRecord("plan-1")]); + vi.spyOn(store, "listPatientNames").mockResolvedValue([{ planId: planId("plan-1"), patientName: "Jordan Nguyen" }]); + + await renderPage(); + + expect(screen.getByRole("heading", { name: "Jordan Nguyen" })).toBeInTheDocument(); + // The whole point of the narrow read: the name arrives without the read that would have + // released the mobile number, the identifiers and the ancestry alongside it. + expect(getEpisode).not.toHaveBeenCalled(); + }); + + it("throws rather than rendering when the names read itself fails, and still records the attempt", async () => { + const { store, recorded } = inMemoryStoreWithSpy(); + vi.spyOn(store, "listPatientNames").mockRejectedValue(new Error("names store unreachable")); + + const { default: PatientsPage } = await import("@/app/caring-contacts/patients/page"); + + await expect(PatientsPage({ searchParams: Promise.resolve({}) })).rejects.toThrow("names store unreachable"); + expect(recorded()).toContainEqual(expect.objectContaining({ objectType: "patientDirectory", outcome: "failed" })); + }); + + it("throws rather than rendering a caseload from a names answer that was never given", async () => { + // Same shape as the plans guard, and unreachable through either real store for the same reason: + // `listPatientNames` returns an array for every actor by contract. A branch that cannot run is + // still read, and is still copied by the next screen. + const { store } = inMemoryStoreWithSpy(); + vi.spyOn(store, "listPatientNames").mockResolvedValue(undefined as unknown as PatientNameProjection[]); + + const { default: PatientsPage } = await import("@/app/caring-contacts/patients/page"); + + await expect(PatientsPage({ searchParams: Promise.resolve({}) })).rejects.toThrow(/names read returned no list/i); + }); +}); diff --git a/tests/caring-contacts-workspace-shell.dom.test.tsx b/tests/caring-contacts-workspace-shell.dom.test.tsx index 4ea1287347..6fe350b6c4 100644 --- a/tests/caring-contacts-workspace-shell.dom.test.tsx +++ b/tests/caring-contacts-workspace-shell.dom.test.tsx @@ -84,11 +84,12 @@ describe("caring-contacts workspace shell", () => { ); }); - it("keeps the frozen rail destination set, in order, with only Today navigable", () => { + it("keeps the frozen rail destination set, in order, with only the built screens navigable", () => { renderShell(); expect(destinationsOf(screen.getByRole("navigation", { name: "Workspace" }))).toEqual([ { label: "Today", kind: "link" }, - { label: "Patients", kind: "unavailable" }, + // Patients became a link in Phase 2B Task 5, in the same change as its page (Ruling 89). + { label: "Patients", kind: "link" }, { label: "Schedule", kind: "unavailable" }, { label: "Templates", kind: "unavailable" }, ]); @@ -98,7 +99,7 @@ describe("caring-contacts workspace shell", () => { renderShell(); expect(destinationsOf(screen.getByRole("navigation", { name: "Phone workspace" }))).toEqual([ { label: "Today", kind: "link" }, - { label: "Patients", kind: "unavailable" }, + { label: "Patients", kind: "link" }, { label: "Schedule", kind: "unavailable" }, { label: "More", kind: "in-page" }, ]); @@ -115,9 +116,9 @@ describe("caring-contacts workspace shell", () => { const { container } = renderShell(); const internalHrefs = [...container.querySelectorAll("a[href^='/']")].map((anchor) => anchor.getAttribute("href")); expect(internalHrefs.length).toBeGreaterThan(0); - // `today` is the only Caring Contacts route with a page. Every other declared - // destination is an unavailable control until Plan 2B builds its page. - expect(new Set(internalHrefs)).toEqual(new Set([CARING_CONTACTS_ROUTES.today])); + // `today` and `patients` are the Caring Contacts routes with a page. Every other + // declared destination is an unavailable control until Plan 2B builds its page. + expect(new Set(internalHrefs)).toEqual(new Set([CARING_CONTACTS_ROUTES.today, CARING_CONTACTS_ROUTES.patients])); }); it("keeps the More panel's destination set, in order, all of them unavailable", () => { @@ -145,9 +146,9 @@ describe("caring-contacts workspace shell", () => { expect(primary!.textContent).toBe("New plan"); expect(destinationKind(primary!)).toBe("unavailable"); expectStatesItsReason(primary!); - // 3 unbuilt rail destinations + 2 on the phone bar + 10 in the More panel + this one. + // 2 unbuilt rail destinations + 1 on the phone bar + 10 in the More panel + this one. expect([...container.querySelectorAll("button")].filter((c) => destinationKind(c) === "unavailable")).toHaveLength( - 16, + 14, ); }); @@ -156,7 +157,9 @@ describe("caring-contacts workspace shell", () => { const unavailable = [...container.querySelectorAll("button")].filter( (control) => destinationKind(control) === "unavailable", ); - // Three unbuilt rail destinations, two more on the phone bar, plus the More panel. + // Two unbuilt rail destinations, one more on the phone bar, plus the More panel. + // The floor stays at 5: it was written as a floor rather than a count, the exact count is + // asserted above, and lowering a floor a change did not breach is loosening for its own sake. expect(unavailable.length).toBeGreaterThanOrEqual(5); for (const control of unavailable) expectStatesItsReason(control); }); diff --git a/tests/design-system-adoption.test.ts b/tests/design-system-adoption.test.ts index 6b27de9c78..4a649fa711 100644 --- a/tests/design-system-adoption.test.ts +++ b/tests/design-system-adoption.test.ts @@ -1412,15 +1412,16 @@ describe("design-system adoption manifest", () => { ["committed", "not-committed", "not-applicable"].includes(surface.baseline.status), ), ).toBe(true); - // 79 = 59 + 6 + 13 + 1: the 59 production pages that preceded both changes, + // 80 = 59 + 6 + 13 + 2: the 59 production pages that preceded both changes, // the six `/search` routes home consolidation split out of the bare paths, // the thirteen-route Ward Flow synthetic patient-flow prototype (mode home, // eight remaining workspace routes, ED/ward/officer role screens, the per-patient - // detail route, and the retired constellation redirect), and the Caring Contacts - // workspace. Redirect stubs keep legacy deep links resolving and still count as + // detail route, and the retired constellation redirect), and the two Caring Contacts + // workspace screens built so far (Today, and the Patients caseload added by Phase 2B + // Task 5). Redirect stubs keep legacy deep links resolving and still count as // declared routes. This is a census, so a route nobody intended to add still // fails the contract. - expect(manifest.routeCoverage.discovered).toHaveLength(79); + expect(manifest.routeCoverage.discovered).toHaveLength(80); expect(manifest.routeCoverage.declared).toEqual(manifest.routeCoverage.discovered); expect(manifest.routeCoverage.undeclared).toEqual([]); expect(manifest.routeCoverage.missing).toEqual([]); diff --git a/tests/helpers/caring-contacts-repository-contract.ts b/tests/helpers/caring-contacts-repository-contract.ts index 19527bec28..0dabcc7e01 100644 --- a/tests/helpers/caring-contacts-repository-contract.ts +++ b/tests/helpers/caring-contacts-repository-contract.ts @@ -21,8 +21,16 @@ import { teamId, } from "@/lib/caring-contacts/ids"; import type { PathwayVersion } from "@/lib/caring-contacts/pathway-versions"; -import type { Actor, CaringContactActor, CaringContactRole, SystemActor } from "@/lib/caring-contacts/permissions"; import { + canPerformCaringContactAction, + type Actor, + type CaringContactActor, + type CaringContactRole, + type SystemActor, +} from "@/lib/caring-contacts/permissions"; +import { + PATIENT_NAME_READ_ACTIONS, + READ_ACTIONS, REPOSITORY_REFUSALS, type AuditSink, type CaringContactRepository, @@ -637,6 +645,119 @@ export function describeCaringContactRepositoryContract(label: string, factory: }); }); + /** + * Ruling 91's names-only projection, held by the SHARED contract so both stores answer it the + * same way. The screen it exists for is the Patients directory, which needs a name per row and + * has no business holding a mobile number, an identifier list or a cultural identity. + */ + describe("listPatientNames — the name, and structurally nothing else (Ruling 91)", () => { + it("releases the patient's name for a plan the actor can list, and no other detail with it", async () => { + const store = await newStore(); + await createPlanParents(store, COORDINATOR_A); + unwrap(await store.createPlan(createInput(), writeContext(COORDINATOR_A, "key-create"))); + + const names = await store.listPatientNames({ actor: COORDINATOR_A }); + + expect(names).toEqual([{ planId: PLAN_ID, patientName: "Jordan Nguyen" }]); + // The shape itself, not merely the values: an extra field would satisfy `toEqual`'s + // subset-free comparison only because it is asserted here as the whole key set. + expect(Object.keys(names[0]).sort()).toEqual(["patientName", "planId"]); + // And the three fields this read exists to leave behind. The fixture's own values, so a + // pass means they were not released rather than that the fixture never held them -- + // `getEpisode` below is the positive control that the store is still holding all of them. + const serialised = JSON.stringify(names); + expect(serialised).not.toContain("491 570 156"); + expect(serialised).not.toContain("UR-00219384"); + const episode = await store.getEpisode(PLAN_ID, { actor: TEAM_LEAD_A }); + expect(episode?.patientMobileNumber).toBe("+61 491 570 156"); + expect(episode?.patientIdentifiers).toEqual(["UR-00219384"]); + }); + + it("names exactly the plans the same actor can list, one entry each", async () => { + const store = await newStore(); + await createPlanParents(store, COORDINATOR_A); + unwrap(await store.createPlan(createInput(), writeContext(COORDINATOR_A, "key-create"))); + await createActivePlan(store); + + const plans = await store.listPlans({ actor: COORDINATOR_A }); + const names = await store.listPatientNames({ actor: COORDINATOR_A }); + + expect(plans).toHaveLength(2); + expect([...names].map((entry) => entry.planId).sort()).toEqual([...plans].map((plan) => plan.plan.id).sort()); + }); + + it("gives an actor from another team the same empty answer an empty store gives", async () => { + const store = await newStore(); + await createPlanParents(store, COORDINATOR_A); + unwrap(await store.createPlan(createInput(), writeContext(COORDINATOR_A, "key-create"))); + + const empty = await newStore(); + + // Positive control: the plan is genuinely readable inside its own team, so the two empties + // below are scoping rather than an absent record. + expect(await store.listPatientNames({ actor: COORDINATOR_A })).toHaveLength(1); + // Identical answers, so nothing here tells a cross-team actor whether the plan exists -- + // the property `getPlan` protects by returning null for both cases. This read takes no + // plan id at all, so there is no question for it to answer either way. + expect(await store.listPatientNames({ actor: COORDINATOR_B })).toEqual([]); + expect(await empty.listPatientNames({ actor: COORDINATOR_B })).toEqual([]); + }); + + it("is empty, never a refusal, for a role that may not list plans — even one holding viewPatientRecord", async () => { + const store = await newStore(); + await createPlanParents(store, COORDINATOR_A); + unwrap(await store.createPlan(createInput(), writeContext(COORDINATOR_A, "key-create"))); + + // The auditor is the case this rule exists for, and the assertion below is what makes the + // empty answer meaningful: the auditor DOES hold the name capability, and still gets + // nothing, because it cannot enumerate this team's plans. A read gated on + // `viewPatientRecord` alone would hand this role every name the team holds -- a widening, + // from a change whose whole purpose is narrowing. + expect( + canPerformCaringContactAction(AUDITOR_A, READ_ACTIONS.patientName, { teamId: AUDITOR_A.teamId }), + ).toEqual({ allowed: true }); + expect(canPerformCaringContactAction(AUDITOR_A, READ_ACTIONS.plan, { teamId: AUDITOR_A.teamId }).allowed).toBe( + false, + ); + expect(PATIENT_NAME_READ_ACTIONS).toContain(READ_ACTIONS.plan); + + expect(await store.listPatientNames({ actor: AUDITOR_A })).toEqual([]); + expect(await store.listPlans({ actor: AUDITOR_A })).toEqual([]); + expect(await store.listPatientNames({ actor: ROLELESS_A })).toEqual([]); + // Software has no reason to know a patient's name; the dispatcher holds no human capability + // at all, and a delivery pipeline that could read names would be a new disclosure surface. + expect(await store.listPatientNames({ actor: DISPATCHER_A })).toEqual([]); + }); + + it("holds no name for a plan a retention clearance has already de-identified", async () => { + const store = await newStore(); + const plan = await createActivePlan(store); + unwrap( + await store.withdrawPlan( + { planId: plan.plan.id, expectedVersion: plan.plan.version, origin: "patient" }, + writeContext(COORDINATOR_A, `names-withdraw-${plan.plan.id}`), + ), + ); + + // Positive control: the name is released right up until the clearance. + const before = await store.listPatientNames({ actor: COORDINATOR_A }); + expect(before.find((entry) => entry.planId === plan.plan.id)?.patientName).toBe("Jordan Nguyen"); + + unwrap( + await store.markRetentionCleared( + { planId: plan.plan.id }, + writeContext(COORDINATOR_A, `names-clear-${plan.plan.id}`), + ), + ); + + const after = await store.listPatientNames({ actor: COORDINATOR_A }); + // The row is still listed -- the plan still exists -- and the cleared value is the empty + // string both stores write, which a caller must read as "no name held". Dropping the entry + // instead would make a cleared plan indistinguishable from one this actor may not see. + expect(after.find((entry) => entry.planId === plan.plan.id)?.patientName).toBe(""); + }); + }); + describe("dispatch never keys off sendAt", () => { it("stores an absorbed contact as terminal, so it can never reach a dispatch list", async () => { const store = await newStore(); diff --git a/tests/route-reachability.test.ts b/tests/route-reachability.test.ts index 808af6651c..0bac2bae1f 100644 --- a/tests/route-reachability.test.ts +++ b/tests/route-reachability.test.ts @@ -5,6 +5,7 @@ import { parse } from "@babel/parser"; import { describe, expect, it } from "vitest"; import { appModeDefinitions, appModeHomeHref } from "@/lib/app-modes"; +import { CARING_CONTACTS_ROUTES, type CaringContactsRouteKey } from "@/lib/caring-contacts-routes"; import { modeSecondaryNavigationRegistry } from "@/lib/mode-secondary-navigation"; import { colourCodingReferenceHref } from "@/lib/reference-routes"; import { tools } from "@/components/tools-page-mockups/tool-fixtures"; @@ -518,6 +519,36 @@ for (const match of tcReservedSegments) { builderTargets.add(`${tcBase}/${match[1]}`); } +// The Caring Contacts workspace owns its own navigation. `shell.tsx` holds one frozen +// destination table and renders each entry as a `` — an identifier, never a +// literal — so the JSX scan above sees no path at all. The table is the source of truth for what +// is linked, and an entry carries an `href` only once its page exists (Ruling 52), so reading it +// here is reading the same fact the shell renders rather than a second copy of it. +// `caring-contacts-routes.ts` is deliberately React-free string data, so importing it is safe. +// `tests/caring-contacts-workspace-shell.dom.test.tsx` independently pins that each of these is +// rendered as a real link, which is what stops this builder from vouching for a dead entry. +const workspaceShellSrc = readFileSync(path.join(srcRoot, "components/caring-contacts/workspace/shell.tsx"), "utf8"); +const workspaceHrefKeys = [...workspaceShellSrc.matchAll(/href:\s*CARING_CONTACTS_ROUTES\.(\w+)/g)].map( + (match) => match[1], +); +// Fail loudly rather than silently covering nothing: an empty parse here would let every built +// workspace destination read as an orphan, or — worse — let a future one go unchecked. +if (workspaceHrefKeys.length === 0) { + throw new Error( + "route-reachability: parsed no `href: CARING_CONTACTS_ROUTES.*` entries from the Caring Contacts shell — " + + "update this parser to match the current source so workspace destinations stay covered.", + ); +} +for (const key of workspaceHrefKeys) { + const href = CARING_CONTACTS_ROUTES[key as CaringContactsRouteKey]; + if (!href) { + throw new Error( + `route-reachability: the Caring Contacts shell links CARING_CONTACTS_ROUTES.${key}, which does not exist.`, + ); + } + builderTargets.add(pathOnly(href)); +} + /** A route is reachable if a builder emits it, or a non-mockup source file links to it. */ function isReachable(route: string, selfFile: string) { if (builderTargets.has(route)) return true; diff --git a/tests/ui-caring-contacts-workspace.spec.ts b/tests/ui-caring-contacts-workspace.spec.ts index f0ec1cf009..e6e96b3238 100644 --- a/tests/ui-caring-contacts-workspace.spec.ts +++ b/tests/ui-caring-contacts-workspace.spec.ts @@ -26,6 +26,29 @@ import { WORKSPACE_WIDTH_BREAKPOINTS, widthStateFor } from "../src/components/ca */ const WORKSPACE_ROUTE = "/caring-contacts"; +const PATIENTS_ROUTE = `${WORKSPACE_ROUTE}/patients`; + +/** + * Every production screen this workspace serves, with the `h1` it must render. + * + * The header above states the rule this list exists to keep true: the adoption + * contract names this file as the sole evidence for all five proof categories of + * the `caring-contacts-workspace` surface, and a proof pointer at a suite that + * never visits a route is a red gate that has been silenced. Phase 2B Task 5 + * added `/caring-contacts/patients` to that surface, so the accessibility-mode + * proofs below run against BOTH screens rather than against Today alone. A + * screen added to that surface without being added here is the same silenced + * gate wearing a newer date. + */ +const WORKSPACE_SCREENS = [ + { name: "Today", route: WORKSPACE_ROUTE, heading: "Today" }, + { name: "Patients", route: PATIENTS_ROUTE, heading: "Patients" }, +] as const; + +type WorkspaceScreen = (typeof WORKSPACE_SCREENS)[number]; + +const TODAY_SCREEN: WorkspaceScreen = WORKSPACE_SCREENS[0]; +const PATIENTS_SCREEN: WorkspaceScreen = WORKSPACE_SCREENS[1]; /** 320/390/430 are the three compact review widths; the rest are the state boundaries. */ const REVIEW_WIDTHS = [320, 390, 430, 768, 1024, 1440] as const; @@ -37,16 +60,21 @@ const VIEWPORT_HEIGHT = 900; * before it existed is unchanged. The service-stop block below passes a shorter viewport for a * reason recorded there. */ -async function openWorkspace(page: Page, width: number, height: number = VIEWPORT_HEIGHT) { +async function openWorkspace( + page: Page, + width: number, + height: number = VIEWPORT_HEIGHT, + screen: WorkspaceScreen = TODAY_SCREEN, +) { await page.setViewportSize({ width, height }); - await page.goto(WORKSPACE_ROUTE, { waitUntil: "load" }); + await page.goto(screen.route, { waitUntil: "load" }); // React streams the segment under `loading.tsx`'s Suspense boundary into a // hidden holder before moving it into place, so a production page sampled too // early carries a second, inert copy of the whole shell. Settle on exactly one // before measuring anything — and assert it, because a shell that genuinely // mounted twice would double every landmark on the page. await expect(page.getByTestId("caring-contacts-rail")).toHaveCount(1); - await expect(page.getByRole("heading", { level: 1, name: "Today" })).toBeVisible(); + await expect(page.getByRole("heading", { level: 1, name: screen.heading })).toBeVisible(); } /** Horizontal overflow of the document against the viewport, in CSS pixels. */ @@ -83,6 +111,29 @@ function markerBorder(page: Page) { }); } +/** + * The Patients screen's own surface and ink, read from the empty-state panel. + * + * `shellColours` reads the rail, the `h1` and the synthetic marker — all shell chrome, identical + * on both routes, so a dark-mode test built only on it proves the SHELL re-resolves and says + * nothing about anything this screen draws. The empty state sits on `--surface-subtle` with its + * own border and muted ink, none of which the shell contributes. + */ +function emptyStateColours(page: Page) { + return page.evaluate(() => { + const group = document.querySelector("[role='group'][aria-label='No patients yet']"); + if (!group) throw new Error("the empty state is missing"); + const style = getComputedStyle(group); + const heading = group.querySelector("p"); + if (!heading) throw new Error("the empty state has no heading"); + return { + surface: style.backgroundColor, + border: style.borderTopColor, + ink: getComputedStyle(heading).color, + }; + }); +} + function shellColours(page: Page) { return page.evaluate(() => { const chrome = document.querySelector("[data-testid='caring-contacts-rail']"); @@ -207,6 +258,153 @@ test.describe("caring-contacts workspace shell", () => { }); }); +/* ------------------------------------------------------------------------- * + * The Patients directory (Phase 2B Task 5) — the workspace's second production + * screen, and the reason this file could no longer prove only one route. + * + * `docs/design-system/adoption-contract.json` added + * `src/app/caring-contacts/patients/page.tsx` to the `caring-contacts-workspace` + * surface, whose five proof categories all cite this file and nothing else. + * Until this block existed, every one of those five was a claim about a route + * this suite had never loaded — a well-formed declaration certifying nothing, + * which is precisely the silenced gate the file header warns about. So the four + * accessibility-mode proofs are repeated here against the real screen rather + * than inherited from Today, and each asserts something that can actually fail. + * + * The demo store holds no plans (the in-memory repository seeds none), so this + * screen's honest state in a browser is the "no patients yet" empty state. That + * is not a fixture convenience: it is the empty-list contract observed end to + * end — an empty caseload served as a page rather than as a missing resource. + * ------------------------------------------------------------------------- */ + +test.describe("caring-contacts patients directory", () => { + test("serves an empty caseload as a page, not a missing resource", async ({ page }) => { + await page.setViewportSize({ width: 1024, height: VIEWPORT_HEIGHT }); + const response = await page.goto(PATIENTS_ROUTE, { waitUntil: "load" }); + + // The empty-list contract observed end to end rather than inferred from a + // render: `listPlans` returning `[]` is a permitted read that released + // something, so this screen is served rather than refused. + // + // READ THE NEXT PARAGRAPH BEFORE TRIMMING THIS TEST TO THE STATUS LINE. + // The status check is NOT what catches a `notFound()` here, and measuring + // that was the point of running the mutation rather than assuming: with + // `if (records.length === 0) notFound()` added to the page, this route still + // answered **200**, because it is dynamic and streams under `loading.tsx`'s + // Suspense boundary — the headers are flushed before the render reaches + // `notFound()`, so the refusal arrives as CONTENT, not as a status code. The + // three content assertions below are the load-bearing ones; the status line + // is kept because it still catches a refusal made before the stream opens + // (the production demo lock, or the route failing to resolve at all). + expect(response?.status(), "the patients route did not serve a page").toBe(200); + await expect(page.getByRole("heading", { level: 1, name: "Patients" })).toBeVisible(); + + // And the empty state states which of the three facts it is, in words. + const empty = page.getByRole("group", { name: "No patients yet" }); + await expect(empty).toBeVisible(); + await expect(empty).toContainText("referral"); + }); + + test("holds the frozen layout at 320px, the narrowest reviewed width", async ({ page }) => { + await openWorkspace(page, 320, VIEWPORT_HEIGHT, PATIENTS_SCREEN); + + expect(await documentOverflow(page), "horizontal document overflow at 320px").toBeLessThanOrEqual(2); + expect(await displayedWidthStates(page), "width state at 320px").toEqual([widthStateFor(320)]); + await expect(page.getByTestId("caring-contacts-phone-dock")).toBeVisible(); + await expect(page.getByTestId("caring-contacts-rail")).toBeHidden(); + + // The state filter is a set of real links, and they meet the production tap + // floor at the width where a thumb is the only pointer. A chip narrowed to + // the generic 44px guidance fails here, which is the point. + const chip = page.getByRole("link", { name: "Active" }); + await expect(chip).toBeVisible(); + const box = await chip.boundingBox(); + expect(box?.height ?? 0, "the state filter chip is under the production tap floor").toBeGreaterThanOrEqual(48); + }); + + test("re-resolves its surfaces and ink in dark rather than leaking a light value", async ({ page }) => { + await page.emulateMedia({ colorScheme: "light" }); + await openWorkspace(page, 1024, VIEWPORT_HEIGHT, PATIENTS_SCREEN); + await expect(page.getByRole("group", { name: "No patients yet" })).toBeVisible(); + const light = await shellColours(page); + const lightEmpty = await emptyStateColours(page); + + await page.emulateMedia({ colorScheme: "dark" }); + await openWorkspace(page, 1024, VIEWPORT_HEIGHT, PATIENTS_SCREEN); + await expect(page.getByRole("group", { name: "No patients yet" })).toBeVisible(); + const dark = await shellColours(page); + const darkEmpty = await emptyStateColours(page); + + expect(dark.chrome, "rail surface did not change in dark").not.toBe(light.chrome); + expect(dark.ink, "heading ink did not change in dark").not.toBe(light.ink); + expect(dark.marker, "synthetic marker ink did not change in dark").not.toBe(light.marker); + for (const value of Object.values(dark)) { + expect(value, "a dark colour resolved to nothing").not.toBe("rgba(0, 0, 0, 0)"); + } + + // Everything above is shell chrome, identical on both routes, so on its own it would prove + // the SHELL re-resolves and claim the category on a screen it had not inspected. These read + // this screen's own surface: the empty state's panel, border and ink, none of them drawn by + // the shell. A hardcoded colour in `list-empty-state.tsx` leaves one of them unchanged. + expect(darkEmpty.surface, "the empty state's surface did not change in dark").not.toBe(lightEmpty.surface); + expect(darkEmpty.border, "the empty state's border did not change in dark").not.toBe(lightEmpty.border); + expect(darkEmpty.ink, "the empty state's ink did not change in dark").not.toBe(lightEmpty.ink); + for (const value of Object.values(darkEmpty)) { + expect(value, "a dark colour on the empty state resolved to nothing").not.toBe("rgba(0, 0, 0, 0)"); + } + }); + + test("states the empty caseload in words once forced colours drop every tint", async ({ page, browserName }) => { + test.skip(browserName !== "chromium", "forced-colors emulation is Chromium-only"); + + await page.emulateMedia({ forcedColors: "active" }); + await openWorkspace(page, 390, VIEWPORT_HEIGHT, PATIENTS_SCREEN); + + // Forced colours drop author backgrounds, so anything this screen said with + // a tint alone says nothing here. Both safeguards must survive in words. + await expect(page.getByTestId("caring-contacts-synthetic-marker")).toBeVisible(); + const empty = page.getByRole("group", { name: "No patients yet" }); + await expect(empty).toBeVisible(); + await expect(empty).toContainText("referral"); + + // The empty state's own delimiter, the same assertion shape the synthetic + // marker uses above and for the same reason: forced colours drop the panel's + // author background, leaving the border as the only thing separating this + // statement from the page around it. + const border = await page.evaluate(() => { + const group = document.querySelector("[role='group'][aria-label='No patients yet']"); + if (!group) throw new Error("the empty state is missing"); + const style = getComputedStyle(group); + return { width: style.borderTopWidth, colour: style.borderTopColor }; + }); + expect(Number.parseFloat(border.width), "the empty state has no border under forced colours").toBeGreaterThan(0); + expect(border.colour, "the empty state border is transparent under forced colours").not.toBe("rgba(0, 0, 0, 0)"); + + expect(await documentOverflow(page), "horizontal overflow under forced colours").toBeLessThanOrEqual(2); + }); + + test("prints with the synthetic marker and the empty state still on the page", async ({ page }) => { + await openWorkspace(page, 1024, VIEWPORT_HEIGHT, PATIENTS_SCREEN); + await page.emulateMedia({ media: "print" }); + + await expect(page.getByTestId("caring-contacts-synthetic-marker")).toBeVisible(); + await expect(page.getByRole("heading", { level: 1, name: "Patients" })).toBeVisible(); + // A printed caseload that has lost the statement of WHY it is empty reads as + // a caseload of zero with no reason given. + await expect(page.getByRole("group", { name: "No patients yet" })).toBeVisible(); + expect(await documentOverflow(page), "horizontal overflow in print").toBeLessThanOrEqual(2); + }); + + test("is reachable from the workspace rail, not only by typing its URL", async ({ page }) => { + await openWorkspace(page, 1024); + + await page.getByRole("navigation", { name: "Workspace" }).getByRole("link", { name: "Patients" }).click(); + + await expect(page.getByRole("heading", { level: 1, name: "Patients" })).toBeVisible(); + expect(new URL(page.url()).pathname).toBe(PATIENTS_ROUTE); + }); +}); + /* ------------------------------------------------------------------------- * * Task 19 — the overlay half, and the accessibility half. *