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.
This commit is contained in:
1 parent
397bb245d8
commit
eb13f98610
2 files changed
+128
-6
No files matched your search
@@ -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");
|
||||
});
|
||||
});
|
||||
@@ -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 }),
|
||||
);
|
||||
}}
|
||||
>
|
||||
<option value="everyNDays">days</option>
|
||||
@@ -169,14 +220,27 @@ export function RecurringSection() {
|
||||
</NativeSelect>
|
||||
</td>
|
||||
<td className="py-2">
|
||||
{/* 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. */}
|
||||
<Input
|
||||
key={`${cost.id}-${cost.startDate}`}
|
||||
className="h-8 w-32"
|
||||
type="text"
|
||||
aria-label={`Start date for ${cost.name}`}
|
||||
value={cost.startDate}
|
||||
onChange={(event) => {
|
||||
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 }),
|
||||
);
|
||||
}}
|
||||
/>
|
||||
</td>
|
||||
|
||||
Reference in new issue
Block a user