fix(purchase): stop offerableCosts crashing render on an empty date
Finding I3 (Critical): PurchaseDialog calls offerableCosts(dataset, draft.date)
unconditionally in its render body. The native <input type="date"> 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) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5
parent
04a089eb06
commit
35caa123cc
@@ -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 <input type="date"> 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("");
|
||||
});
|
||||
});
|
||||
|
||||
@@ -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 `<input type="date">` 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 `<input type="date">` 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,
|
||||
|
||||
Reference in New Issue
Block a user