fix(domain): review round 1 — name the cost, cover the anchored throw, pin two reachable-but-untested paths
C1 (Critical): a month-only startDate on an anchored everyNMonths cost with
no charges yet reached addMonthsToDate directly and threw a bare parsing
error naming no cost, several frames below foldBalances. anchoredOccurrences
now checks its starting cursor's granularity before the walk begins and
throws a cost-named, actionable error instead. No new validation at either
write boundary — that stays out of scope here. stepFrom's docstring no longer
asserts a guarantee ("required to carry a day-granular startDate") that
nothing in the code enforces; it now says what actually protects it.
I1 (Important): the anchored walk's own termination throw had zero test
coverage — both surviving "termination" tests use the cost() fixture, whose
default anchoring is calendar, so they only ever exercised gridOccurrences.
Proven by mutation (replacing the anchored throw with break left every test
passing). Added a test that actually walks anchoredOccurrences, and corrected
the inline comment that falsely claimed coverage existed.
I2 (Important): confirmationsFor's throw is reachable from data the JSON
importer currently accepts (a calendar-cost purchase missing occurrenceDate),
not just a hypothetical programming error — its docstring said "programming
error upstream," which isn't true yet. Docstring corrected, and the throw is
now pinned by a test instead of only asserted in prose.
I3 (Important): nothing asserted that editing a charge's date backwards
re-anchors the series instead of throwing — exactly the behaviour deleting
requireConfirmationAdvances made structurally safe, and exactly the gap left
by deleting round-1's "a date more than one cycle before the slot it claims
is refused" test. Added the inverse: same fixture, but asserting the edit now
succeeds, the fold still folds cleanly, and the series re-anchors to the
edited date.
Minor: round-1 deleted ExpectedBand.test.ts's "stays suppressed once X3 no
longer applies" (X1-only), leaving its underlying scenario (day-granular
anchored cost, slot beyond the displayed month, X3 already satisfied)
completely untested even though still reachable. Added a test pinning what
expectedGroups actually does with it now that X1 is gone: the row is offered,
not suppressed — so a future reintroduction of suppression here is a visible,
deliberate change instead of a silent one.
Left untouched, as instructed: ExpectedBand.tsx's stale X1 docstring and the
Settings anchoring dropdown (both belong to later tasks).
This commit is contained in:
@@ -150,9 +150,30 @@ describe("occurrencesOf — confirmation (calendar)", () => {
|
||||
expect(found[0]!.confirmedBy).toBe("pY");
|
||||
expect(found[1]!.confirmedBy).toBeNull();
|
||||
});
|
||||
|
||||
test("REVIEW ROUND 2 / I2: a calendar-cost purchase with no occurrenceDate throws, naming the purchase", () => {
|
||||
// Reachable from legal-looking imported data, not just a programming
|
||||
// error: the JSON importer currently accepts a purchase referencing a
|
||||
// recurring cost with no occurrenceDate, so a file built that way loads
|
||||
// clean and throws on the very next fold. Pinned here so the throw's
|
||||
// behaviour (and its message naming the purchase, not a bare "undefined")
|
||||
// doesn't drift silently.
|
||||
const rent = cost({ id: "rent", anchoring: "calendar" });
|
||||
const noSlot: Purchase = {
|
||||
id: "bad1",
|
||||
date: "2026-01-05",
|
||||
description: "Rent",
|
||||
total: { amount: 8000, currency: "DKK" },
|
||||
splitMode: "percent",
|
||||
splits: [{ postId: "housing", value: 100, absorbsRemainder: true }],
|
||||
schedule: null,
|
||||
source: { recurringId: "rent" }, // no occurrenceDate
|
||||
};
|
||||
expect(() => occurrencesOf(rent, [noSlot], "2026-02")).toThrow(/bad1/);
|
||||
});
|
||||
});
|
||||
|
||||
describe("occurrencesOf — termination", () => {
|
||||
describe("occurrencesOf — termination (grid walk)", () => {
|
||||
// CONTROLLER RULING: the round-1 version of this block also had "a
|
||||
// confirmation dated absurdly early throws rather than walking backwards",
|
||||
// which modelled the OLD lastCharge slot-rebasing mechanism (a confirmation
|
||||
@@ -163,8 +184,15 @@ describe("occurrencesOf — termination", () => {
|
||||
// before startDate cannot pull the series backwards" below) — so the throw
|
||||
// that test asserted is no longer reachable by the scenario it built.
|
||||
// Deleted rather than adapted, because there is no reachable input left that
|
||||
// exercises the same path; the anchored walk's own termination throw is
|
||||
// still covered by the two tests kept below, which only need n < 1.
|
||||
// exercises the same path.
|
||||
//
|
||||
// CORRECTION (round 2 review): the two tests below both build their cost
|
||||
// with `cost({...})`, whose fixture default is `anchoring: "calendar"` — so
|
||||
// they only ever exercise `gridOccurrences`'s termination throw, not
|
||||
// `anchoredOccurrences`'s. A prior version of this comment claimed the
|
||||
// anchored throw was "still covered by the two tests kept below"; that was
|
||||
// false. The anchored walk's own termination throw has its own test, in
|
||||
// "the anchored walk (lastCharge)" below.
|
||||
test("a recurrence that cannot advance throws rather than hanging", () => {
|
||||
const broken = cost({ recurrence: { kind: "everyNDays", n: 0 }, startDate: "2026-01-01" });
|
||||
expect(() => occurrencesOf(broken, none, "2026-02")).toThrow(/did not advance/);
|
||||
@@ -309,6 +337,28 @@ describe("the anchored walk (lastCharge)", () => {
|
||||
expect(dates).toEqual(["2026-10-12", "2026-11-11"]);
|
||||
});
|
||||
|
||||
test("REVIEW ROUND 2 / I1: a broken recurrence throws through the ANCHORED walk too, not just the grid one", () => {
|
||||
// Proven missing by mutation: replacing the anchored walk's termination
|
||||
// throw with a `break` left every existing test passing, because the two
|
||||
// tests in "occurrencesOf — termination (grid walk)" both default to
|
||||
// `anchoring: "calendar"` via the `cost()` fixture and so only exercise
|
||||
// `gridOccurrences`. `phone()` here defaults to `anchoring: "lastCharge"`,
|
||||
// so this one actually walks `anchoredOccurrences`.
|
||||
const broken = phone({ recurrence: { kind: "everyNDays", n: 0 } });
|
||||
expect(() => occurrencesOf(broken, [], "2026-11")).toThrow(/did not advance/);
|
||||
});
|
||||
|
||||
test("REVIEW ROUND 2 / C1: a month-only startDate throws a cost-named error, not a bare date-parsing one", () => {
|
||||
// No charges, so the walk's starting cursor is `cost.startDate` directly
|
||||
// — "2026-09", month-only. Before this fix, that reached `stepFrom` ->
|
||||
// `addMonthsToDate` and threw "Not a day-granular IsoDate: 2026-09",
|
||||
// naming no cost, three frames below foldBalances. Now `anchoredOccurrences`
|
||||
// catches it first and names the bill.
|
||||
const untimed = phone({ startDate: "2026-09" });
|
||||
expect(() => occurrencesOf(untimed, [], "2026-11")).toThrow(/Phone plan/);
|
||||
expect(() => occurrencesOf(untimed, [], "2026-11")).not.toThrow(/day-granular IsoDate/);
|
||||
});
|
||||
|
||||
test("unconfirmed across many months, a day-granular anchored cost just keeps stepping", () => {
|
||||
// CONTROLLER RULING: replaces round-1's "unconfirmed, it behaves exactly
|
||||
// like a calendar series" (old describe "lastCharge anchoring: the phone
|
||||
|
||||
+41
-15
@@ -42,6 +42,9 @@ function isBefore(a: IsoDate, b: IsoDate): boolean {
|
||||
return a < b;
|
||||
}
|
||||
|
||||
/** "YYYY-MM-DD" vs "YYYY-MM" — whether a date carries a day at all. */
|
||||
const DAY_GRANULAR_DATE = /^\d{4}-\d{2}-\d{2}$/;
|
||||
|
||||
/** Moves forward to the next occurrence of `weekday`, or stays put if already on it. */
|
||||
function snapForward(date: IsoDate, weekday: number): IsoDate {
|
||||
const delta = (weekday - weekdayOf(date) + 7) % 7;
|
||||
@@ -58,13 +61,16 @@ function snapForward(date: IsoDate, weekday: number): IsoDate {
|
||||
* step is day-precise and clamps (`addMonthsToDate`). The other two kinds are
|
||||
* day-granular either way and ignore the argument.
|
||||
*
|
||||
* The anchored walk (`anchoredOccurrences`) only ever calls this with a
|
||||
* day-granular `date`: its cursor starts at `cost.startDate` or at a real
|
||||
* charge's date, and an anchored `everyNMonths` cost is required to carry a
|
||||
* day-granular `startDate` for exactly this reason. If a month-only date ever
|
||||
* reaches this branch, `addMonthsToDate` throwing is CORRECT — something
|
||||
* upstream produced a cursor this function was never meant to receive, and a
|
||||
* visible error beats silently treating "2026-09" as "2026-09-01".
|
||||
* `anchoredOccurrences` checks its starting cursor is day-granular before
|
||||
* ever calling this for an `everyNMonths` cost, and throws an error naming
|
||||
* the cost if it is not. That check exists BECAUSE nothing currently
|
||||
* guarantees a day-granular `startDate` on an anchored `everyNMonths` cost —
|
||||
* neither write boundary rejects a month-only one, and a charge's own date
|
||||
* can be month-only too (`Purchase.date`'s documented shape). Without that
|
||||
* check, a month-only date reaching this branch would still throw, but from
|
||||
* `addMonthsToDate` with a bare parsing message that names no cost — a
|
||||
* failure the owner cannot act on, several frames below `foldBalances`, that
|
||||
* would take every money route down with it.
|
||||
*
|
||||
* Adding a recurrence kind means adding a case here and a variant to
|
||||
* `Recurrence`. Nothing else in the app switches on the kind.
|
||||
@@ -96,14 +102,17 @@ export function stepFrom(
|
||||
*
|
||||
* Calendar-only. A `lastCharge` cost has no slots (`Purchase.source`'s doc in
|
||||
* types.ts) — its charges are walked by `chargesFor`/`anchoredOccurrences`
|
||||
* off `Purchase.date` directly, and never reach this function. A purchase
|
||||
* that DOES reach here (its `recurringId` matches) but carries no
|
||||
* `occurrenceDate` is therefore not a data problem to skip past quietly: it
|
||||
* means some caller attached a `lastCharge`-shaped purchase (or one with a
|
||||
* dangling/mismatched source) to a cost this function is walking as
|
||||
* `calendar`, which is a programming error upstream. Throwing says so
|
||||
* directly instead of the old scaffolding `continue`, which would have
|
||||
* silently dropped the confirmation and left the slot looking unpaid.
|
||||
* off `Purchase.date` directly, and never reach this function BY DESIGN.
|
||||
*
|
||||
* A purchase that DOES reach here (its `recurringId` names a `calendar` cost)
|
||||
* but carries no `occurrenceDate` is not only a hypothetical programming
|
||||
* error: the JSON importer currently accepts a purchase missing one, and
|
||||
* import replaces the whole dataset, so a file built that way loads clean and
|
||||
* throws on the very next fold — the app goes down immediately, from data the
|
||||
* import boundary let in. Throwing here, naming the purchase, is still better
|
||||
* than the old scaffolding `continue`, which would have silently dropped the
|
||||
* confirmation and left the slot looking unpaid; closing the import boundary
|
||||
* itself is a separate piece of work.
|
||||
*/
|
||||
export function confirmationsFor(
|
||||
purchases: readonly Purchase[],
|
||||
@@ -183,6 +192,23 @@ function anchoredOccurrences(
|
||||
// `max(latest, startDate)`: a charge back-dated before the cost existed must
|
||||
// not pull the series behind its own start.
|
||||
const anchor = latest === null ? null : isBefore(latest, cost.startDate) ? cost.startDate : latest;
|
||||
const startingPoint = anchor ?? cost.startDate;
|
||||
|
||||
// `stepFrom`'s `everyNMonths`/`lastCharge` branch hands its date straight
|
||||
// to `addMonthsToDate`, which requires "YYYY-MM-DD" and would otherwise
|
||||
// throw a bare parsing error naming no cost at all, several frames below
|
||||
// `foldBalances` — taking every money route down with a message the owner
|
||||
// cannot act on. Nothing at either write boundary rejects a month-only
|
||||
// `startDate` on an anchored `everyNMonths` cost today (and a charge's own
|
||||
// date can be month-only too, per `Purchase.date`'s documented shape), so
|
||||
// check here, before the walk starts, and name the cost.
|
||||
if (cost.recurrence.kind === "everyNMonths" && !DAY_GRANULAR_DATE.test(startingPoint)) {
|
||||
throw new Error(
|
||||
`Recurring cost "${cost.name}" is anchored (lastCharge) with a monthly recurrence, so every ` +
|
||||
`date it steps from must be day-granular ("YYYY-MM-DD") — got "${startingPoint}". Give it a ` +
|
||||
`specific day: on its startDate if it has no charges yet, or on its latest charge otherwise.`,
|
||||
);
|
||||
}
|
||||
|
||||
let cursor = anchor === null ? cost.startDate : stepFrom(anchor, cost.recurrence, cost.anchoring);
|
||||
|
||||
|
||||
@@ -833,3 +833,56 @@ describe("confirmOccurrence", () => {
|
||||
// always succeeds — because the anchored walk always re-derives its cursor
|
||||
// from whichever charge is latest, never from a `Purchase.date` a caller is
|
||||
// trying to move "behind" a slot that no longer exists.
|
||||
|
||||
// REVIEW ROUND 2 / I3: that claim ("now behaves identically — the edit always
|
||||
// succeeds") was asserted above but never actually pinned by a test — the
|
||||
// deleted test #10 ("a date more than one cycle before the slot it claims is
|
||||
// refused") half-covered this shape and was removed along with it, leaving a
|
||||
// silent gap. Inverted rather than left out: same fixture the old refused-edit
|
||||
// test used, but now asserting the edit SUCCEEDS, the fold still folds
|
||||
// cleanly afterwards, and the series re-anchors to the edited date rather
|
||||
// than the original confirmation date.
|
||||
describe("editing a charge's date backwards re-anchors instead of throwing", () => {
|
||||
test("moving a confirmed charge's date more than one cycle earlier re-anchors the series to it", () => {
|
||||
const data = createSeedDataset("2026-08");
|
||||
data.posts[0]!.id = "housing";
|
||||
const car = actions.addRecurringCost(data, {
|
||||
name: "Car insurance",
|
||||
archived: false,
|
||||
amount: { amount: 900, currency: "DKK" },
|
||||
startDate: "2026-08-14",
|
||||
recurrence: { kind: "everyNDays", n: 30 },
|
||||
anchoring: "lastCharge",
|
||||
splitMode: "percent",
|
||||
splits: [{ postId: "housing", value: 100, absorbsRemainder: true }],
|
||||
});
|
||||
|
||||
const purchase = actions.confirmOccurrence(data, car.id, "2026-09-13", {
|
||||
date: "2026-09-13",
|
||||
});
|
||||
expect(() => occurrencesByMonth(data, "2026-12")).not.toThrow();
|
||||
|
||||
// Under the old guard, an edit like this ("more than one cycle before the
|
||||
// slot it claims") was refused. Now it just succeeds. (2026-08-20, not
|
||||
// something before the cost's own 2026-08-14 startDate — an edit that far
|
||||
// back would hit the DELIBERATE "can't pull the series before startDate"
|
||||
// clamp tested in occurrences.test.ts, which is a different behaviour
|
||||
// than the one this test is pinning.)
|
||||
actions.updatePurchase(data, purchase.id, { date: "2026-08-20" });
|
||||
expect(data.purchases.find((p) => p.id === purchase.id)!.date).toBe("2026-08-20");
|
||||
|
||||
// The fold still works...
|
||||
expect(() => occurrencesByMonth(data, "2026-12")).not.toThrow();
|
||||
|
||||
// ...and the series re-anchors to the EDITED date: the next projected
|
||||
// occurrence is one cycle (30 days) after 2026-08-20, not after the
|
||||
// original 2026-09-13.
|
||||
const byMonth = occurrencesByMonth(data, "2026-12");
|
||||
const dates = [...byMonth.values()]
|
||||
.flat()
|
||||
.filter((o) => o.recurringId === car.id)
|
||||
.map((o) => o.date)
|
||||
.sort();
|
||||
expect(dates[0]).toBe("2026-09-19"); // 2026-08-20 + 30 days
|
||||
});
|
||||
});
|
||||
|
||||
@@ -298,6 +298,33 @@ describe("expectedGroups — X3 (a cost with an outstanding slot never also show
|
||||
// throws — is still true, and now trivially so: nothing in the anchored walk
|
||||
// can throw from a confirmation's date any more.
|
||||
describe("expectedGroups — coming-up confirmations never break the fold (formerly X1)", () => {
|
||||
test("REVIEW ROUND 2 (Minor): a day-granular anchored cost's future slot IS now offered once this month is settled — X1's suppression is gone, not replaced", () => {
|
||||
// This is deleted test #8's scenario ("stays suppressed even once X3 no
|
||||
// longer applies") — still perfectly reachable, and now completely
|
||||
// untested. Pinning what actually happens today rather than leaving the
|
||||
// gap silent: September's slot is settled on time, so X3 has nothing left
|
||||
// to suppress, and there is no anchored-model X1 replacement, so the
|
||||
// October row (39 days from `today`, more than one 30-day cycle) IS
|
||||
// offered. If a future task reintroduces some suppression here, this test
|
||||
// will fail loudly and force that change to be deliberate.
|
||||
const data = baseDataset();
|
||||
data.recurring = [
|
||||
cost({
|
||||
id: "car",
|
||||
name: "Car insurance",
|
||||
amount: { amount: 349, currency: "DKK" },
|
||||
startDate: "2026-08-14",
|
||||
recurrence: { kind: "everyNDays", n: 30 },
|
||||
anchoring: "lastCharge",
|
||||
}),
|
||||
];
|
||||
confirmOccurrence(data, "car", "2026-09-13", { date: "2026-09-13" });
|
||||
|
||||
const groups = expectedGroups(data, "2026-09", "2026-09-04");
|
||||
expect(groups.pending).toEqual([]);
|
||||
expect(groups.comingUp.map((o) => o.date)).toEqual(["2026-10-13"]);
|
||||
});
|
||||
|
||||
test("regression: whatever expectedGroups offers as 'coming up', confirming it all today and folding never throws (demo Car insurance shape)", () => {
|
||||
const data = baseDataset();
|
||||
data.recurring = [
|
||||
|
||||
Reference in New Issue
Block a user