Skip to content
64 changes: 64 additions & 0 deletions frontend/src/__tests__/dashboard.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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, loadYtdSparkline, setupSavingsTrendHandlers, setupDashboardHandlers, formatTrendAxisTick } from '../dashboard';
import { formatTrendTooltipTitle } from '../modules/chart-utils';
Expand Down Expand Up @@ -801,6 +804,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('\u2014');
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 () => {
Expand Down
51 changes: 51 additions & 0 deletions frontend/src/__tests__/recommendations.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -358,6 +358,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
Expand All @@ -375,6 +380,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
Expand All @@ -383,6 +391,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<HTMLTableCellElement>('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
Expand Down
43 changes: 38 additions & 5 deletions frontend/src/dashboard.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import { groupRecsByCell, pageLevelRange, formatSavingsRange, triggerAutoRefresh
import { getRecommendationsFreshness } from './api/recommendations';
import { buildFirstRunEmptyState, isScopeFiltered } from './first-run-empty-state';
import { showSkeletonTiles, showSkeletonBlock, teardownSkeleton } from './lib/skeleton';
import { filterByEnabledProviders } from './lib/enabled-providers';
import { buildKpiTile } from './lib/kpi-tile';
import { scopeUpcomingToProvider, upcomingEmptyMessage, UPCOMING_ACCOUNT_SCOPE_NOTE } from './lib/upcoming-scope';
import { formatTrendAxisTick, formatTrendTooltipTitle, formatSparseTrendMessage, MIN_TREND_POINTS } from './modules/chart-utils';
Expand Down Expand Up @@ -151,10 +152,11 @@ export async function loadDashboard(): Promise<void> {
// 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, accountsResult] = await Promise.allSettled([
const [summaryResult, upcomingResult, recsResult, enabledProviders, accountsResult] = await Promise.allSettled([
api.getDashboardSummary(currentProvider, currentAccountIDs),
api.getUpcomingPurchases(),
api.getRecommendations({ provider: currentProvider, account_ids: currentAccountIDs }),
loadEnabledProviders(),
// Only feeds the first-run onboarding card (issue #756); wrapped so a
// missing or failing endpoint degrades to "unknown" instead of
// breaking the dashboard.
Expand All @@ -181,7 +183,15 @@ export async function loadDashboard(): Promise<void> {
: (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;
Expand All @@ -194,7 +204,10 @@ export async function loadDashboard(): Promise<void> {
// on the Home chart. Mirrors the pattern from loadSavingsTrendChart (#747).
const filterDesc = buildFilterDesc(currentProvider, currentAccountIDs);

renderDashboardSummary(summaryData, recs);
renderDashboardSummary(summaryData, recs, recsCount);
// `recs` is already narrowed to Enabled Providers (#180), matching the
// Opportunities first-run check, which also runs on the preference-
// filtered list.
renderHomeOnboarding(
recsResult.status === 'fulfilled' && recs.length === 0 && !summaryData.active_commitments,
accountsResult.status === 'fulfilled' && Array.isArray(accountsResult.value)
Expand Down Expand Up @@ -244,6 +257,20 @@ export async function loadDashboard(): Promise<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<readonly string[] | null> {
try {
const cfg = await api.getConfig();
return cfg?.global?.enabled_providers ?? null;
} catch {
return null;
}
}

/**
* Issue #756: first-run onboarding card at the top of Home. Shown when no
* recommendations loaded and nothing is committed yet, so a new user gets a
Expand All @@ -266,7 +293,11 @@ function renderHomeOnboarding(show: boolean, hasAccounts: boolean | null, scopeF
}));
}

function renderDashboardSummary(data: DashboardSummary, recs: readonly LocalRecommendation[]): void {
function renderDashboardSummary(
data: DashboardSummary,
recs: readonly LocalRecommendation[],
recsCount: number | null,
): void {
const summary = document.getElementById('summary');
if (!summary) return;

Expand Down Expand Up @@ -323,7 +354,9 @@ function renderDashboardSummary(data: DashboardSummary, recs: readonly LocalReco
detail: string;
}> = [
{ kpi: 'savings', title: 'Potential Monthly Savings', value: savingsDisplay, valueSavings: true,
detail: `${data.total_recommendations ?? NO_VALUE} 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 ?? NO_VALUE} recommendations` },
{ kpi: 'commitments', title: 'Active Commitments', value: data.active_commitments != null ? String(data.active_commitments) : NO_VALUE,
detail: `${formatCurrency(data.committed_monthly)}/mo committed` },
{ kpi: 'coverage', title: 'Current Coverage', value: coverageValue, detail: coverageDetail },
Expand Down
26 changes: 26 additions & 0 deletions frontend/src/lib/enabled-providers.ts
Original file line number Diff line number Diff line change
@@ -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<T extends { provider: string }>(
recs: readonly T[],
enabledProviders: readonly string[] | null | undefined,
): T[] {
if (!enabledProviders || enabledProviders.length === 0) return [...recs];
return recs.filter((r) => enabledProviders.includes(r.provider));
}
Loading
Loading