mirror of
https://github.com/MBombeck/HealthLog.git
synced 2026-11-14 14:40:52 +00:00
fix(documents): surface confirm failures and harden the library edges
Close the gaps the final verification pass flagged on the documents surface: - Confirm no longer claims success when the route rejects a fact into its `failed[]` array (HTTP 200). A unit-mismatch fact left silently PENDING with a green toast now raises a warning naming how many couldn't be saved and why. - Unify the German documents copy on "Angaben": the discard-confirm body, the provider-unsupported note, and the module description still said "Fakten". - After upload, sort by date-added rather than document date so a back-dated upload always lands on the first page where its detail panel can open. - Give the list its own load-error state with a retry instead of rendering the "no documents yet" empty state on a failed fetch. - Classify the local-OCR-disabled extract error to a clear message. - Drop the orphaned settings.sections.notifications.inboxLink key (no call site).
This commit is contained in:
1 parent
2ec4828c1f
commit
679f8a05c9
9 files changed
+254
-33
No files matched your search
@@ -2,16 +2,35 @@ import { describe, it, expect } from "vitest";
|
||||
|
||||
import { ApiError } from "@/lib/api/api-fetch";
|
||||
import type { InboundDocumentDto } from "@/lib/validations/inbound-documents";
|
||||
import enMessages from "../../../../messages/en.json";
|
||||
import {
|
||||
buildDocumentListSearch,
|
||||
classifyUploadError,
|
||||
commitFailureReasonKey,
|
||||
confirmFailureReasonKey,
|
||||
documentDateKey,
|
||||
formatDateGroupLabel,
|
||||
groupDocumentsByDate,
|
||||
isAlreadyConfirmedError,
|
||||
isLocalOcrDisabledError,
|
||||
isProviderUnsupportedError,
|
||||
} from "../library-utils";
|
||||
|
||||
/** Resolve a dotted i18n key against a bundle (the dynamic reason keys aren't
|
||||
* literal `t()` call sites, so the call-site coverage guard can't see them —
|
||||
* assert here that each one actually resolves to a string). */
|
||||
function resolveKey(bundle: unknown, key: string): unknown {
|
||||
return key
|
||||
.split(".")
|
||||
.reduce<unknown>(
|
||||
(acc, part) =>
|
||||
acc && typeof acc === "object"
|
||||
? (acc as Record<string, unknown>)[part]
|
||||
: undefined,
|
||||
bundle,
|
||||
);
|
||||
}
|
||||
|
||||
function doc(
|
||||
id: string,
|
||||
over: Partial<InboundDocumentDto> = {},
|
||||
@@ -182,6 +201,67 @@ describe("classifyUploadError", () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe("isLocalOcrDisabledError", () => {
|
||||
it("matches the extract 422 local-OCR-disabled signal", () => {
|
||||
const err = new ApiError("local ocr off", 422, {
|
||||
errorCode: "documents.inbound.localOcrDisabled",
|
||||
});
|
||||
expect(isLocalOcrDisabledError(err)).toBe(true);
|
||||
});
|
||||
|
||||
it("rejects other API errors and non-errors", () => {
|
||||
expect(
|
||||
isLocalOcrDisabledError(
|
||||
new ApiError("no provider", 422, {
|
||||
errorCode: "documents.inbound.providerUnsupported",
|
||||
}),
|
||||
),
|
||||
).toBe(false);
|
||||
expect(isLocalOcrDisabledError(new Error("boom"))).toBe(false);
|
||||
expect(isLocalOcrDisabledError(null)).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe("commitFailureReasonKey", () => {
|
||||
it("maps each commit-failure code to a reason key that resolves", () => {
|
||||
const cases: Record<string, string> = {
|
||||
"observation.unitMismatch": "documents.review.commitError.unitMismatch",
|
||||
"observation.unitRequired": "documents.review.commitError.unitRequired",
|
||||
"something.unknown": "documents.review.commitError.generic",
|
||||
};
|
||||
for (const [code, key] of Object.entries(cases)) {
|
||||
expect(commitFailureReasonKey(code)).toBe(key);
|
||||
// Every reason the toast can surface resolves to a real string.
|
||||
expect(typeof resolveKey(enMessages, key)).toBe("string");
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
describe("confirmFailureReasonKey", () => {
|
||||
it("surfaces the shared reason when every failed fact agrees", () => {
|
||||
const key = confirmFailureReasonKey([
|
||||
{ reason: "observation.unitMismatch" },
|
||||
{ reason: "observation.unitMismatch" },
|
||||
]);
|
||||
expect(key).toBe("documents.review.commitError.unitMismatch");
|
||||
expect(typeof resolveKey(enMessages, key)).toBe("string");
|
||||
});
|
||||
|
||||
it("falls back to the generic reason when failures disagree", () => {
|
||||
const key = confirmFailureReasonKey([
|
||||
{ reason: "observation.unitMismatch" },
|
||||
{ reason: "observation.unitRequired" },
|
||||
]);
|
||||
expect(key).toBe("documents.review.commitError.generic");
|
||||
});
|
||||
|
||||
it("uses the single failure's reason for a one-item batch", () => {
|
||||
expect(
|
||||
confirmFailureReasonKey([{ reason: "observation.unitRequired" }]),
|
||||
).toBe("documents.review.commitError.unitRequired");
|
||||
});
|
||||
});
|
||||
|
||||
describe("formatDateGroupLabel", () => {
|
||||
it("delegates the calendar day to the supplied app formatter", () => {
|
||||
const seen: string[] = [];
|
||||
|
||||
@@ -34,6 +34,7 @@ import {
|
||||
Info,
|
||||
Loader2,
|
||||
Pencil,
|
||||
RefreshCw,
|
||||
Search,
|
||||
Sparkles,
|
||||
Upload,
|
||||
@@ -85,9 +86,11 @@ import {
|
||||
import {
|
||||
buildDocumentListSearch,
|
||||
classifyUploadError,
|
||||
confirmFailureReasonKey,
|
||||
formatDateGroupLabel,
|
||||
groupDocumentsByDate,
|
||||
isAlreadyConfirmedError,
|
||||
isLocalOcrDisabledError,
|
||||
isProviderUnsupportedError,
|
||||
MAX_UPLOAD_BYTES,
|
||||
} from "./library-utils";
|
||||
@@ -96,6 +99,14 @@ type ListResponse = {
|
||||
documents: InboundDocumentDto[];
|
||||
nextCursor: string | null;
|
||||
};
|
||||
/** Shape the confirm route returns (HTTP 200) — `failed[]` carries per-fact
|
||||
* commit misses the client must surface, not silently swallow. */
|
||||
type ConfirmResponse = {
|
||||
approved: { factId: string; recordType: string; recordId: string }[];
|
||||
rejected: string[];
|
||||
needsReview: string[];
|
||||
failed: { factId: string; reason: string }[];
|
||||
};
|
||||
type Decision = "approve" | "reject";
|
||||
|
||||
export function InboundDocumentsView() {
|
||||
@@ -184,13 +195,15 @@ export function InboundDocumentsView() {
|
||||
},
|
||||
onSuccess: (doc) => {
|
||||
toast.success(t("documents.toast.uploaded"));
|
||||
// Drop any active filter / search / sort so the just-stored document is
|
||||
// in the list — otherwise selecting it opens a detail panel for a row the
|
||||
// current filter excludes, and the user lands on a dead end. A fresh
|
||||
// upload files under today, so the default documentDate-desc view shows it
|
||||
// at the top.
|
||||
// Drop any active filter / search so the just-stored document is in the
|
||||
// list — otherwise selecting it opens a detail panel for a row the current
|
||||
// filter excludes, and the user lands on a dead end. Sort by date-added
|
||||
// (createdAt) desc, NOT documentDate: a user-set past documentDate would
|
||||
// otherwise file the fresh upload deep in the list, off page 1, where the
|
||||
// detail panel never renders. The newest upload always has the newest
|
||||
// createdAt, so this guarantees it lands first and its panel opens.
|
||||
clearFilters();
|
||||
setSort("documentDate");
|
||||
setSort("createdAt");
|
||||
setOrder("desc");
|
||||
void invalidateKeys(queryClient, [queryKeys.documents()]);
|
||||
setSelectedId(doc.id);
|
||||
@@ -447,6 +460,27 @@ export function InboundDocumentsView() {
|
||||
<Skeleton key={i} className="h-12 w-full rounded-md" />
|
||||
))}
|
||||
</div>
|
||||
) : list.isError ? (
|
||||
// A failed list load is its own state with a retry — never the
|
||||
// "no documents yet" empty state, which would falsely imply the
|
||||
// library is empty.
|
||||
<div
|
||||
role="alert"
|
||||
data-slot="documents-list-error"
|
||||
className="text-muted-foreground flex flex-col items-start gap-3 py-4 text-sm sm:flex-row sm:items-center sm:justify-between"
|
||||
>
|
||||
<span>{t("documents.list.loadError")}</span>
|
||||
<Button
|
||||
type="button"
|
||||
size="sm"
|
||||
variant="outline"
|
||||
onClick={() => void list.refetch()}
|
||||
className="gap-1.5"
|
||||
>
|
||||
<RefreshCw className="h-3.5 w-3.5" aria-hidden />
|
||||
<span>{t("common.retry")}</span>
|
||||
</Button>
|
||||
</div>
|
||||
) : documents.length === 0 ? (
|
||||
isFiltered ? (
|
||||
<div
|
||||
@@ -610,22 +644,44 @@ export function DocumentDetail({
|
||||
// A missing provider — or a document already confirmed elsewhere — is not
|
||||
// an error the user acts on here. Surface the calm inline note (rendered
|
||||
// below) instead of an alarming toast.
|
||||
if (!isProviderUnsupportedError(err) && !isAlreadyConfirmedError(err)) {
|
||||
toast.error(t("documents.toast.extractFailed"));
|
||||
if (isProviderUnsupportedError(err) || isAlreadyConfirmedError(err)) {
|
||||
return;
|
||||
}
|
||||
// Local OCR switched off in settings: name the cause rather than the
|
||||
// generic "couldn't extract".
|
||||
if (isLocalOcrDisabledError(err)) {
|
||||
toast.error(t("documents.toast.localOcrDisabled"));
|
||||
return;
|
||||
}
|
||||
toast.error(t("documents.toast.extractFailed"));
|
||||
},
|
||||
});
|
||||
|
||||
const confirm = useMutation({
|
||||
mutationFn: () =>
|
||||
apiPost(`/api/documents/inbound/${documentId}/confirm`, {
|
||||
mutationFn: (): Promise<ConfirmResponse> =>
|
||||
apiPost<ConfirmResponse>(`/api/documents/inbound/${documentId}/confirm`, {
|
||||
decisions: Object.entries(decisions).map(([factId, action]) => ({
|
||||
factId,
|
||||
action,
|
||||
})),
|
||||
}),
|
||||
onSuccess: () => {
|
||||
toast.success(t("documents.toast.confirmed"));
|
||||
onSuccess: (data) => {
|
||||
// The route returns HTTP 200 even when it rejects a fact into `failed[]`
|
||||
// (e.g. a stated unit that disagrees with the saved marker). A green
|
||||
// "saved" toast would lie — the fact stays PENDING. Surface the count +
|
||||
// reason so the user knows to reconcile it, and only show the plain
|
||||
// success when nothing failed.
|
||||
const failed = data.failed ?? [];
|
||||
if (failed.length > 0) {
|
||||
toast.warning(
|
||||
t("documents.toast.confirmedPartial", {
|
||||
count: failed.length,
|
||||
reason: t(confirmFailureReasonKey(failed)),
|
||||
}),
|
||||
);
|
||||
} else {
|
||||
toast.success(t("documents.toast.confirmed"));
|
||||
}
|
||||
setDecisions({});
|
||||
void invalidateKeys(queryClient, [queryKeys.documents()]);
|
||||
},
|
||||
|
||||
@@ -101,6 +101,49 @@ export function isAlreadyConfirmedError(error: unknown): boolean {
|
||||
);
|
||||
}
|
||||
|
||||
/**
|
||||
* True when an extract failure is the route's "local OCR not enabled" signal
|
||||
* (`documents.inbound.localOcrDisabled`, 422). The TEXT extract path refuses
|
||||
* when in-browser OCR is switched off in settings; surface a clear message that
|
||||
* names the setting rather than the generic "couldn't extract" toast.
|
||||
*/
|
||||
export function isLocalOcrDisabledError(error: unknown): boolean {
|
||||
return (
|
||||
error instanceof ApiError &&
|
||||
error.meta?.errorCode === "documents.inbound.localOcrDisabled"
|
||||
);
|
||||
}
|
||||
|
||||
/**
|
||||
* Map a per-fact commit-failure code to the i18n key for its plain-language
|
||||
* reason. The confirm route returns HTTP 200 even when it rejects a fact into
|
||||
* its `failed[]` array (e.g. a stated unit that disagrees with the saved
|
||||
* marker), so the client must surface why — otherwise the fact silently stays
|
||||
* PENDING with the user told nothing.
|
||||
*/
|
||||
export function commitFailureReasonKey(code: string): string {
|
||||
switch (code) {
|
||||
case "observation.unitMismatch":
|
||||
return "documents.review.commitError.unitMismatch";
|
||||
case "observation.unitRequired":
|
||||
return "documents.review.commitError.unitRequired";
|
||||
default:
|
||||
return "documents.review.commitError.generic";
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Pick the single reason key that best describes a batch of failed facts: the
|
||||
* shared reason when every failure agrees, else a generic fallback. Used to
|
||||
* build the partial-failure toast after a confirm that rejected one or more
|
||||
* facts server-side.
|
||||
*/
|
||||
export function confirmFailureReasonKey(failed: { reason: string }[]): string {
|
||||
const codes = new Set(failed.map((f) => f.reason));
|
||||
if (codes.size === 1) return commitFailureReasonKey([...codes][0]);
|
||||
return "documents.review.commitError.generic";
|
||||
}
|
||||
|
||||
/**
|
||||
* Client-side upload ceiling. Mirrors the server's `OCR_MAX_BYTES` (12 MB) so an
|
||||
* oversized file is rejected before it is sent, instead of after a full upload
|
||||
|
||||
Reference in new issue
Block a user