fix(import): stop the Apple Health card ticking an archive that landed nothing

Same shape as the CSV and JSON cards: reaching the done state rendered the
success tick whether or not a single row was upserted. The archive's count
picks the tone now, and an archive that wrote nothing says so.

The clinical-record count stays out of that decision. Skipping
electrocardiograms and lab documents is a documented, deliberate exclusion,
not a refused row, so it must not read as a warning.

Refs #640
This commit is contained in:
Marc-André Bombeck committed 2026-07-25 21:45:10 +02:00
1 parent 0c9832eb8d
commit 4e95bd28f8
13 files changed
+719 -22

No files matched your search

@@ -0,0 +1,75 @@
/**
* #640 structural guard — "wrote nothing, looked successful" cannot come back.
*
* The classifier property in `import-result-state.test.ts` proves the rule.
* This proves the rule is the only route to the affordance: the success icon
* and the success colour exist in exactly one file in the import panel, on
* exactly one key of the outcome table, and no card reaches for them
* directly. A future card that hand-rolls a green tick fails here rather than
* shipping and lying to a self-hoster about their sensor history.
*/
import { describe, it, expect } from "vitest";
import { readFileSync, readdirSync } from "node:fs";
import { join } from "node:path";
const PANEL_DIR = join(__dirname, "..");
const PRESENTATION_FILE = "import-result-view.tsx";
/** The success affordance: the tick component and the success colour token. */
const SUCCESS_MARKERS = ["CheckCircle2", "text-success"] as const;
function panelSourceFiles(): string[] {
return readdirSync(PANEL_DIR).filter(
(name) => name.endsWith(".ts") || name.endsWith(".tsx"),
);
}
/** Code only — a comment naming the marker is prose, not an affordance. */
function readCode(name: string): string {
return readFileSync(join(PANEL_DIR, name), "utf8")
.replace(/\/\*[\s\S]*?\*\//g, "")
.replace(/^\s*\/\/.*$/gm, "");
}
describe("import panel — success affordance is owned by the outcome table", () => {
it("mentions the success markers in the presentation module only", () => {
const offenders = panelSourceFiles()
.filter((name) => name !== PRESENTATION_FILE)
.filter((name) => {
const source = readCode(name);
return SUCCESS_MARKERS.some((marker) => source.includes(marker));
});
expect(offenders).toEqual([]);
});
it("binds each success marker to the success outcome inside that module", () => {
const source = readCode(PRESENTATION_FILE);
for (const marker of SUCCESS_MARKERS) {
// Once as the lucide import (the tick only), once on the table row.
const uses = source.split(marker).length - 1;
expect(uses).toBe(marker === "CheckCircle2" ? 2 : 1);
}
// The table row itself: `success:` carries both markers, and no other
// outcome key does.
const successRow = source.match(/success:\s*\{[^}]*\}/)?.[0] ?? "";
expect(successRow).toContain("CheckCircle2");
expect(successRow).toContain("text-success");
for (const other of ["partial", "failed", "empty"]) {
const row = source.match(new RegExp(`${other}:\\s*\\{[^}]*\\}`))?.[0];
expect(row).toBeTruthy();
for (const marker of SUCCESS_MARKERS) {
expect(row).not.toContain(marker);
}
}
});
it("routes every card's result line through the shared outcome line", () => {
for (const name of [
"csv-import-card.tsx",
"json-import-card.tsx",
"apple-health-import-card.tsx",
]) {
expect(readCode(name)).toMatch(/CsvImportResultView|ImportOutcomeLine/);
}
});
});
@@ -0,0 +1,94 @@
/**
* #640 — an import that wrote nothing must never read as a success.
*
* The reported failure was not the parser refusing rows; it was the card
* answering that refusal with a green tick, which is what turned a
* recoverable problem into a silent one. These pin the classifier that both
* import cards render from, and the structural guard that keeps the success
* affordance attached to the success outcome alone lives alongside in
* `import-result-guard.test.ts`.
*/
import { describe, it, expect } from "vitest";
import {
classifyImportOutcome,
groupSkipReasons,
} from "../import-result-state";
describe("classifyImportOutcome", () => {
it("calls the reported production result a failure", () => {
// The audit envelope from the report: 1 597 rows in, nothing written.
expect(classifyImportOutcome({ written: 0, skipped: 1597 })).toBe("failed");
});
it("calls a file with no data rows empty, not a success", () => {
expect(classifyImportOutcome({ written: 0, skipped: 0 })).toBe("empty");
});
it("calls a mixed result partial", () => {
expect(classifyImportOutcome({ written: 3, skipped: 2 })).toBe("partial");
});
it("calls a clean result a success", () => {
expect(classifyImportOutcome({ written: 5, skipped: 0 })).toBe("success");
});
it("never reaches success while nothing was written", () => {
for (let written = 0; written <= 4; written++) {
for (let skipped = 0; skipped <= 4; skipped++) {
const outcome = classifyImportOutcome({ written, skipped });
if (written === 0) {
expect(outcome).not.toBe("success");
expect(outcome).not.toBe("partial");
}
if (outcome === "success") {
expect(written).toBeGreaterThan(0);
expect(skipped).toBe(0);
}
}
}
});
});
describe("groupSkipReasons", () => {
it("collapses a repeated reason to one entry with a count", () => {
const rows = Array.from({ length: 1597 }, (_, i) => ({
line: i + 2,
status: "skipped",
reason: "invalid_glucose_context",
}));
expect(groupSkipReasons(rows)).toEqual([
{ reason: "invalid_glucose_context", count: 1597 },
]);
});
it("ignores rows that were written", () => {
expect(
groupSkipReasons([
{ line: 2, status: "inserted" },
{ line: 3, status: "updated" },
{ line: 4, status: "skipped", reason: "unknown_unit" },
]),
).toEqual([{ reason: "unknown_unit", count: 1 }]);
});
it("orders by count descending, then by reason", () => {
const rows = [
{ line: 2, status: "skipped", reason: "unknown_unit" },
{ line: 3, status: "skipped", reason: "duplicate" },
{ line: 4, status: "skipped", reason: "duplicate" },
{ line: 5, status: "skipped", reason: "notes_too_long" },
];
expect(groupSkipReasons(rows)).toEqual([
{ reason: "duplicate", count: 2 },
{ reason: "notes_too_long", count: 1 },
{ reason: "unknown_unit", count: 1 },
]);
});
it("buckets a skipped row that carries no reason", () => {
expect(groupSkipReasons([{ line: 2, status: "skipped" }])).toEqual([
{ reason: "unknown", count: 1 },
]);
});
});
@@ -0,0 +1,180 @@
/**
* #640 — the CSV import result renders what actually happened.
*
* Project convention is SSR-only component tests (vitest runs `node`;
* `@testing-library/react` is not installed), so the view takes the result as
* a prop and each state is rendered directly.
*
* The states pinned here are the ones the report separated: a file where
* everything was refused must not carry the success icon or success wording,
* a mixed file reads as a warning, a clean file reads as a success, and a
* file with no data rows gets its own message rather than either. Skip
* reasons render grouped with a count, in plain language, inside the card's
* `aria-live` region so a screen reader hears the outcome too.
*/
import { describe, expect, it, vi } from "vitest";
import { renderToStaticMarkup } from "react-dom/server";
vi.mock("next/navigation", () => ({
useRouter: () => ({ push: vi.fn(), replace: vi.fn() }),
usePathname: () => "/settings/export",
}));
import { I18nProvider } from "@/lib/i18n/context";
import {
CsvImportResultView,
type CsvImportResult,
} from "../import-result-view";
import { CsvImportCard } from "../csv-import-card";
function render(node: React.ReactElement) {
return renderToStaticMarkup(
<I18nProvider initialLocale="en">{node}</I18nProvider>,
);
}
function result(over: Partial<CsvImportResult> = {}): CsvImportResult {
return {
inserted: 0,
updated: 0,
skipped: 0,
total: 0,
dryRun: false,
rows: [],
...over,
};
}
/** The production envelope from the report: nothing written, everything refused. */
const ALL_SKIPPED = result({
skipped: 1597,
total: 1597,
rows: Array.from({ length: 1597 }, (_, i) => ({
line: i + 2,
status: "skipped",
reason: "invalid_glucose_context",
})),
});
describe("<CsvImportResultView> — result states", () => {
it("renders an all-skipped import as a failure, not a success", () => {
const html = render(<CsvImportResultView result={ALL_SKIPPED} />);
expect(html).toContain('data-outcome="failed"');
expect(html).not.toContain('data-outcome="success"');
expect(html).not.toContain("text-success");
expect(html).toContain("Nothing was imported");
expect(html).toContain("All 1597 rows were skipped");
});
it("renders a mixed import as a warning", () => {
const html = render(
<CsvImportResultView
result={result({
inserted: 3,
skipped: 2,
total: 5,
rows: [
{ line: 2, status: "inserted" },
{ line: 3, status: "inserted" },
{ line: 4, status: "inserted" },
{ line: 5, status: "skipped", reason: "unknown_unit" },
{ line: 6, status: "skipped", reason: "duplicate" },
],
})}
/>,
);
expect(html).toContain('data-outcome="partial"');
expect(html).toContain("text-warning");
expect(html).not.toContain("text-success");
});
it("renders a clean import as a success", () => {
const html = render(
<CsvImportResultView
result={result({
inserted: 4,
updated: 1,
total: 5,
rows: [
{ line: 2, status: "inserted" },
{ line: 3, status: "inserted" },
{ line: 4, status: "inserted" },
{ line: 5, status: "inserted" },
{ line: 6, status: "updated" },
],
})}
/>,
);
expect(html).toContain('data-outcome="success"');
expect(html).toContain("text-success");
expect(html).toContain("Imported 4 new and updated 1.");
expect(html).not.toContain("skipped");
});
it("gives a file with no data rows its own message", () => {
const html = render(<CsvImportResultView result={result()} />);
expect(html).toContain('data-outcome="empty"');
expect(html).not.toContain("text-success");
expect(html).toContain("no data rows");
});
it("reports a preview that would import nothing as a failure", () => {
const html = render(
<CsvImportResultView result={{ ...ALL_SKIPPED, dryRun: true }} />,
);
expect(html).toContain('data-outcome="failed"');
expect(html).toContain("nothing would import");
});
});
describe("<CsvImportResultView> — skip reasons", () => {
it("groups a repeated reason into one counted line, in plain language", () => {
const html = render(<CsvImportResultView result={ALL_SKIPPED} />);
expect(html).toContain(
"1597 rows skipped: glucose context is not one of the allowed values",
);
// One grouped line, not 1 597 (and not the old first-fifty list).
const groups = html.split('data-testid="import-csv-skip-groups"')[1] ?? "";
expect(groups.split("</li>").length - 1).toBeGreaterThan(0);
expect(html.match(/1597 rows skipped/g)).toHaveLength(1);
});
it("keeps the per-line detail behind a disclosure", () => {
const html = render(
<CsvImportResultView
result={result({
skipped: 1,
total: 1,
rows: [{ line: 2, status: "skipped", reason: "unknown_unit" }],
})}
/>,
);
expect(html).toContain("<details");
expect(html).toContain("Show every skipped row");
expect(html).toContain("Line 2: unit not recognised for this type");
});
it("labels a reason it does not know rather than leaking the raw token", () => {
const html = render(
<CsvImportResultView
result={result({
skipped: 1,
total: 1,
rows: [{ line: 2, status: "skipped", reason: "brand_new_reason" }],
})}
/>,
);
expect(html).not.toContain("brand_new_reason");
expect(html).toContain("1 row skipped: skipped");
});
});
describe("<CsvImportCard> — announcement", () => {
it("keeps the result slot inside the polite live region", () => {
const html = render(<CsvImportCard />);
expect(html).toContain('aria-live="polite"');
// No result yet: the region is present and empty, so the first outcome to
// land is announced rather than being read as initial content.
expect(html).not.toContain('data-testid="import-csv-result"');
});
});
@@ -2,7 +2,7 @@
import { useCallback, useId, useRef, useState } from "react";
import { useQuery } from "@tanstack/react-query";
import { AlertCircle, CheckCircle2, Loader2, Upload } from "lucide-react";
import { AlertCircle, Loader2, Upload } from "lucide-react";
import { Button } from "@/components/ui/button";
import { Progress } from "@/components/ui/progress";
@@ -11,6 +11,8 @@ import { useTranslations } from "@/lib/i18n/context";
import { cn } from "@/lib/utils";
import { apiFetchRaw, apiGet } from "@/lib/api/api-fetch";
import { ImportCardShell } from "./import-card-shell";
import { classifyImportOutcome } from "./import-result-state";
import { ImportOutcomeLine } from "./import-result-view";
/** Terminal states the status poll stops on. */
const TERMINAL_STATES: readonly string[] = ["done", "failed"];
@@ -237,22 +239,28 @@ export function AppleHealthImportCard() {
</div>
)}
{isDone && (
<div
data-testid="import-apple-health-result"
className="text-foreground flex items-start gap-2 text-xs"
>
<CheckCircle2
className="text-success mt-0.5 h-3.5 w-3.5 shrink-0"
aria-hidden="true"
/>
<span>
{t("settings.sections.export.import.appleHealth.doneSummary", {
imported: status?.result?.totals?.rowsUpserted ?? 0,
read: status?.result?.totals?.recordsRead ?? 0,
skipped: status?.result?.clinical?.skipped ?? 0,
})}
</span>
</div>
<ImportOutcomeLine
outcome={classifyImportOutcome({
written: status?.result?.totals?.rowsUpserted ?? 0,
// The clinical count is a documented, deliberate exclusion
// (electrocardiograms, lab documents), not a refused row — it
// must not tip the archive into a warning. Only "nothing
// landed" changes the tone here.
skipped: 0,
})}
message={
(status?.result?.totals?.rowsUpserted ?? 0) > 0
? t("settings.sections.export.import.appleHealth.doneSummary", {
imported: status?.result?.totals?.rowsUpserted ?? 0,
read: status?.result?.totals?.recordsRead ?? 0,
skipped: status?.result?.clinical?.skipped ?? 0,
})
: t("settings.sections.export.import.appleHealth.doneNothing", {
read: status?.result?.totals?.recordsRead ?? 0,
})
}
testId="import-apple-health-result"
/>
)}
{isDone && (status?.result?.cumulativeEstimates?.days ?? 0) > 0 && (
<AppleHealthEstimateWarning