diff --git a/src/store/snapshot.test.ts b/src/store/snapshot.test.ts index 0d41120..e91d324 100644 --- a/src/store/snapshot.test.ts +++ b/src/store/snapshot.test.ts @@ -83,6 +83,73 @@ describe("mutate", () => { ).rejects.toThrow("disk full"); expect(store.get().posts[0]!.name).toBe("Video Games"); }); + + test("two concurrent mutate() calls both survive (no lost update)", async () => { + // Simulate real persistence latency: each write takes ~20ms, so if the + // two mutations clone concurrently instead of queueing, the second + // commit silently overwrites the first's change. + let stored = createSeedDataset("2026-09"); + const persistence: Persistence = { + async read() { + return stored; + }, + async write(dataset) { + await new Promise((resolve) => setTimeout(resolve, 20)); + stored = dataset; + }, + }; + const store = createSnapshotStore(persistence, "2026-09"); + await store.load(); + + const first = store.mutate((draft) => { + draft.months[0]!.income = { amount: 20000, currency: "DKK" }; + }); + const second = store.mutate((draft) => { + draft.posts[0]!.name = "Renamed"; + }); + await Promise.all([first, second]); + + expect(store.get().months[0]!.income.amount).toBe(20000); + expect(store.get().posts[0]!.name).toBe("Renamed"); + expect(stored.months[0]!.income.amount).toBe(20000); + expect(stored.posts[0]!.name).toBe("Renamed"); + }); + + test("a failing write mid-queue rejects only its own caller and does not wedge later mutations", async () => { + const fake = fakePersistence(createSeedDataset("2026-09")); + // The queue processes mutations strictly in order, so failing the + // SECOND write call (regardless of when each mutate() is invoked) + // reliably targets the middle mutation below. + let writeCount = 0; + const persistence: Persistence = { + read: fake.persistence.read, + async write(dataset) { + writeCount++; + if (writeCount === 2) throw new Error("disk full"); + await fake.persistence.write(dataset); + }, + }; + const store = createSnapshotStore(persistence, "2026-09"); + await store.load(); + + const okBefore = store.mutate((draft) => { + draft.months[0]!.income = { amount: 111, currency: "DKK" }; + }); + const failing = store.mutate((draft) => { + draft.posts[0]!.name = "Should not stick"; + }); + const okAfter = store.mutate((draft) => { + draft.posts[1]!.name = "Should stick"; + }); + + await okBefore; + await expect(failing).rejects.toThrow("disk full"); + await okAfter; + + expect(store.get().months[0]!.income.amount).toBe(111); + expect(store.get().posts[0]!.name).not.toBe("Should not stick"); + expect(store.get().posts[1]!.name).toBe("Should stick"); + }); }); test("replace swaps the whole dataset, as JSON import needs", async () => { diff --git a/src/store/snapshot.ts b/src/store/snapshot.ts index b013344..43fd5e1 100644 --- a/src/store/snapshot.ts +++ b/src/store/snapshot.ts @@ -23,6 +23,22 @@ export function createSnapshotStore( let snapshot: Dataset | null = null; const listeners = new Set(); + // All mutations run through this chain, one at a time, so a mutation + // always clones from the snapshot the previous mutation committed rather + // than from a stale snapshot captured before it. Without this, two + // mutations issued before either's write resolves both clone the same + // starting point and the one that commits last silently discards the + // other's change. + let queue: Promise = Promise.resolve(); + + function enqueue(work: () => Promise): Promise { + const run = queue.then(work); + // Keep the chain alive after a failure, but the caller still sees the + // rejection via `run`, which is returned unmodified below. + queue = run.catch(() => undefined); + return run; + } + function notify() { for (const listener of listeners) listener(); } @@ -53,14 +69,16 @@ export function createSnapshotStore( get, - async mutate(fn) { - const draft = structuredClone(get()); - fn(draft); - await commit(draft); + mutate(fn) { + return enqueue(async () => { + const draft = structuredClone(get()); + fn(draft); + await commit(draft); + }); }, - async replace(dataset) { - await commit(dataset); + replace(dataset) { + return enqueue(() => commit(dataset)); }, subscribe(listener) { diff --git a/src/ui/components/PlanEditor.tsx b/src/ui/components/PlanEditor.tsx index 9ef729d..66941ef 100644 --- a/src/ui/components/PlanEditor.tsx +++ b/src/ui/components/PlanEditor.tsx @@ -16,19 +16,42 @@ interface Props { onChange: (next: PurchaseDraft) => void; } +// A native reports "" until the user has filled in every +// field, and monthOf() throws on anything that isn't a well-formed +// YYYY-MM-DD. This is UI-level input validation, not a domain concern — the +// domain is right to throw on malformed input; the caller just must not call +// it with malformed input. +const ISO_DATE_PATTERN = /^\d{4}-\d{2}-\d{2}$/; + +function hasValidDate(draft: PurchaseDraft): boolean { + return ISO_DATE_PATTERN.test(draft.date); +} + export function PlanEditor({ draft, onChange }: Props) { const balance = planBalance(draft); if (!draft.plan) { + const dateReady = hasValidDate(draft); return ( - +
+ + {!dateReady && ( +

+ Pick a date for this purchase first. +

+ )} +
); } @@ -41,11 +64,12 @@ export function PlanEditor({ draft, onChange }: Props) { { const months = Number(event.target.value); - if (months >= 1) { + if (Number.isInteger(months) && months >= 1) { onChange(withPlan(draft, draft.plan!.startMonth, months)); } }} diff --git a/src/ui/routes/settings/DataSection.tsx b/src/ui/routes/settings/DataSection.tsx index 91a1b25..fba6e99 100644 --- a/src/ui/routes/settings/DataSection.tsx +++ b/src/ui/routes/settings/DataSection.tsx @@ -18,20 +18,44 @@ export function DataSection() { const [pending, setPending] = useState(null); const [error, setError] = useState(null); + // Shared by every export button and by the pre-import backup below. + // `buildOds` in particular can throw (e.g. MissingRateError when a + // currency in use has no configured rate), and a throw out of a plain + // onClick handler is invisible to React's error boundary — so every + // export path funnels through here and reports failure via the same + // error state the import flow already uses. Returns whether it succeeded, + // so callers that depend on the export (the import backup) can tell. + function runExport(build: () => { filename: string; blob: Blob }): boolean { + try { + setError(null); + const { filename, blob } = build(); + downloadBlob(filename, blob); + return true; + } catch (cause) { + const message = cause instanceof Error ? cause.message : String(cause); + setError(`Could not export: ${message}`); + return false; + } + } + + function buildJsonExport() { + return { + filename: exportFilename(currentMonth), + blob: new Blob([exportDatasetJson(dataset)], { type: "application/json" }), + }; + } + function exportJson() { - downloadBlob( - exportFilename(currentMonth), - new Blob([exportDatasetJson(dataset)], { type: "application/json" }), - ); + runExport(buildJsonExport); } function exportOds() { - downloadBlob( - odsFilename(currentMonth), - new Blob([buildOds(dataset) as BlobPart], { + runExport(() => ({ + filename: odsFilename(currentMonth), + blob: new Blob([buildOds(dataset) as BlobPart], { type: "application/vnd.oasis.opendocument.spreadsheet", }), - ); + })); } async function chooseFile(file: File) { @@ -40,7 +64,8 @@ export function DataSection() { setPending(parseDatasetJson(await file.text())); } catch (cause) { setPending(null); - setError(cause instanceof Error ? cause.message : String(cause)); + const message = cause instanceof Error ? cause.message : String(cause); + setError(`Could not import: ${message}`); } } @@ -48,8 +73,9 @@ export function DataSection() { if (!pending) return; // Back up first: import replaces everything, and this is the user's only // copy. Order matters — the backup must download before the destructive - // replace, never after. - exportJson(); + // replace, never after. If the backup itself fails, abort rather than + // destroy the only copy of the user's data. + if (!runExport(buildJsonExport)) return; await store.replace(pending); setPending(null); } @@ -87,7 +113,7 @@ export function DataSection() { /> - {error &&

Could not import: {error}

} + {error &&

{error}

} {pending && (