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;