From 35caa123cc1505e4b1eca475e97b4a2b8ab6c136 Mon Sep 17 00:00:00 2001 From: Lucas Winther Date: Sat, 5 Sep 2026 23:55:54 +0200 Subject: [PATCH] fix(purchase): stop offerableCosts crashing render on an empty date MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Finding I3 (Critical): PurchaseDialog calls offerableCosts(dataset, draft.date) unconditionally in its render body. The native lets the owner clear it, setting draft.date to "" — monthOf("") throws, uncaught, during render. Unlike an event-handler throw an error boundary can react to, this crashes the dialog with no interaction to undo. monthOf itself stays strict per AGENTS.md (no softening a domain throw into a fallback): offerableCosts now wraps it in a UI-layer isParseableMonth check and returns no costs for an empty (or otherwise unparseable) date, rather than reporting an error for a form the owner simply hasn't finished. Checked the other recurringForm.ts helpers that take a date for the same hole: - newCostOptionIsSelectable had it too — "everyNMonths"+"calendar" read as selectable for ANY date length, including empty, though seedCostFromDraft still hands that combo's date to monthOf. Fixed the same way, so every "+ New recurring cost..." option is disabled while the date is empty. - sourceFor and seedCostFromDraft are only ever called at save time, inside the guarded store.mutate, never from render — left strict, since a bad date there already surfaces as a caught save-time error exactly like any other bad-date save (ensureMonth's monthOf in addPurchase). Documented and tested that determination rather than inventing a second rule for it. Co-Authored-By: Claude Opus 5 (1M context) --- src/ui/recurringForm.test.ts | 86 ++++++++++++++++++++++++++++++++++++ src/ui/recurringForm.ts | 50 +++++++++++++++++++++ 2 files changed, 136 insertions(+) diff --git a/src/ui/recurringForm.test.ts b/src/ui/recurringForm.test.ts index c6042c5..9e1073a 100644 --- a/src/ui/recurringForm.test.ts +++ b/src/ui/recurringForm.test.ts @@ -53,6 +53,21 @@ describe("offerableCosts", () => { actions.endRecurringCost(d, d.recurring[0]!.id, "2026-09"); expect(offerableCosts(d, "2026-09-20").map((c) => c.name)).not.toContain("Rent"); }); + + // I3 (Critical): the purchase dialog calls this unconditionally on every + // render, and the native lets the owner clear it, + // making `date` "". `monthOf("")` throws — uncaught, that took the render + // down with no interaction to undo. An empty date is a half-finished form, + // not an error, so this returns no costs rather than throwing. + test("an empty date offers no costs, rather than throwing", () => { + const d = withCosts(); + expect(offerableCosts(d, "")).toEqual([]); + }); + + test("any other unparseable date also offers no costs, rather than throwing", () => { + const d = withCosts(); + expect(offerableCosts(d, "not-a-date")).toEqual([]); + }); }); describe("sourceFor", () => { @@ -103,6 +118,25 @@ describe("sourceFor", () => { occurrenceDate: "2026-09-01", }); }); + + // I3 follow-up: unlike `offerableCosts`, `sourceFor` is only ever called at + // save time (inside store.mutate), never from a render body — so it is NOT + // required to tolerate an empty date the way `offerableCosts` is. It stays + // strict and throws, exactly like every other bad-date save already does + // (ensureMonth's monthOf in addPurchase) — a caught save-time error, not an + // uncaught render crash. An anchored cost carries no slot at all and so + // never touches monthOf, and is unaffected by an empty date. + test("a calendar cost throws on an empty date, same as any other bad-date save", () => { + const d = withCosts(); + const rent = d.recurring.find((c) => c.name === "Rent")!; + expect(() => sourceFor(d, rent, "")).toThrow(); + }); + + test("an anchored cost is unaffected by an empty date — it carries no slot", () => { + const d = withCosts(); + const phone = d.recurring.find((c) => c.name === "Phone")!; + expect(sourceFor(d, phone, "")).toEqual({ recurringId: phone.id }); + }); }); // I2 fix: a month-only purchase date can only ever seed "everyNMonths" + @@ -137,6 +171,18 @@ describe("newCostOptionIsSelectable", () => { expect(newCostOptionIsSelectable("everyNWeeks", "calendar", "2026-09-20")).toBe(true); expect(newCostOptionIsSelectable("everyNWeeks", "lastCharge", "2026-09-20")).toBe(true); }); + + // I3 follow-up: an empty date fails EVERY pairing, including + // "everyNMonths" + "calendar" — the one combination that is otherwise + // always fine. That combination still hands the date to `monthOf` inside + // `seedCostFromDraft`, and an empty string does not parse as a month + // either, so it must not be offered as selectable while the date is empty. + test("an empty date is never selectable, not even everyNMonths + calendar", () => { + expect(newCostOptionIsSelectable("everyNMonths", "calendar", "")).toBe(false); + expect(newCostOptionIsSelectable("everyNMonths", "lastCharge", "")).toBe(false); + expect(newCostOptionIsSelectable("everyNDays", "calendar", "")).toBe(false); + expect(newCostOptionIsSelectable("everyNWeeks", "lastCharge", "")).toBe(false); + }); }); describe("seedCostFromDraft", () => { @@ -190,4 +236,44 @@ describe("seedCostFromDraft", () => { seedCostFromDraft(draft, { kind: "everyNMonths", n: 1 }, "lastCharge").startDate, ).toBe("2026-09-20"); }); + + // I3 follow-up: like `sourceFor`, this is only ever called at save time + // (inside store.mutate), never from a render body — `newCostOptionIsSelectable` + // already disables every "+ New" option for an empty date, so reaching + // this with one would mean the combo was chosen before the date was + // cleared. It stays strict and throws, exactly like any other bad-date + // save (ensureMonth's monthOf in addPurchase) — a caught save-time error, + // not an uncaught render crash. + test("a month-granular pairing throws on an empty date, same as any other bad-date save", () => { + const draft = { + date: "", + description: "Rent", + note: "", + amount: 8200, + currency: "DKK" as const, + splitMode: "percent" as const, + splits: [{ postId: "p1", value: 100, absorbsRemainder: true }], + plan: null, + }; + expect(() => seedCostFromDraft(draft, { kind: "everyNMonths", n: 1 }, "calendar")).toThrow(); + }); + + test("a day-granular pairing carries an empty date straight through, unvalidated here", () => { + // seedCostFromDraft itself does not call monthOf for this pairing — the + // store's own requireStartDateGranularity is what would refuse it, at + // save time, inside the same guarded mutate call. + const draft = { + date: "", + description: "Phone", + note: "", + amount: 300, + currency: "DKK" as const, + splitMode: "percent" as const, + splits: [{ postId: "p1", value: 100, absorbsRemainder: true }], + plan: null, + }; + expect( + seedCostFromDraft(draft, { kind: "everyNMonths", n: 1 }, "lastCharge").startDate, + ).toBe(""); + }); }); diff --git a/src/ui/recurringForm.ts b/src/ui/recurringForm.ts index 1426db1..470aa29 100644 --- a/src/ui/recurringForm.ts +++ b/src/ui/recurringForm.ts @@ -10,6 +10,26 @@ import type { } from "../domain/types.ts"; import type { PurchaseDraft } from "./purchaseForm.ts"; +/** + * Whether `date` parses as a month at all. `monthOf` stays strict (it is a + * domain function used across the app, and AGENTS.md forbids softening a + * domain throw into a fallback) — this wraps it rather than loosening it, so + * the UI layer can tell "not a date yet" apart from "a bug" without touching + * `src/domain/`. + * + * A purchase's date starts as a whole month and can be widened to a real day, + * but the native `` lets the owner clear it outright — + * that is a half-finished FORM, not a domain value, and belongs handled here. + */ +function isParseableMonth(date: IsoDate): boolean { + try { + monthOf(date); + return true; + } catch { + return false; + } +} + /** * The costs a purchase dated `date` could be a charge for. * @@ -18,8 +38,15 @@ import type { PurchaseDraft } from "./purchaseForm.ts"; * refresh. A `calendar` cost is offerable only where its grid actually has a * slot in that purchase's month; tagging one outside its grid would record a * charge settling nothing. + * + * `date` can be empty: the purchase dialog calls this unconditionally on + * every render, and the native `` lets the owner clear it. + * An empty (or otherwise unparseable) date is not an error to report here — + * it is just a form the owner has not finished — so nothing is offered rather + * than throwing `monthOf`'s parse error out of a render body. */ export function offerableCosts(dataset: Dataset, date: IsoDate): RecurringCost[] { + if (!isParseableMonth(date)) return []; const month = monthOf(date); return dataset.recurring .filter((cost) => !cost.archived) @@ -50,6 +77,14 @@ export function offerableCosts(dataset: Dataset, date: IsoDate): RecurringCost[] * falls there. That is correct for both a month-granular grid and a * day-granular calendar cost. If nothing falls in that month, the cost * should never have been offered — throw rather than invent a slot. + * + * Unlike `offerableCosts`, this does NOT guard against an empty + * `purchaseDate` by returning something inert — it is only ever called at + * save time (inside `store.mutate`, via `useMutate`), never from a render + * body, so a bad date here surfaces as a caught save-time error, exactly like + * every other bad-date save already does (`ensureMonth`'s `monthOf` in + * `addPurchase`). Softening it would invent a second rule for the same + * situation rather than reuse the one that already exists. */ export function sourceFor( dataset: Dataset, @@ -96,16 +131,31 @@ export function sourceFor( * (PurchaseDialog's "+ New recurring cost…" unit and anchoring selects). * Exported so each option can check this without duplicating the rule, and so * it is testable on its own — a DOM test cannot exercise a predicate. + * + * An empty `purchaseDate` (the native date input, cleared) fails EVERY + * pairing, including "everyNMonths" + "calendar": that combination is + * otherwise always fine, but it still hands `purchaseDate` to `monthOf` in + * `seedCostFromDraft`, and an empty string does not even parse as a month. */ export function newCostOptionIsSelectable( kind: Recurrence["kind"], anchoring: Anchoring, purchaseDate: IsoDate, ): boolean { + if (!isParseableMonth(purchaseDate)) return false; const needsDay = kind !== "everyNMonths" || anchoring === "lastCharge"; return !needsDay || purchaseDate.length > 7; } +/** + * Like `sourceFor`, this does NOT guard against an empty `draft.date` — it is + * only ever called at save time, inside `store.mutate`. The dialog's + * "+ New recurring cost…" options are already disabled for an empty date via + * `newCostOptionIsSelectable`, so reaching this with one would mean the + * combo was selected before the date was cleared; the resulting throw (from + * `monthOf` here, or from the store's own `requireStartDateGranularity`) is + * caught the same way any other bad-date save already is. + */ export function seedCostFromDraft( draft: PurchaseDraft, recurrence: Recurrence,