From 0c9df88a45bf1797215df0769ab442d0c34f5547 Mon Sep 17 00:00:00 2001 From: Lucas Winther Date: Fri, 4 Sep 2026 23:55:38 +0200 Subject: [PATCH] fix(recurring): refuse a confirmation date that stops the series A confirmation is an ordinary Purchase, and the ordinary purchase dialog can edit its date. Under lastCharge anchoring that date is what the walk measures the next occurrence from, so moving it back more than a cycle made the walk step backwards and throw out of foldBalances -- every month route, the year view and the summary down at once, from an edit that never mentioned recurring costs. updatePurchase is the write that can invalidate an already-valid confirmation, so it now asks wouldAdvancePast -- the same helper ExpectedBand asks before offering a row -- and refuses the edit instead. The walk's throw is unchanged: a step that lands backwards really is bad data, and this stops it being created. A purchase with no source is not checked at all, and a source naming a cost that no longer exists falls through. PurchaseDialog was closing on save regardless of the outcome, with its useMutate error rendered nowhere, so a refusal looked like a save that did nothing. mutate now reports whether the write landed, and the dialog stays open and shows the reason when it did not. Co-Authored-By: Claude Opus 5 (1M context) --- src/store/actions.test.ts | 99 ++++++++++++++++++++++++++++ src/store/actions.ts | 47 +++++++++++++ src/ui/components/PurchaseDialog.tsx | 45 ++++++++----- src/ui/hooks/useMutate.ts | 18 +++-- 4 files changed, 188 insertions(+), 21 deletions(-) diff --git a/src/store/actions.test.ts b/src/store/actions.test.ts index a6a9c60..aef132a 100644 --- a/src/store/actions.test.ts +++ b/src/store/actions.test.ts @@ -820,3 +820,102 @@ describe("confirmOccurrence", () => { expect(() => actions.confirmOccurrence(draft(), "nope", "2026-03")).toThrow(/Unknown recurring cost/); }); }); + +/** + * Confirming a slot writes an ORDINARY purchase, and the ordinary purchase + * dialog can then edit its date — an action with nothing on its face to do + * with recurring costs. Under `lastCharge` anchoring that date is what the + * walk measures the next occurrence from, so moving it back far enough makes + * the walk step backwards and throw out of `foldBalances`, taking every month + * route, the year view and the summary down together. The write is refused + * here so that state cannot be reached. + */ +describe("editing a confirmation's date", () => { + function carDraft(anchoring: "calendar" | "lastCharge" = "lastCharge"): Dataset { + const data = createSeedDataset("2026-08"); + data.posts[0]!.id = "housing"; + actions.addRecurringCost(data, { + name: "Car insurance", + archived: false, + amount: { amount: 900, currency: "DKK" }, + startDate: "2026-08-14", + recurrence: { kind: "everyNDays", n: 30 }, + anchoring, + splitMode: "percent", + splits: [{ postId: "housing", value: 100, absorbsRemainder: true }], + }); + return data; + } + + /** The walk every month route, the year view and the summary go through. */ + function folds(data: Dataset): () => unknown { + return () => occurrencesByMonth(data, "2026-12"); + } + + function dateOf(data: Dataset, purchaseId: string): string { + return data.purchases.find((p) => p.id === purchaseId)!.date; + } + + test("a date more than one cycle before the slot it claims is refused", () => { + const data = carDraft(); + const purchase = actions.confirmOccurrence(data, data.recurring[0]!.id, "2026-09-13", { + date: "2026-09-13", + }); + expect(folds(data)).not.toThrow(); + + expect(() => actions.updatePurchase(data, purchase.id, { date: "2026-08-01" })).toThrow( + /Car insurance/, + ); + expect(dateOf(data, purchase.id)).toBe("2026-09-13"); + expect(folds(data)).not.toThrow(); + }); + + test("a date that keeps the series advancing is still allowed", () => { + const data = carDraft(); + const purchase = actions.confirmOccurrence(data, data.recurring[0]!.id, "2026-09-13", { + date: "2026-09-13", + }); + + actions.updatePurchase(data, purchase.id, { date: "2026-09-08" }); + expect(dateOf(data, purchase.id)).toBe("2026-09-08"); + expect(folds(data)).not.toThrow(); + }); + + test("a purchase with no slot is an ordinary purchase and moves freely", () => { + const data = carDraft(); + const purchase = actions.addPurchase(data, { + date: "2026-09-13", + description: "Sofa", + total: { amount: 100, currency: "DKK" }, + splitMode: "percent", + splits: [{ postId: "housing", value: 100, absorbsRemainder: true }], + schedule: null, + }); + + actions.updatePurchase(data, purchase.id, { date: "2026-01-01" }); + expect(dateOf(data, purchase.id)).toBe("2026-01-01"); + expect(folds(data)).not.toThrow(); + }); + + test("a calendar-anchored cost's confirmation moves freely, because it never rebases", () => { + const data = carDraft("calendar"); + const purchase = actions.confirmOccurrence(data, data.recurring[0]!.id, "2026-09-13", { + date: "2026-09-13", + }); + + actions.updatePurchase(data, purchase.id, { date: "2026-08-01" }); + expect(dateOf(data, purchase.id)).toBe("2026-08-01"); + expect(folds(data)).not.toThrow(); + }); + + test("a slot naming a cost that no longer exists does not block the edit", () => { + const data = carDraft(); + const purchase = actions.confirmOccurrence(data, data.recurring[0]!.id, "2026-09-13", { + date: "2026-09-13", + }); + data.recurring = []; + + actions.updatePurchase(data, purchase.id, { date: "2026-08-01" }); + expect(dateOf(data, purchase.id)).toBe("2026-08-01"); + }); +}); diff --git a/src/store/actions.ts b/src/store/actions.ts index c117c63..59093ac 100755 --- a/src/store/actions.ts +++ b/src/store/actions.ts @@ -3,6 +3,7 @@ import { compareMonths, monthOf } from "../domain/months.ts"; import { toDayOrdinal } from "../domain/days.ts"; import { roundMoney } from "../domain/money.ts"; import { currencyUsage, normalizeCurrencyCode } from "../domain/currencies.ts"; +import { wouldAdvancePast } from "../domain/occurrences.ts"; import type { Currency, CurrencyDef, @@ -220,12 +221,58 @@ export function addPurchase( return created; } +/** + * A confirmation's `date` is TRUTH, and under `lastCharge` anchoring it is + * what `occurrencesOf` measures the NEXT occurrence from. Moved back far + * enough, the step from it lands at or before the slot the purchase claims, + * and the walk throws its "did not advance" error — out of `foldBalances`, so + * every month route, the year view and the summary go down at once, from an + * edit made in the ordinary purchase dialog that never mentioned recurring + * costs. Refusing the write is the only place that state can be prevented: + * the walk's throw is correct and must not be softened, and by the time it + * fires the bad data is already saved. + * + * `wouldAdvancePast` answers exactly this question and is what `ExpectedBand` + * asks before OFFERING a row; both must agree with the walk, so neither + * re-derives the condition. + * + * Two deliberate pass-throughs. A purchase with no slot is an ordinary + * purchase and is not checked at all — this is the hot path for every + * ordinary edit. A slot naming a cost that no longer exists is not checked + * either: a dangling source is a separate concern, and refusing here would + * only strand the purchase. + */ +function requireConfirmationAdvances( + draft: Dataset, + purchase: Purchase, + source: Purchase["source"], + date: IsoDate, +): void { + if (!source) return; + const cost = draft.recurring.find((c) => c.id === source.recurringId); + if (!cost) return; + if (wouldAdvancePast(cost, source.occurrenceDate, date)) return; + throw new Error( + `Purchase ${purchase.id} confirms "${cost.name}" for ${source.occurrenceDate}, and ` + + `dating it ${date} would not move the series past that occurrence — ` + + `pick a later date, or delete the purchase to un-confirm the occurrence`, + ); +} + export function updatePurchase( draft: Dataset, purchaseId: PurchaseId, changes: Partial>, ): void { const purchase = requirePurchase(draft, purchaseId); + if (changes.date !== undefined && changes.date !== purchase.date) { + requireConfirmationAdvances( + draft, + purchase, + "source" in changes ? changes.source : purchase.source, + changes.date, + ); + } const resolved: Partial> = { ...changes }; if (changes.total) { resolved.total = roundMoneyValue(draft, changes.total); diff --git a/src/ui/components/PurchaseDialog.tsx b/src/ui/components/PurchaseDialog.tsx index d924217..ec4a814 100644 --- a/src/ui/components/PurchaseDialog.tsx +++ b/src/ui/components/PurchaseDialog.tsx @@ -43,7 +43,7 @@ interface Props { export function PurchaseDialog({ monthId, purchase, trigger }: Props) { const dataset = useDataset(); - const { mutate } = useMutate(); + const { mutate, error, clearError } = useMutate(); const activePosts = dataset.posts .filter((p) => !p.archived) .sort((a, b) => a.order - b.order); @@ -74,23 +74,25 @@ export function PurchaseDialog({ monthId, purchase, trigger }: Props) { setBulk(emptyBulkDraft(monthId, activePosts[0]?.id ?? "")); } + // A write can be REFUSED, not merely fail on a full disk: editing the date + // of a purchase that confirms a recurring occurrence is rejected when the + // new date would stop the series advancing. Closing the dialog regardless + // would show the user a save that silently did nothing, so the dialog stays + // open and shows `error` instead. `mutate` never rejects. function save() { if (errors.length > 0) return; - if (bulkMode) { - // One mutate for the whole batch: a single write, one queue entry, and no - // way to half-commit a shopping trip. - const created = toPurchases(bulk); - mutate((data) => { - for (const p of created) addPurchase(data, p); - }); - } else { - mutate((data) => { - if (purchase) updatePurchase(data, purchase.id, toPurchase(draft)); - else addPurchase(data, toPurchase(draft)); - }); - } - setOpen(false); - if (!purchase) reset(); + // One mutate for the whole batch: a single write, one queue entry, and no + // way to half-commit a shopping trip. + const created = bulkMode ? toPurchases(bulk) : null; + void mutate((data) => { + if (created) for (const p of created) addPurchase(data, p); + else if (purchase) updatePurchase(data, purchase.id, toPurchase(draft)); + else addPurchase(data, toPurchase(draft)); + }).then((saved) => { + if (!saved) return; + setOpen(false); + if (!purchase) reset(); + }); } return ( @@ -98,7 +100,10 @@ export function PurchaseDialog({ monthId, purchase, trigger }: Props) { open={open} onOpenChange={(next) => { setOpen(next); - if (next) reset(); + if (next) { + reset(); + clearError(); + } }} > {trigger} @@ -321,6 +326,12 @@ export function PurchaseDialog({ monthId, purchase, trigger }: Props) { ))} )} + + {error && ( +

+ Could not save: {error} +

+ )} diff --git a/src/ui/hooks/useMutate.ts b/src/ui/hooks/useMutate.ts index 57c4a99..c076f04 100644 --- a/src/ui/hooks/useMutate.ts +++ b/src/ui/hooks/useMutate.ts @@ -6,14 +6,24 @@ import type { Dataset } from "../../domain/types.ts"; * Wraps store.mutate so a failed write surfaces to the user instead of * vanishing into an unhandled rejection. The snapshot is unchanged on failure, * so the UI keeps showing the last saved state. + * + * `mutate` returns whether the write landed, and the returned promise never + * rejects — the error is already in `error` by the time it settles. A caller + * that only fires and forgets can ignore it; one that would otherwise dismiss + * its own UI (a dialog, say) can wait and keep it open, because a refusal the + * user cannot see is barely better than one that never happened. */ export function useMutate() { const [error, setError] = useState(null); - const mutate = useCallback((fn: (draft: Dataset) => void) => { - store.mutate(fn).catch((cause: unknown) => { - setError(cause instanceof Error ? cause.message : String(cause)); - }); + const mutate = useCallback((fn: (draft: Dataset) => void): Promise => { + return store.mutate(fn).then( + () => true, + (cause: unknown) => { + setError(cause instanceof Error ? cause.message : String(cause)); + return false; + }, + ); }, []); return { mutate, error, clearError: useCallback(() => setError(null), []) };