mirror of
https://github.com/MBombeck/HealthLog.git
synced 2026-11-09 13:01:00 +00:00
test(medications): close the gaps the mutation pass found
Three things the tests said were covered and were not. A column flipped from honoured to ignored moved both sides of its own assertion, because the check read the verdict off the table it was checking. The verdicts are now written out as literals, so changing one is a decision somebody states twice. The preview block was unreachable from an SSR-only test, so nothing would have noticed it rendering through the outcome view — which would classify what a preview *would* write as a success and put a tick on a run that never ran. It moves into its own component that takes the verdict as a prop, the way the CSV result view already does, and the preview and the archived-row report are asserted directly. The account-wide import narrows its in-progress check to account-wide jobs, so an import running on one medication's page does not refuse it. Nothing pinned that; now something does. Also correct two comments about the earlier replay-key fault. The repeated value in the reported file was not a dose quantity — it was copied from the example this dialog itself displays. The field never carried identity, and the submission was never at fault.
This commit is contained in:
1 parent
8a36117a0c
commit
36d589e11b
8 files changed
+274
-107
No files matched your search
@@ -28,12 +28,17 @@ import { isPlausibleEntryInstant } from "@/lib/validations/entry-instant";
|
||||
|
||||
// The entry shape carried a third field, `zaehler`, and the replay key was
|
||||
// built from it whenever it was present. That reads as a per-row counter,
|
||||
// which is what the documented example showed — but the field was optional,
|
||||
// its uniqueness was never stated and never checked, and an export that puts
|
||||
// the dose quantity there (1 tablet, on every row) collapsed a month of
|
||||
// history onto one key: the first row imported, every later one lost the
|
||||
// `(userId, idempotencyKey)` unique and came back reported as a duplicate of
|
||||
// a dose it had nothing to do with.
|
||||
// which is what the example beside this dialog showed — but the field was
|
||||
// optional, its uniqueness was never stated and never checked, and nothing
|
||||
// told anyone it carried identity. A submission that repeated one value down
|
||||
// the column collapsed a month of history onto one key: the first row
|
||||
// imported, every later one lost the `(userId, idempotencyKey)` unique and
|
||||
// came back reported as a duplicate of a dose it had nothing to do with.
|
||||
//
|
||||
// The reported file repeated `1.0` because it was modelled on the example this
|
||||
// dialog itself displayed, which counted upward and explained nothing. So the
|
||||
// fault was never in the submission; it was in a field that decided identity
|
||||
// while presenting itself as a quantity.
|
||||
//
|
||||
// `zaehler` is gone from the contract rather than accepted-and-ignored. A
|
||||
// payload that still carries it imports unchanged — `z.object` drops
|
||||
|
||||
@@ -241,6 +241,30 @@ describe("POST /api/medications/intake/dose-history-import", () => {
|
||||
expect(prisma.medicationIntakeImportJob.create).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("looks only at account-wide work when deciding whether one is already running", async () => {
|
||||
await POST(postReq(CSV));
|
||||
// The in-progress check and the stale sweep both narrow on
|
||||
// `medicationId: null`. Without that narrowing, an import running on a
|
||||
// single medication's own page would refuse this one — and the sweep would
|
||||
// reach across and fail it.
|
||||
expect(prisma.medicationIntakeImportJob.findFirst).toHaveBeenCalledWith({
|
||||
where: {
|
||||
userId: "user-1",
|
||||
medicationId: null,
|
||||
status: { in: ["queued", "running"] },
|
||||
},
|
||||
select: { id: true },
|
||||
});
|
||||
expect(prisma.medicationIntakeImportJob.updateMany).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
where: expect.objectContaining({
|
||||
userId: "user-1",
|
||||
medicationId: null,
|
||||
}),
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
it("fails the job and answers 503 when the worker cannot take it", async () => {
|
||||
vi.mocked(getGlobalBoss).mockReturnValue(null as never);
|
||||
const res = await POST(postReq(CSV));
|
||||
|
||||
@@ -27,6 +27,10 @@ import { IntakeImportSkipGroups } from "@/components/medications/intake-import-r
|
||||
|
||||
import { DoseHistoryColumnRulings } from "../dose-history-columns";
|
||||
import { DoseHistoryImportCard } from "../dose-history-import-card";
|
||||
import {
|
||||
DoseHistoryVerdictView,
|
||||
type DoseHistoryFileVerdict,
|
||||
} from "../dose-history-verdict";
|
||||
|
||||
function render(node: React.ReactElement) {
|
||||
return renderToStaticMarkup(
|
||||
@@ -82,6 +86,72 @@ describe("<DoseHistoryImportCard>", () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe("<DoseHistoryVerdictView>", () => {
|
||||
const verdict: DoseHistoryFileVerdict = {
|
||||
rowsRead: 3395,
|
||||
queued: 3387,
|
||||
refused: 8,
|
||||
refusedByReason: [{ reason: "status_not_recorded", count: 8 }],
|
||||
unmatchedMedications: [],
|
||||
ambiguousMedications: [],
|
||||
mirroredMedications: [],
|
||||
unknownColumns: [],
|
||||
codingsNotRead: 0,
|
||||
fromArchivedMedications: 1462,
|
||||
};
|
||||
|
||||
it("shows a preview's refusals without claiming anything was written", () => {
|
||||
const html = render(
|
||||
<DoseHistoryVerdictView verdict={verdict} previewOnly />,
|
||||
);
|
||||
expect(html).toContain('data-preview="true"');
|
||||
expect(html).toContain('data-testid="dose-history-refusal-groups"');
|
||||
expect(html).toContain("8 entries skipped");
|
||||
// A preview has written nothing. Rendering it through the outcome view
|
||||
// would classify 3,387-would-be-written as a success and put a tick on a
|
||||
// run that never ran.
|
||||
expect(html).not.toContain("data-outcome");
|
||||
expect(html).not.toContain("text-success");
|
||||
expect(html).not.toContain("intake-import-outcome");
|
||||
});
|
||||
|
||||
it("reports the archived rows as imported rather than as a reason to drop them", () => {
|
||||
const html = render(
|
||||
<DoseHistoryVerdictView verdict={verdict} previewOnly={false} />,
|
||||
);
|
||||
expect(html).toContain('data-testid="dose-history-archived"');
|
||||
expect(html).toContain("1462");
|
||||
expect(html).toContain("imported anyway");
|
||||
// Archived rows are reported, never refused.
|
||||
expect(html).not.toContain("dose-history-unmatched");
|
||||
});
|
||||
|
||||
it("names each medication it could not use, and why it could not", () => {
|
||||
const html = render(
|
||||
<DoseHistoryVerdictView
|
||||
previewOnly
|
||||
verdict={{
|
||||
...verdict,
|
||||
unmatchedMedications: ["Ramipril"],
|
||||
ambiguousMedications: ["Metformin"],
|
||||
mirroredMedications: ["Vitamin D"],
|
||||
unknownColumns: ["Mood"],
|
||||
codingsNotRead: 4,
|
||||
}}
|
||||
/>,
|
||||
);
|
||||
expect(html).toContain("Ramipril");
|
||||
expect(html).toContain("Metformin");
|
||||
expect(html).toContain("Vitamin D");
|
||||
expect(html).toContain("Mood");
|
||||
expect(html).toContain("No medication on record matches");
|
||||
expect(html).toContain("Matches more than one medication");
|
||||
expect(html).toContain("Mirrored from another app");
|
||||
expect(html).toContain("no rule for");
|
||||
expect(html).toContain("Codings value that was not read");
|
||||
});
|
||||
});
|
||||
|
||||
describe("refusal reason sentences", () => {
|
||||
it("has plain language for every reason the importer can report", () => {
|
||||
const html = render(
|
||||
|
||||
@@ -52,6 +52,10 @@ const IMPORT_SURFACES: ReadonlyArray<{ path: string; through: RegExp }> = [
|
||||
path: "src/components/settings/import-panel/dose-history-import-card.tsx",
|
||||
through: /IntakeImportResultView/,
|
||||
},
|
||||
{
|
||||
path: "src/components/settings/import-panel/dose-history-verdict.tsx",
|
||||
through: /IntakeImportSkipGroups/,
|
||||
},
|
||||
];
|
||||
|
||||
/** The success affordance: the tick component and the success colour token. */
|
||||
|
||||
@@ -23,7 +23,6 @@ import { AlertCircle, Loader2, PillBottle, Upload } from "lucide-react";
|
||||
|
||||
import {
|
||||
IntakeImportResultView,
|
||||
IntakeImportSkipGroups,
|
||||
type IntakeImportResultState,
|
||||
} from "@/components/medications/intake-import-result";
|
||||
import { Button } from "@/components/ui/button";
|
||||
@@ -38,23 +37,13 @@ import {
|
||||
import { invalidateMedicationReads } from "@/lib/query-keys";
|
||||
|
||||
import { DoseHistoryColumnRulings } from "./dose-history-columns";
|
||||
import {
|
||||
DoseHistoryVerdictView,
|
||||
type DoseHistoryFileVerdict,
|
||||
} from "./dose-history-verdict";
|
||||
import { ImportCardShell } from "./import-card-shell";
|
||||
import { MAX_PASTE_CHARS } from "./constants";
|
||||
|
||||
/** The file-level verdict, exactly as the route returns it. */
|
||||
interface DoseHistoryFileVerdict {
|
||||
rowsRead: number;
|
||||
queued: number;
|
||||
refused: number;
|
||||
refusedByReason: Array<{ reason: string; count: number }>;
|
||||
unmatchedMedications: string[];
|
||||
ambiguousMedications: string[];
|
||||
mirroredMedications: string[];
|
||||
unknownColumns: string[];
|
||||
codingsNotRead: number;
|
||||
fromArchivedMedications: number;
|
||||
}
|
||||
|
||||
interface DoseHistoryKickoff {
|
||||
dryRun: boolean;
|
||||
jobId: string | null;
|
||||
@@ -255,86 +244,7 @@ export function DoseHistoryImportCard() {
|
||||
|
||||
<div aria-live="polite" className="space-y-2">
|
||||
{verdict && (
|
||||
<div
|
||||
data-testid="dose-history-verdict"
|
||||
className="space-y-1 text-xs"
|
||||
data-preview={previewOnly ? "true" : "false"}
|
||||
>
|
||||
<p className="text-foreground">
|
||||
{previewOnly
|
||||
? t("settings.sections.export.import.doseHistory.previewLine", {
|
||||
rows: verdict.rowsRead,
|
||||
queued: verdict.queued,
|
||||
refused: verdict.refused,
|
||||
})
|
||||
: t(
|
||||
"settings.sections.export.import.doseHistory.submittedLine",
|
||||
{
|
||||
rows: verdict.rowsRead,
|
||||
queued: verdict.queued,
|
||||
},
|
||||
)}
|
||||
</p>
|
||||
{/* The reason lines, never the outcome sentence: a preview has
|
||||
written nothing, and classifying it by the counts it would
|
||||
produce would put a success tick on a run that never ran. */}
|
||||
<IntakeImportSkipGroups
|
||||
groups={verdict.refusedByReason}
|
||||
testId="dose-history-refusal-groups"
|
||||
/>
|
||||
{verdict.unmatchedMedications.length > 0 && (
|
||||
<p
|
||||
className="text-muted-foreground"
|
||||
data-testid="dose-history-unmatched"
|
||||
>
|
||||
{t("settings.sections.export.import.doseHistory.unmatched", {
|
||||
names: verdict.unmatchedMedications.join(", "),
|
||||
})}
|
||||
</p>
|
||||
)}
|
||||
{verdict.ambiguousMedications.length > 0 && (
|
||||
<p className="text-muted-foreground">
|
||||
{t("settings.sections.export.import.doseHistory.ambiguous", {
|
||||
names: verdict.ambiguousMedications.join(", "),
|
||||
})}
|
||||
</p>
|
||||
)}
|
||||
{verdict.mirroredMedications.length > 0 && (
|
||||
<p className="text-muted-foreground">
|
||||
{t("settings.sections.export.import.doseHistory.mirrored", {
|
||||
names: verdict.mirroredMedications.join(", "),
|
||||
})}
|
||||
</p>
|
||||
)}
|
||||
{verdict.unknownColumns.length > 0 && (
|
||||
<p className="text-muted-foreground">
|
||||
{t(
|
||||
"settings.sections.export.import.doseHistory.unknownColumns",
|
||||
{ names: verdict.unknownColumns.join(", ") },
|
||||
)}
|
||||
</p>
|
||||
)}
|
||||
{verdict.codingsNotRead > 0 && (
|
||||
<p className="text-muted-foreground">
|
||||
{t(
|
||||
"settings.sections.export.import.doseHistory.codingsNotRead",
|
||||
{
|
||||
count: verdict.codingsNotRead,
|
||||
},
|
||||
)}
|
||||
</p>
|
||||
)}
|
||||
{verdict.fromArchivedMedications > 0 && (
|
||||
<p
|
||||
className="text-muted-foreground"
|
||||
data-testid="dose-history-archived"
|
||||
>
|
||||
{t("settings.sections.export.import.doseHistory.archivedRows", {
|
||||
count: verdict.fromArchivedMedications,
|
||||
})}
|
||||
</p>
|
||||
)}
|
||||
</div>
|
||||
<DoseHistoryVerdictView verdict={verdict} previewOnly={previewOnly} />
|
||||
)}
|
||||
{outcome && <IntakeImportResultView result={outcome} />}
|
||||
{error && (
|
||||
|
||||
@@ -0,0 +1,113 @@
|
||||
"use client";
|
||||
|
||||
/**
|
||||
* What the importer found in the file, before or alongside what it wrote.
|
||||
*
|
||||
* Split out of the card so each state can be rendered from a prop and asserted
|
||||
* directly — the card itself owns a file input, a query client and a poll loop,
|
||||
* and the thing that has to be right is what this block claims.
|
||||
*
|
||||
* It deliberately does NOT render the outcome sentence. A preview has written
|
||||
* nothing, so classifying it by the counts it *would* produce would put a success
|
||||
* tick on a run that never ran — the same defect, pointed one step earlier, as
|
||||
* the one that made an import of nothing look finished. The reason lines are
|
||||
* shared with the real outcome; the verdict is not.
|
||||
*/
|
||||
import { IntakeImportSkipGroups } from "@/components/medications/intake-import-result";
|
||||
import { useTranslations } from "@/lib/i18n/context";
|
||||
|
||||
/** The file-level verdict, exactly as the route returns it. */
|
||||
export interface DoseHistoryFileVerdict {
|
||||
rowsRead: number;
|
||||
queued: number;
|
||||
refused: number;
|
||||
refusedByReason: Array<{ reason: string; count: number }>;
|
||||
unmatchedMedications: string[];
|
||||
ambiguousMedications: string[];
|
||||
mirroredMedications: string[];
|
||||
unknownColumns: string[];
|
||||
codingsNotRead: number;
|
||||
fromArchivedMedications: number;
|
||||
}
|
||||
|
||||
export function DoseHistoryVerdictView({
|
||||
verdict,
|
||||
previewOnly,
|
||||
}: {
|
||||
verdict: DoseHistoryFileVerdict;
|
||||
previewOnly: boolean;
|
||||
}) {
|
||||
const { t } = useTranslations();
|
||||
return (
|
||||
<div
|
||||
data-testid="dose-history-verdict"
|
||||
className="space-y-1 text-xs"
|
||||
data-preview={previewOnly ? "true" : "false"}
|
||||
>
|
||||
<p className="text-foreground">
|
||||
{previewOnly
|
||||
? t("settings.sections.export.import.doseHistory.previewLine", {
|
||||
rows: verdict.rowsRead,
|
||||
queued: verdict.queued,
|
||||
refused: verdict.refused,
|
||||
})
|
||||
: t("settings.sections.export.import.doseHistory.submittedLine", {
|
||||
rows: verdict.rowsRead,
|
||||
queued: verdict.queued,
|
||||
})}
|
||||
</p>
|
||||
<IntakeImportSkipGroups
|
||||
groups={verdict.refusedByReason}
|
||||
testId="dose-history-refusal-groups"
|
||||
/>
|
||||
{verdict.unmatchedMedications.length > 0 && (
|
||||
<p
|
||||
className="text-muted-foreground"
|
||||
data-testid="dose-history-unmatched"
|
||||
>
|
||||
{t("settings.sections.export.import.doseHistory.unmatched", {
|
||||
names: verdict.unmatchedMedications.join(", "),
|
||||
})}
|
||||
</p>
|
||||
)}
|
||||
{verdict.ambiguousMedications.length > 0 && (
|
||||
<p className="text-muted-foreground">
|
||||
{t("settings.sections.export.import.doseHistory.ambiguous", {
|
||||
names: verdict.ambiguousMedications.join(", "),
|
||||
})}
|
||||
</p>
|
||||
)}
|
||||
{verdict.mirroredMedications.length > 0 && (
|
||||
<p className="text-muted-foreground">
|
||||
{t("settings.sections.export.import.doseHistory.mirrored", {
|
||||
names: verdict.mirroredMedications.join(", "),
|
||||
})}
|
||||
</p>
|
||||
)}
|
||||
{verdict.unknownColumns.length > 0 && (
|
||||
<p className="text-muted-foreground">
|
||||
{t("settings.sections.export.import.doseHistory.unknownColumns", {
|
||||
names: verdict.unknownColumns.join(", "),
|
||||
})}
|
||||
</p>
|
||||
)}
|
||||
{verdict.codingsNotRead > 0 && (
|
||||
<p className="text-muted-foreground">
|
||||
{t("settings.sections.export.import.doseHistory.codingsNotRead", {
|
||||
count: verdict.codingsNotRead,
|
||||
})}
|
||||
</p>
|
||||
)}
|
||||
{verdict.fromArchivedMedications > 0 && (
|
||||
<p
|
||||
className="text-muted-foreground"
|
||||
data-testid="dose-history-archived"
|
||||
>
|
||||
{t("settings.sections.export.import.doseHistory.archivedRows", {
|
||||
count: verdict.fromArchivedMedications,
|
||||
})}
|
||||
</p>
|
||||
)}
|
||||
</div>
|
||||
);
|
||||
}
|
||||
@@ -26,6 +26,43 @@ import {
|
||||
import { AUTO_EXPORT_REFUSAL_REASONS } from "../auto-export-parse";
|
||||
|
||||
describe("export column rulings", () => {
|
||||
/**
|
||||
* The verdicts, spelled out.
|
||||
*
|
||||
* Every other assertion here reads the verdict off the same table it is
|
||||
* checking, so a column flipped from `honoured` to `ignored` would move both
|
||||
* sides of the comparison and pass — the importer would quietly stop using a
|
||||
* column and the card would quietly say so, with nothing failing in between.
|
||||
* Restating them as literals is what makes such a flip a decision somebody has
|
||||
* to write down twice.
|
||||
*/
|
||||
const EXPECTED_VERDICTS: Record<string, string> = {
|
||||
Date: "honoured",
|
||||
"Scheduled Date": "honoured",
|
||||
Medication: "honoured",
|
||||
Nickname: "honoured",
|
||||
Dosage: "honoured",
|
||||
"Scheduled Dosage": "honoured",
|
||||
Unit: "honoured",
|
||||
Status: "honoured",
|
||||
Archived: "reported",
|
||||
Codings: "ignored",
|
||||
form: "ignored",
|
||||
start: "ignored",
|
||||
end: "ignored",
|
||||
};
|
||||
|
||||
it("holds the verdict each column was decided to have", () => {
|
||||
expect(
|
||||
Object.fromEntries(
|
||||
AUTO_EXPORT_COLUMN_RULINGS.map((ruling) => [
|
||||
ruling.column,
|
||||
ruling.verdict,
|
||||
]),
|
||||
),
|
||||
).toEqual(EXPECTED_VERDICTS);
|
||||
});
|
||||
|
||||
it("rules on every CSV column exactly once", () => {
|
||||
const ruled = AUTO_EXPORT_COLUMN_RULINGS.filter(
|
||||
(ruling) => !ruling.jsonOnly,
|
||||
|
||||
@@ -19,15 +19,19 @@ export interface MedicationImportQueueEntry {
|
||||
}
|
||||
|
||||
/**
|
||||
* An imported dose is identified by when it was taken, and by nothing else.
|
||||
* An imported dose is identified by its medication and its instant, and by
|
||||
* nothing else.
|
||||
*
|
||||
* The key used to be built from the row's optional `zaehler` field whenever it
|
||||
* was present. That reads as a per-row counter, but nothing required it to be
|
||||
* one, and an export that put the dose quantity there — 1 tablet, on every row
|
||||
* — collapsed a month of distinct doses onto a single key. Deriving from the
|
||||
* instant makes the key a function of the fact it identifies, and matches the
|
||||
* live-row unique on `(user_id, medication_id, scheduled_for, source)` that the
|
||||
* database enforces regardless.
|
||||
* one and nothing checked. The file that surfaced this repeated `1.0` down the
|
||||
* column because it was modelled on the example the import dialog itself
|
||||
* displayed — so the value came from us, and a field we presented as a quantity
|
||||
* silently decided identity. A month of distinct doses collapsed onto one key.
|
||||
*
|
||||
* Deriving from the instant makes the key a function of the fact it identifies,
|
||||
* and matches the live-row unique on `(user_id, medication_id, scheduled_for,
|
||||
* source)` that the database enforces regardless.
|
||||
*/
|
||||
export function buildMedicationImportEntries(
|
||||
medicationId: string,
|
||||
|
||||
Reference in new issue
Block a user