From b9220c0745a1723c1fa2fd6be7070e67af54ff83 Mon Sep 17 00:00:00 2001 From: Astrid Gealer Date: Fri, 23 Oct 2026 22:43:14 +0000 Subject: [PATCH 1/3] fix(frontend): make Home and Opportunities agree on Potential Monthly Savings Both cards sum the same per-cell range, but over different rows. Opportunities drops recommendations for providers switched off in Settings -> Enabled Providers (#463); Home summed every provider, so with a provider disabled the two same-named KPIs disagreed. Home's "N recommendations" line also came from the backend total, which ignores that setting. Home now reads the config and applies the same filter through a shared helper (lib/enabled-providers.ts), and counts the rows it summed. A config fetch failure falls back to no filter, as on Opportunities. When column filters narrow the Opportunities list, its savings card now says "Filtered: X of Y recommendations", so a deliberately narrower scope is labeled instead of looking like a disagreement. Refs #180 Co-Authored-By: Claude Opus 5.5 --- frontend/src/__tests__/dashboard.test.ts | 64 +++++++++++++++++ .../src/__tests__/recommendations.test.ts | 8 +++ frontend/src/dashboard.ts | 40 +++++++++-- frontend/src/lib/enabled-providers.ts | 26 +++++++ frontend/src/recommendations.ts | 18 +++-- frontend/tests-e2e/savings-kpi-parity.spec.ts | 68 +++++++++++++++++++ 6 files changed, 214 insertions(+), 10 deletions(-) create mode 100644 frontend/src/lib/enabled-providers.ts create mode 100644 frontend/tests-e2e/savings-kpi-parity.spec.ts diff --git a/frontend/src/__tests__/dashboard.test.ts b/frontend/src/__tests__/dashboard.test.ts index 9a296bec6..72fc76db8 100644 --- a/frontend/src/__tests__/dashboard.test.ts +++ b/frontend/src/__tests__/dashboard.test.ts @@ -70,6 +70,9 @@ jest.mock('../api', () => ({ // empty list so existing tests that don't care about the savings card // value don't need to be updated. getRecommendations: jest.fn().mockResolvedValue([]), + // #180: Home applies the Settings -> Enabled Providers filter, like the + // Opportunities page. Default to "no preference" (no filter). + getConfig: jest.fn().mockResolvedValue({ global: {} }), })); import { loadSavingsTrendChart, setupSavingsTrendHandlers, setupDashboardHandlers, formatTrendAxisTick } from '../dashboard'; @@ -707,6 +710,67 @@ describe('Dashboard Module', () => { expect(savingsCard?.textContent).not.toContain('$700'); }); + // #180: Home and Opportunities must sum the same rows, so Home applies the + // same Enabled Providers filter and counts the rows it summed. + describe('Enabled Providers scope parity with Opportunities (#180)', () => { + const scopedRecs = [ + { id: 'a1', provider: 'aws', service: 'ec2', region: 'us-east-1', resource_type: 't3.medium', term: 1, savings: 150, upfront_cost: 0, count: 1 }, + { id: 'a2', provider: 'aws', service: 'rds', region: 'us-east-1', resource_type: 'db.t3.medium', term: 1, savings: 100, upfront_cost: 0, count: 1 }, + { id: 'z1', provider: 'azure', service: 'aks', region: 'eastus', resource_type: 'Standard_D2_v3', term: 1, savings: 90, upfront_cost: 0, count: 1 }, + ]; + const summaryFor = (total: number) => ({ + potential_monthly_savings: 0, total_recommendations: total, active_commitments: 0, + committed_monthly: 0, current_coverage: 0, target_coverage: 80, ytd_savings: 0, by_service: {}, + }); + const savingsTile = (): Element | null => document.querySelector('[data-kpi="savings"]'); + const rangeInputIds = (): string[] => + (mockGroupRecsByCell.mock.calls[0]![0] as Array<{ id: string }>).map((r) => r.id); + + beforeEach(() => { + (api.getRecommendations as jest.Mock).mockResolvedValue(scopedRecs); + (api.getDashboardSummary as jest.Mock).mockResolvedValue(summaryFor(3)); + (api.getUpcomingPurchases as jest.Mock).mockResolvedValue({ purchases: [] }); + }); + + test('drops recommendations for disabled providers before computing the range', async () => { + (api.getConfig as jest.Mock).mockResolvedValueOnce({ global: { enabled_providers: ['aws'] } }); + + await loadDashboard(); + + expect(rangeInputIds()).toEqual(['a1', 'a2']); + // The count matches the rows summed, not the backend total (3). + expect(savingsTile()?.querySelector('.kpi-tile-detail')?.textContent).toBe('2 recommendations'); + }); + + test('empty enabled_providers keeps every recommendation (permissive default)', async () => { + (api.getConfig as jest.Mock).mockResolvedValueOnce({ global: { enabled_providers: [] } }); + + await loadDashboard(); + + expect(rangeInputIds()).toEqual(['a1', 'a2', 'z1']); + expect(savingsTile()?.querySelector('.kpi-tile-detail')?.textContent).toBe('3 recommendations'); + }); + + test('a config fetch failure does not block the dashboard and applies no filter', async () => { + (api.getConfig as jest.Mock).mockRejectedValueOnce(new Error('403 Forbidden')); + + await loadDashboard(); + + expect(rangeInputIds()).toEqual(['a1', 'a2', 'z1']); + expect(savingsTile()?.querySelector('.kpi-tile-value')?.textContent).toBe('$300 – $400'); + }); + + test('falls back to the backend count when the recommendations fetch fails', async () => { + (api.getRecommendations as jest.Mock).mockRejectedValueOnce(new Error('boom')); + (api.getDashboardSummary as jest.Mock).mockResolvedValue(summaryFor(7)); + + await loadDashboard(); + + expect(savingsTile()?.querySelector('.kpi-tile-value')?.textContent).toBe('--'); + expect(savingsTile()?.querySelector('.kpi-tile-detail')?.textContent).toBe('7 recommendations'); + }); + }); + // #978: Current Coverage KPI must render with exactly 1 decimal place so it // cannot overflow the tile box at narrower screen widths. test('#978: Current Coverage KPI rounds to 1 decimal place', async () => { diff --git a/frontend/src/__tests__/recommendations.test.ts b/frontend/src/__tests__/recommendations.test.ts index c62b3342c..9d2461338 100644 --- a/frontend/src/__tests__/recommendations.test.ts +++ b/frontend/src/__tests__/recommendations.test.ts @@ -335,6 +335,11 @@ describe('Recommendations Module', () => { }; expect(cardValue()).toContain('$300'); expect(cardValue()).toContain('$400'); + // #180: unfiltered, the card covers every loaded row (same scope as + // Home), so it carries no "Filtered" scope line. + const scopeLine = (): string | null => + document.querySelector('#recommendations-summary [data-testid="savings-scope"]')?.textContent ?? null; + expect(scopeLine()).toBeNull(); // Apply a real column filter restricting to service=ec2 (cell1 // only). renderRecommendationsList re-runs applyColumnFilters @@ -352,6 +357,9 @@ describe('Recommendations Module', () => { expect(cardValue()).toContain('$150'); expect(cardValue()).not.toContain('$300'); expect(cardValue()).not.toContain('$400'); + // #180: the narrowed scope is labeled so it can't be mistaken for + // Home's all-recommendations figure. + expect(scopeLine()).toBe('Filtered: 2 of 4 recommendations'); // Restore the filter mock to empty so the leaked return value // doesn't bleed into subsequent tests in this file. jest.clearAllMocks diff --git a/frontend/src/dashboard.ts b/frontend/src/dashboard.ts index 2a8e61716..a9de0185b 100644 --- a/frontend/src/dashboard.ts +++ b/frontend/src/dashboard.ts @@ -13,6 +13,7 @@ import { confirmDialog } from './confirmDialog'; import { canAccess } from './permissions'; import { groupRecsByCell, pageLevelRange, formatSavingsRange, triggerAutoRefreshIfStale } from './recommendations'; import { showSkeletonTiles, showSkeletonBlock, teardownSkeleton } from './lib/skeleton'; +import { filterByEnabledProviders } from './lib/enabled-providers'; import { formatTrendAxisTick } from './modules/chart-utils'; // Re-export for backward compatibility (tests and other callers import from dashboard.ts). export { formatTrendAxisTick } from './modules/chart-utils'; @@ -208,10 +209,11 @@ export async function loadDashboard(): Promise { // move the range computation server-side if needed. // Promise.allSettled ensures the dashboard still renders if any individual // fetch fails -- each card falls back gracefully. - const [summaryResult, upcomingResult, recsResult] = await Promise.allSettled([ + const [summaryResult, upcomingResult, recsResult, enabledProviders] = await Promise.allSettled([ api.getDashboardSummary(currentProvider, currentAccountIDs), api.getUpcomingPurchases(), api.getRecommendations({ provider: currentProvider, account_ids: currentAccountIDs }), + loadEnabledProviders(), ]); const summaryData = summaryResult.status === 'fulfilled' ? (summaryResult.value as DashboardSummary) : null; @@ -234,7 +236,15 @@ export async function loadDashboard(): Promise { : (rawRecs != null && typeof rawRecs === 'object' && Array.isArray((rawRecs as { recommendations?: unknown }).recommendations)) ? (rawRecs as { recommendations: unknown[] }).recommendations : []; - const recs: readonly LocalRecommendation[] = recsArray as unknown as LocalRecommendation[]; + // #180: apply the same Settings -> Enabled Providers filter as the + // Opportunities page, so the two Potential Monthly Savings cards are + // computed from the same rows. A failed recs fetch stays distinguishable + // (null) so the tile can fall back to the backend count. + const recs: readonly LocalRecommendation[] = filterByEnabledProviders( + recsArray as unknown as LocalRecommendation[], + enabledProviders.status === 'fulfilled' ? enabledProviders.value : null, + ); + const recsCount = recsResult.status === 'fulfilled' ? recs.length : null; if (summaryResult.status === 'rejected') { throw summaryResult.reason as Error; @@ -247,7 +257,7 @@ export async function loadDashboard(): Promise { // on the Home chart. Mirrors the pattern from loadSavingsTrendChart (#747). const filterDesc = buildFilterDesc(currentProvider, currentAccountIDs); - renderDashboardSummary(summaryData, recs); + renderDashboardSummary(summaryData, recs, recsCount); // Single merged per-service chart (#908): potential range from recs + // current-savings underlay from the summary's by_service map. renderSavingsByService(recs, summaryData.by_service || {}, filterDesc); @@ -289,7 +299,25 @@ export async function loadDashboard(): Promise { } } -function renderDashboardSummary(data: DashboardSummary, recs: readonly LocalRecommendation[]): void { +/** + * Settings -> Enabled Providers, or null when the config can't be read (a + * missing permission or network error must not block Home; null means "no + * filter", the same permissive fallback the Opportunities page uses). + */ +async function loadEnabledProviders(): Promise { + try { + const cfg = await api.getConfig(); + return cfg?.global?.enabled_providers ?? null; + } catch { + return null; + } +} + +function renderDashboardSummary( + data: DashboardSummary, + recs: readonly LocalRecommendation[], + recsCount: number | null, +): void { const summary = document.getElementById('summary'); if (!summary) return; @@ -346,7 +374,9 @@ function renderDashboardSummary(data: DashboardSummary, recs: readonly LocalReco detail: string; }> = [ { kpi: 'savings', title: 'Potential Monthly Savings', value: savingsDisplay, valueSavings: true, - detail: `${data.total_recommendations ?? '--'} recommendations` }, + // #180: count the same rows the range sums (and the Opportunities page + // counts); the backend total ignores the Enabled Providers setting. + detail: `${recsCount ?? data.total_recommendations ?? '--'} recommendations` }, { kpi: 'commitments', title: 'Active Commitments', value: data.active_commitments != null ? String(data.active_commitments) : '--', detail: `${formatCurrency(data.committed_monthly)}/mo committed` }, { kpi: 'coverage', title: 'Current Coverage', value: coverageValue, detail: coverageDetail }, diff --git a/frontend/src/lib/enabled-providers.ts b/frontend/src/lib/enabled-providers.ts new file mode 100644 index 000000000..e34e939be --- /dev/null +++ b/frontend/src/lib/enabled-providers.ts @@ -0,0 +1,26 @@ +/** + * Settings -> General -> Enabled Providers scoping for recommendation lists + * (issues #463 and #180). + * + * The backend returns every collected recommendation irrespective of the + * `enabled_providers` preference, so each screen that sums recommendations + * must apply the same client-side filter. Home and Opportunities both use + * this helper so their "Potential Monthly Savings" figures are computed + * from the same set of rows and cannot drift apart. + * + * Permissive default: an empty or absent list (older configs, or a tenant + * that never touched the toggles) means "no filter", matching the Settings + * load path, rather than hiding every recommendation. + */ + +/** + * Keep only the recommendations whose provider is enabled. Returns a copy of + * the input when `enabledProviders` is empty, null or undefined. + */ +export function filterByEnabledProviders( + recs: readonly T[], + enabledProviders: readonly string[] | null | undefined, +): T[] { + if (!enabledProviders || enabledProviders.length === 0) return [...recs]; + return recs.filter((r) => enabledProviders.includes(r.provider)); +} diff --git a/frontend/src/recommendations.ts b/frontend/src/recommendations.ts index 307ff2d7a..d4c3fa9da 100644 --- a/frontend/src/recommendations.ts +++ b/frontend/src/recommendations.ts @@ -6,6 +6,7 @@ import * as api from './api'; import * as state from './state'; import type { CostPeriod } from './state'; import { formatCurrency, formatTerm, escapeHtml, CURRENCY_DEFAULT_DIGITS } from './utils'; +import { filterByEnabledProviders } from './lib/enabled-providers'; import { getRecommendationsFreshness, refreshRecommendations as refreshRecommendationsAPI } from './api/recommendations'; import type { RecommendationsFreshness } from './api/recommendations'; import { showToast } from './toast'; @@ -641,9 +642,8 @@ export async function loadRecommendations(): Promise { // permissive default on the checkboxes (line ~2510). const enabledProviders = cfgResponse?.global?.enabled_providers; const rawRecs = (data.recommendations || []) as unknown as api.Recommendation[]; - const visibleByPreference: api.Recommendation[] = enabledProviders && enabledProviders.length > 0 - ? rawRecs.filter(r => enabledProviders.includes(r.provider as api.Provider)) - : rawRecs; + // Shared with Home's savings card (#180) so both screens sum the same rows. + const visibleByPreference = filterByEnabledProviders(rawRecs, enabledProviders); state.setRecommendations(visibleByPreference); state.clearSelectedRecommendations(); @@ -832,6 +832,7 @@ async function onLookbackChange(rawValue: string): Promise { function renderRecommendationsSummary( _summary: RecommendationsSummary | null, recommendations: readonly LocalRecommendation[], + loadedCount: number = recommendations.length, ): void { const container = document.getElementById('recommendations-summary'); if (!container) return; @@ -901,6 +902,13 @@ function renderRecommendationsSummary( const sfx = periodSuffix(period); return isSelectionView ? `Selected Savings ${sfx}` : `Potential Savings ${sfx}`; })(); + // #180: Home's Potential Monthly Savings always covers every loaded + // recommendation, so when column filters narrow this page the card must say + // so; otherwise the two same-named KPIs disagree with no visible reason. + const isFilteredView = !isSelectionView && variantCount < loadedCount; + const savingsScopeDetail = isFilteredView + ? `

Filtered: ${variantCount} of ${loadedCount} recommendations

` + : ''; const upfrontLabel = isSelectionView ? 'Selected Upfront Cost' : 'Total Upfront Cost'; const paybackLabel = 'Payback Period'; @@ -911,7 +919,7 @@ function renderRecommendationsSummary(

${savingsLabel}

-

${savingsText}

+

${savingsText}

${savingsScopeDetail}

${upfrontLabel}

@@ -4802,7 +4810,7 @@ function renderRecommendationsList(loadedRecs: LocalRecommendation[]): void { // total_upfront_cost, avg_payback_months); the savings figure itself is // recomputed from `recommendations` inside renderRecommendationsSummary // via the same pageLevelRange the banner uses. - renderRecommendationsSummary(lastRecommendationsSummary, recommendations); + renderRecommendationsSummary(lastRecommendationsSummary, recommendations, loadedRecs?.length ?? 0); // Mount once; update is per-render below. mountBottomActionBox(); diff --git a/frontend/tests-e2e/savings-kpi-parity.spec.ts b/frontend/tests-e2e/savings-kpi-parity.spec.ts new file mode 100644 index 000000000..20a95c013 --- /dev/null +++ b/frontend/tests-e2e/savings-kpi-parity.spec.ts @@ -0,0 +1,68 @@ +/** + * Issue #180: Home and Opportunities both show "Potential Monthly Savings" + * for the same recommendations, so the two figures must agree. Opportunities + * honors Settings -> Enabled Providers (#463); Home used to sum every + * provider, so a tenant with a provider switched off saw two different + * numbers. When the Opportunities card is narrowed by column filters, the + * scope difference must be labeled on the card itself. + */ +import { test, expect, type Page } from '@playwright/test'; +import { mockApi, seedAuth, RECS } from './fixtures/recs'; + +// Every fixture row is its own cell with a single variant, so each page-level +// range collapses to the plain sum of the enabled rows. +const AWS_TOTAL = RECS.filter((r) => r.provider === 'aws').reduce((s, r) => s + r.savings, 0); +const AWS_COUNT = RECS.filter((r) => r.provider === 'aws').length; +const AWS_EC2_COUNT = RECS.filter((r) => r.provider === 'aws' && r.service === 'ec2').length; +const usd = (n: number): string => `$${n.toLocaleString('en-US')}`; + +async function setup(page: Page): Promise { + await seedAuth(page); + await mockApi(page); + // Azure and GCP are switched off in Settings. + await page.route('**/api/config', (route) => + route.fulfill({ json: { global: { enabled_providers: ['aws'] }, services: {} } }), + ); + await page.route('**/api/dashboard/upcoming', (route) => route.fulfill({ json: { purchases: [] } })); + await page.route('**/api/dashboard/summary**', (route) => + route.fulfill({ + json: { + potential_monthly_savings: 0, total_recommendations: RECS.length, active_commitments: 0, + committed_monthly: 0, current_coverage: 0, target_coverage: 80, ytd_savings: 0, by_service: {}, + }, + }), + ); +} + +function opportunitiesSavingsCard(page: Page) { + return page.locator('#recommendations-summary .card').filter({ hasText: 'Potential Monthly Savings' }); +} + +test('Home and Opportunities agree on Potential Monthly Savings when a provider is disabled', async ({ page }) => { + await setup(page); + + await page.goto('/home'); + const homeTile = page.locator('#summary [data-kpi="savings"]'); + await expect(homeTile.locator('.kpi-tile-value')).toHaveText(usd(AWS_TOTAL)); + await expect(homeTile.locator('.kpi-tile-detail')).toHaveText(`${AWS_COUNT} recommendations`); + + await page.goto('/opportunities'); + const card = opportunitiesSavingsCard(page); + await expect(card.locator('.value.savings')).toHaveText(usd(AWS_TOTAL)); + // Unfiltered: same scope as Home, so no scope label. + await expect(card.getByTestId('savings-scope')).toHaveCount(0); +}); + +test('a column-filtered Opportunities card labels its narrower scope', async ({ page }) => { + await page.addInitScript(() => { + localStorage.setItem('cudly.recs.columnFilters.v1', JSON.stringify({ + schemaVersion: 1, + filters: { service: { kind: 'set', values: ['ec2'] } }, + })); + }); + await setup(page); + + await page.goto('/opportunities'); + await expect(opportunitiesSavingsCard(page).getByTestId('savings-scope')) + .toHaveText(`Filtered: ${AWS_EC2_COUNT} of ${AWS_COUNT} recommendations`); +}); From 0be9d7889459d9dc25cf29cafbaf3edc4e1b2cbd Mon Sep 17 00:00:00 2001 From: Astrid Gealer Date: Sat, 24 Oct 2026 18:26:04 +0000 Subject: [PATCH 2/3] fix(frontend): label the Opportunities savings card when a filter is pushed to the server The "Filtered: X of Y" line only appeared when the visible count was below the loaded count. A single-value service or region filter is sent to the server (#162), so the loaded list is already narrowed, the two counts match, and the card showed a smaller total than Home with no label: the #180 symptom on every load after the first. The card now decides "filtered" from the active column filters and from the filters the last fetch pushed to the server. When the loaded list was narrowed server-side the unfiltered total isn't known, so the line names the filter instead of inventing a total, e.g. "Filtered: 6 recommendations (Service: ec2)". The e2e recs mock now applies service/region like the backend does. Refs #180 Co-Authored-By: Claude Opus 5.5 --- .../src/__tests__/recommendations.test.ts | 43 +++++++++++++++++++ frontend/src/recommendations.ts | 39 +++++++++++++++-- frontend/tests-e2e/fixtures/recs.ts | 5 +++ frontend/tests-e2e/savings-kpi-parity.spec.ts | 25 +++++++++++ 4 files changed, 108 insertions(+), 4 deletions(-) diff --git a/frontend/src/__tests__/recommendations.test.ts b/frontend/src/__tests__/recommendations.test.ts index d8a0cd87b..a2b84aefb 100644 --- a/frontend/src/__tests__/recommendations.test.ts +++ b/frontend/src/__tests__/recommendations.test.ts @@ -386,6 +386,49 @@ describe('Recommendations Module', () => { (state.getRecommendationsColumnFilters as jest.Mock).mockReturnValue({}); }); + test('savings card is labeled Filtered when the service filter is pushed to the server (#180)', async () => { + // A single-value service filter is sent as ?service= (#162) and the + // backend returns only matching rows, so the visible and loaded counts + // are equal. The card must still say it is filtered; otherwise it shows + // a smaller total than Home with no explanation. + const ec2Recs = [ + { id: 'cell1-cheap', provider: 'aws', cloud_account_id: 'a1', service: 'ec2', resource_type: 't3.medium', region: 'us-east-1', count: 1, term: 1, payment_option: 'no-upfront', savings: 100, upfront_cost: 0 }, + { id: 'cell1-pricey', provider: 'aws', cloud_account_id: 'a1', service: 'ec2', resource_type: 't3.medium', region: 'us-east-1', count: 1, term: 1, payment_option: 'all-upfront', savings: 150, upfront_cost: 1000 }, + ]; + (api.getRecommendations as jest.Mock).mockResolvedValue({ + summary: { total_count: 2, total_monthly_savings: 250, total_upfront_cost: 1000, avg_payback_months: 1 }, + recommendations: ec2Recs, + regions: [], + }); + (state.getRecommendations as jest.Mock).mockReturnValue(ec2Recs); + (state.getVisibleRecommendations as jest.Mock).mockReturnValue(ec2Recs); + (state.getRecommendationsColumnFilters as jest.Mock).mockReturnValue({ + service: { kind: 'set', values: ['ec2'] }, + }); + const scopeLine = (): string | null => + document.querySelector('#recommendations-summary [data-testid="savings-scope"]')?.textContent ?? null; + + try { + await loadRecommendations(); + + expect(api.getRecommendations).toHaveBeenLastCalledWith(expect.objectContaining({ service: 'ec2' })); + expect(scopeLine()).toBe('Filtered: 2 recommendations (Service: ec2)'); + + // A further client-side filter is counted, not named. + (state.getRecommendationsColumnFilters as jest.Mock).mockReturnValue({ + service: { kind: 'set', values: ['ec2'] }, + term: { kind: 'set', values: ['1'] }, + }); + document.querySelector('th[data-sort="service"]')?.click(); + expect(scopeLine()).toBe('Filtered: 2 recommendations (Service: ec2, +1 more filter)'); + } finally { + // Reload unfiltered so the pushed-down scope doesn't leak into later tests. + (state.getRecommendationsColumnFilters as jest.Mock).mockReturnValue({}); + await loadRecommendations(); + } + expect(scopeLine()).toBeNull(); + }); + test('summary cards narrow to selection in real time (closes #279)', async () => { // 4 recs / 2 cells. With no selection: cards reflect the full // visible set (savings $300–$400, upfront $0–$3000). After ticking diff --git a/frontend/src/recommendations.ts b/frontend/src/recommendations.ts index c630b15f9..51cb92899 100644 --- a/frontend/src/recommendations.ts +++ b/frontend/src/recommendations.ts @@ -106,6 +106,12 @@ const expandedSpGroups = new Set(); // in sync with the per-cell banner range under the table). let lastRecommendationsSummary: RecommendationsSummary | null = null; +// #180: the single-value column filters the last successful fetch sent to the +// server (#162 pushdown). The loaded list is already narrowed by these, so the +// unfiltered total is unknown and the savings card must still say "Filtered" +// even though the visible and loaded counts match. +let loadedServerFilters: ReadonlyArray<{ column: state.RecommendationsColumnId; value: string }> = []; + // #284 (CR follow-up): guard against concurrent stale-load refreshes. If a // background refresh is already in flight, any subsequent stale detection // skips kicking off a second request (and duplicate toasts). @@ -650,6 +656,10 @@ export async function loadRecommendations(): Promise { state.setRecommendations(visibleByPreference); state.clearSelectedRecommendations(); + loadedServerFilters = [ + ...(pushedService !== undefined ? [{ column: 'service' as const, value: pushedService }] : []), + ...(pushedRegion !== undefined ? [{ column: 'region' as const, value: pushedRegion }] : []), + ]; // Cache the API-derived summary so renderRecommendationsList can // recompute the savings card from the visible (post-filter) set on @@ -841,6 +851,27 @@ async function onLookbackChange(rawValue: string): Promise { } } +/** + * #180: the savings card's scope line, or null when the card covers every + * recommendation (the same scope as Home). "Filtered" is decided by whether a + * column filter is active or the fetch itself was narrowed server-side, not by + * comparing counts: a pushed-down service/region filter returns only matching + * rows, so the visible and loaded counts are equal. In that case the + * unfiltered total isn't known, so the line names the server-side filter + * instead of an "of Y" figure. + */ +function savingsScopeText(visibleCount: number, loadedCount: number): string | null { + const active = Object.keys(state.getRecommendationsColumnFilters()); + if (loadedServerFilters.length > 0) { + const pushed = loadedServerFilters.map((f) => `${columnLabel(f.column)}: ${f.value}`); + const others = active.filter((col) => !loadedServerFilters.some((f) => f.column === col)).length; + if (others > 0) pushed.push(`+${others} more ${others === 1 ? 'filter' : 'filters'}`); + return `Filtered: ${visibleCount} recommendations (${pushed.join(', ')})`; + } + if (active.length === 0 && visibleCount >= loadedCount) return null; + return `Filtered: ${visibleCount} of ${loadedCount} recommendations`; +} + function renderRecommendationsSummary( _summary: RecommendationsSummary | null, recommendations: readonly LocalRecommendation[], @@ -914,12 +945,12 @@ function renderRecommendationsSummary( const sfx = periodSuffix(period); return isSelectionView ? `Selected Savings ${sfx}` : `Potential Savings ${sfx}`; })(); - // #180: Home's Potential Monthly Savings always covers every loaded + // #180: Home's Potential Monthly Savings always covers every // recommendation, so when column filters narrow this page the card must say // so; otherwise the two same-named KPIs disagree with no visible reason. - const isFilteredView = !isSelectionView && variantCount < loadedCount; - const savingsScopeDetail = isFilteredView - ? `

Filtered: ${variantCount} of ${loadedCount} recommendations

` + const scopeText = isSelectionView ? null : savingsScopeText(variantCount, loadedCount); + const savingsScopeDetail = scopeText + ? `

${escapeHtml(scopeText)}

` : ''; const upfrontLabel = isSelectionView ? 'Selected Upfront Cost' : 'Total Upfront Cost'; const paybackLabel = 'Payback Period'; diff --git a/frontend/tests-e2e/fixtures/recs.ts b/frontend/tests-e2e/fixtures/recs.ts index 47b31284c..e7626d6af 100644 --- a/frontend/tests-e2e/fixtures/recs.ts +++ b/frontend/tests-e2e/fixtures/recs.ts @@ -293,6 +293,11 @@ export async function mockApi(page: Page): Promise { if (provider && provider !== 'all' && provider !== '') { rows = RECS.filter((r) => r.provider === provider); } + // Like the backend, apply the single-value service/region pushdown (#162). + const service = u.searchParams.get('service'); + if (service) rows = rows.filter((r) => r.service === service); + const region = u.searchParams.get('region'); + if (region) rows = rows.filter((r) => r.region === region); if (pendingDelayMs > 0) { const wait = pendingDelayMs; pendingDelayMs = 0; diff --git a/frontend/tests-e2e/savings-kpi-parity.spec.ts b/frontend/tests-e2e/savings-kpi-parity.spec.ts index 20a95c013..bade659aa 100644 --- a/frontend/tests-e2e/savings-kpi-parity.spec.ts +++ b/frontend/tests-e2e/savings-kpi-parity.spec.ts @@ -66,3 +66,28 @@ test('a column-filtered Opportunities card labels its narrower scope', async ({ await expect(opportunitiesSavingsCard(page).getByTestId('savings-scope')) .toHaveText(`Filtered: ${AWS_EC2_COUNT} of ${AWS_COUNT} recommendations`); }); + +test('the card stays labeled when the service filter is sent to the server', async ({ page }) => { + // The persisted filter is seeded after the first fetch, so only a later + // load (here: leaving and returning to Opportunities) sends ?service=ec2. + // The server then returns only ec2 rows and the counts match, but the card + // still covers less than Home's figure. + await page.addInitScript(() => { + localStorage.setItem('cudly.recs.columnFilters.v1', JSON.stringify({ + schemaVersion: 1, + filters: { service: { kind: 'set', values: ['ec2'] } }, + })); + }); + await setup(page); + + await page.goto('/opportunities'); + const scope = opportunitiesSavingsCard(page).getByTestId('savings-scope'); + await expect(scope).toHaveText(`Filtered: ${AWS_EC2_COUNT} of ${AWS_COUNT} recommendations`); + + await page.locator('#home-tab-btn').click(); + const pushed = page.waitForRequest((req) => + req.url().includes('/api/recommendations') && new URL(req.url()).searchParams.get('service') === 'ec2'); + await page.locator('#opportunities-tab-btn').click(); + await pushed; + await expect(scope).toHaveText(`Filtered: ${AWS_EC2_COUNT} recommendations (Service: ec2)`); +}); From 6982f14ca6f002f4519145b90a2a4576d356ddbc Mon Sep 17 00:00:00 2001 From: Astrid Gealer Date: Sat, 24 Oct 2026 22:47:56 +0000 Subject: [PATCH 3/3] test(frontend): expect the em dash placeholder on Home's failed-fetch savings tile Main's #1376 replaced the '--' placeholder with NO_VALUE (an em dash). This branch's #180 fallback test still expected '--' for the savings tile value when the recommendations fetch fails. Refs #180 Co-Authored-By: Claude Opus 5.5 --- frontend/src/__tests__/dashboard.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frontend/src/__tests__/dashboard.test.ts b/frontend/src/__tests__/dashboard.test.ts index 3c52b76c4..226dd234a 100644 --- a/frontend/src/__tests__/dashboard.test.ts +++ b/frontend/src/__tests__/dashboard.test.ts @@ -860,7 +860,7 @@ describe('Dashboard Module', () => { await loadDashboard(); - expect(savingsTile()?.querySelector('.kpi-tile-value')?.textContent).toBe('--'); + expect(savingsTile()?.querySelector('.kpi-tile-value')?.textContent).toBe('\u2014'); expect(savingsTile()?.querySelector('.kpi-tile-detail')?.textContent).toBe('7 recommendations'); }); });