From 0c3729bb093639017a0527df9a47c42faf0122df Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Thu, 28 May 2026 00:44:47 +0200 Subject: [PATCH 1/2] feat(home/chart): repoint per-service range bars to potential savings + move to Potential section - Move #savings-by-service-section below #savings-chart-section so both live in the Potential Savings area of the Home page. Previously the range bars sat in the current/historical savings group despite showing forward-looking data. - Re-data the chart: renderSavingsByService now accepts LocalRecommendation[] instead of SavingsDataPoint[]. A new computeServiceStatsFromRecs helper groups recommendation rows by service.service and computes min/max of rec.savings within each group. Floor = min potential savings for that service; upside = max - min. Wired in loadDashboard alongside renderSavingsChart (both have recs in scope). Removed the now-redundant calls from loadSavingsTrendChart. - Update copy: heading -> "Potential savings range per service", dataset labels -> "Min potential"/"Upside", tooltip lines use "Min potential / Max potential / Options / Min option / Max option". Empty state -> "No recommendations available yet." - Tests: replace SavingsDataPoint fixtures with Recommendation fixtures throughout the renderSavingsByService DOM suite. Add computeServiceStatsFromRecs unit tests (empty, single, multi-rec, minLabel/maxLabel tracking, samples). Add integration test asserting loadDashboard wires the bar chart to recs not analytics data. Remove obsolete loadSavingsTrendChart re-render tests. - Drop medianOf helper (no longer called after tooltip update). NOTE: current recommendations carry a single savings field per row, not per-payment-option variants. Min/max within a service therefore reflects the range across different recommendation rows (e.g. different regions or resource types). Per-variant breakdowns (1yr/3yr x no-upfront/all-upfront) will further widen the range automatically once the backend ships them. Closes #769 --- frontend/src/__tests__/dashboard.test.ts | 244 ++++++++++++++--------- frontend/src/dashboard.ts | 110 ++++++---- frontend/src/index.html | 10 +- 3 files changed, 230 insertions(+), 134 deletions(-) diff --git a/frontend/src/__tests__/dashboard.test.ts b/frontend/src/__tests__/dashboard.test.ts index 69cc38ce9..112411dbe 100644 --- a/frontend/src/__tests__/dashboard.test.ts +++ b/frontend/src/__tests__/dashboard.test.ts @@ -1130,13 +1130,14 @@ describe('Dashboard Module', () => { }); // Issue #765: per-service savings-range bar chart. - describe('renderSavingsByService (issue #765)', () => { + describe('renderSavingsByService (issue #769)', () => { // Import the public helpers from dashboard. The module is already // loaded above via the jest.mock chain, so we can import directly. // eslint-disable-next-line @typescript-eslint/no-var-requires - const { renderSavingsByService, computeServiceStats } = require('../dashboard') as { - renderSavingsByService: (dataPoints: unknown[]) => void; + const { renderSavingsByService, computeServiceStats, computeServiceStatsFromRecs } = require('../dashboard') as { + renderSavingsByService: (recs: unknown[]) => void; computeServiceStats: (dataPoints: unknown[]) => Map; + computeServiceStatsFromRecs: (recs: unknown[]) => Map; }; function buildDOM(): void { @@ -1148,13 +1149,32 @@ describe('Dashboard Module', () => { const section = document.createElement('section'); section.id = 'savings-by-service-section'; const h3 = document.createElement('h3'); - h3.textContent = 'Savings range by service'; + h3.textContent = 'Potential savings range per service'; section.appendChild(h3); section.appendChild(canvas); section.appendChild(empty); document.body.appendChild(section); } + /** Build a minimal recommendation fixture. */ + function rec(service: string, savings: number, term = 1, payment = 'no_upfront'): unknown { + return { + id: `${service}-${savings}`, + provider: 'aws', + service, + region: 'us-east-1', + resource_type: 'Standard', + count: 1, + term, + payment, + upfront_cost: 0, + monthly_cost: 0, + savings, + selected: false, + purchased: false, + }; + } + beforeEach(() => { document.body.innerHTML = ''; jest.clearAllMocks(); @@ -1168,8 +1188,8 @@ describe('Dashboard Module', () => { (api.getRecommendations as jest.Mock).mockResolvedValue([]); }); - // computeServiceStats unit tests. - describe('computeServiceStats', () => { + // computeServiceStats unit tests (retained: function still exported for legacy compatibility). + describe('computeServiceStats (legacy data-points path)', () => { test('returns empty map for empty data points', () => { const result = computeServiceStats([]); expect(result.size).toBe(0); @@ -1234,9 +1254,72 @@ describe('Dashboard Module', () => { }); }); - // renderSavingsByService DOM behaviour tests. + // computeServiceStatsFromRecs unit tests. + describe('computeServiceStatsFromRecs', () => { + test('returns empty map for empty recommendations', () => { + expect(computeServiceStatsFromRecs([]).size).toBe(0); + }); + + test('single rec produces min === max (zero upside)', () => { + const result = computeServiceStatsFromRecs([rec('ec2', 100)]); + const ec2 = result.get('ec2'); + expect(ec2?.min).toBe(100); + expect(ec2?.max).toBe(100); + expect(ec2?.count).toBe(1); + }); + + test('two recs for same service: min is lower value, max is higher value', () => { + const result = computeServiceStatsFromRecs([ + rec('ec2', 200, 1, 'no_upfront'), + rec('ec2', 500, 3, 'all_upfront'), + ]); + const ec2 = result.get('ec2'); + expect(ec2?.min).toBe(200); + expect(ec2?.max).toBe(500); + expect(ec2?.count).toBe(2); + }); + + test('tracks minLabel and maxLabel from term/payment option', () => { + const result = computeServiceStatsFromRecs([ + rec('ec2', 200, 1, 'no_upfront'), + rec('ec2', 500, 3, 'all_upfront'), + ]); + const ec2 = result.get('ec2'); + expect(ec2?.minLabel).toBe('1yr no_upfront'); + expect(ec2?.maxLabel).toBe('3yr all_upfront'); + }); + + test('accumulates stats for multiple services independently', () => { + const result = computeServiceStatsFromRecs([ + rec('ec2', 100, 1, 'no_upfront'), + rec('ec2', 400, 3, 'all_upfront'), + rec('rds', 50, 1, 'no_upfront'), + rec('rds', 80, 3, 'all_upfront'), + ]); + expect(result.size).toBe(2); + expect(result.get('ec2')?.min).toBe(100); + expect(result.get('ec2')?.max).toBe(400); + expect(result.get('rds')?.min).toBe(50); + expect(result.get('rds')?.max).toBe(80); + }); + + test('stores raw sample values', () => { + const result = computeServiceStatsFromRecs([ + rec('ec2', 100), + rec('ec2', 300), + rec('ec2', 200), + ]); + const ec2 = result.get('ec2'); + expect(ec2?.samples).toHaveLength(3); + expect(ec2?.samples).toContain(100); + expect(ec2?.samples).toContain(200); + expect(ec2?.samples).toContain(300); + }); + }); + + // renderSavingsByService DOM behaviour tests (now driven by recommendation fixtures). describe('DOM behaviour', () => { - test('shows empty state and hides canvas when no data points', () => { + test('shows empty state and hides canvas when no recommendations', () => { buildDOM(); renderSavingsByService([]); const canvas = document.getElementById('savings-by-service-chart'); @@ -1245,37 +1328,31 @@ describe('Dashboard Module', () => { expect(empty?.classList.contains('hidden')).toBe(false); }); - test('shows empty state when all data points have zero savings', () => { + test('shows empty state when all recommendations have zero savings', () => { buildDOM(); - renderSavingsByService([ - { timestamp: 't1', total_savings: 0, total_upfront: 0, purchase_count: 0, cumulative_savings: 0, by_service: { ec2: 0 } }, - ]); + renderSavingsByService([rec('ec2', 0)]); expect(document.getElementById('savings-by-service-chart')?.classList.contains('hidden')).toBe(true); expect(document.getElementById('savings-by-service-empty')?.classList.contains('hidden')).toBe(false); }); test('resets heading text to default when dataset becomes empty after a truncated render', () => { buildDOM(); - // First render with 2 services to get the default heading. - const points = [ - { timestamp: 't1', total_savings: 0, total_upfront: 0, purchase_count: 0, cumulative_savings: 0, - by_service: { ec2: 100, rds: 50 } }, - ]; - renderSavingsByService(points); + renderSavingsByService([rec('ec2', 100), rec('rds', 50)]); const h3 = document.querySelector('#savings-by-service-section h3') as HTMLElement; - h3.textContent = 'Savings range by service (+3 more)'; // simulate stale suffix + h3.textContent = 'Potential savings range per service (+3 more)'; // simulate stale suffix // Second render with empty data -- heading must be reset. renderSavingsByService([]); - expect(h3.textContent).toBe('Savings range by service'); + expect(h3.textContent).toBe('Potential savings range per service'); }); test('renders chart with exactly two services when two services have positive savings', () => { buildDOM(); - const points = [ - { timestamp: 't1', total_savings: 0, total_upfront: 0, purchase_count: 0, cumulative_savings: 0, by_service: { ec2: 100, rds: 50 } }, - { timestamp: 't2', total_savings: 0, total_upfront: 0, purchase_count: 0, cumulative_savings: 0, by_service: { ec2: 300, rds: 80 } }, - ]; - renderSavingsByService(points); + renderSavingsByService([ + rec('ec2', 100, 1, 'no_upfront'), + rec('ec2', 300, 3, 'all_upfront'), + rec('rds', 50, 1, 'no_upfront'), + rec('rds', 80, 3, 'all_upfront'), + ]); // Chart must be constructed (canvas visible, empty hidden). expect(document.getElementById('savings-by-service-chart')?.classList.contains('hidden')).toBe(false); expect(document.getElementById('savings-by-service-empty')?.classList.contains('hidden')).toBe(true); @@ -1288,33 +1365,33 @@ describe('Dashboard Module', () => { expect(chartData.data.labels).toContain('rds'); }); - test('bar floor dataset uses min, range dataset uses (max - min)', () => { + test('bar floor dataset uses min potential, upside dataset uses (max - min)', () => { buildDOM(); - const points = [ - { timestamp: 't1', total_savings: 0, total_upfront: 0, purchase_count: 0, cumulative_savings: 0, by_service: { ec2: 100 } }, - { timestamp: 't2', total_savings: 0, total_upfront: 0, purchase_count: 0, cumulative_savings: 0, by_service: { ec2: 400 } }, - ]; - renderSavingsByService(points); + // ec2: two recs with savings 100 and 400 -> floor=100, upside=300. + renderSavingsByService([ + rec('ec2', 100, 1, 'no_upfront'), + rec('ec2', 400, 3, 'all_upfront'), + ]); const chartCtor = Chart as unknown as jest.Mock; const lastCall = chartCtor.mock.calls[chartCtor.mock.calls.length - 1]; const datasets = (lastCall?.[1] as { data: { datasets: { label: string; data: number[] }[] } }).data.datasets; - const floorDs = datasets.find((d) => d.label === 'Floor'); - const rangeDs = datasets.find((d) => d.label === 'Range'); - expect(floorDs?.data[0]).toBe(100); // min - expect(rangeDs?.data[0]).toBe(300); // max - min + const floorDs = datasets.find((d) => d.label === 'Min potential'); + const rangeDs = datasets.find((d) => d.label === 'Upside'); + expect(floorDs?.data[0]).toBe(100); // min potential savings + expect(rangeDs?.data[0]).toBe(300); // max - min }); - test('services are sorted by max savings descending', () => { + test('services are sorted by max potential savings descending', () => { buildDOM(); - const points = [ - { timestamp: 't1', total_savings: 0, total_upfront: 0, purchase_count: 0, cumulative_savings: 0, - by_service: { ec2: 100, rds: 500, lambda: 50 } }, - ]; - renderSavingsByService(points); + // ec2: max=200, rds: max=500, lambda: max=50 -- expected order: rds, ec2, lambda. + renderSavingsByService([ + rec('ec2', 200), + rec('rds', 500), + rec('lambda', 50), + ]); const chartCtor = Chart as unknown as jest.Mock; const lastCall = chartCtor.mock.calls[chartCtor.mock.calls.length - 1]; const labels = (lastCall?.[1] as { data: { labels: string[] } }).data.labels; - // rds has max 500, ec2 100, lambda 50 — rds must be first. expect(labels[0]).toBe('rds'); expect(labels[1]).toBe('ec2'); expect(labels[2]).toBe('lambda'); @@ -1325,71 +1402,52 @@ describe('Dashboard Module', () => { const mockDestroyA = jest.fn(); (Chart as unknown as jest.Mock).mockImplementationOnce(() => ({ destroy: mockDestroyA })); - const points = [ - { timestamp: 't1', total_savings: 0, total_upfront: 0, purchase_count: 0, cumulative_savings: 0, by_service: { ec2: 100 } }, - ]; - renderSavingsByService(points); + const recs = [rec('ec2', 100)]; + renderSavingsByService(recs); // Second call must destroy the first chart. - renderSavingsByService(points); + renderSavingsByService(recs); expect(mockDestroyA).toHaveBeenCalled(); }); test('no-ops gracefully when canvas is missing from DOM', () => { // No buildDOM() call — canvas absent. - expect(() => renderSavingsByService([ - { timestamp: 't1', total_savings: 0, total_upfront: 0, purchase_count: 0, cumulative_savings: 0, by_service: { ec2: 100 } }, - ])).not.toThrow(); + expect(() => renderSavingsByService([rec('ec2', 100)])).not.toThrow(); }); - }); - // Filter chip change re-renders via loadSavingsTrendChart. - describe('filter integration via loadSavingsTrendChart', () => { - beforeEach(() => { + test('loadDashboard wires range bars to recommendations, not trend data', async () => { + // The range bar chart must receive the recs from getRecommendations, + // not from getSavingsAnalytics. Verify by providing recs but no analytics. buildDOM(); - const canvas = document.createElement('canvas'); - canvas.id = 'savings-trend-chart'; - const empty = document.createElement('div'); - empty.id = 'savings-trend-empty'; - empty.className = 'hidden'; - document.body.appendChild(canvas); - document.body.appendChild(empty); - }); - - test('re-renders bar chart when trend chart is re-fetched due to filter change', async () => { - (api.getSavingsAnalytics as jest.Mock).mockResolvedValue({ - data_points: [ - { timestamp: 't1', total_savings: 100, total_upfront: 0, purchase_count: 1, cumulative_savings: 100, - by_service: { ec2: 100, rds: 50 } }, - ], + const summaryEl = document.createElement('section'); + summaryEl.id = 'summary'; + const upcomingEl = document.createElement('div'); + upcomingEl.id = 'upcoming-list'; + const savingsChartSection = document.createElement('section'); + savingsChartSection.id = 'savings-chart-section'; + const savingsCanvas = document.createElement('canvas'); + savingsCanvas.id = 'savings-chart'; + savingsChartSection.appendChild(savingsCanvas); + document.body.appendChild(summaryEl); + document.body.appendChild(savingsChartSection); + document.body.appendChild(upcomingEl); + + (api.getDashboardSummary as jest.Mock).mockResolvedValue({ + potential_monthly_savings: 0, current_monthly_savings: 0, + total_recommendations: 1, active_reservations: 0, + target_coverage: 80, ytd_savings: 0, + by_service: {}, }); - - await loadSavingsTrendChart(); - - const barCanvas = document.getElementById('savings-by-service-chart'); - expect(barCanvas?.classList.contains('hidden')).toBe(false); - const chartCtor = Chart as unknown as jest.Mock; - // At least one Chart call must have been for the bar chart. - const barChartCall = chartCtor.mock.calls.find( - (call: unknown[]) => (call[1] as { type: string })?.type === 'bar' - ); - expect(barChartCall).toBeDefined(); - }); - - test('re-renders bar chart empty state when analytics data is empty', async () => { + (api.getUpcomingPurchases as jest.Mock).mockResolvedValue({ purchases: [] }); + // Provide a rec so the bar chart should render (not show empty state). + (api.getRecommendations as jest.Mock).mockResolvedValue([rec('ec2', 150)]); + // Analytics deliberately absent/empty -- bar chart should still render from recs. (api.getSavingsAnalytics as jest.Mock).mockResolvedValue({ data_points: [] }); - await loadSavingsTrendChart(); + await loadDashboard(); - expect(document.getElementById('savings-by-service-chart')?.classList.contains('hidden')).toBe(true); - expect(document.getElementById('savings-by-service-empty')?.classList.contains('hidden')).toBe(false); + expect(document.getElementById('savings-by-service-chart')?.classList.contains('hidden')).toBe(false); + expect(document.getElementById('savings-by-service-empty')?.classList.contains('hidden')).toBe(true); }); - - // Note: provider is intentionally NOT forwarded to getSavingsAnalytics - // because the /history/analytics handler does not yet support a - // provider-scoped query (see dashboard.ts comment near the fetch call - // for the original CR finding and the deliberate decision). A test - // asserting the negative would be brittle; the comment in the - // production code is the source of truth. }); }); }); diff --git a/frontend/src/dashboard.ts b/frontend/src/dashboard.ts index dac651b8d..289ef5a51 100644 --- a/frontend/src/dashboard.ts +++ b/frontend/src/dashboard.ts @@ -156,6 +156,7 @@ export async function loadDashboard(): Promise { renderDashboardSummary(summaryData!, recs); renderSavingsChart(summaryData!.by_service || {}); + renderSavingsByService(recs); renderUpcomingPurchases(upcomingData?.purchases || []); // Load the savings-over-time widget independently -- failure shouldn't // block the rest of the dashboard (e.g. analytics not configured). @@ -634,7 +635,8 @@ async function cancelScheduledPurchase(executionId: string): Promise { } /** - * Per-service savings statistics derived from a window of data points. + * Per-service savings statistics derived from a window of data points or + * from a recommendations list. * Exported for unit testing only. */ export interface ServiceSavingsStats { @@ -643,6 +645,10 @@ export interface ServiceSavingsStats { sum: number; count: number; samples: number[]; + /** Label of the recommendation option that produced the minimum savings (e.g. "1yr no-upfront"). */ + minLabel?: string; + /** Label of the recommendation option that produced the maximum savings (e.g. "3yr all-upfront"). */ + maxLabel?: string; } /** @@ -650,6 +656,9 @@ export interface ServiceSavingsStats { * data point carries a by_service map of { serviceName: savingsValue }. * Points with no by_service entry (omitempty from backend) are skipped. * Exported for unit testing. + * + * @deprecated Used only by historical-data tests. New production code uses + * computeServiceStatsFromRecs for forward-looking potential savings. */ export function computeServiceStats( dataPoints: readonly SavingsDataPoint[], @@ -675,31 +684,68 @@ export function computeServiceStats( } /** - * Compute the median of a sorted sample array. - * Returns 0 for an empty array. + * Compute per-service potential-savings stats from a recommendations list. + * + * For each service, collects every recommendation row and treats each row's + * `savings` value as one "option". The bar's: + * - floor = min(savings) across all rows for that service + * - upside = max(savings) - min(savings) + * + * When a service has only one recommendation row, the bar collapses to a + * single point (floor only, zero upside) — that is the correct visual. + * + * NOTE: The current recommendations response carries a single `savings` field + * per row rather than per-variant breakdowns (e.g. 1yr/3yr × no-upfront/ + * all-upfront columns). When per-variant rows are shipped, min/max will + * automatically reflect the full option range; no code change is required. + * See #769 for context. + * + * Exported for unit testing. */ -function medianOf(values: number[]): number { - if (values.length === 0) return 0; - const sorted = [...values].sort((a, b) => a - b); - const mid = Math.floor(sorted.length / 2); - return sorted.length % 2 === 0 ? (sorted[mid - 1]! + sorted[mid]!) / 2 : sorted[mid]!; +export function computeServiceStatsFromRecs( + recs: readonly LocalRecommendation[], +): Map { + const stats = new Map(); + for (const rec of recs) { + const svc = rec.service; + const val = typeof rec.savings === 'number' ? rec.savings : 0; + const label = `${rec.term}yr ${rec.payment}`; + const existing = stats.get(svc); + if (existing) { + if (val < existing.min) { + existing.min = val; + existing.minLabel = label; + } + if (val > existing.max) { + existing.max = val; + existing.maxLabel = label; + } + existing.sum += val; + existing.count += 1; + existing.samples.push(val); + } else { + stats.set(svc, { min: val, max: val, sum: val, count: 1, samples: [val], minLabel: label, maxLabel: label }); + } + } + return stats; } + /** - * Render the per-service savings-range stacked bar chart (issue #765). - * Accepts the same data_points array as the trend line chart. + * Render the per-service potential-savings range stacked bar chart (issue #769). + * Accepts the recommendations array from loadDashboard. * * Each bar is split into two stacked datasets: - * - "Floor" (solid): height = min savings observed across the window. - * - "Range" (translucent 35% opacity): height = max - min (the upside). + * - "Min potential" (solid): height = min potential savings across recommendation rows for that service. + * - "Upside" (translucent 35% opacity): height = max - min (the additional potential). * - * Services are sorted by max savings descending; the top SAVINGS_BY_SERVICE_MAX + * Services are sorted by max potential savings descending; the top SAVINGS_BY_SERVICE_MAX * are shown. When truncated, the section heading notes "+N more". * - * Empty state: when no service has positive savings, the canvas is hidden and + * Empty state: when no recommendations are available, the canvas is hidden and * the empty-state paragraph is shown. */ -export function renderSavingsByService(dataPoints: readonly SavingsDataPoint[]): void { +export function renderSavingsByService(recs: readonly LocalRecommendation[]): void { const canvas = document.getElementById('savings-by-service-chart') as HTMLCanvasElement | null; const emptyEl = document.getElementById('savings-by-service-empty'); const section = document.getElementById('savings-by-service-section'); @@ -711,7 +757,7 @@ export function renderSavingsByService(dataPoints: readonly SavingsDataPoint[]): savingsByServiceChart = null; } - const stats = computeServiceStats(dataPoints); + const stats = computeServiceStatsFromRecs(recs); const heading = section?.querySelector('h3'); // Filter to services with positive savings, then sort by max desc. @@ -719,7 +765,7 @@ export function renderSavingsByService(dataPoints: readonly SavingsDataPoint[]): positive.sort((a, b) => b[1].max - a[1].max); if (positive.length === 0) { - if (heading) heading.textContent = 'Savings range by service'; + if (heading) heading.textContent = 'Potential savings range per service'; canvas.classList.add('hidden'); emptyEl?.classList.remove('hidden'); return; @@ -733,8 +779,8 @@ export function renderSavingsByService(dataPoints: readonly SavingsDataPoint[]): const visible = positive.slice(0, SAVINGS_BY_SERVICE_MAX); if (heading) { heading.textContent = truncated - ? `Savings range by service (+${positive.length - SAVINGS_BY_SERVICE_MAX} more)` - : 'Savings range by service'; + ? `Potential savings range per service (+${positive.length - SAVINGS_BY_SERVICE_MAX} more)` + : 'Potential savings range per service'; } const labels = visible.map(([svc]) => svc); @@ -759,14 +805,14 @@ export function renderSavingsByService(dataPoints: readonly SavingsDataPoint[]): labels, datasets: [ { - label: 'Floor', + label: 'Min potential', data: floorData, backgroundColor: bgColors, borderRadius: { topLeft: 0, topRight: 0, bottomLeft: 4, bottomRight: 4 }, stack: 'savings', }, { - label: 'Range', + label: 'Upside', data: rangeData, backgroundColor: rangeColors, borderRadius: { topLeft: 4, topRight: 4, bottomLeft: 0, bottomRight: 0 }, @@ -790,15 +836,16 @@ export function renderSavingsByService(dataPoints: readonly SavingsDataPoint[]): const s = stats.get(svc); if (!s) return ''; const pct = totalSavings > 0 ? ((s.max / totalSavings) * 100).toFixed(1) : '0.0'; - const med = medianOf(s.samples); - return [ + const lines = [ `Service: ${svc}`, - `Min: $${s.min.toLocaleString()}`, - `Median: $${med.toLocaleString()}`, - `Max: $${s.max.toLocaleString()}`, - `Samples: ${s.count}`, + `Min potential: $${s.min.toLocaleString()}`, + `Max potential: $${s.max.toLocaleString()}`, + `Options: ${s.count}`, `% of total: ${pct}%`, ]; + if (s.minLabel) lines.push(`Min option: ${s.minLabel}`); + if (s.maxLabel) lines.push(`Max option: ${s.maxLabel}`); + return lines; }, }, }, @@ -931,18 +978,11 @@ export async function loadSavingsTrendChart(): Promise { } if (savingsTrendChart) { savingsTrendChart.destroy(); savingsTrendChart = null; } attachSparkline('ytd', []); - // Show empty state for the per-service bar chart as well. - renderSavingsByService([]); return; } canvas.classList.remove('hidden'); empty?.classList.add('hidden'); - // Per-service savings-range bar chart (issue #765). Shares the same - // data_points response so no extra fetch is needed. The YTD sparkline - // is already populated earlier in this function via attachSparkline. - renderSavingsByService(data.data_points); - if (savingsTrendChart) savingsTrendChart.destroy(); savingsTrendChart = new Chart(canvas, { @@ -998,8 +1038,6 @@ export async function loadSavingsTrendChart(): Promise { empty.textContent = 'Savings history is not available yet.'; empty.classList.remove('hidden'); } - // Degrade the per-service bar chart to its empty state too. - renderSavingsByService([]); } } diff --git a/frontend/src/index.html b/frontend/src/index.html index 5b8c3a2d2..136490f06 100644 --- a/frontend/src/index.html +++ b/frontend/src/index.html @@ -93,15 +93,15 @@

Savings over time

-
-

Savings range by service

- - -

Potential Savings by Service

+
+

Potential savings range per service

+ + +

Upcoming Scheduled Purchases

From 54b932b2f0bade0b0465afa6a138537e259d36ef Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Thu, 28 May 2026 13:59:49 +0200 Subject: [PATCH 2/2] fix(home/chart): tighten tooltip label fallback + correct empty-state copy + update test fixture Guard rec.payment in computeServiceStatsFromRecs: when the field is absent or whitespace-only the tooltip label previously rendered as "1yr undefined"; it now falls back to "1yr unspecified". A new unit test asserts the fallback for both undefined and empty-string payment values, and verifies the string "undefined" never appears in the label. Correct the savings-by-service empty-state paragraph: the previous text ("No recommendations available yet.") was misleading because the element is also shown when recommendations exist but none carry positive potential savings; the updated copy ("No positive potential savings found for current recommendations.") matches the actual emit condition. Align the getDashboardSummary mock fixture in dashboard.test.ts with the current DashboardSummary contract: drop the removed fields current_monthly_savings and active_reservations, and add the current fields active_commitments and committed_monthly. --- frontend/src/__tests__/dashboard.test.ts | 16 ++++++++++++++-- frontend/src/dashboard.ts | 3 ++- frontend/src/index.html | 2 +- 3 files changed, 17 insertions(+), 4 deletions(-) diff --git a/frontend/src/__tests__/dashboard.test.ts b/frontend/src/__tests__/dashboard.test.ts index 112411dbe..db91a25c3 100644 --- a/frontend/src/__tests__/dashboard.test.ts +++ b/frontend/src/__tests__/dashboard.test.ts @@ -1289,6 +1289,18 @@ describe('Dashboard Module', () => { expect(ec2?.maxLabel).toBe('3yr all_upfront'); }); + test('uses "unspecified" label when payment is undefined or empty string', () => { + const recNoPayment = { ...rec('ec2', 100) as Record }; + delete recNoPayment['payment']; + const recEmptyPayment = { ...rec('ec2', 200) as Record, payment: '' }; + const result = computeServiceStatsFromRecs([recNoPayment, recEmptyPayment] as unknown as Parameters[0]); + const ec2 = result.get('ec2'); + expect(ec2?.minLabel).toContain('unspecified'); + expect(ec2?.maxLabel).toContain('unspecified'); + expect(ec2?.minLabel).not.toContain('undefined'); + expect(ec2?.maxLabel).not.toContain('undefined'); + }); + test('accumulates stats for multiple services independently', () => { const result = computeServiceStatsFromRecs([ rec('ec2', 100, 1, 'no_upfront'), @@ -1432,8 +1444,8 @@ describe('Dashboard Module', () => { document.body.appendChild(upcomingEl); (api.getDashboardSummary as jest.Mock).mockResolvedValue({ - potential_monthly_savings: 0, current_monthly_savings: 0, - total_recommendations: 1, active_reservations: 0, + potential_monthly_savings: 0, + total_recommendations: 1, active_commitments: 0, committed_monthly: 0, target_coverage: 80, ytd_savings: 0, by_service: {}, }); diff --git a/frontend/src/dashboard.ts b/frontend/src/dashboard.ts index 289ef5a51..f1e40fa81 100644 --- a/frontend/src/dashboard.ts +++ b/frontend/src/dashboard.ts @@ -709,7 +709,8 @@ export function computeServiceStatsFromRecs( for (const rec of recs) { const svc = rec.service; const val = typeof rec.savings === 'number' ? rec.savings : 0; - const label = `${rec.term}yr ${rec.payment}`; + const paymentLabel = rec.payment && rec.payment.trim().length > 0 ? rec.payment : 'unspecified'; + const label = `${rec.term}yr ${paymentLabel}`; const existing = stats.get(svc); if (existing) { if (val < existing.min) { diff --git a/frontend/src/index.html b/frontend/src/index.html index 136490f06..3e4f178a5 100644 --- a/frontend/src/index.html +++ b/frontend/src/index.html @@ -100,7 +100,7 @@

Potential Savings by Service

Potential savings range per service

- +

Upcoming Scheduled Purchases