From 3a4086807aec17ef8312d3a4201a5ae9a03c7535 Mon Sep 17 00:00:00 2001 From: Lucas Winther Date: Sun, 6 Sep 2026 18:32:17 +0200 Subject: [PATCH] fix(purchase): do not offer a tag the write will refuse MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `offerableCosts` offered every non-archived anchored cost whatever the date, so the default new-purchase flow — month-only, since `emptyDraft` seeds `date: monthId` — offered the owner a choice that now fails at the boundary. An anchored cost needs a day, the same way `newCostOptionIsSelectable` already gates the new-cost pairings. A calendar cost is unaffected: its charge names the slot it settles and its own date is never stepped from. An anchored cost stays SELECTED when the date drops to month-only, because a selection is never taken away out from under the owner — so the dialog says what is wrong there rather than letting Save be the first place it comes up. Two more in the same file and its neighbour: - A recurring cost that has vanished since the dialog opened (deleted from Settings in another tab) made the purchase save with no association and no message, leaving the bill still showing as expected. It is refused now. - The comment above `save()` described `requireConfirmationAdvances`, deleted in this branch. A write can still be refused; only the stated reason was stale. `normalizedStartDateFor` and `isValidStartDate` both asked their own copy of the granularity rule, and the first had `anchoring` optional — which the unit handler omitted, silently answering "calendar" for a cost anchored to its last charge. Both call the domain predicate now, with anchoring required. Co-Authored-By: Claude Opus 5 (1M context) --- src/ui/components/PurchaseDialog.tsx | 38 +++++++++-- src/ui/recurringForm.test.ts | 37 ++++++++++- src/ui/recurringForm.ts | 43 +++++++------ .../routes/settings/RecurringSection.test.ts | 51 +++++++++------ src/ui/routes/settings/RecurringSection.tsx | 64 ++++++++++--------- 5 files changed, 156 insertions(+), 77 deletions(-) diff --git a/src/ui/components/PurchaseDialog.tsx b/src/ui/components/PurchaseDialog.tsx index ff5b76c..df4b95f 100644 --- a/src/ui/components/PurchaseDialog.tsx +++ b/src/ui/components/PurchaseDialog.tsx @@ -116,11 +116,14 @@ export function PurchaseDialog({ monthId, purchase, trigger }: Props) { setNewAnchoring("calendar"); } - // 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. + // A write can be REFUSED, not merely fail on a full disk. Tagging a + // purchase to a cost anchored to its last charge is rejected when the + // purchase's date carries no day, because that date is what the next charge + // is measured from; a cost that has vanished since the dialog opened is + // rejected below; and any of the store's other pairing rules can refuse + // too. 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; // One mutate for the whole batch: a single write, one queue entry, and no @@ -142,7 +145,19 @@ export function PurchaseDialog({ monthId, purchase, trigger }: Props) { source = sourceFor(data, seeded, draft.date); } else if (recurringSelection !== "none") { const cost = data.recurring.find((c) => c.id === recurringSelection); - if (cost) source = sourceFor(data, cost, draft.date); + // Throw rather than save untagged. The cost can genuinely be gone — + // deleted from Settings in another tab while this dialog sat open — + // and dropping the association silently would save a purchase the + // owner believes is a charge for a bill, leaving that bill still + // showing as expected. `useMutate` catches this and the dialog stays + // open with the message. + if (!cost) { + throw new Error( + `The recurring cost this purchase was tagged to no longer exists. Choose another one ` + + `(or "None") and save again.`, + ); + } + source = sourceFor(data, cost, draft.date); } if (purchase) { // `source` is passed explicitly, even when undefined: updatePurchase @@ -401,6 +416,17 @@ export function PurchaseDialog({ monthId, purchase, trigger }: Props) { measured from the last charge, that also restarts its cycle from today.

+ {/* An anchored cost stays SELECTED when the date drops to + month-only (the selection is never taken away out from under + the owner), but the store will refuse the save: that date is + what the bill's next charge is measured from. Say so here + rather than letting Save be the first place it comes up. */} + {selected?.anchoring === "lastCharge" && draft.date.length <= 7 && ( +

+ “{selected.name}” is measured from its last charge, so this + purchase needs an exact date — use “set exact date” above. +

+ )} {recurringSelection === NEW_COST && ( diff --git a/src/ui/recurringForm.test.ts b/src/ui/recurringForm.test.ts index 9e1073a..83fb2ab 100644 --- a/src/ui/recurringForm.test.ts +++ b/src/ui/recurringForm.test.ts @@ -36,11 +36,46 @@ function withCosts(): Dataset { } describe("offerableCosts", () => { - test("an anchored cost is offerable on any date", () => { + test("an anchored cost is offerable on any DAY — it has no grid to miss", () => { const names = offerableCosts(withCosts(), "2027-05-11").map((c) => c.name); expect(names).toContain("Phone"); }); + // `emptyDraft` seeds `date: monthId`, so month-only is the DEFAULT for a new + // purchase. Offering an anchored cost there offered a tag both write + // boundaries now refuse — and before they refused it, one that saved and + // then took every money route down on the next fold, because an anchored + // cost's next due date is stepped from its latest charge's own date. + test("an anchored cost is NOT offered for a month-only date", () => { + const names = offerableCosts(withCosts(), "2026-09").map((c) => c.name); + expect(names).not.toContain("Phone"); + }); + + test("a monthly anchored cost is not offered for a month-only date either", () => { + const d = withCosts(); + const post = d.posts[0]!; + actions.addRecurringCost(d, { + name: "Streaming", + archived: false, + amount: { amount: 129, currency: "DKK" }, + startDate: "2026-09-01", + recurrence: { kind: "everyNMonths", n: 1 }, + anchoring: "lastCharge", + splitMode: "percent", + splits: [{ postId: post.id, value: 100, absorbsRemainder: true }], + }); + expect(offerableCosts(d, "2026-09").map((c) => c.name)).not.toContain("Streaming"); + expect(offerableCosts(d, "2026-09-20").map((c) => c.name)).toContain("Streaming"); + }); + + // A calendar cost's charge names the slot it settles in + // `source.occurrenceDate`; its own `Purchase.date` is never stepped from, so + // a month-only one carries no granularity rule at all. + test("a calendar cost IS still offered for a month-only date", () => { + const names = offerableCosts(withCosts(), "2026-09").map((c) => c.name); + expect(names).toContain("Rent"); + }); + test("a calendar cost is offerable only when its grid has a slot in that month", () => { const d = withCosts(); expect(offerableCosts(d, "2026-09-20").map((c) => c.name)).toContain("Rent"); diff --git a/src/ui/recurringForm.ts b/src/ui/recurringForm.ts index 470aa29..bd354ac 100644 --- a/src/ui/recurringForm.ts +++ b/src/ui/recurringForm.ts @@ -1,3 +1,8 @@ +import { + chargeDateNeedsDay, + isDayGranularDate, + startDateNeedsDay, +} from "../domain/granularity.ts"; import { monthOf } from "../domain/months.ts"; import { occurrencesOf } from "../domain/occurrences.ts"; import type { @@ -33,11 +38,18 @@ function isParseableMonth(date: IsoDate): boolean { /** * The costs a purchase dated `date` could be a charge for. * - * An anchored (`lastCharge`) cost is always offerable: it has no grid of - * slots, so any date can be the moment its cycle restarted — that IS the - * 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. + * An anchored (`lastCharge`) cost has no grid of slots, so any DAY can be the + * moment its cycle restarted — that IS the refresh. It needs a day, though: + * the purchase's own date becomes the anchor the series steps forward from + * (`chargeDateNeedsDay`), so a month-only date makes the tag unsaveable, and + * both write boundaries refuse it. The owner must not be offered a choice that + * then fails — the same reason `newCostOptionIsSelectable` disables the + * new-cost pairings a month-only date cannot express. + * + * 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. Its charge's own date carries no granularity rule at all — + * the slot it settles lives in `source.occurrenceDate`. * * `date` can be empty: the purchase dialog calls this unconditionally on * every render, and the native `` lets the owner clear it. @@ -51,7 +63,7 @@ export function offerableCosts(dataset: Dataset, date: IsoDate): RecurringCost[] return dataset.recurring .filter((cost) => !cost.archived) .filter((cost) => { - if (cost.anchoring === "lastCharge") return true; + if (chargeDateNeedsDay(cost.anchoring)) return isDayGranularDate(date); return occurrencesOf(cost, dataset.purchases, month).some( (occurrence) => monthOf(occurrence.date) === month, ); @@ -106,23 +118,15 @@ export function sourceFor( return { recurringId: cost.id, occurrenceDate: occurrence.date }; } -/** - * A new cost seeded from the purchase being typed. - * - * A calendar monthly cost is month-granular; every other combination needs a - * real day — an anchored monthly cost's next due date is computed - * day-precisely from the charge, and the two day-granular recurrence kinds - * hand their `startDate` straight to `addDays` regardless of anchoring. - */ /** * Whether a NEW cost's kind+anchoring pairing can be seeded from a purchase * dated `purchaseDate`. * * `seedCostFromDraft` hands `purchaseDate` straight to `startDate` whenever - * the pairing is not month-granular, so this mirrors - * `requireStartDateGranularity`'s `needsDay` in `store/actions.ts`: every - * pairing needs a real day EXCEPT "everyNMonths" anchored to "calendar" — - * rent occurs IN a month, so a month-only date is fine there. + * the pairing is not month-granular, so this asks the domain's own + * `startDateNeedsDay` rather than restating it: every pairing needs a real day + * EXCEPT "everyNMonths" anchored to "calendar" — rent occurs IN a month, so a + * month-only date is fine there. * * A month-only `purchaseDate` reaching any OTHER pairing would reach the * store's own validation and throw text naming a field ("startDate") the @@ -143,8 +147,7 @@ export function newCostOptionIsSelectable( purchaseDate: IsoDate, ): boolean { if (!isParseableMonth(purchaseDate)) return false; - const needsDay = kind !== "everyNMonths" || anchoring === "lastCharge"; - return !needsDay || purchaseDate.length > 7; + return !startDateNeedsDay(kind, anchoring) || isDayGranularDate(purchaseDate); } /** diff --git a/src/ui/routes/settings/RecurringSection.test.ts b/src/ui/routes/settings/RecurringSection.test.ts index 1f2e0d7..f18a9f4 100644 --- a/src/ui/routes/settings/RecurringSection.test.ts +++ b/src/ui/routes/settings/RecurringSection.test.ts @@ -12,35 +12,44 @@ import { // 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); + expect(isValidStartDate("", "everyNMonths", "calendar")).toBe(false); + expect(isValidStartDate("", "everyNDays", "calendar")).toBe(false); + expect(isValidStartDate("", "everyNWeeks", "calendar")).toBe(false); }); test("everyNMonths accepts either granularity", () => { - expect(isValidStartDate("2026-09", "everyNMonths")).toBe(true); - expect(isValidStartDate("2026-09-17", "everyNMonths")).toBe(true); + expect(isValidStartDate("2026-09", "everyNMonths", "calendar")).toBe(true); + expect(isValidStartDate("2026-09-17", "everyNMonths", "calendar")).toBe(true); }); test("everyNMonths still rejects garbage", () => { - expect(isValidStartDate("not-a-date", "everyNMonths")).toBe(false); - expect(isValidStartDate("2026-13", "everyNMonths")).toBe(false); + expect(isValidStartDate("not-a-date", "everyNMonths", "calendar")).toBe(false); + expect(isValidStartDate("2026-13", "everyNMonths", "calendar")).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); + expect(isValidStartDate("2026-09", "everyNDays", "calendar")).toBe(false); + expect(isValidStartDate("2026-09", "everyNWeeks", "calendar")).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); + expect(isValidStartDate("2026-09-17", "everyNDays", "calendar")).toBe(true); + expect(isValidStartDate("2026-09-17", "everyNWeeks", "calendar")).toBe(true); + }); + + // The field asks the same predicate the store does, so it refuses exactly + // what the write would refuse. Anchoring is half of that rule: an + // everyNMonths cost measured from its last charge steps day-precisely, so + // a month-only value there is a doomed mutate, not a valid entry. + test("everyNMonths rejects a month-only value once it is anchored to lastCharge", () => { + expect(isValidStartDate("2026-09", "everyNMonths", "lastCharge")).toBe(false); + expect(isValidStartDate("2026-09-17", "everyNMonths", "lastCharge")).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 + expect(isValidStartDate("2026-02-30", "everyNDays", "calendar")).toBe(false); // no Feb 30 }); // Residual C1: `monthOf` only extracts the month and never checks the day, @@ -48,13 +57,13 @@ describe("isValidStartDate", () => { // has 30 days) used to pass here even under everyNMonths, and could then be // stored, only for foldBalances to throw three months later. test("everyNMonths still rejects a day-granular value with an impossible day", () => { - expect(isValidStartDate("2026-09-31", "everyNMonths")).toBe(false); + expect(isValidStartDate("2026-09-31", "everyNMonths", "calendar")).toBe(false); }); test("everyNMonths accepts a real leap-day and rejects a non-leap Feb 29", () => { - expect(isValidStartDate("2024-02-29", "everyNMonths")).toBe(true); // 2024 is a leap year - expect(isValidStartDate("2026-02-29", "everyNMonths")).toBe(false); // 2026 is not - expect(isValidStartDate("2026-02-28", "everyNMonths")).toBe(true); + expect(isValidStartDate("2024-02-29", "everyNMonths", "calendar")).toBe(true); // 2024 is a leap year + expect(isValidStartDate("2026-02-29", "everyNMonths", "calendar")).toBe(false); // 2026 is not + expect(isValidStartDate("2026-02-28", "everyNMonths", "calendar")).toBe(true); }); }); @@ -91,17 +100,17 @@ describe("isValidEndedFrom", () => { // 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"); + expect(normalizedStartDateFor("everyNDays", "2026-09", "calendar")).toBe("2026-09-01"); + expect(normalizedStartDateFor("everyNWeeks", "2026-09", "calendar")).toBe("2026-09-01"); }); test("leaves an already day-granular date untouched", () => { - expect(normalizedStartDateFor("everyNDays", "2026-09-17")).toBe("2026-09-17"); + expect(normalizedStartDateFor("everyNDays", "2026-09-17", "calendar")).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"); + expect(normalizedStartDateFor("everyNMonths", "2026-09", "calendar")).toBe("2026-09"); + expect(normalizedStartDateFor("everyNMonths", "2026-09-17", "calendar")).toBe("2026-09-17"); }); // The carried anchoring item: an everyNMonths cost switching to lastCharge diff --git a/src/ui/routes/settings/RecurringSection.tsx b/src/ui/routes/settings/RecurringSection.tsx index a833bf5..ed64f58 100644 --- a/src/ui/routes/settings/RecurringSection.tsx +++ b/src/ui/routes/settings/RecurringSection.tsx @@ -19,6 +19,7 @@ import { SplitEditor } from "../../components/SplitEditor.tsx"; import type { SplitTarget } from "../../purchaseForm.ts"; import { monthOf } from "../../../domain/months.ts"; import { toDayOrdinal } from "../../../domain/days.ts"; +import { startDateNeedsDay } from "../../../domain/granularity.ts"; import type { IsoDate, Recurrence, RecurringCost, Split } from "../../../domain/types.ts"; /** Also used by `PurchaseDialog`'s "+ New recurring cost…" control. */ @@ -30,11 +31,13 @@ export function isDayGranular(date: IsoDate): boolean { } /** - * 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. + * True only for a string this cost's SCHEDULE could actually walk: whether + * that needs a day is `startDateNeedsDay`, the same predicate the store's own + * validation asks, so the field refuses here exactly what the write would + * refuse rather than issuing a doomed mutate. Only "everyNMonths" anchored to + * "calendar" tolerates a month-only value (the walk normalises it through + * `monthOf`); everything else reaches `addDays` or `addMonthsToDate`. Reuses + * the domain's own parsers rather than a second, possibly-diverging regex. * * A day-SHAPED value is routed through `toDayOrdinal` even under * `everyNMonths`: `monthOf` only extracts the month and never checks the day, @@ -43,14 +46,19 @@ export function isDayGranular(date: IsoDate): boolean { * months later out of the fold. * * 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. + * between the "Starts" field and 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 { +export function isValidStartDate( + value: string, + kind: Recurrence["kind"], + anchoring: RecurringCost["anchoring"], +): boolean { + if (startDateNeedsDay(kind, anchoring) && !isDayGranular(value)) return false; try { - if (kind === "everyNMonths" && !isDayGranular(value)) monthOf(value); - else toDayOrdinal(value); + if (isDayGranular(value)) toDayOrdinal(value); + else monthOf(value); return true; } catch { return false; @@ -85,26 +93,25 @@ export function isValidEndedFrom(value: string): boolean { /** * What the "Starts" value becomes when the owner switches to a day-granular * unit, or to `lastCharge` anchoring: a month-only value expands to the first - * of that month, in the SAME mutate call that changes the kind or anchoring - * (C1, and the carried anchoring item) — the walk must never see the old - * pairing even for one render. + * of that month, in the SAME mutate call that changes the kind or anchoring — + * the walk must never see the old pairing even for one render. * - * Mirrors `requireStartDateGranularity`'s `needsDay` in `store/actions.ts`: - * any kind other than `everyNMonths` always needs a day-granular date - * regardless of anchoring (so `anchoring` can be omitted by the kind - * handler, which never loosens that requirement), and `everyNMonths` needs - * one only once it is anchored to `lastCharge`. An already day-granular - * value always passes through untouched. + * Which pairings need a day is `startDateNeedsDay` in + * `domain/granularity.ts` — the same predicate the store's own validation + * asks, so this control cannot expand a date the store would then refuse, or + * leave one it would. `anchoring` is REQUIRED for that reason: it was + * optional once and the kind handler omitted it, which silently answered + * "calendar" for a cost anchored to its last charge. An already day-granular + * value passes through untouched. * * Exported for `RecurringSection.test.ts`. */ export function normalizedStartDateFor( kind: Recurrence["kind"], startDate: IsoDate, - anchoring?: RecurringCost["anchoring"], + anchoring: RecurringCost["anchoring"], ): IsoDate { - const needsDay = kind !== "everyNMonths" || anchoring === "lastCharge"; - if (!needsDay || isDayGranular(startDate)) return startDate; + if (!startDateNeedsDay(kind, anchoring) || isDayGranular(startDate)) return startDate; return `${startDate}-01`; } @@ -214,11 +221,11 @@ export function RecurringSection() { ? { kind, n: cost.recurrence.n, weekday: 1 } : { kind, n: cost.recurrence.n }; // 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 + // startDate is one of the ways this 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); + const startDate = normalizedStartDateFor(kind, cost.startDate, cost.anchoring); mutate((draft) => updateRecurringCost(draft, cost.id, { recurrence, startDate }), ); @@ -312,8 +319,7 @@ export function RecurringSection() { } onChange={(event) => { const anchoring = event.target.value as RecurringCost["anchoring"]; - // Same C1-shaped normalisation the kind handler below - // does: an everyNMonths cost anchored to lastCharge + // Same normalisation the kind handler below does: an everyNMonths cost anchored to lastCharge // needs a day-granular startDate, so expand a // month-only one in the SAME mutate call that changes // the anchoring — the store validates the pairing on @@ -347,7 +353,7 @@ export function RecurringSection() { defaultValue={cost.startDate} onBlur={(event) => { const typed = event.target.value; - if (!isValidStartDate(typed, cost.recurrence.kind)) { + if (!isValidStartDate(typed, cost.recurrence.kind, cost.anchoring)) { event.target.value = cost.startDate; return; }