From 7852184d21d0f2f068eba276abbd63f1df40186e Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Fri, 19 Jun 2026 16:45:09 +0200 Subject: [PATCH 1/2] fix(dashboard): fix Home savings 'All' range and label chart tooltip date QA 3.1: The 'All' timeframe sent '1970-01-01T00:00:00Z' as the start param, triggering HTTP 400 "date range too large" from the analytics handler (which caps at 366 days). The catch block rendered all errors as "Savings history is not available yet." Replace the epoch sentinel with a rolling ~365-day window (now - 365d) that fits the backend cap. The existing axisMinMs anchor-to-earliest-point logic is preserved. QA 3.2: The tooltip had no title callback, so Chart.js rendered the raw 13-digit millisecond x value as the tooltip header. Add a title callback that formats the x timestamp via the existing formatTrendAxisTick helper. Closes #1248 --- frontend/src/__tests__/dashboard.test.ts | 54 ++++++++++++++++++++++-- frontend/src/dashboard.ts | 30 ++++++------- 2 files changed, 65 insertions(+), 19 deletions(-) diff --git a/frontend/src/__tests__/dashboard.test.ts b/frontend/src/__tests__/dashboard.test.ts index 8896d78e6..e33b8c4b1 100644 --- a/frontend/src/__tests__/dashboard.test.ts +++ b/frontend/src/__tests__/dashboard.test.ts @@ -872,7 +872,11 @@ describe('Dashboard Module', () => { expect(Math.round(span / 86400_000)).toBe(30); }); - test('All range sends epoch sentinel as start (not a client-side 3650d ceiling)', async () => { + // QA 3.1: 'all' range must NOT send the epoch sentinel (1970-01-01) as + // the start param. The backend caps the date range at 366 days and returns + // HTTP 400 for a 1970 start, which the catch block rendered as an empty- + // state error. The fix uses a rolling ~365-day window that fits the cap. + test('All range sends a start within ~365 days of now, not the epoch sentinel (QA 3.1)', async () => { // Add an 'all' button and make it active. const bAll = document.createElement('button'); bAll.className = 'trend-range'; @@ -880,15 +884,29 @@ describe('Dashboard Module', () => { bAll.textContent = 'All'; document.body.appendChild(bAll); setupSavingsTrendHandlers(); + (api.getSavingsAnalytics as jest.Mock).mockResolvedValue({ + data_points: [{ timestamp: new Date().toISOString(), cumulative_savings: 500, total_savings: 50, total_upfront: 0, purchase_count: 1 }], + }); (api.getSavingsAnalytics as jest.Mock).mockClear(); + (api.getSavingsAnalytics as jest.Mock).mockResolvedValue({ + data_points: [{ timestamp: new Date().toISOString(), cumulative_savings: 500, total_savings: 50, total_upfront: 0, purchase_count: 1 }], + }); bAll.click(); await new Promise(r => setTimeout(r, 0)); const call = (api.getSavingsAnalytics as jest.Mock).mock.calls[0]?.[0]; - // Must send the epoch sentinel so the backend returns unbounded history. - // A computed 'now - 3650d' would silently cap accounts with older data. - expect(call.start).toBe('1970-01-01T00:00:00Z'); + // Must NOT be the epoch sentinel -- that causes HTTP 400 from the backend. + expect(call.start).not.toBe('1970-01-01T00:00:00Z'); + // Start must be within ~365 days of now (within 366 days to fit the cap). + const startMs = new Date(call.start as string).getTime(); + const nowMs = Date.now(); + const ageMs = nowMs - startMs; + expect(ageMs).toBeGreaterThan(0); + expect(ageMs).toBeLessThanOrEqual(366 * 86400_000 + 5_000); // 5s clock tolerance + // The chart must render (not show empty-state) when the API returns data. + const empty = document.getElementById('savings-trend-empty'); + expect(empty?.classList.contains('hidden')).toBe(true); }); // QA row 405, step 3.1 — x-axis windowing behaviour. @@ -967,6 +985,34 @@ describe('Dashboard Module', () => { expect(canvas?.classList.contains('hidden')).toBe(true); expect(empty?.classList.contains('hidden')).toBe(false); }); + + // QA 3.2: tooltip title callback must return a formatted date string, not + // the raw 13-digit millisecond timestamp that Chart.js renders by default + // when no title callback is configured. + test('tooltip title callback returns a formatted date string, not a raw ms timestamp (QA 3.2)', async () => { + const purchaseTs = '2024-06-15T12:00:00Z'; + (api.getSavingsAnalytics as jest.Mock).mockResolvedValue({ + data_points: [{ timestamp: purchaseTs, cumulative_savings: 250, total_savings: 10, total_upfront: 500, purchase_count: 1 }], + }); + + await loadSavingsTrendChart(); + + const chartCall = (Chart as unknown as jest.Mock).mock.calls[0]; + const titleCb = chartCall[1].options.plugins.tooltip.callbacks.title as + (items: Array<{ raw: { x: number; y: number } }>) => string; + + const xMs = new Date(purchaseTs).getTime(); + const result = titleCb([{ raw: { x: xMs, y: 250 } }]); + + // Must be a non-empty string. + expect(typeof result).toBe('string'); + expect(result.length).toBeGreaterThan(0); + // Must NOT be the raw numeric millisecond value. + expect(result).not.toBe(String(xMs)); + expect(result).not.toMatch(/^\d{13}$/); + // Must contain recognizable date text (month abbreviation). + expect(result).toMatch(/Jun\s+\d+/); + }); }); describe('formatTrendAxisTick (QA 3.1)', () => { diff --git a/frontend/src/dashboard.ts b/frontend/src/dashboard.ts index 351b0a899..767eba2df 100644 --- a/frontend/src/dashboard.ts +++ b/frontend/src/dashboard.ts @@ -1017,19 +1017,15 @@ export async function loadSavingsTrendChart(): Promise { const now = new Date(); const nowMs = now.getTime(); const isAllRange = savingsTrendRange === 'all'; - // For 'all', pass the Unix epoch as the start sentinel so the backend - // returns every data point it holds. parseDateRange on the backend - // defaults a missing start to (end - 7d), so we must send an explicit - // floor rather than omitting the param — epoch is the lowest valid - // RFC3339 value and has no practical upper bound on history length. - // A client-side 3650-day ceiling would silently truncate accounts with - // purchase history older than ~10 years. - const epochStart = '1970-01-01T00:00:00Z'; const days = isAllRange ? null : parseInt(savingsTrendRange, 10); - // windowStart is the left edge of the axis; for 'all' it is overridden - // below to the earliest purchase timestamp (or now-365d if no purchases). + // For 'all', use a rolling ~365-day window — the maximum history the + // analytics API allows (~366-day cap in handler_analytics.go). Sending a + // 1970 epoch sentinel caused HTTP 400 "date range too large" which the + // catch block rendered as an empty-state error (QA 3.1). The axisMinMs + // anchor-to-earliest-point logic below still applies so sparse data fills + // the full chart width correctly. const windowStartMs = isAllRange ? nowMs - 365 * 86400_000 : nowMs - (days as number) * 86400_000; - const intervalDays = isAllRange ? 3650 : (days as number); + const intervalDays = isAllRange ? 365 : (days as number); const interval: 'hourly' | 'daily' | 'weekly' = intervalDays <= 7 ? 'hourly' : intervalDays <= 90 ? 'daily' : 'weekly'; try { @@ -1043,10 +1039,7 @@ export async function loadSavingsTrendChart(): Promise { const accountIDs = state.getCurrentAccountIDs(); const provider = state.getCurrentProvider(); const data = await api.getSavingsAnalytics({ - // For 'all': send the epoch sentinel so the backend returns unbounded - // history. Omitting start would cause parseDateRange to default to - // (end - 7d), silently clipping the chart (see handler_analytics.go). - start: isAllRange ? epochStart : new Date(windowStartMs).toISOString(), + start: new Date(windowStartMs).toISOString(), end: now.toISOString(), interval, ...(provider ? { provider } : {}), @@ -1128,6 +1121,13 @@ export async function loadSavingsTrendChart(): Promise { legend: { display: false }, tooltip: { callbacks: { + // Format the x timestamp as a human-readable date so the tooltip + // header shows a date string rather than the raw 13-digit + // millisecond value (QA 3.2). + title: (items) => { + const raw = items[0]?.raw as { x: number; y: number } | undefined; + return raw?.x != null ? formatTrendAxisTick(raw.x, interval) : ''; + }, label: (ctx) => `Cumulative savings: $${((ctx.raw as { x: number; y: number }).y).toLocaleString()}`, }, }, From 8008bc0e293b5ea5b5dd48ddf93f683d0ba59523 Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Fri, 26 Jun 2026 16:31:53 +0200 Subject: [PATCH 2/2] chore(dashboard): fix stale trend-chart docstring + dedupe All-range test mock Adversarial-review follow-ups to PR #1257: - frontend/src/dashboard.ts: the loadSavingsTrendChart docstring still described the pre-QA-2.3 "empty windows render labelled axes" behaviour; the shipped code (lines 1083-1098) shows a filter-aware empty-state banner instead. Reconciled the docstring to describe the actual three branches (data / empty / fetch-error). - frontend/src/__tests__/dashboard.test.ts: the new "All range sends ~365d start" test set mockResolvedValue twice with a mockClear() in between. mockClear() resets call history but NOT mock return values, so the first set was dead code. Collapsed to a single mockResolvedValue + mockClear with a clarifying comment. 97 dashboard tests pass, tsc clean. No behaviour change. --- frontend/src/__tests__/dashboard.test.ts | 5 ++--- frontend/src/dashboard.ts | 13 ++++++++----- 2 files changed, 10 insertions(+), 8 deletions(-) diff --git a/frontend/src/__tests__/dashboard.test.ts b/frontend/src/__tests__/dashboard.test.ts index e33b8c4b1..eb242f1cf 100644 --- a/frontend/src/__tests__/dashboard.test.ts +++ b/frontend/src/__tests__/dashboard.test.ts @@ -884,13 +884,12 @@ describe('Dashboard Module', () => { bAll.textContent = 'All'; document.body.appendChild(bAll); setupSavingsTrendHandlers(); + // Override the beforeEach default of empty data_points so the chart + // actually renders -- the test asserts the empty-state stays hidden. (api.getSavingsAnalytics as jest.Mock).mockResolvedValue({ data_points: [{ timestamp: new Date().toISOString(), cumulative_savings: 500, total_savings: 50, total_upfront: 0, purchase_count: 1 }], }); (api.getSavingsAnalytics as jest.Mock).mockClear(); - (api.getSavingsAnalytics as jest.Mock).mockResolvedValue({ - data_points: [{ timestamp: new Date().toISOString(), cumulative_savings: 500, total_savings: 50, total_upfront: 0, purchase_count: 1 }], - }); bAll.click(); await new Promise(r => setTimeout(r, 0)); diff --git a/frontend/src/dashboard.ts b/frontend/src/dashboard.ts index 767eba2df..ffab5b4db 100644 --- a/frontend/src/dashboard.ts +++ b/frontend/src/dashboard.ts @@ -1003,11 +1003,14 @@ export function formatTrendAxisTick(tsMs: number, intervalHint: 'hourly' | 'dail /** * Load the savings-over-time trend chart for the dashboard. Fetches the - * history analytics endpoint with the currently-selected range and - * renders a line chart of cumulative savings spanning the full selected - * window on the x-axis (QA row 405, step 3.1). Empty windows render - * labelled axes rather than a "no data" stub; only fetch failures use - * the error stub. + * history analytics endpoint with the currently-selected range and renders + * a line chart of cumulative savings spanning the full selected window on + * the x-axis (QA row 405, step 3.1). When the response is empty (QA 2.3 + * supersedes QA 3.1's earlier empty-axes suggestion), the canvas is hidden + * and an empty-state banner is shown: a filter-aware "No savings history + * for " when a provider or account chip is active, otherwise the + * generic "No purchase history yet." Fetch failures hide the canvas and + * show the "Savings history is not available yet." stub instead. */ export async function loadSavingsTrendChart(): Promise { const canvas = document.getElementById('savings-trend-chart') as HTMLCanvasElement | null;