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; }