From 07d19b42b1a608829f41d5c5484694bf1bcbf2ac Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Wed, 27 May 2026 13:28:55 +0200 Subject: [PATCH] fix(home/filter): chart honors global Account filter (QA 2.3) The Home page Savings-over-time chart showed "No purchase history yet" for accounts that DID have purchases when selected via the topbar Account filter. Root cause: three issues compounded: 1. Backend SQL (fixed by PR #741): QueryHistory matched only the legacy account_id VARCHAR(20) column, not cloud_account_id UUID FK. The topbar chip sends cloud_accounts.id (a UUID), so all rows returned 0 and the chart fell into the empty-state branch. 2. Missing coalescing: setupDashboardHandlers subscribed directly with () => void loadDashboard(), causing two concurrent loadDashboard() calls on provider change (topbar fires both account-cleared and provider-changed synchronously). Added queueMicrotask coalescing matching the scheduleReload pattern in modules/savings-history.ts. 3. Missing home-tab guard: subscriptions fired even when the user was on a different tab, wasting API calls. Added isHomeTabActive() guard matching isPurchasesTabActive() in modules/savings-history.ts. 4. Non-informative empty state: when the API returned 0 data points due to an active filter, the chart showed the generic "No purchase history yet" message rather than indicating the filter caused the empty result. Added buildTrendFilterDesc() and filter-aware empty.textContent, mirroring showEmptyState() in savings-history.ts. Tests: 7 new tests covering subscription wiring, active-tab guard, coalescing, account_id forwarding, and filter-aware empty-state copy. All 63 frontend test suites pass; all Go tests pass. Mirrors PR #741 fix for the Purchases page (same filter-not-wired pattern). Verified by selecting each account in turn and confirming the chart reflects only that account's history. Refs QA row 384, step 2.3. --- frontend/src/__tests__/dashboard.test.ts | 135 ++++++++++++++++++++++- frontend/src/dashboard.ts | 59 +++++++++- 2 files changed, 190 insertions(+), 4 deletions(-) diff --git a/frontend/src/__tests__/dashboard.test.ts b/frontend/src/__tests__/dashboard.test.ts index bc75f4ec9..d03caa03b 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,139 @@ describe('Dashboard Module', () => { }); }); + // QA row 384 step 2.3: Home page Savings-over-time chart must honor the + // global Account filter (issue #701). These tests verify that: + // 1. setupDashboardHandlers subscribes to both filter chips. + // 2. An account-chip change re-fetches when Home tab is active. + // 3. No re-fetch fires when Home tab is inactive. + // 4. Back-to-back provider+account fires coalesce into one reload. + // 5. The account_id is forwarded to getSavingsAnalytics. + // 6. Filter-aware empty-state copy is shown when a filter is active. + describe('setupDashboardHandlers — filter chip subscriptions (QA 2.3)', () => { + function addHomeTab(active: boolean): void { + const div = document.createElement('div'); + div.id = 'home-tab'; + if (active) div.classList.add('active'); + document.body.appendChild(div); + } + + beforeEach(() => { + (state.subscribeProvider as jest.Mock).mockClear(); + (state.subscribeAccount as jest.Mock).mockClear(); + (api.getSavingsAnalytics as jest.Mock).mockClear(); + (api.getDashboardSummary as jest.Mock).mockResolvedValue({ + potential_monthly_savings: 0, total_recommendations: 0, + active_commitments: 0, committed_monthly: 0, current_coverage: 0, + target_coverage: 80, ytd_savings: 0, by_service: {}, + }); + (api.getUpcomingPurchases as jest.Mock).mockResolvedValue({ purchases: [] }); + (api.getRecommendations as jest.Mock).mockResolvedValue([]); + (api.getSavingsAnalytics as jest.Mock).mockResolvedValue({ data_points: [] }); + + // Ensure a canvas for savings-trend-chart exists so loadSavingsTrendChart + // does not bail out at the early-return guard. + const canvas = document.createElement('canvas'); + canvas.id = 'savings-trend-chart'; + const empty = document.createElement('p'); + empty.id = 'savings-trend-empty'; + empty.className = 'hidden'; + document.body.appendChild(canvas); + document.body.appendChild(empty); + }); + + test('registers one callback each with subscribeProvider and 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 triggers loadDashboard (and getSavingsAnalytics) when home tab is active', async () => { + addHomeTab(true); + (state.getCurrentAccountIDs as jest.Mock).mockReturnValue(['uuid-acct-1']); + + setupDashboardHandlers(); + const accountCb = (state.subscribeAccount as jest.Mock).mock.calls[0]?.[0] as () => void; + + (api.getSavingsAnalytics as jest.Mock).mockClear(); + accountCb(); + await new Promise((r) => setTimeout(r, 0)); + + expect(api.getSavingsAnalytics).toHaveBeenCalledTimes(1); + expect(api.getSavingsAnalytics).toHaveBeenCalledWith( + expect.objectContaining({ account_ids: ['uuid-acct-1'] }) + ); + }); + + test('does NOT fire when home tab is inactive (active-tab guard)', async () => { + addHomeTab(false); + + setupDashboardHandlers(); + const accountCb = (state.subscribeAccount as jest.Mock).mock.calls[0]?.[0] as () => void; + const providerCb = (state.subscribeProvider as jest.Mock).mock.calls[0]?.[0] as () => void; + + (api.getSavingsAnalytics as jest.Mock).mockClear(); + (api.getDashboardSummary as jest.Mock).mockClear(); + accountCb(); + providerCb(); + await new Promise((r) => setTimeout(r, 0)); + + expect(api.getDashboardSummary).not.toHaveBeenCalled(); + }); + + test('back-to-back provider+account fires coalesce into one reload', async () => { + addHomeTab(true); + + 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.getDashboardSummary as jest.Mock).mockClear(); + // Simulate topbar provider-change: clears accounts then sets provider, + // per the #185 ordering rule — both fire synchronously. + accountCb(); + providerCb(); + await new Promise((r) => setTimeout(r, 0)); + + expect(api.getDashboardSummary).toHaveBeenCalledTimes(1); + }); + + test('loadSavingsTrendChart forwards account_id to the analytics API', async () => { + (state.getCurrentAccountIDs as jest.Mock).mockReturnValue(['uuid-acct-2']); + + await loadSavingsTrendChart(); + + expect(api.getSavingsAnalytics).toHaveBeenCalledWith( + expect.objectContaining({ account_ids: ['uuid-acct-2'] }) + ); + }); + + test('empty-state shows filter name when account chip is active (QA 2.3)', async () => { + (state.getCurrentAccountIDs as jest.Mock).mockReturnValue(['uuid-acct-3']); + (api.getSavingsAnalytics as jest.Mock).mockResolvedValue({ data_points: [] }); + + await loadSavingsTrendChart(); + + const empty = document.getElementById('savings-trend-empty'); + expect(empty?.classList.contains('hidden')).toBe(false); + expect(empty?.textContent).toContain('uuid-acct-3'); + }); + + test('empty-state shows generic message when no filter is active', async () => { + (state.getCurrentAccountIDs as jest.Mock).mockReturnValue([]); + (state.getCurrentProvider as jest.Mock).mockReturnValue(''); + (api.getSavingsAnalytics as jest.Mock).mockResolvedValue({ data_points: [] }); + + await loadSavingsTrendChart(); + + const empty = document.getElementById('savings-trend-empty'); + expect(empty?.classList.contains('hidden')).toBe(false); + expect(empty?.textContent).toContain('No purchase history yet'); + }); + }); + // 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..dec4d6f83 100644 --- a/frontend/src/dashboard.ts +++ b/frontend/src/dashboard.ts @@ -32,6 +32,17 @@ let savingsTrendRange: '7' | '30' | '90' | 'all' = '90'; // enumerates real pending executions to surface execution_id properly. let upcomingPurchasesIndex: Map = new Map(); +/** + * True when the Home tab is the currently-visible tab. Used by the + * reload-on-filter-change subscriptions below to skip the fetch (and + * the resulting skeleton flash) when the user is on another tab — + * switchTab('home') calls loadDashboard() on entry anyway. + * Mirrors the isPurchasesTabActive() guard in modules/savings-history.ts. + */ +function isHomeTabActive(): boolean { + return document.getElementById('home-tab')?.classList.contains('active') === true; +} + /** * Setup dashboard event handlers */ @@ -41,8 +52,27 @@ export function setupDashboardHandlers(): void { // 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()); + // + // Coalescing: the provider-change handler in topbar-filters.ts fires + // BOTH the account subscriber (setCurrentAccountIDs([])) AND the + // provider subscriber (setCurrentProvider(newProv)) synchronously. + // Without coalescing, two loadDashboard() calls race back-to-back. + // queueMicrotask defers the actual fetch to after the current call + // stack clears, so the two fires collapse into one reload. + // Active-tab guard: skip the fetch when the Home tab is not visible; + // switchTab('home') triggers loadDashboard() on entry. + // Mirrors the scheduleReload pattern in modules/savings-history.ts. + let dashboardReloadQueued = false; + const scheduleDashboardReload = (): void => { + if (!isHomeTabActive() || dashboardReloadQueued) return; + dashboardReloadQueued = true; + queueMicrotask(() => { + dashboardReloadQueued = false; + if (isHomeTabActive()) void loadDashboard(); + }); + }; + state.subscribeProvider(scheduleDashboardReload); + state.subscribeAccount(scheduleDashboardReload); setupSavingsTrendHandlers(); } @@ -576,6 +606,19 @@ async function cancelScheduledPurchase(executionId: string): Promise { } } +/** + * Build a short human-readable description of the active topbar filter + * for use in the Savings Trend empty-state message. Returns '' when no + * filter is active so callers can distinguish "unfiltered empty" from + * "filtered empty". Mirrors buildFilterDesc() in modules/savings-history.ts. + */ +function buildTrendFilterDesc(provider: string, accountIDs: readonly string[]): string { + const parts: string[] = []; + if (provider && provider.toLowerCase() !== 'all') parts.push(provider.toUpperCase()); + if (accountIDs.length > 0) parts.push(accountIDs[0] ?? ''); + return parts.join(', '); +} + /** * Load the savings-over-time trend chart for the dashboard. Fetches the * history analytics endpoint with the currently-selected range and @@ -597,6 +640,7 @@ export async function loadSavingsTrendChart(): Promise { // filter is single-select so we pass the only selected ID or omit to // query all accessible accounts. const accountIDs = state.getCurrentAccountIDs(); + const currentProvider = state.getCurrentProvider(); const data = await api.getSavingsAnalytics({ start: start.toISOString(), end: end.toISOString(), @@ -606,7 +650,16 @@ export async function loadSavingsTrendChart(): Promise { if (!data.data_points || data.data_points.length === 0) { if (savingsTrendChart) { savingsTrendChart.destroy(); savingsTrendChart = null; } canvas.classList.add('hidden'); - empty?.classList.remove('hidden'); + if (empty) { + // Build a short description of the active filter so the empty-state + // copy distinguishes "no purchases exist yet" from "nothing in the + // selected scope". Mirrors modules/savings-history.ts showEmptyState(). + const filterDesc = buildTrendFilterDesc(currentProvider, accountIDs); + empty.textContent = filterDesc + ? `No savings data for the selected filter (${filterDesc}).` + : 'No purchase history yet — the chart will populate once you start executing plans.'; + empty.classList.remove('hidden'); + } attachSparkline('ytd', []); return; }