From 358aeb2a8249828b78d322ccdcc67a5792908b80 Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Wed, 20 May 2026 01:01:40 +0200 Subject: [PATCH 1/2] fix(ux/home): re-query Savings-over-time chart on filter chip change The Home tab's Savings-over-time chart rendered identical data across account selections because the dashboard subscribers fired loadDashboard() on every filter change without an active-tab guard or coalescing, mirroring a gap the Opportunities page had before PR #488 (closes #477). Apply the same pattern to dashboard.ts: - Active-tab guard: only fire loadDashboard() when #home-tab is active. switchTab('home') already reloads on entry, so a filter change while the user is on Opportunities/Plans/Purchases can defer; this avoids burning /dashboard/summary, /recommendations, and /history/analytics round-trips for a section the user is not looking at. - Microtask coalescing: topbar-filters.ts updates BOTH state slots on a provider chip change (clear accounts then set provider, per the #185 ordering rule), firing both subscribers from one user action. Without coalescing this would trigger two loadDashboard() calls back-to-back plus a stale-overwrite risk if responses race. queueMicrotask runs once after the synchronous state-mutation chain settles. - Re-check the active-tab flag inside the microtask so a tab-switch between chip click and microtask flush skips the now-unneeded fetch. URL persistence is already global via topbar-filters.ts (PR #488); Home inherits it for free. Test coverage (dashboard.test.ts, new describe 'subscriber wiring (#498)'): - Wiring assertion: setupDashboardHandlers registers callbacks with state.subscribeProvider and state.subscribeAccount. - Account chip change re-queries getSavingsAnalytics when home-tab is active. - Provider chip change re-queries getSavingsAnalytics when home-tab is active. - Inactive-tab guard: no fetch when home-tab is not .active. - Coalescing: back-to-back provider+account fires trigger one reload. - Bug-was-fixed: two consecutive account changes produce two fetches with distinct account_ids ([A] then [B]), proving the chart re-queries with fresh data rather than the same payload. Closes #498. --- frontend/src/__tests__/dashboard.test.ts | 174 ++++++++++++++++++++++- frontend/src/dashboard.ts | 56 ++++++-- 2 files changed, 221 insertions(+), 9 deletions(-) diff --git a/frontend/src/__tests__/dashboard.test.ts b/frontend/src/__tests__/dashboard.test.ts index bc75f4ec9..e5c318a02 100644 --- a/frontend/src/__tests__/dashboard.test.ts +++ b/frontend/src/__tests__/dashboard.test.ts @@ -71,7 +71,7 @@ jest.mock('../api', () => ({ // value don't need to be updated. getRecommendations: jest.fn().mockResolvedValue([]), })); -import { loadSavingsTrendChart, setupSavingsTrendHandlers } from '../dashboard'; +import { loadSavingsTrendChart, setupSavingsTrendHandlers, setupDashboardHandlers } from '../dashboard'; // Mock state module jest.mock('../state', () => ({ @@ -730,6 +730,178 @@ describe('Dashboard Module', () => { }); }); + // Issue #498: Home tab subscriber wiring. Chip changes must re-query + // the Savings-over-time chart with the new filter; inactive-tab guard + // and microtask coalescing mirror PR #488's recommendations.ts pattern. + describe('subscriber wiring (issue #498)', () => { + function makeDiv(id: string): HTMLDivElement { + const el = document.createElement('div'); + el.id = id; + return el; + } + + beforeEach(() => { + // Fresh DOM with the elements setupDashboardHandlers + loadDashboard + // + loadSavingsTrendChart touch. #home-tab is the active-tab guard + // target; the canvas and empty-state nodes are the chart targets; + // summary + upcoming-list are loadDashboard's render destinations. + while (document.body.firstChild) document.body.removeChild(document.body.firstChild); + document.body.appendChild(makeDiv('home-tab')); + document.body.appendChild(makeDiv('summary')); + document.body.appendChild(makeDiv('upcoming-list')); + const canvas = document.createElement('canvas'); + canvas.id = 'savings-trend-chart'; + document.body.appendChild(canvas); + const empty = document.createElement('p'); + empty.id = 'savings-trend-empty'; + empty.className = 'hidden'; + document.body.appendChild(empty); + + // Default mock shape: getDashboardSummary returns a minimal payload + // so loadDashboard's render path doesn't throw. getSavingsAnalytics + // returns an empty data_points list (chart hides + early-returns). + (api.getDashboardSummary as jest.Mock).mockResolvedValue({ + by_service: {}, + active_commitments: 0, + active_plans: 0, + upcoming_purchases: 0, + potential_monthly_savings: 0, + }); + (api.getUpcomingPurchases as jest.Mock).mockResolvedValue({ purchases: [] }); + (api.getRecommendations as jest.Mock).mockResolvedValue([]); + (api.getSavingsAnalytics as jest.Mock).mockResolvedValue({ data_points: [] }); + }); + + test('registers callbacks with state.subscribeProvider and state.subscribeAccount', () => { + setupDashboardHandlers(); + + expect(state.subscribeProvider).toHaveBeenCalledTimes(1); + expect(state.subscribeAccount).toHaveBeenCalledTimes(1); + expect(typeof (state.subscribeProvider as jest.Mock).mock.calls[0]?.[0]).toBe('function'); + expect(typeof (state.subscribeAccount as jest.Mock).mock.calls[0]?.[0]).toBe('function'); + }); + + test('account chip change re-queries the savings chart when home-tab is active', async () => { + const tab = document.getElementById('home-tab')!; + tab.classList.add('active'); + + setupDashboardHandlers(); + const accountCb = (state.subscribeAccount as jest.Mock).mock.calls[0]?.[0] as () => void; + expect(typeof accountCb).toBe('function'); + + (api.getSavingsAnalytics as jest.Mock).mockClear(); + (api.getDashboardSummary as jest.Mock).mockClear(); + accountCb(); + // queueMicrotask + loadDashboard's awaited Promise.allSettled + the + // fire-and-forget loadSavingsTrendChart all need to settle. A pair + // of macrotask flushes covers the chain. + await new Promise((r) => setTimeout(r, 0)); + await new Promise((r) => setTimeout(r, 0)); + + expect(api.getDashboardSummary).toHaveBeenCalledTimes(1); + expect(api.getSavingsAnalytics).toHaveBeenCalledTimes(1); + }); + + test('provider chip change re-queries the savings chart when home-tab is active', async () => { + const tab = document.getElementById('home-tab')!; + tab.classList.add('active'); + + setupDashboardHandlers(); + const providerCb = (state.subscribeProvider as jest.Mock).mock.calls[0]?.[0] as () => void; + expect(typeof providerCb).toBe('function'); + + (api.getSavingsAnalytics as jest.Mock).mockClear(); + providerCb(); + await new Promise((r) => setTimeout(r, 0)); + await new Promise((r) => setTimeout(r, 0)); + + expect(api.getSavingsAnalytics).toHaveBeenCalledTimes(1); + }); + + test('does NOT re-query when home-tab is inactive (active-tab guard)', async () => { + // #home-tab present but no .active class: user is on another tab. + setupDashboardHandlers(); + const providerCb = (state.subscribeProvider as jest.Mock).mock.calls[0]?.[0] as () => void; + const accountCb = (state.subscribeAccount as jest.Mock).mock.calls[0]?.[0] as () => void; + + (api.getSavingsAnalytics as jest.Mock).mockClear(); + (api.getDashboardSummary as jest.Mock).mockClear(); + providerCb(); + accountCb(); + await new Promise((r) => setTimeout(r, 0)); + await new Promise((r) => setTimeout(r, 0)); + + expect(api.getDashboardSummary).not.toHaveBeenCalled(); + expect(api.getSavingsAnalytics).not.toHaveBeenCalled(); + }); + + test('coalesces back-to-back provider+account fires into a single reload', async () => { + const tab = document.getElementById('home-tab')!; + tab.classList.add('active'); + + setupDashboardHandlers(); + const providerCb = (state.subscribeProvider as jest.Mock).mock.calls[0]?.[0] as () => void; + const accountCb = (state.subscribeAccount as jest.Mock).mock.calls[0]?.[0] as () => void; + + (api.getSavingsAnalytics as jest.Mock).mockClear(); + (api.getDashboardSummary as jest.Mock).mockClear(); + // The topbar provider-change handler updates BOTH state slots in + // sequence (clear accounts then set provider, per the #185 ordering + // rule), so both subscribers fire synchronously from one user + // action. Without coalescing this would trigger two fetches. + providerCb(); + accountCb(); + await new Promise((r) => setTimeout(r, 0)); + await new Promise((r) => setTimeout(r, 0)); + + expect(api.getDashboardSummary).toHaveBeenCalledTimes(1); + expect(api.getSavingsAnalytics).toHaveBeenCalledTimes(1); + }); + + test('bug-was-fixed: switching accounts triggers fetches with distinct account_ids and chart re-queries', async () => { + const tab = document.getElementById('home-tab')!; + tab.classList.add('active'); + + // Distinct response per account proves the chart isn't just re-firing + // with the same data: the new account flows through to the request + // and the response is fresh data. + const responseForA = { data_points: [{ timestamp: '2026-01-01T00:00:00Z', cumulative_savings: 100, total_savings: 100 }] }; + const responseForB = { data_points: [{ timestamp: '2026-01-01T00:00:00Z', cumulative_savings: 500, total_savings: 500 }] }; + (api.getSavingsAnalytics as jest.Mock) + .mockResolvedValueOnce(responseForA) + .mockResolvedValueOnce(responseForB); + (state.getCurrentAccountIDs as jest.Mock) + .mockReturnValueOnce(['acct-A']) + .mockReturnValueOnce(['acct-A']) + .mockReturnValueOnce(['acct-A']) + .mockReturnValueOnce(['acct-B']) + .mockReturnValueOnce(['acct-B']) + .mockReturnValueOnce(['acct-B']); + + setupDashboardHandlers(); + const accountCb = (state.subscribeAccount as jest.Mock).mock.calls[0]?.[0] as () => void; + + // First chip change: select acct-A. + accountCb(); + await new Promise((r) => setTimeout(r, 0)); + await new Promise((r) => setTimeout(r, 0)); + // Second chip change: select acct-B. Microtask flag is reset inside + // the previous microtask so this fire schedules a fresh reload. + accountCb(); + await new Promise((r) => setTimeout(r, 0)); + await new Promise((r) => setTimeout(r, 0)); + + // Two distinct fetches, each carrying the account that was active + // at the time of the fetch. The chart loader passes account_ids + // only when exactly one account is selected, which matches the + // single-account chip semantics. + const calls = (api.getSavingsAnalytics as jest.Mock).mock.calls; + expect(calls).toHaveLength(2); + expect(calls[0]?.[0]).toMatchObject({ account_ids: ['acct-A'] }); + expect(calls[1]?.[0]).toMatchObject({ account_ids: ['acct-B'] }); + }); + }); + // Issue #185 invariant — clear account state BEFORE awaiting the // account-list refetch on provider change — moved from dashboard.ts // to topbar-filters.ts as part of issue #344 T2. The dashboard no diff --git a/frontend/src/dashboard.ts b/frontend/src/dashboard.ts index 102d866cc..d60c21fb2 100644 --- a/frontend/src/dashboard.ts +++ b/frontend/src/dashboard.ts @@ -33,16 +33,56 @@ let savingsTrendRange: '7' | '30' | '90' | 'all' = '90'; let upcomingPurchasesIndex: Map = new Map(); /** - * Setup dashboard event handlers + * True when the Home tab is the currently-visible tab. The reload-on- + * filter-change subscriptions below skip the fetch when this is false so + * we don't burn an API call (and a skeleton flash) for a section the user + * isn't looking at: `switchTab('home')` runs loadDashboard() on next + * entry anyway, so the user always sees data matching the active filter. + */ +function isHomeTabActive(): boolean { + return document.getElementById('home-tab')?.classList.contains('active') === true; +} + +/** + * Setup dashboard event handlers (issue #498). + * + * Filter source-of-truth lives in state.ts (mutated by the global topbar + * chips). Subscribe to filter changes and reload the dashboard; the issue + * #185 ordering rule (clear accounts before refetching for a new provider) + * is enforced by topbar-filters.ts at the source so loadDashboard() always + * sees consistent state. + * + * Mirrors the recommendations.ts pattern from PR #488 (closes #477): + * - Active-tab guard: only fire loadDashboard() when home-tab is active. + * - Coalesce duplicate reloads via queueMicrotask: the provider-change + * handler in topbar-filters.ts updates BOTH state slots (clear + * accounts then set provider, per the #185 ordering rule), firing + * account- AND provider-subscribers from one user action. Without + * coalescing we'd kick off two loadDashboard() calls back-to-back: + * extra API load plus a stale-overwrite risk if the first response + * lands after the second. + * + * Microtask scheduling: both subscriber fires are synchronous within the + * same setCurrentProvider/setCurrentAccountIDs call chain, so a microtask + * runs once after the chain settles. setTimeout(_, 0) would also work but + * adds a macrotask delay the user could perceive on slow machines. */ export function setupDashboardHandlers(): void { - // Filter source-of-truth lives in state.ts (mutated by the global - // topbar chips). Subscribe to filter changes and reload the dashboard; - // the issue #185 ordering rule (clear accounts before refetching for a - // new provider) is enforced by topbar-filters.ts at the source so the - // dashboard's loadDashboard() always sees consistent state. - state.subscribeProvider(() => void loadDashboard()); - state.subscribeAccount(() => void loadDashboard()); + let reloadQueued = false; + const scheduleReload = (): void => { + if (!isHomeTabActive() || reloadQueued) return; + reloadQueued = true; + queueMicrotask(() => { + reloadQueued = false; + // Re-check active-tab inside the microtask: the user could have + // switched tabs between the chip change and the microtask flushing, + // in which case the fetch is now unneeded (switchTab on next entry + // will reload). + if (isHomeTabActive()) void loadDashboard(); + }); + }; + state.subscribeProvider(scheduleReload); + state.subscribeAccount(scheduleReload); setupSavingsTrendHandlers(); } From 8ae19d36c28108b7549192f9dbc70ffd5e0d2e63 Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Wed, 20 May 2026 01:06:46 +0200 Subject: [PATCH 2/2] test(dashboard): cover mid-flight tab-switch cancels queued reload Address CodeRabbit nitpick on PR #500: lock in the inside-microtask active-tab re-check by exercising the path where the chip change fires while Home is active but the user switches tabs before the microtask flushes. The existing tests verified active/inactive at callback time but not the mid-flight cancellation guarded by lines 77-81 of dashboard.ts. --- frontend/src/__tests__/dashboard.test.ts | 25 ++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/frontend/src/__tests__/dashboard.test.ts b/frontend/src/__tests__/dashboard.test.ts index e5c318a02..de02d1446 100644 --- a/frontend/src/__tests__/dashboard.test.ts +++ b/frontend/src/__tests__/dashboard.test.ts @@ -858,6 +858,31 @@ describe('Dashboard Module', () => { expect(api.getSavingsAnalytics).toHaveBeenCalledTimes(1); }); + test('cancels queued reload when Home becomes inactive before microtask flush', async () => { + // Covers the re-check inside the microtask: a chip change fires + // while Home is active, but the user switches tabs before the + // microtask runs. The deferred fetch should be cancelled. + const tab = document.getElementById('home-tab')!; + tab.classList.add('active'); + + setupDashboardHandlers(); + const accountCb = (state.subscribeAccount as jest.Mock).mock.calls[0]?.[0] as () => void; + + (api.getDashboardSummary as jest.Mock).mockClear(); + (api.getSavingsAnalytics as jest.Mock).mockClear(); + + // Queue the reload while Home is active. + accountCb(); + // Synchronously deactivate Home before the microtask flushes. + tab.classList.remove('active'); + + await new Promise((r) => setTimeout(r, 0)); + await new Promise((r) => setTimeout(r, 0)); + + expect(api.getDashboardSummary).not.toHaveBeenCalled(); + expect(api.getSavingsAnalytics).not.toHaveBeenCalled(); + }); + test('bug-was-fixed: switching accounts triggers fetches with distinct account_ids and chart re-queries', async () => { const tab = document.getElementById('home-tab')!; tab.classList.add('active');