From 04d30c25700a9b5eb24e739a7a59900d2ceee5a1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Sun, 2 Aug 2026 14:39:55 +0200 Subject: [PATCH] test(sharing): break every control, then keep the ones that noticed MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Each behaviour here was verified by removing the code that provides it and confirming the test went red, then putting it back. Two of them did not, and both were rewritten rather than kept: The restore-side persister check asserted that the delegate's own dashboard could not be read back inside a shared record. Deleting the guard left it green, because the record-scoped hash already makes a snapshot written in one record unreadable in another. It now counts what landed in the cache, so the guard it names is the thing it measures — and the weaker line is kept beside it with a note saying why. The switcher's account rows were asserted from server-rendered markup. Radix portals do not render server-side at all, so an earlier draft was measuring the renderer. Those rows moved to the browser suite; what stays here is the gate, which fails sharply — with the `canSwitch` bail removed the two cases do not return different markup, they throw. Two more shapes worth naming. The cache dimension is exercised through a real QueryClient built from the app's own defaults, because "the hash string changed" and "one record's data cannot be read while inside another" are different claims. And the invalidation is a plain function over a client with three live entries seeded, with a test that asserts they start valid — without it, a client that failed to seed and one where everything was invalidated look the same from every other assertion. --- .../__tests__/nav-model-shared-record.test.ts | 137 ++++++++++ .../__tests__/shared-record-chrome.test.tsx | 174 ++++++++++++ .../shared-record-grant-loss-bridge.tsx | 26 +- .../access/__tests__/access-panel.test.tsx | 254 ++++++++++++++++++ src/hooks/__tests__/grant-loss.test.ts | 124 +++++++++ src/hooks/use-account-switch.ts | 32 ++- .../query-persister-shared-record.test.ts | 213 +++++++++++++++ .../account-grants-invalidation.test.ts | 81 ++++++ src/lib/queries/use-account-grants.ts | 27 +- .../query-keys/__tests__/record-scope.test.ts | 154 +++++++++++ src/lib/query-keys/record-scope.ts | 10 +- 11 files changed, 1204 insertions(+), 28 deletions(-) create mode 100644 src/components/layout/__tests__/nav-model-shared-record.test.ts create mode 100644 src/components/layout/__tests__/shared-record-chrome.test.tsx create mode 100644 src/components/settings/access/__tests__/access-panel.test.tsx create mode 100644 src/hooks/__tests__/grant-loss.test.ts create mode 100644 src/lib/pwa/__tests__/query-persister-shared-record.test.ts create mode 100644 src/lib/queries/__tests__/account-grants-invalidation.test.ts create mode 100644 src/lib/query-keys/__tests__/record-scope.test.ts diff --git a/src/components/layout/__tests__/nav-model-shared-record.test.ts b/src/components/layout/__tests__/nav-model-shared-record.test.ts new file mode 100644 index 000000000..36061a96c --- /dev/null +++ b/src/components/layout/__tests__/nav-model-shared-record.test.ts @@ -0,0 +1,137 @@ +/** + * What the navigation offers inside somebody else's record. + * + * Paint, not enforcement — every route behind a hidden entry refuses on its + * own, from a frozen allowlist this model cannot reach. What these pin is that + * the line is drawn in ONE place: the mobile bar's fixed slots, the sidebar, + * the More hub and the shell's deep-link guard all ask the same list, so they + * cannot end up disagreeing about what a delegate is offered. + * + * The default is the safe one. A destination that says nothing about sharing + * drops out, so a surface added later is hidden until somebody has thought + * about it rather than exposed until somebody notices. + * + * Mutation check, run: flip the default in `isNavDestinationVisible` so an + * unflagged destination survives a switch → "hides everything that has not + * been classified" goes red naming Insights and the Coach. + */ +import { describe, expect, it } from "vitest"; + +import { + BOTTOM_NAV_PRIMARY_SLOT_HREFS, + NAV_DESTINATIONS, + isDestinationInSharedRecord, + mobileMoreHubDestinations, + visibleNavDestinations, + visibleUtilityDestinations, +} from "../nav-model"; + +/** Everything enabled, so the module gate never masks the sharing gate. */ +const ALL_MODULES = Object.fromEntries( + NAV_DESTINATIONS.filter((d) => d.requiresModule).map((d) => [ + d.requiresModule as string, + true, + ]), +); + +function hrefsInSharedRecord(): string[] { + return visibleNavDestinations(ALL_MODULES, true, true).map((d) => d.href); +} + +describe("the destination list under a switch", () => { + it("still offers the health record itself", () => { + const hrefs = hrefsInSharedRecord(); + // The non-zero proof for every negative assertion below: a gate that hid + // everything would satisfy all of them and leave a delegate with a blank + // app. + expect(hrefs.length).toBeGreaterThan(5); + expect(hrefs).toContain("/"); + expect(hrefs).toContain("/measurements"); + expect(hrefs).toContain("/medications"); + expect(hrefs).toContain("/labs"); + }); + + it("hides everything that has not been classified", () => { + const hrefs = hrefsInSharedRecord(); + const unflagged = NAV_DESTINATIONS.filter((d) => !d.sharedRecord).map( + (d) => d.href, + ); + // Non-zero: if every destination were flagged this loop would assert + // nothing at all. + expect(unflagged.length).toBeGreaterThan(0); + for (const href of unflagged) { + expect( + hrefs, + `${href} should not be offered inside a shared record`, + ).not.toContain(href); + } + }); + + it("hides the AI surfaces by name", () => { + // Not incidental. Server-managed LLM egress of a person's health data + // rides the consent THAT person gave for their own use, so a delegate + // triggering it would create a consent-shaped act the owner never made. + const hrefs = hrefsInSharedRecord(); + expect(hrefs).not.toContain("/insights"); + expect(hrefs).not.toContain("/coach"); + }); + + it("changes nothing for a session in its own record", () => { + const own = visibleNavDestinations(ALL_MODULES, true).map((d) => d.href); + expect(own).toEqual(NAV_DESTINATIONS.map((d) => d.href)); + }); +}); + +describe("the utility tail under a switch", () => { + it("disappears entirely", () => { + // Settings holds credentials, integrations, notification channels and + // grant management; Notifications is the delegate's own device business. + expect(visibleUtilityDestinations({ sharedRecord: true })).toEqual([]); + expect(visibleUtilityDestinations().length).toBeGreaterThan(0); + }); +}); + +describe("the mobile bar under a switch", () => { + it("drops the same entries from the More hub", () => { + const hub = mobileMoreHubDestinations({ + modules: ALL_MODULES, + sharedRecord: true, + }).map((d) => d.href); + expect(hub.length).toBeGreaterThan(0); + expect(hub).not.toContain("/coach"); + expect(hub).toContain("/measurements"); + for (const slot of BOTTOM_NAV_PRIMARY_SLOT_HREFS) { + expect(hub).not.toContain(slot); + } + }); + + it("answers for the fixed Insights slot from the same list", () => { + // The bar carries its own literal for the fixed slots. Having it ASK the + // model rather than carry a second answer is what stops the two surfaces + // drifting — which is the whole reason this module exists. + expect(isDestinationInSharedRecord("/insights")).toBe(false); + expect(isDestinationInSharedRecord("/medications")).toBe(true); + }); +}); + +describe("the deep-link guard reads the same list", () => { + it("covers a destination and everything beneath it", () => { + expect(isDestinationInSharedRecord("/measurements")).toBe(true); + expect(isDestinationInSharedRecord("/measurements/abc123")).toBe(true); + expect(isDestinationInSharedRecord("/labs/panel/7")).toBe(true); + }); + + it("matches the dashboard exactly, so it does not swallow the app", () => { + // `"/"` is a destination like any other, and a prefix match on it would + // report every path in the product as covered — including /settings. + expect(isDestinationInSharedRecord("/")).toBe(true); + expect(isDestinationInSharedRecord("/settings/account")).toBe(false); + expect(isDestinationInSharedRecord("/notifications")).toBe(false); + }); + + it("answers false for a path no destination claims", () => { + // A surface nobody has classified is not one to open inside somebody + // else's record. + expect(isDestinationInSharedRecord("/some/new/surface")).toBe(false); + }); +}); diff --git a/src/components/layout/__tests__/shared-record-chrome.test.tsx b/src/components/layout/__tests__/shared-record-chrome.test.tsx new file mode 100644 index 000000000..36c589b56 --- /dev/null +++ b/src/components/layout/__tests__/shared-record-chrome.test.tsx @@ -0,0 +1,174 @@ +/** + * The chrome that tells somebody whose record they are in. + * + * These render the real components to markup rather than asserting on their + * source, because the property that matters is what a person sees. The suite + * runs SSR-only (`@testing-library/react` is not a dependency here), so what + * is proven is the paint: the banner appears exactly when the server says the + * session is inside a record, it names the person, and the switcher offers + * exactly the accounts the server published. What is NOT proven here is the + * click — the switch itself is driven end to end in + * `e2e/account-sharing.spec.ts`. + * + * Mutation checks, run: + * - make the banner render unconditionally (drop the `if (!active)` bail) → + * "renders nothing in the caller's own record" goes red. + * - make the switcher ignore `canSwitch` → "renders nothing when nothing is + * shared" goes red. + */ +import { describe, expect, it, vi } from "vitest"; +import { renderToStaticMarkup } from "react-dom/server"; +import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; + +import { I18nProvider } from "@/lib/i18n/context"; +import type { AccountAccess } from "@/lib/sharing/account-access-view"; + +const mockAccessRef: { value: AccountAccess } = { + value: { accounts: [], active: null, canSwitch: false }, +}; + +vi.mock("@/hooks/use-auth", () => ({ + useAuth: () => ({ + user: { + id: "delegate", + username: "delegate", + email: null, + role: "USER", + avatarUrl: null, + modules: {}, + accountAccess: mockAccessRef.value, + }, + isAuthenticated: true, + isLoading: false, + refetch: vi.fn(), + }), + useLogout: () => ({ mutate: vi.fn(), isPending: false }), +})); + +vi.mock("@/hooks/use-account-switch", () => ({ + useAccountSwitch: () => ({ mutate: vi.fn(), isPending: false }), +})); + +import { AccountSwitcherMenuItems } from "../account-switcher-menu"; +import { SharedRecordBanner } from "../shared-record-banner"; + +const OWNER = { + accountId: "acct-owner", + username: "grandma", + displayName: "Margarethe", + access: "read" as const, + canWrite: false, +}; + +function render(access: AccountAccess, node: React.ReactNode): string { + mockAccessRef.value = access; + return renderToStaticMarkup( + + {node} + , + ); +} + +describe("", () => { + it("renders nothing in the caller's own record", () => { + const html = render( + { accounts: [OWNER], active: null, canSwitch: true }, + , + ); + // Holding a grant is not being inside the record. A banner that appeared + // on the strength of the list would claim a context the session is not in. + expect(html).toBe(""); + }); + + it("names the person whose record is open", () => { + const html = render( + { accounts: [OWNER], active: OWNER, canSwitch: true }, + , + ); + expect(html).toContain('data-slot="shared-record-banner"'); + expect(html).toContain('data-account-id="acct-owner"'); + // The display name, not the username: this is the line somebody reads + // before they log a blood pressure. + expect(html).toContain("Margarethe"); + }); + + it("falls back to the username when the account set no display name", () => { + const html = render( + { + accounts: [{ ...OWNER, displayName: null }], + active: { ...OWNER, displayName: null }, + canSwitch: true, + }, + , + ); + // Never an unnamed banner. "You are in 's record" is worse than useless. + expect(html).toContain("grandma"); + }); + + it("says the access is read-only, and carries the way out", () => { + const html = render( + { accounts: [OWNER], active: OWNER, canSwitch: true }, + , + ); + expect(html).toContain('data-slot="shared-record-banner-exit"'); + expect(html).toContain("read it, not change it"); + }); + + it("drops the read-only line when the server says the grant can write", () => { + // Resolved server-side. The banner never decides what a grant permits; the + // day a WRITE grant exists this line stops claiming otherwise on its own. + const writable = { ...OWNER, access: "write" as const, canWrite: true }; + const html = render( + { accounts: [writable], active: writable, canSwitch: true }, + , + ); + expect(html).toContain('data-slot="shared-record-banner"'); + expect(html).not.toContain("read it, not change it"); + }); + + it("uses the warning register, not a subtle tint", () => { + // Somebody who stops noticing the banner logs their own reading into + // another person's record. The visual register is part of the contract. + const html = render( + { accounts: [OWNER], active: OWNER, canSwitch: true }, + , + ); + expect(html).toContain("bg-warning/15"); + expect(html).toContain("border-warning/40"); + }); +}); + +describe("", () => { + it("renders nothing when nothing is shared", () => { + const html = render( + { accounts: [], active: null, canSwitch: false }, + , + ); + // An account that has never used sharing sees the user menu it always saw. + expect(html).toBe(""); + }); + + it("binds canSwitch rather than the list length", () => { + // The server publishes the boolean; the client renders it. A menu that + // derived the affordance from `accounts.length` would be a second program + // deciding whether this person may switch. + const html = render( + { accounts: [OWNER], active: null, canSwitch: false }, + , + ); + expect(html).toBe(""); + }); + + // WHAT IS NOT PROVEN HERE. Nothing below renders the switcher's account + // rows. Radix mounts `DropdownMenuSubContent` — and the `MenuContent` its + // trigger requires — inside a portal, and portals do not render server-side + // at all, so any assertion about the rows would be measuring the renderer + // rather than the component. An earlier draft asserted on that markup and + // passed for the wrong reason. The rows, their labels, the active tick and + // the click are driven against a real browser in + // `e2e/account-sharing.spec.ts`. + // + // What the two cases above DO prove is the gate, and they prove it sharply: + // with the `canSwitch` bail removed they do not merely return different + // markup, they throw, because the Radix sub-menu has no menu root here. +}); diff --git a/src/components/layout/shared-record-grant-loss-bridge.tsx b/src/components/layout/shared-record-grant-loss-bridge.tsx index ce21ad625..f948dbf10 100644 --- a/src/components/layout/shared-record-grant-loss-bridge.tsx +++ b/src/components/layout/shared-record-grant-loss-bridge.tsx @@ -1,10 +1,10 @@ "use client"; import { useQueryClient } from "@tanstack/react-query"; -import { useEffect, useRef } from "react"; +import { useEffect } from "react"; import { - isSharingAccessDenied, + subscribeToGrantLoss, useLeaveSharedRecordOnGrantLoss, } from "@/hooks/use-account-switch"; @@ -25,26 +25,18 @@ import { * code is the point: a per-call-site handler is a rule the next feature * forgets, and forgetting it here means somebody is stuck. * - * Fires once. `landOnRecord` never resolves (the document is being replaced), - * so a second trigger would queue a second reload behind a navigation already - * in flight; the ref makes the first one the only one. + * Fires once — `subscribeToGrantLoss` owns that, because `landOnRecord` never + * resolves (the document is being replaced) and a second trigger would queue a + * second reload behind a navigation already in flight. */ export function SharedRecordGrantLossBridge() { const queryClient = useQueryClient(); const leave = useLeaveSharedRecordOnGrantLoss(); - const leaving = useRef(false); - useEffect(() => { - const unsubscribe = queryClient.getQueryCache().subscribe((event) => { - if (leaving.current) return; - if (event.type !== "updated") return; - if (event.query.state.status !== "error") return; - if (!isSharingAccessDenied(event.query.state.error)) return; - leaving.current = true; - void leave(); - }); - return unsubscribe; - }, [queryClient, leave]); + useEffect( + () => subscribeToGrantLoss(queryClient, () => void leave()), + [queryClient, leave], + ); return null; } diff --git a/src/components/settings/access/__tests__/access-panel.test.tsx b/src/components/settings/access/__tests__/access-panel.test.tsx new file mode 100644 index 000000000..2c82c6b6d --- /dev/null +++ b/src/components/settings/access/__tests__/access-panel.test.tsx @@ -0,0 +1,254 @@ +/** + * The shared-access panel: what each row says, and what the activity view + * admits about itself. + * + * SSR renders of the real cards with the reads stubbed at the hook boundary, + * so what is asserted is the paint. The mutations (revoke, renounce, accept) + * are proven where they can be: their invalidation contract in + * `src/lib/queries/__tests__/account-grants-invalidation.test.ts`, their + * confirmation gates in `src/__tests__/destructive-control-guard.test.ts`, and + * the end-to-end click in `e2e/account-sharing.spec.ts`. + */ +import { describe, expect, it, vi } from "vitest"; +import { renderToStaticMarkup } from "react-dom/server"; + +import { I18nProvider } from "@/lib/i18n/context"; +import type { + GrantList, + GrantRow, + RecordActivity, +} from "@/lib/queries/use-account-grants"; + +const grantsRef: { value: GrantList } = { value: { given: [], received: [] } }; +const activityRef: { value: RecordActivity | undefined } = { value: undefined }; + +vi.mock("@/lib/queries/use-account-grants", async (importOriginal) => { + const actual = + await importOriginal(); + return { + ...actual, + useAccountGrants: () => ({ + data: grantsRef.value, + isLoading: false, + isError: false, + refetch: vi.fn(), + }), + useRecordActivity: () => ({ + data: activityRef.value, + isLoading: false, + isError: false, + refetch: vi.fn(), + }), + useRevokeGrant: () => ({ mutate: vi.fn(), isPending: false }), + useRenounceGrant: () => ({ mutate: vi.fn(), isPending: false }), + useAcceptGrant: () => ({ mutate: vi.fn(), isPending: false }), + }; +}); + +vi.mock("@/hooks/use-account-switch", () => ({ + useAccountSwitch: () => ({ mutate: vi.fn(), isPending: false }), +})); + +import { GrantsGivenCard } from "../grants-given-card"; +import { GrantsReceivedCard } from "../grants-received-card"; +import { RecordActivityCard } from "../record-activity-card"; + +function render(node: React.ReactNode): string { + return renderToStaticMarkup( + {node}, + ); +} + +const BASE: GrantRow = { + id: "g1", + account: { id: "acct-1", username: "housemate", displayName: "Jo" }, + access: "READ", + state: "ACTIVE", + invitedAt: "2026-01-02T10:00:00.000Z", + acceptedAt: "2026-01-02T11:00:00.000Z", + expiresAt: null, + lastUsedAt: null, + revokedAt: null, + revokedBy: null, +}; + +describe("who can open my record", () => { + it("says so honestly when nobody can", () => { + grantsRef.value = { given: [], received: [] }; + const html = render(); + expect(html).toContain("Nobody else can open your record"); + expect(html).not.toContain('data-slot="grants-given-list"'); + }); + + it("renders the state the server resolved, not one it worked out", () => { + // Every row carries a `state` the request resolver's own predicate + // produced. A panel that compared `expiresAt` to the clock would be a + // second opinion about somebody's access to a health record, and the two + // would disagree first in the dangerous direction. + grantsRef.value = { + given: [ + { ...BASE, id: "a", state: "ACTIVE" }, + { ...BASE, id: "p", state: "PENDING", acceptedAt: null }, + { + ...BASE, + id: "e", + state: "EXPIRED", + expiresAt: "2026-01-03T00:00:00.000Z", + }, + ], + received: [], + }; + const html = render(); + expect(html).toContain('data-grant-state="ACTIVE"'); + expect(html).toContain('data-grant-state="PENDING"'); + expect(html).toContain('data-grant-state="EXPIRED"'); + }); + + it("keeps an ended grant in the list, with who ended it", () => { + // The row IS the consent record. "Who had access, from when to when, and + // who ended it" is the question the panel exists to answer, and a list + // that dropped the ended rows could not answer it. + grantsRef.value = { + given: [ + { + ...BASE, + state: "REVOKED", + revokedAt: "2026-02-01T09:00:00.000Z", + revokedBy: "GRANTOR", + }, + ], + received: [], + }; + const html = render(); + expect(html).toContain('data-grant-state="REVOKED"'); + expect(html).toContain("You ended it"); + }); + + it("distinguishes a withdrawal from a handover", () => { + grantsRef.value = { + given: [ + { + ...BASE, + state: "REVOKED", + revokedAt: "2026-02-01T09:00:00.000Z", + revokedBy: "GRANTEE", + }, + ], + received: [], + }; + const html = render(); + // Two facts, one column. A trail that conflated them would misread an + // ordinary handover as a falling-out. + expect(html).toContain("Handed back"); + expect(html).not.toContain("You ended it"); + }); + + it("offers no way to end a grant that has already ended", () => { + grantsRef.value = { + given: [{ ...BASE, state: "REVOKED", revokedAt: "2026-02-01T09:00:00Z" }], + received: [], + }; + const ended = render(); + expect(ended).not.toContain('data-slot="grant-revoke"'); + + grantsRef.value = { given: [BASE], received: [] }; + const live = render(); + expect(live).toContain('data-slot="grant-revoke"'); + }); + + it("says a live grant has never been used rather than leaving a blank", () => { + grantsRef.value = { given: [{ ...BASE, lastUsedAt: null }], received: [] }; + const html = render(); + expect(html).toContain("never"); + }); +}); + +describe("records I can open", () => { + it("offers acceptance on an invitation and nothing else", () => { + grantsRef.value = { + given: [], + received: [{ ...BASE, state: "PENDING", acceptedAt: null }], + }; + const html = render(); + expect(html).toContain('data-slot="grant-accept"'); + // Nothing to open and nothing to hand back: an invitation confers nothing. + expect(html).not.toContain('data-slot="grant-open"'); + expect(html).not.toContain('data-slot="grant-renounce"'); + }); + + it("offers opening and handing back once it is active", () => { + grantsRef.value = { given: [], received: [BASE] }; + const html = render(); + expect(html).toContain('data-slot="grant-open"'); + expect(html).toContain('data-account-id="acct-1"'); + expect(html).toContain('data-slot="grant-renounce"'); + expect(html).not.toContain('data-slot="grant-accept"'); + }); +}); + +describe("when my record was opened", () => { + it("states its own window, from the number the server resolved", () => { + // A view that showed "the activity" without saying how far back it can + // see would imply a completeness it does not have: somebody checking last + // spring would read an empty list as "no" when the honest answer is + // "further back than this reaches". + activityRef.value = { entries: [], retentionDays: 90 }; + const html = render(); + expect(html).toContain('data-slot="record-activity-retention"'); + expect(html).toContain("last 90 days"); + }); + + it("prints the operator's window, never a compiled-in 365", () => { + activityRef.value = { entries: [], retentionDays: 30 }; + const html = render(); + expect(html).toContain("last 30 days"); + expect(html).not.toContain("365"); + }); + + it("says nothing about a window it has not been told", () => { + // A placeholder here would be exactly the invented value the sentence + // exists to avoid. + activityRef.value = undefined; + const html = render(); + expect(html).not.toContain('data-slot="record-activity-retention"'); + }); + + it("counts the openings rather than listing them one by one", () => { + activityRef.value = { + retentionDays: 365, + entries: [ + { + id: "a1", + actor: { id: "acct-1", username: "housemate", displayName: "Jo" }, + action: "sharing.record.accessed", + accesses: 40, + createdAt: "2026-02-03T08:00:00.000Z", + }, + ], + }; + const html = render(); + expect(html).toContain('data-slot="record-activity-row"'); + expect(html).toContain("Jo"); + expect(html).toContain("40"); + }); + + it("names a deleted account as one", () => { + // The actor column is deliberately not a foreign key, so the id outlives + // the person. Rendering nothing would read as though the owner had done + // it themselves. + activityRef.value = { + retentionDays: 365, + entries: [ + { + id: "a2", + actor: null, + action: "sharing.record.accessed", + accesses: 3, + createdAt: "2026-02-03T08:00:00.000Z", + }, + ], + }; + const html = render(); + expect(html).toContain("A deleted account"); + }); +}); diff --git a/src/hooks/__tests__/grant-loss.test.ts b/src/hooks/__tests__/grant-loss.test.ts new file mode 100644 index 000000000..c7a6740de --- /dev/null +++ b/src/hooks/__tests__/grant-loss.test.ts @@ -0,0 +1,124 @@ +/** + * Leaving a record the server has stopped honouring. + * + * Revocation clears the delegate's acting sessions in the same transaction, so + * that path puts the browser back by itself. Expiry does not — nothing sweeps, + * the grant simply stops being live on the next request, and the session keeps + * its stamp. From then on every read is a correct 403 under a banner that says + * the person is somewhere they are not. + * + * The subscriber is what closes that. It runs against a real `QueryClient` + * here, driven into the error state the way a failing read drives it, because + * "the effect calls subscribe" is a claim about source and "the callback fires + * when a read is refused" is the behaviour somebody depends on. + * + * Mutation check, run: widen the guard to fire on any error (drop the + * `isSharingAccessDenied` test) → "ignores an ordinary failure" goes red. + */ +import { describe, expect, it, vi } from "vitest"; +import { QueryClient } from "@tanstack/react-query"; + +import { + isSharingAccessDenied, + isSharingNotPermitted, + subscribeToGrantLoss, +} from "@/hooks/use-account-switch"; +import { ApiError } from "@/lib/api/api-fetch"; +import { QUERY_CLIENT_DEFAULT_OPTIONS } from "@/lib/pwa/query-client-options"; + +function client(): QueryClient { + return new QueryClient({ defaultOptions: QUERY_CLIENT_DEFAULT_OPTIONS }); +} + +/** Drive one query to the error state, the way a refused read does. */ +async function failWith(qc: QueryClient, error: unknown): Promise { + await qc + .fetchQuery({ + queryKey: ["probe", Math.random()], + queryFn: () => Promise.reject(error), + retry: false, + }) + .catch(() => {}); +} + +const DENIED = new ApiError("Account access denied", 403, { + errorCode: "sharing.access.denied", +}); +const NOT_PERMITTED = new ApiError("Not permitted", 403, { + errorCode: "sharing.not_permitted", +}); + +describe("the two sharing error codes", () => { + it("tells the two apart", () => { + // Different facts, different responses. One means the caller may no longer + // act as that account and must leave; the other means the surface is not + // part of what sharing covers and leaving would be the wrong answer. + expect(isSharingAccessDenied(DENIED)).toBe(true); + expect(isSharingNotPermitted(DENIED)).toBe(false); + expect(isSharingNotPermitted(NOT_PERMITTED)).toBe(true); + expect(isSharingAccessDenied(NOT_PERMITTED)).toBe(false); + }); + + it("reads the code, not the status or the prose", () => { + expect(isSharingAccessDenied(new ApiError("Forbidden", 403))).toBe(false); + expect(isSharingAccessDenied(new Error("Account access denied"))).toBe( + false, + ); + expect(isSharingAccessDenied(null)).toBe(false); + }); +}); + +describe("subscribeToGrantLoss", () => { + it("leaves when a read is refused for a grant that has ended", async () => { + const qc = client(); + const onLoss = vi.fn(); + const stop = subscribeToGrantLoss(qc, onLoss); + + await failWith(qc, DENIED); + stop(); + + expect(onLoss).toHaveBeenCalledTimes(1); + }); + + it("ignores an ordinary failure", async () => { + const qc = client(); + const onLoss = vi.fn(); + const stop = subscribeToGrantLoss(qc, onLoss); + + // A 500, a network blip and a not-permitted surface are all reasons to + // stay put. Reloading the app out of the record on any of them would turn + // a transient error into a lost context. + await failWith(qc, new ApiError("Server error", 500)); + await failWith(qc, NOT_PERMITTED); + await failWith(qc, new TypeError("fetch failed")); + stop(); + + expect(onLoss).not.toHaveBeenCalled(); + }); + + it("fires once, however many reads are refused", async () => { + // A switched page has several reads in flight and they all fail together. + // The callback replaces the document; a second one would queue a second + // navigation behind it. + const qc = client(); + const onLoss = vi.fn(); + const stop = subscribeToGrantLoss(qc, onLoss); + + await failWith(qc, DENIED); + await failWith(qc, DENIED); + await failWith(qc, DENIED); + stop(); + + expect(onLoss).toHaveBeenCalledTimes(1); + }); + + it("stops listening once unsubscribed", async () => { + const qc = client(); + const onLoss = vi.fn(); + subscribeToGrantLoss(qc, onLoss)(); + + await failWith(qc, DENIED); + + expect(onLoss).not.toHaveBeenCalled(); + }); +}); diff --git a/src/hooks/use-account-switch.ts b/src/hooks/use-account-switch.ts index 547dcbfe0..6d11b18dc 100644 --- a/src/hooks/use-account-switch.ts +++ b/src/hooks/use-account-switch.ts @@ -26,7 +26,11 @@ * Callers therefore do not need a success state, only a pending one. */ -import { useMutation, useQueryClient } from "@tanstack/react-query"; +import { + useMutation, + useQueryClient, + type QueryClient, +} from "@tanstack/react-query"; import { useCallback } from "react"; import { toast } from "sonner"; @@ -154,6 +158,32 @@ export function useAccountSwitch() { * down) the reload still happens: the stamp survives, but so does the honest * refusal, and the alternative is a page that neither recovers nor says why. */ +/** + * Watch the cache for the moment the server stops honouring the switch. + * + * A plain function over a QueryClient rather than an effect body, so a test + * can drive a real cache into the error state and check what happens — which + * is the only way to tell a subscriber that fires from one that was written + * and never wired. + * + * Fires `onLoss` at most once. The caller reloads the document, and a second + * trigger would queue a second navigation behind one already in flight. + */ +export function subscribeToGrantLoss( + queryClient: QueryClient, + onLoss: () => void, +): () => void { + let fired = false; + return queryClient.getQueryCache().subscribe((event) => { + if (fired) return; + if (event.type !== "updated") return; + if (event.query.state.status !== "error") return; + if (!isSharingAccessDenied(event.query.state.error)) return; + fired = true; + onLoss(); + }); +} + export function useLeaveSharedRecordOnGrantLoss() { return useCallback(async () => { try { diff --git a/src/lib/pwa/__tests__/query-persister-shared-record.test.ts b/src/lib/pwa/__tests__/query-persister-shared-record.test.ts new file mode 100644 index 000000000..9b7626450 --- /dev/null +++ b/src/lib/pwa/__tests__/query-persister-shared-record.test.ts @@ -0,0 +1,213 @@ +/** + * Nothing of somebody else's record reaches this device's disk, and nothing on + * it is painted while somebody else's record is open. + * + * The full page reload a switch performs clears the in-memory cache. These two + * layers are the ones it cannot reach: the IndexedDB snapshot, and (through + * the same wipe helper) the service worker's read-data cache. Between them + * they are the reason a switched browser can paint the previous record's + * numbers with nothing in the page's own code being wrong — the answer would + * not have come from the page. + * + * The IndexedDB here is a small fake rather than a mock of the persister's own + * calls. A test that asserted "idbSet was not called" would prove the code + * took a branch; this one proves the store is empty, which is the fact + * somebody's privacy depends on. + * + * Mutation checks, both run: + * - delete the `isReadingSharedRecord()` guard in `flush` → "writes nothing + * to disk while inside another record" goes red, with the snapshot found + * in the store. + * - delete the guard in `restorePersistedQueryCache` → "restores nothing + * while inside another record" goes red on the cache-size line, with one + * entry hydrated. The line above it stays GREEN under that mutation, which + * is the point of having both: the record-scoped hash already makes the + * restored entry unreadable, so an assertion about readability alone would + * have reported success about a control that was no longer there. + */ +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { QueryClient } from "@tanstack/react-query"; + +import { QUERY_CLIENT_DEFAULT_OPTIONS } from "@/lib/pwa/query-client-options"; +import { + restorePersistedQueryCache, + startPersistingQueryCache, +} from "@/lib/pwa/query-persister"; +import { queryKeys } from "@/lib/query-keys"; +import { + __resetRecordScopeForTests, + setRecordScope, +} from "@/lib/query-keys/record-scope"; + +const BUILD = "test-build"; + +// ── A minimal in-memory IndexedDB ──────────────────────────────────────────── +// +// Only the three calls the persister makes: `open`, `get`, `put`/`delete` +// inside a transaction. Small enough to read in one sitting, real enough that +// the assertions below are about stored bytes rather than about call counts. + +const store = new Map(); + +function fakeRequest(result: T) { + const req: { + result: T; + onsuccess: (() => void) | null; + onerror: (() => void) | null; + onupgradeneeded: (() => void) | null; + } = { result, onsuccess: null, onerror: null, onupgradeneeded: null }; + queueMicrotask(() => req.onsuccess?.()); + return req; +} + +function fakeObjectStore() { + return { + get: (key: string) => fakeRequest(store.get(key)), + put: (value: unknown, key: string) => { + store.set(key, value); + return fakeRequest(undefined); + }, + delete: (key: string) => { + store.delete(key); + return fakeRequest(undefined); + }, + }; +} + +function installFakeIndexedDb() { + (globalThis as { indexedDB?: unknown }).indexedDB = { + open: () => { + const req: { + result: unknown; + onsuccess: (() => void) | null; + onerror: (() => void) | null; + onupgradeneeded: (() => void) | null; + } = { + result: { + createObjectStore: () => {}, + close: () => {}, + transaction: () => { + const tx: { + objectStore: () => ReturnType; + oncomplete: (() => void) | null; + onerror: (() => void) | null; + } = { + objectStore: fakeObjectStore, + oncomplete: null, + onerror: null, + }; + queueMicrotask(() => tx.oncomplete?.()); + return tx; + }, + }, + onsuccess: null, + onerror: null, + onupgradeneeded: null, + }; + queueMicrotask(() => req.onsuccess?.()); + return req; + }, + }; +} + +/** Let the persister's microtask chains and its 1 s debounce run. */ +async function settle(): Promise { + await vi.advanceTimersByTimeAsync(1500); +} + +const originalIdb = (globalThis as { indexedDB?: unknown }).indexedDB; + +beforeEach(() => { + vi.useFakeTimers(); + store.clear(); + __resetRecordScopeForTests(); + installFakeIndexedDb(); +}); + +afterEach(() => { + vi.useRealTimers(); + if (originalIdb === undefined) { + delete (globalThis as { indexedDB?: unknown }).indexedDB; + } else { + (globalThis as { indexedDB?: unknown }).indexedDB = originalIdb; + } +}); + +function appClient(): QueryClient { + return new QueryClient({ defaultOptions: QUERY_CLIENT_DEFAULT_OPTIONS }); +} + +describe("persisting while inside another record", () => { + it("writes the dashboard to disk in the caller's own record", async () => { + // The positive control, and the reason the refusal below means anything: + // without it, "the store is empty" and "the persister does nothing at all + // in this test harness" are the same sentence. + const client = appClient(); + const stop = startPersistingQueryCache(client, BUILD); + client.setQueryData(queryKeys.dashboardSnapshot(), { weightKg: 71 }); + await settle(); + stop(); + + expect(store.size).toBeGreaterThan(0); + }); + + it("writes nothing to disk while inside another record", async () => { + setRecordScope("acct-owner"); + const client = appClient(); + const stop = startPersistingQueryCache(client, BUILD); + client.setQueryData(queryKeys.dashboardSnapshot(), { weightKg: 92 }); + await settle(); + stop(); + + // Not "a smaller payload" and not "the wrong key" — nothing. The + // allowlist decides which families are low-sensitivity enough to keep on + // this device, and that judgement was made about the account's own data. + expect(store.size).toBe(0); + }); +}); + +describe("restoring while inside another record", () => { + /** Seed the store the way a session in its own record would have left it. */ + async function seedOwnSnapshot(): Promise { + const writer = appClient(); + const stop = startPersistingQueryCache(writer, BUILD); + writer.setQueryData(queryKeys.dashboardSnapshot(), { weightKg: 71 }); + await settle(); + stop(); + expect(store.size).toBeGreaterThan(0); + } + + it("hydrates the persisted dashboard in the caller's own record", async () => { + await seedOwnSnapshot(); + + const reader = appClient(); + await restorePersistedQueryCache(reader, BUILD); + await vi.advanceTimersByTimeAsync(0); + + expect(reader.getQueryData(queryKeys.dashboardSnapshot())).toEqual({ + weightKg: 71, + }); + }); + + it("restores nothing while inside another record", async () => { + await seedOwnSnapshot(); + setRecordScope("acct-owner"); + + const reader = appClient(); + await restorePersistedQueryCache(reader, BUILD); + await vi.advanceTimersByTimeAsync(0); + + // The disk holds the DELEGATE's own dashboard. Painting it under a banner + // saying "you are in her record" is the mis-context failure this feature + // exists to prevent, in the one place a full reload cannot clear. + expect(reader.getQueryData(queryKeys.dashboardSnapshot())).toBeUndefined(); + + // And the entry is not merely unreachable — it was never hydrated. The + // line above passes with the guard deleted, because the record-scoped + // hash already makes a snapshot written in one record unreadable in + // another; asserting only that would be a check that cannot fail for the + // control it names. This one counts what landed in the cache, so removing + // the guard leaves exactly one entry here and turns it red. + expect(reader.getQueryCache().getAll()).toHaveLength(0); + }); +}); diff --git a/src/lib/queries/__tests__/account-grants-invalidation.test.ts b/src/lib/queries/__tests__/account-grants-invalidation.test.ts new file mode 100644 index 000000000..df165898f --- /dev/null +++ b/src/lib/queries/__tests__/account-grants-invalidation.test.ts @@ -0,0 +1,81 @@ +/** + * A grant transition refreshes everything it can have changed. + * + * The standing rule this checks is not "the mutation calls invalidate" — that + * is a claim about the source. It is "the three reads a grant transition makes + * stale come back invalidated", and the only thing that can answer it is a + * real `QueryClient` with those three reads in it. + * + * The third one is the interesting one. The grant list and the activity feed + * are the surface a person is looking at; `["auth","me"]` is the account + * payload, which carries `accountAccess` — the switcher's menu and the banner. + * Forget it and a revoked record keeps its entry in the menu: the affordance + * still on screen, the click already refused by the server. + * + * Mutation check, run: drop `queryKeys.authMe()` from `grantDependentKeys` + * and "refreshes the account payload" goes red. + */ +import { beforeEach, describe, expect, it } from "vitest"; +import { QueryClient } from "@tanstack/react-query"; + +import { QUERY_CLIENT_DEFAULT_OPTIONS } from "@/lib/pwa/query-client-options"; +import { invalidateGrantReads } from "@/lib/queries/use-account-grants"; +import { queryKeys } from "@/lib/query-keys"; +import { __resetRecordScopeForTests } from "@/lib/query-keys/record-scope"; + +beforeEach(() => { + __resetRecordScopeForTests(); +}); + +function seeded(): QueryClient { + const client = new QueryClient({ + defaultOptions: QUERY_CLIENT_DEFAULT_OPTIONS, + }); + client.setQueryData(queryKeys.accountGrants(), { given: [], received: [] }); + client.setQueryData(queryKeys.accountActivity(), { + entries: [], + retentionDays: 365, + }); + client.setQueryData(queryKeys.authMe(), { id: "u1" }); + return client; +} + +describe("invalidateGrantReads", () => { + it("refreshes the grant list", () => { + const client = seeded(); + invalidateGrantReads(client); + expect( + client.getQueryCache().find({ queryKey: queryKeys.accountGrants() }) + ?.state.isInvalidated, + ).toBe(true); + }); + + it("refreshes the owner's record-activity feed", () => { + const client = seeded(); + invalidateGrantReads(client); + expect( + client.getQueryCache().find({ queryKey: queryKeys.accountActivity() }) + ?.state.isInvalidated, + ).toBe(true); + }); + + it("refreshes the account payload, which carries the switcher", () => { + const client = seeded(); + invalidateGrantReads(client); + expect( + client.getQueryCache().find({ queryKey: queryKeys.authMe() })?.state + .isInvalidated, + ).toBe(true); + }); + + it("starts from three live, valid entries", () => { + // Non-zero proof. Without this, a client that failed to seed and a client + // whose entries were all invalidated look identical from the assertions + // above — every `find()` would return undefined and `?.state` would make + // each one a quiet `undefined !== true`. + const client = seeded(); + const all = client.getQueryCache().getAll(); + expect(all).toHaveLength(3); + expect(all.every((q) => q.state.isInvalidated)).toBe(false); + }); +}); diff --git a/src/lib/queries/use-account-grants.ts b/src/lib/queries/use-account-grants.ts index 2041c447f..8ea22df03 100644 --- a/src/lib/queries/use-account-grants.ts +++ b/src/lib/queries/use-account-grants.ts @@ -17,7 +17,12 @@ * cannot render "active" on a row the server has stopped honouring. */ -import { useMutation, useQuery, useQueryClient } from "@tanstack/react-query"; +import { + useMutation, + useQuery, + useQueryClient, + type QueryClient, +} from "@tanstack/react-query"; import { apiDelete, apiGet, apiPost } from "@/lib/api/api-fetch"; import { grantDependentKeys, queryKeys } from "@/lib/query-keys"; @@ -88,17 +93,27 @@ export function useRecordActivity(enabled = true) { /** * Refresh everything a grant transition can have changed. * + * A plain function over a QueryClient rather than a hook body, so a test can + * hand it a real client, seed the three reads and check they came back + * invalidated — which is the only way to tell an invalidation that runs from + * one that was written and never wired. + */ +export function invalidateGrantReads(queryClient: QueryClient): void { + for (const key of grantDependentKeys) { + void queryClient.invalidateQueries({ queryKey: key }); + } +} + +/** + * The same refresh, bound to the mounted client. + * * Shared by all four mutations rather than written out per hook: the day one * of them forgets the account payload is the day a revoked record keeps its * switcher entry. */ function useInvalidateGrants() { const queryClient = useQueryClient(); - return () => { - for (const key of grantDependentKeys) { - void queryClient.invalidateQueries({ queryKey: key }); - } - }; + return () => invalidateGrantReads(queryClient); } export interface InviteInput { diff --git a/src/lib/query-keys/__tests__/record-scope.test.ts b/src/lib/query-keys/__tests__/record-scope.test.ts new file mode 100644 index 000000000..befe0bc56 --- /dev/null +++ b/src/lib/query-keys/__tests__/record-scope.test.ts @@ -0,0 +1,154 @@ +/** + * The account dimension of the query cache. + * + * These run against a REAL `QueryClient` configured with the app's own default + * options, not against the hash function in isolation. The question worth + * answering is not "does the string change" — it is "can a value written while + * reading one record be read back while reading another", and only the cache + * itself can answer that. + * + * Mutation check, verified: delete `queryKeyHashFn` from + * `QUERY_CLIENT_DEFAULT_OPTIONS` and the two cross-record leaks below go red + * with the previous record's data returned verbatim. + */ +import { beforeEach, describe, expect, it } from "vitest"; +import { QueryClient } from "@tanstack/react-query"; + +import { QUERY_CLIENT_DEFAULT_OPTIONS } from "@/lib/pwa/query-client-options"; +import { queryKeys } from "@/lib/query-keys"; +import { + __resetRecordScopeForTests, + getRecordScope, + isReadingSharedRecord, + recordScopedQueryKeyHashFn, + setRecordScope, +} from "@/lib/query-keys/record-scope"; + +function appClient(): QueryClient { + return new QueryClient({ defaultOptions: QUERY_CLIENT_DEFAULT_OPTIONS }); +} + +beforeEach(() => { + __resetRecordScopeForTests(); +}); + +describe("the record scope", () => { + it("starts in the caller's own record", () => { + expect(getRecordScope()).toBeNull(); + expect(isReadingSharedRecord()).toBe(false); + }); + + it("moves, and comes back", () => { + setRecordScope("acct-owner"); + expect(getRecordScope()).toBe("acct-owner"); + expect(isReadingSharedRecord()).toBe(true); + + setRecordScope(null); + expect(getRecordScope()).toBeNull(); + expect(isReadingSharedRecord()).toBe(false); + }); +}); + +describe("the hash function", () => { + it("is TanStack's own hash while reading the caller's own record", () => { + // The overwhelming majority of sessions never switch. Nothing about their + // cache identity may change, or every persisted entry in the field is + // orphaned by this release. + expect(recordScopedQueryKeyHashFn(["dashboard", "snapshot"])).toBe( + JSON.stringify(["dashboard", "snapshot"]), + ); + }); + + it("sorts object keys, like the hash it replaces", () => { + // TanStack's `hashKey` is stable across key order inside an object so two + // spellings of one filter share a slot. Losing that would fragment the + // cache in a way no test of ours would otherwise notice. + const a = recordScopedQueryKeyHashFn(["k", { b: 2, a: 1 }]); + const b = recordScopedQueryKeyHashFn(["k", { a: 1, b: 2 }]); + expect(a).toBe(b); + }); + + it("gives one key two identities in two records", () => { + const own = recordScopedQueryKeyHashFn(queryKeys.dashboardSnapshot()); + setRecordScope("acct-owner"); + const shared = recordScopedQueryKeyHashFn(queryKeys.dashboardSnapshot()); + expect(shared).not.toBe(own); + }); +}); + +describe("the cache cannot serve one record's data while reading another", () => { + it("hides an entry written in the caller's own record", () => { + const client = appClient(); + client.setQueryData(queryKeys.dashboardSnapshot(), { weightKg: 71 }); + expect(client.getQueryData(queryKeys.dashboardSnapshot())).toEqual({ + weightKg: 71, + }); + + setRecordScope("acct-owner"); + + // The delegate's own numbers must be unreachable from inside somebody + // else's record. This is the assertion the whole hash exists for. + expect(client.getQueryData(queryKeys.dashboardSnapshot())).toBeUndefined(); + }); + + it("hides an entry written in a shared record once the browser leaves", () => { + const client = appClient(); + setRecordScope("acct-owner"); + client.setQueryData(queryKeys.dashboardSnapshot(), { weightKg: 92 }); + + setRecordScope(null); + + expect(client.getQueryData(queryKeys.dashboardSnapshot())).toBeUndefined(); + }); + + it("keeps two records' entries apart at the same time", () => { + const client = appClient(); + setRecordScope("acct-a"); + client.setQueryData(queryKeys.dashboardSnapshot(), { record: "a" }); + setRecordScope("acct-b"); + client.setQueryData(queryKeys.dashboardSnapshot(), { record: "b" }); + + expect(client.getQueryData(queryKeys.dashboardSnapshot())).toEqual({ + record: "b", + }); + setRecordScope("acct-a"); + expect(client.getQueryData(queryKeys.dashboardSnapshot())).toEqual({ + record: "a", + }); + }); +}); + +describe("prefix invalidation still reaches every slot", () => { + it("invalidates by key ARRAY, which the hash never touched", () => { + // The reason the dimension lives on the hash and not on the key arrays. + // `measurementDependentKeys` and every other bundle in the factory were + // built at import time, before any record was known; had the arrays grown + // a scope element, none of them would match anything ever again — and + // nothing would have failed, the caches would simply have stopped + // refreshing after a write. + const client = appClient(); + setRecordScope("acct-owner"); + client.setQueryData(queryKeys.measurementsList(LIST_PARAMS), [1, 2, 3]); + + client.invalidateQueries({ queryKey: queryKeys.measurements() }); + + const entry = client + .getQueryCache() + .find({ queryKey: queryKeys.measurementsList(LIST_PARAMS) }); + expect(entry).toBeDefined(); + expect(entry?.state.isInvalidated).toBe(true); + }); +}); + +const LIST_PARAMS = { + type: undefined, + sourceEq: undefined, + from: undefined, + to: undefined, + valueMin: undefined, + valueMax: undefined, + page: 1, + sortBy: "measuredAt", + sortDir: "desc", + mode: "raw", +} as const; diff --git a/src/lib/query-keys/record-scope.ts b/src/lib/query-keys/record-scope.ts index a3bb61d86..93468e317 100644 --- a/src/lib/query-keys/record-scope.ts +++ b/src/lib/query-keys/record-scope.ts @@ -75,13 +75,15 @@ function readMirror(): string | null { * The record this browser is currently reading, or null when it is reading its * own. * - * Server-side this is always null. `Providers` renders on the server during - * SSR, where there is no storage to read and no cache to partition. + * Server-side this stays null for the life of the process: the mirror is only + * ever seeded from browser storage, and `setRecordScope` is called from client + * code alone. That matters more than it looks — the value is a module-level + * variable, so a server that could write it would be sharing one scope across + * every request it serves. */ export function getRecordScope(): string | null { - if (typeof window === "undefined") return null; if (!seeded) { - scope = readMirror(); + scope = typeof window === "undefined" ? null : readMirror(); seeded = true; } return scope;