mirror of
https://github.com/MBombeck/HealthLog.git
synced 2026-11-09 04:50:59 +00:00
fix(02-12.1): fetch a document's original once, and only to show it
The detail sheet loaded the original twice on open — the inline preview and the download href each fetched and decrypted the whole blob — and for a non-inline document the old href had cost nothing until clicked. - one loader, lifted to the sheet, and only for a document that renders inline; everything else fetches at click time - make the download a real disabled control that reflects a failed fetch: href="#" with aria-disabled announces unavailable and still activates, and it ignored the failure entirely
This commit is contained in:
1 parent
120323d995
commit
73e73469e3
2 files changed
+222
-42
No files matched your search
@@ -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(/<InlinePreview\s+src=\{inlineSrc\}/);
|
||||
// And the preview declares it takes bytes, not an id to fetch from.
|
||||
expect(SHEET).toMatch(/function InlinePreview\(\{\s*\n\s*src,/);
|
||||
expect(SHEET).not.toMatch(
|
||||
/function InlinePreview\([\s\S]{0,400}documentId/,
|
||||
);
|
||||
});
|
||||
|
||||
it("downloads at click time when the bytes are not already in hand", () => {
|
||||
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\(\), \[\]\)/,
|
||||
);
|
||||
});
|
||||
});
|
||||
@@ -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<typeof Button>) {
|
||||
const { t } = useTranslations();
|
||||
const [pending, setPending] = useState(false);
|
||||
const [failed, setFailed] = useState(false);
|
||||
const loaderRef = useRef<ReturnType<typeof createFencedBlobLoader> | 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 (
|
||||
<Button
|
||||
{...buttonProps}
|
||||
data-slot={slot}
|
||||
disabled={blocked || pending}
|
||||
aria-label={t("documents.detail.download")}
|
||||
title={blocked ? t("documents.detail.loadError") : undefined}
|
||||
onClick={() => void onClick()}
|
||||
>
|
||||
<Download className="size-4" aria-hidden />
|
||||
<span className="hidden sm:inline">
|
||||
{blocked
|
||||
? t("documents.detail.loadError")
|
||||
: t("documents.detail.download")}
|
||||
</span>
|
||||
</Button>
|
||||
);
|
||||
}
|
||||
|
||||
/** 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 `<iframe src="/api/…">` carries no record-session assertion, so the
|
||||
// fence refuses it once the session has been inside a shared record; the
|
||||
// bytes therefore come through the app transport as an object URL. The
|
||||
// narrow path is the point: a non-inline document (a large scan, a
|
||||
// download-only format) fetched nothing until somebody clicked, and it must
|
||||
// stay that way — this used to fetch for every document on open, twice.
|
||||
const { url: inlineSrc, failed: inlineFailed } = useFencedObjectUrl(
|
||||
doc && doc.servingClass === "inline"
|
||||
? `/api/documents/inbound/${doc.id}/original`
|
||||
: null,
|
||||
);
|
||||
|
||||
return (
|
||||
@@ -607,23 +713,14 @@ export function DocumentDetailSheet({
|
||||
</span>
|
||||
</Button>
|
||||
)}
|
||||
<Button
|
||||
<DocumentDownloadButton
|
||||
documentId={doc.id}
|
||||
filename={doc.filename}
|
||||
readyUrl={inlineSrc}
|
||||
unavailable={inlineFailed}
|
||||
variant="outline"
|
||||
asChild
|
||||
data-slot="document-download"
|
||||
aria-label={t("documents.detail.download")}
|
||||
>
|
||||
<a
|
||||
href={originalHref ?? "#"}
|
||||
download={doc.filename ?? undefined}
|
||||
aria-disabled={originalHref === null}
|
||||
>
|
||||
<Download className="size-4" aria-hidden />
|
||||
<span className="hidden sm:inline">
|
||||
{t("documents.detail.download")}
|
||||
</span>
|
||||
</a>
|
||||
</Button>
|
||||
slot="document-download"
|
||||
/>
|
||||
</div>
|
||||
</div>
|
||||
) : undefined
|
||||
@@ -647,7 +744,8 @@ export function DocumentDetailSheet({
|
||||
<div className="space-y-6">
|
||||
{doc.servingClass === "inline" ? (
|
||||
<InlinePreview
|
||||
documentId={doc.id}
|
||||
src={inlineSrc}
|
||||
failed={inlineFailed}
|
||||
mimeType={doc.mimeType}
|
||||
title={title}
|
||||
/>
|
||||
@@ -663,16 +761,15 @@ export function DocumentDetailSheet({
|
||||
{t("documents.detail.previewUnavailable")} ·{" "}
|
||||
{formatBytes(doc.byteSize, locale)}
|
||||
</p>
|
||||
<Button asChild size="sm" className="mt-1">
|
||||
<a
|
||||
href={originalHref ?? "#"}
|
||||
download={doc.filename ?? undefined}
|
||||
aria-disabled={originalHref === null}
|
||||
>
|
||||
<Download className="size-4" aria-hidden />
|
||||
{t("documents.detail.download")}
|
||||
</a>
|
||||
</Button>
|
||||
<DocumentDownloadButton
|
||||
documentId={doc.id}
|
||||
filename={doc.filename}
|
||||
readyUrl={null}
|
||||
unavailable={false}
|
||||
size="sm"
|
||||
className="mt-1"
|
||||
slot="document-download-inline"
|
||||
/>
|
||||
</div>
|
||||
)}
|
||||
|
||||
|
||||
Reference in new issue
Block a user