diff --git a/src/components/documents/__tests__/document-detail-download.test.ts b/src/components/documents/__tests__/document-detail-download.test.ts new file mode 100644 index 000000000..e3d331e5e --- /dev/null +++ b/src/components/documents/__tests__/document-detail-download.test.ts @@ -0,0 +1,83 @@ +/** + * The detail sheet fetches the original once, and only when it will show it. + * + * Two regressions this pins, both introduced by the fix that moved these + * controls off a bare `/api/…` href and neither visible in any rendered + * assertion: + * + * * **Two eager fetches per open.** `InlinePreview` loaded the original and + * the sheet-level download href loaded it AGAIN, so opening a document + * decrypted and held two copies of it — including for a non-inline + * document, where the old href cost nothing until somebody clicked. The + * vault meets large scans. + * * **A download control that could not be pressed and would not say so.** + * `href="#"` plus `aria-disabled` announces "unavailable" and still + * activates, and it ignored the failure flag entirely, so a permanently + * unfetchable original left a control that silently did nothing. + * + * Asserted on source shape rather than by rendering: the project's component + * convention is SSR-only (no DOM, no testing-library), and both properties are + * about which code paths EXIST — how many loaders there are, and whether the + * disabled state is real. A render could not see either. + */ +import { describe, expect, it } from "vitest"; +import { readFileSync } from "node:fs"; +import { join } from "node:path"; + +const SHEET = readFileSync( + join(process.cwd(), "src/components/documents/document-detail-sheet.tsx"), + "utf8", +); + +describe("the detail sheet's original-document transport", () => { + it("has exactly one eager loader for the original", () => { + const eager = [...SHEET.matchAll(/useFencedObjectUrl\(/g)]; + // Non-zero proof first: a sheet that fetched nothing would satisfy a bare + // "not more than one" check. + expect(eager.length).toBe(1); + }); + + it("only fetches eagerly for a document it is going to render inline", () => { + // The whole point of the single loader: a non-inline document — a large + // scan, a download-only format — fetches and decrypts nothing on open. + expect(SHEET).toMatch( + /useFencedObjectUrl\(\s*\n?\s*doc && doc\.servingClass === "inline"/, + ); + }); + + it("hands the blob down rather than letting the preview fetch its own", () => { + expect(SHEET).toMatch(/ { + expect(SHEET).toContain("createFencedBlobLoader()"); + expect(SHEET).toMatch( + /const onClick = async \(\) => \{[\s\S]{0,200}readyUrl/, + ); + }); + + it("gives the download a real disabled state that reflects failure", () => { + // `aria-disabled` on an anchor is an announcement, not a behaviour. + expect(SHEET).not.toMatch(/aria-disabled=/); + expect(SHEET).not.toMatch(/href=\{originalHref/); + expect(SHEET).toMatch(/disabled=\{blocked \|\| pending\}/); + expect(SHEET).toMatch(/const blocked = unavailable \|\| failed;/); + // And it says why, with the existing unavailable messaging. + expect(SHEET).toMatch( + /blocked\s*\n?\s*\?\s*t\("documents\.detail\.loadError"\)/, + ); + }); + + it("revokes what it fetched", () => { + // A decrypted health document left in an object URL lives as long as the + // tab does, and nothing in the UI ever shows it. + expect(SHEET).toMatch( + /useEffect\(\(\) => \(\) => loaderRef\.current\?\.dispose\(\), \[\]\)/, + ); + }); +}); diff --git a/src/components/documents/document-detail-sheet.tsx b/src/components/documents/document-detail-sheet.tsx index 2ddb73d4d..4c5a55876 100644 --- a/src/components/documents/document-detail-sheet.tsx +++ b/src/components/documents/document-detail-sheet.tsx @@ -64,7 +64,10 @@ import { import { useFormatters, useTranslations } from "@/lib/i18n/context"; import { useCoachLaunch } from "@/lib/insights/coach-launch-context"; import { invalidateKeys, queryKeys } from "@/lib/query-keys"; -import { useFencedObjectUrl } from "@/hooks/use-fenced-object-url"; +import { + createFencedBlobLoader, + useFencedObjectUrl, +} from "@/hooks/use-fenced-object-url"; import { cn } from "@/lib/utils"; import { INBOUND_DOCUMENT_KINDS, @@ -95,23 +98,119 @@ type PatchInput = { episodeIds?: string[]; }; +/** + * Download the stored original. + * + * v1.37.0 — a real button, not an anchor wearing `aria-disabled`. + * + * Two things were wrong with the anchor. `aria-disabled` tells a screen reader + * the control is unavailable and stops nothing: `href="#"` is still activated + * by a click and by Enter, so a person got a jump to the top of the page + * instead of a file. And it ignored the failure flag entirely, so a document + * whose original could not be fetched left a control that looked fine and + * silently did nothing, for good. + * + * It also fetches at CLICK time unless the bytes are already in hand. The + * sheet loads the original eagerly only for documents it is going to render + * inline; for everything else — a large scan, a download-only format — nothing + * is fetched or decrypted until somebody asks for it, which is what the plain + * `href` used to cost and what the first fix accidentally gave up. + */ +function DocumentDownloadButton({ + documentId, + filename, + readyUrl, + unavailable, + slot, + ...buttonProps +}: { + documentId: string; + filename: string | null; + /** Already-fetched bytes to reuse (the inline preview's), or null. */ + readyUrl: string | null; + /** A prior fetch of these bytes failed for good. */ + unavailable: boolean; + slot: string; +} & React.ComponentProps) { + const { t } = useTranslations(); + const [pending, setPending] = useState(false); + const [failed, setFailed] = useState(false); + const loaderRef = useRef | null>( + null, + ); + + useEffect(() => () => loaderRef.current?.dispose(), []); + + const save = (url: string) => { + const anchor = document.createElement("a"); + anchor.href = url; + anchor.download = filename ?? "document"; + anchor.click(); + }; + + const onClick = async () => { + if (readyUrl) { + save(readyUrl); + return; + } + setPending(true); + setFailed(false); + // One loader for the lifetime of the button: loading again revokes the + // previous object URL, and unmounting revokes the live one. Without that a + // decrypted health document stays in the tab's memory until it closes. + loaderRef.current ??= createFencedBlobLoader(); + const result = await loaderRef.current.load( + `/api/documents/inbound/${documentId}/original`, + ); + setPending(false); + if (result.kind === "loaded") save(result.url); + else if (result.kind === "failed") setFailed(true); + }; + + const blocked = unavailable || failed; + + return ( + + ); +} + /** Inline preview for Class A documents; skeleton until the blob paints. */ function InlinePreview({ - documentId, + src, + failed, mimeType, title, }: { - documentId: string; + /** + * v1.37.0 — the object URL, fetched ONCE by the sheet and handed down. + * + * The bytes come through the app transport because it carries the + * record-session assertion and a browser-issued `src="/api/…"` cannot. What + * changed since: this component used to fetch them itself while the sheet + * fetched the SAME original again for the download link, so opening a + * document decrypted and held two copies of it — and a large scan is a large + * copy. One loader now, lifted to the sheet. + */ + src: string | null; + failed: boolean; mimeType: string; title: string; }) { const [loaded, setLoaded] = useState(false); - // v1.37.0 — the bytes come through the app transport, which carries the - // record-session assertion; a browser-issued `src="/api/…"` cannot, and the - // fence refuses it on any session that has been inside a shared record. - const { url: src, failed } = useFencedObjectUrl( - `/api/documents/inbound/${documentId}/original`, - ); const isPdf = mimeType === "application/pdf"; // Chromium's PDF viewer grabs focus while it initialises — BEFORE the // frame's `load` event — stranding keyboard events inside the embedded @@ -504,12 +603,19 @@ export function DocumentDetailSheet({ const title = doc?.title ?? doc?.filename ?? t("documents.card.untitled"); const Icon = doc ? DOCUMENT_KIND_ICONS[doc.kind] : null; - // v1.37.0 — an object URL over transport-fetched bytes rather than a bare - // `/api/…` href. A download navigation carries no record-session assertion, - // so the fence refuses it once the session has been inside a shared record. - // Null until the blob lands; the anchor is disabled rather than dead. - const { url: originalHref } = useFencedObjectUrl( - doc ? `/api/documents/inbound/${doc.id}/original` : null, + // v1.37.0 — the ONE eager fetch of the original, and only for a document + // that is going to render it. + // + // A `