From abc9076d234cee156ad0f081aef076a6a644bc7b Mon Sep 17 00:00:00 2001 From: le king fu Date: Thu, 8 Oct 2026 21:05:30 -0400 Subject: [PATCH] fix(budget): drop the grid's target once the grid leaves the page MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit On a profile without transactions whose import steps were skipped, the rows can land before the transaction count, both reads queueing behind the DB lock: the grid mounted, reported "Expand all", then gave way to the empty state, and the card kept "Click Expand all…" above it with no grid on the page. The page now passes no target while it shows the empty state, and a wiring test replays that sequence. Invisible in dev, where StrictMode reorders the two reads. The month test sets the clock to 23:00 on December 31, which is still December in UTC: under the CI runner's zone, a month read in UTC passed. The zone is pinned west of Greenwich, as in useImportWizard.test.ts. Review of #400 (PR #406). Co-Authored-By: Claude Opus 5.5 --- .../budget/BudgetTable.render.test.tsx | 22 +++++++++++++--- src/pages/BudgetPage.tsx | 9 +++++-- src/pages/BudgetPage.wiring.test.ts | 25 +++++++++++++++++++ 3 files changed, 50 insertions(+), 6 deletions(-) diff --git a/src/components/budget/BudgetTable.render.test.tsx b/src/components/budget/BudgetTable.render.test.tsx index d943c06..f747d00 100644 --- a/src/components/budget/BudgetTable.render.test.tsx +++ b/src/components/budget/BudgetTable.render.test.tsx @@ -5,7 +5,7 @@ // subtotal-position preference is read from a stubbed localStorage. What is // pinned is where the one highlight lands: "Expand all" or this month's cell // of the first editable row shown. -import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; +import { describe, it, expect, vi, beforeAll, afterAll, beforeEach, afterEach } from "vitest"; import { renderToStaticMarkup } from "react-dom/server"; import type { BudgetYearRow } from "../../shared/types"; import type { CollapseAccessors } from "../../utils/collapsibleRows"; @@ -172,9 +172,23 @@ describe("BudgetTable — the getting-started card's target (#400)", () => { expect(count(html, HIGHLIGHT)).toBe(1); }); - it("the cell follows the clock's month", () => { - vi.setSystemTime(new Date(2026, DECEMBER, 31, 23, 0)); - expect(ringedMonth(rowOf(render(FLAT, true), "Divers"))).toBe(DECEMBER); + describe("in a timezone west of Greenwich", () => { + // Under TZ=UTC (the CI runner), 23:00 on December 31 is December both in + // local time and in UTC, so a month read in UTC would pass unnoticed. The + // zone is pinned as in useImportWizard.test.ts (#356). + const originalTz = process.env.TZ; + beforeAll(() => { + process.env.TZ = "America/Toronto"; + }); + afterAll(() => { + if (originalTz === undefined) delete process.env.TZ; + else process.env.TZ = originalTz; + }); + + it("the cell follows the clock's month", () => { + vi.setSystemTime(new Date(2026, DECEMBER, 31, 23, 0)); + expect(ringedMonth(rowOf(render(FLAT, true), "Divers"))).toBe(DECEMBER); + }); }); it("subtotals at the bottom: still the first editable row shown", () => { diff --git a/src/pages/BudgetPage.tsx b/src/pages/BudgetPage.tsx index 764017d..2a2feac 100644 --- a/src/pages/BudgetPage.tsx +++ b/src/pages/BudgetPage.tsx @@ -43,8 +43,13 @@ export default function BudgetPage() { // The getting-started card follows the grid (#400). Its target is // highlighted only while the card is on screen: a ring needs the card that - // explains it. - const cardModel = budgetCardModel({ target: gridTarget }, onboarding.checklist); + // explains it. The grid's last report does not outlive the grid: the rows + // can land before the transaction count (both reads queue behind the DB + // lock), so the grid may report, then give way to the empty state. + const cardModel = budgetCardModel( + { target: isEmptyProfile ? null : gridTarget }, + onboarding.checklist, + ); const highlight = cardModel.target !== null && showsStepCard(onboarding.mode, onboarding.cardPage, pathname); diff --git a/src/pages/BudgetPage.wiring.test.ts b/src/pages/BudgetPage.wiring.test.ts index 6023293..fd3b1e3 100644 --- a/src/pages/BudgetPage.wiring.test.ts +++ b/src/pages/BudgetPage.wiring.test.ts @@ -183,4 +183,29 @@ describe("BudgetPage — the grid's target and the card (#400)", () => { it("hands the grid the same callback at every render, so the grid does not report at every render", () => { expect(render().report).toBe(render().report); }); + + it("a profile without transactions: the grid's last report does not outlive the grid", () => { + // Steps 3 to 5 skipped: the budget step is next on a profile without any + // transaction. The rows can land before the count (both reads queue + // behind db.ts's lock): the grid mounts, reports "Expand all", then gives + // way to the empty state (review of #400). + const counters = { ...BUDGET_LEFT, imports: 0, manualCategorized: 0, transactions: 0 }; + const checklist = computeChecklist(counters, { + importFolderSet: true, + budgetAllowed: true, + skipped: ["source", "import", "manualCategory"], + }); + harness.onboarding = path(counters, { checklist, nextStep: nextStep(checklist) }); + const budget = harness.budget as { state: BudgetState }; + budget.state = { ...budget.state, totalInDb: null }; + render().report("expandAll"); + expect(render().model.text?.key).toBe("onboarding.card.budget.collapsed"); + + budget.state = { ...budget.state, totalInDb: 0 }; + harness.cursor = 0; + const page = BudgetPage() as AnyElement; + expect(find(page, (el) => el.type === BudgetTable)).toHaveLength(0); + const [card] = find(page, (el) => el.type === OnboardingStepCard); + expect(card.props.model).toEqual({ text: null, done: false, target: null }); + }); });