From eb13f986103df2301166902747b506e67ab514f4 Mon Sep 17 00:00:00 2001 From: Lucas Winther Date: Fri, 4 Sep 2026 07:57:37 +0200 Subject: [PATCH] fix(ui): normalise startDate on a kind switch, validate the Starts field (C1 UI, C2) The unit dropdown could rewrite a cost's recurrence kind to a day-granular one without touching its month-only startDate, committing exactly the pairing the store now refuses. It now expands startDate to the first of the month in the SAME mutate call that changes the kind. The Starts field itself was free text committed on every keystroke, including an emptied field ("" reaches monthOf() and throws with the same blast radius). It is now an uncontrolled input that only commits on blur, and only when the text parses as an IsoDate of the granularity the cost's recurrence kind requires; an invalid edit is discarded rather than stored. isValidStartDate and normalizedStartDateFor are exported and covered directly in RecurringSection.test.ts, since this is what stands between this field and a repeat of C1/C2. --- .../routes/settings/RecurringSection.test.ts | 58 ++++++++++++++ src/ui/routes/settings/RecurringSection.tsx | 76 +++++++++++++++++-- 2 files changed, 128 insertions(+), 6 deletions(-) create mode 100644 src/ui/routes/settings/RecurringSection.test.ts diff --git a/src/ui/routes/settings/RecurringSection.test.ts b/src/ui/routes/settings/RecurringSection.test.ts new file mode 100644 index 0000000..4b011b9 --- /dev/null +++ b/src/ui/routes/settings/RecurringSection.test.ts @@ -0,0 +1,58 @@ +import { describe, expect, test } from "bun:test"; +import { isValidStartDate, normalizedStartDateFor } from "./RecurringSection.tsx"; + +// C2: the "Starts" field must never be able to persist an empty or +// malformed value — `isValidStartDate` is the guard the onBlur handler uses +// to decide whether to call `mutate` at all. +describe("isValidStartDate", () => { + test("an empty string is never valid, for any kind", () => { + expect(isValidStartDate("", "everyNMonths")).toBe(false); + expect(isValidStartDate("", "everyNDays")).toBe(false); + expect(isValidStartDate("", "everyNWeeks")).toBe(false); + }); + + test("everyNMonths accepts either granularity", () => { + expect(isValidStartDate("2026-09", "everyNMonths")).toBe(true); + expect(isValidStartDate("2026-09-17", "everyNMonths")).toBe(true); + }); + + test("everyNMonths still rejects garbage", () => { + expect(isValidStartDate("not-a-date", "everyNMonths")).toBe(false); + expect(isValidStartDate("2026-13", "everyNMonths")).toBe(false); + }); + + test("a day-granular kind rejects a month-only value", () => { + // This is the exact C1/C2 crash: monthOf() would happily hand this back, + // but occurrencesOf hands it to addDays, which throws. + expect(isValidStartDate("2026-09", "everyNDays")).toBe(false); + expect(isValidStartDate("2026-09", "everyNWeeks")).toBe(false); + }); + + test("a day-granular kind accepts a day-granular value", () => { + expect(isValidStartDate("2026-09-17", "everyNDays")).toBe(true); + expect(isValidStartDate("2026-09-17", "everyNWeeks")).toBe(true); + }); + + test("a day-granular kind rejects a malformed day-granular-shaped value", () => { + expect(isValidStartDate("2026-02-30", "everyNDays")).toBe(false); // no Feb 30 + }); +}); + +// C1: switching the unit dropdown to a day-granular kind must expand a +// month-only startDate in the same mutate call, never leaving the pairing +// unvalidated for even one render. +describe("normalizedStartDateFor", () => { + test("expands a month-only date when switching to a day-granular kind", () => { + expect(normalizedStartDateFor("everyNDays", "2026-09")).toBe("2026-09-01"); + expect(normalizedStartDateFor("everyNWeeks", "2026-09")).toBe("2026-09-01"); + }); + + test("leaves an already day-granular date untouched", () => { + expect(normalizedStartDateFor("everyNDays", "2026-09-17")).toBe("2026-09-17"); + }); + + test("leaves the date untouched when switching to everyNMonths", () => { + expect(normalizedStartDateFor("everyNMonths", "2026-09")).toBe("2026-09"); + expect(normalizedStartDateFor("everyNMonths", "2026-09-17")).toBe("2026-09-17"); + }); +}); diff --git a/src/ui/routes/settings/RecurringSection.tsx b/src/ui/routes/settings/RecurringSection.tsx index 1917e70..ae8c7c8 100644 --- a/src/ui/routes/settings/RecurringSection.tsx +++ b/src/ui/routes/settings/RecurringSection.tsx @@ -14,10 +14,53 @@ import { import { parseMoneyInput } from "../../moneyInput.ts"; import { currentMonth } from "../../../store/index.ts"; import { Section } from "../../components/Section.tsx"; -import type { Recurrence, RecurringCost } from "../../../domain/types.ts"; +import { monthOf } from "../../../domain/months.ts"; +import { toDayOrdinal } from "../../../domain/days.ts"; +import type { IsoDate, Recurrence, RecurringCost } from "../../../domain/types.ts"; const WEEKDAYS = ["Sunday", "Monday", "Tuesday", "Wednesday", "Thursday", "Friday", "Saturday"]; +/** "YYYY-MM" is 7 characters; "YYYY-MM-DD" is 10. Same idiom PurchaseDialog uses. */ +export function isDayGranular(date: IsoDate): boolean { + return date.length > 7; +} + +/** + * True only for a string this cost's recurrence kind could actually walk: + * `everyNMonths` tolerates either granularity (the walk normalises through + * `monthOf`), the other two require a day-granular date because they reach + * `addDays` directly. Reuses the domain's own parsers rather than a second, + * possibly-diverging regex. + * + * Exported for `RecurringSection.test.ts` — this is the one thing standing + * between the "Starts" field and C2 (an empty or malformed value bricking + * every route the fold touches), so it is tested directly rather than only + * through a DOM interaction. + */ +export function isValidStartDate(value: string, kind: Recurrence["kind"]): boolean { + try { + if (kind === "everyNMonths") monthOf(value); + else toDayOrdinal(value); + return true; + } catch { + return false; + } +} + +/** + * What the "Starts" value becomes when the owner switches to a day-granular + * unit: a month-only value expands to the first of that month, in the SAME + * mutate call that changes the kind (C1) — the walk must never see the old + * pairing even for one render. `everyNMonths` and an already day-granular + * value pass through untouched. + * + * Exported for `RecurringSection.test.ts`. + */ +export function normalizedStartDateFor(kind: Recurrence["kind"], startDate: IsoDate): IsoDate { + if (kind === "everyNMonths" || isDayGranular(startDate)) return startDate; + return `${startDate}-01`; +} + export function RecurringSection() { const dataset = useDataset(); const { mutate } = useMutate(); @@ -82,7 +125,15 @@ export function RecurringSection() { kind === "everyNWeeks" ? { kind, n: cost.recurrence.n, weekday: 1 } : { kind, n: cost.recurrence.n }; - mutate((draft) => updateRecurringCost(draft, cost.id, { recurrence })); + // Switching to a day-granular kind against a month-only + // startDate is exactly how C1 bricked the app: the walk hands + // "2026-09" to addDays and throws. Expand it to the first of + // that month IN THIS SAME mutate call, so the pairing the store + // now validates never has a chance to go wrong. + const startDate = normalizedStartDateFor(kind, cost.startDate); + mutate((draft) => + updateRecurringCost(draft, cost.id, { recurrence, startDate }), + ); }} > @@ -169,14 +220,27 @@ export function RecurringSection() { + {/* Uncontrolled: committing only happens on blur, and only + when the text parses as an IsoDate of the granularity + this cost's recurrence kind requires. Keyed on the + stored value so an external change (kind flip + auto-expanding it, or a rejected edit) is reflected + rather than left showing stale typed text. */} { - const startDate = event.target.value; - mutate((draft) => updateRecurringCost(draft, cost.id, { startDate })); + defaultValue={cost.startDate} + onBlur={(event) => { + const typed = event.target.value; + if (!isValidStartDate(typed, cost.recurrence.kind)) { + event.target.value = cost.startDate; + return; + } + mutate((draft) => + updateRecurringCost(draft, cost.id, { startDate: typed }), + ); }} />