Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 9 additions & 2 deletions frontend/src/__tests__/crosstab-session.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -63,11 +63,18 @@ describe('cross-tab logout sync (issue #493)', () => {
expect(reload).not.toHaveBeenCalled();
});

test('ignores events with null key (e.g. localStorage.clear())', () => {
test('localStorage.clear() in another tab (key === null) signs this tab out (#317)', () => {
const { api, handler, reload } = loadAuth();
api.setAuthToken('t');
handler(evt(null, null));
expect(api.isAuthenticated()).toBe(true);
expect(api.isAuthenticated()).toBe(false);
expect(reload).toHaveBeenCalledTimes(1);
});

test('localStorage.clear() does not reload a tab that is already signed out', () => {
const { api, handler, reload } = loadAuth();
handler(evt(null, null));
expect(api.isAuthenticated()).toBe(false);
expect(reload).not.toHaveBeenCalled();
});

Expand Down
90 changes: 85 additions & 5 deletions frontend/src/__tests__/dashboard.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ jest.mock('../api', () => ({
// value don't need to be updated.
getRecommendations: jest.fn().mockResolvedValue([]),
}));
import { loadSavingsTrendChart, setupSavingsTrendHandlers, setupDashboardHandlers, formatTrendAxisTick } from '../dashboard';
import { loadSavingsTrendChart, loadYtdSparkline, setupSavingsTrendHandlers, setupDashboardHandlers, formatTrendAxisTick } from '../dashboard';
import { formatTrendTooltipTitle } from '../modules/chart-utils';

// Mock state module
Expand Down Expand Up @@ -1256,10 +1256,11 @@ describe('Dashboard Module', () => {
accountCb();
await new Promise((r) => setTimeout(r, 0));

expect(api.getSavingsAnalytics).toHaveBeenCalledTimes(1);
expect(api.getSavingsAnalytics).toHaveBeenCalledWith(
expect.objectContaining({ account_ids: ['uuid-acct-1'] })
);
// Two scoped series: the trend chart and the YTD tile sparkline (#317).
expect(api.getSavingsAnalytics).toHaveBeenCalledTimes(2);
for (const [filters] of (api.getSavingsAnalytics as jest.Mock).mock.calls) {
expect(filters).toEqual(expect.objectContaining({ account_ids: ['uuid-acct-1'] }));
}
});

test('does NOT fire when home tab is inactive (active-tab guard)', async () => {
Expand Down Expand Up @@ -2176,3 +2177,82 @@ describe('Dashboard Module', () => {
});
});
});

describe('YTD Savings tile sparkline (#317)', () => {
function addYtdTile(): SVGSVGElement {
const svg = document.createElementNS('http://www.w3.org/2000/svg', 'svg');
svg.classList.add('kpi-tile-spark', 'hidden');
svg.dataset['sparkKey'] = 'ytd';
const canvas = document.createElement('canvas');
canvas.id = 'savings-trend-chart';
const b7 = document.createElement('button');
b7.className = 'trend-range';
b7.dataset['range'] = '7';
document.body.replaceChildren(svg, canvas, b7);
return svg;
}

beforeEach(() => {
jest.clearAllMocks();
(state.getCurrentProvider as jest.Mock).mockReturnValue('aws');
(state.getCurrentAccountIDs as jest.Mock).mockReturnValue(['uuid-acct-1']);
});

test('requests 1 January (UTC) to now, scoped by the chips, whatever the trend range', async () => {
addYtdTile();
(api.getSavingsAnalytics as jest.Mock).mockResolvedValue({ data_points: [] });

await loadYtdSparkline();

expect(api.getSavingsAnalytics).toHaveBeenCalledTimes(1);
const filters = (api.getSavingsAnalytics as jest.Mock).mock.calls[0][0];
const start = new Date(filters.start);
expect(start.toISOString()).toBe(new Date(Date.UTC(new Date().getUTCFullYear(), 0, 1)).toISOString());
expect(filters).toEqual(expect.objectContaining({ provider: 'aws', account_ids: ['uuid-acct-1'] }));
});

test('draws the YTD series into the tile', async () => {
const svg = addYtdTile();
(api.getSavingsAnalytics as jest.Mock).mockResolvedValue({
data_points: [
{ timestamp: '2026-01-05T00:00:00Z', cumulative_savings: 10 },
{ timestamp: '2026-05-05T00:00:00Z', cumulative_savings: 40 },
],
});

await loadYtdSparkline();

expect(svg.classList.contains('hidden')).toBe(false);
expect(svg.querySelector('polyline')).not.toBeNull();
});

test('the trend chart (7-day range) no longer redraws the YTD tile', async () => {
const svg = addYtdTile();
(api.getSavingsAnalytics as jest.Mock).mockResolvedValue({
data_points: [
{ timestamp: new Date(Date.now() - 2 * 86400_000).toISOString(), cumulative_savings: 1 },
{ timestamp: new Date().toISOString(), cumulative_savings: 2 },
],
});
setupSavingsTrendHandlers();
(document.querySelector('.trend-range[data-range="7"]') as HTMLButtonElement).click();
await new Promise((r) => setTimeout(r, 0));

expect(api.getSavingsAnalytics).toHaveBeenCalledTimes(1);
expect(svg.querySelector('polyline')).toBeNull();
});

test('a failed fetch hides the sparkline and a missing tile skips the request', async () => {
const svg = addYtdTile();
const warn = jest.spyOn(console, 'warn').mockImplementation(() => {});
(api.getSavingsAnalytics as jest.Mock).mockRejectedValue(new Error('503'));
await loadYtdSparkline();
expect(svg.classList.contains('hidden')).toBe(true);

document.body.replaceChildren();
(api.getSavingsAnalytics as jest.Mock).mockClear();
await loadYtdSparkline();
expect(api.getSavingsAnalytics).not.toHaveBeenCalled();
warn.mockRestore();
});
});
12 changes: 12 additions & 0 deletions frontend/src/__tests__/inventory.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -456,6 +456,18 @@ describe('loadCoverageBreakdown — fetch + render flow', () => {
clearDOM();
});

test('a provider named like an Object.prototype member renders as text, not a function (#317)', async () => {
(api.getCoverageBreakdown as jest.Mock).mockResolvedValue({
providers: [makeProviderSection('constructor', null, null), makeProviderSection('toString', null, null)],
});

await loadCoverageBreakdown();

const titles = Array.from(document.querySelectorAll('#coverage-providers .coverage-provider-card h3'))
.map((h) => h.textContent);
expect(titles).toEqual(['CONSTRUCTOR', 'TOSTRING']);
});

test('renders per-provider sections with service rows', async () => {
(api.getCoverageBreakdown as jest.Mock).mockResolvedValue({
providers: [
Expand Down
17 changes: 17 additions & 0 deletions frontend/src/__tests__/own-property.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
import { ownValue } from '../lib/own-property';

describe('ownValue (#317)', () => {
const table: Record<string, string> = { aws: 'AWS', 'no-upfront': 'No Upfront' };

test('returns own entries', () => {
expect(ownValue(table, 'aws')).toBe('AWS');
expect(ownValue(table, 'no-upfront')).toBe('No Upfront');
});

test.each(['constructor', 'toString', '__proto__', 'hasOwnProperty', 'valueOf', ''])(
'returns undefined for %p instead of an Object.prototype member',
(key) => {
expect(ownValue(table, key)).toBeUndefined();
},
);
});
17 changes: 12 additions & 5 deletions frontend/src/api/client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -93,11 +93,18 @@ function installStorageListener(): void {
if (storageListenerInstalled) return;
if (typeof window === 'undefined' || !window.addEventListener) return;
window.addEventListener('storage', (e: StorageEvent) => {
if (!e.key || !(STORAGE_KEYS as readonly string[]).includes(e.key)) return;
// Only react to a *cleared* key (newValue === null); partial
// updates (e.g. token refresh) propagate via the in-memory state
// and don't need a reload.
if (e.newValue !== null) return;
if (e.key === null) {
// localStorage.clear() in another tab (a browser privacy control,
// devtools, a future clear() call) fires with key === null: every
// key, auth included, is gone. Signed-out tabs have nothing to drop.
if (!authToken && !apiKey && !csrfToken) return;
} else {
if (!(STORAGE_KEYS as readonly string[]).includes(e.key)) return;
// Only react to a *cleared* key (newValue === null); partial
// updates (e.g. token refresh) propagate via the in-memory state
// and don't need a reload.
if (e.newValue !== null) return;
}
authToken = '';
apiKey = '';
csrfToken = '';
Expand Down
43 changes: 36 additions & 7 deletions frontend/src/dashboard.ts
Original file line number Diff line number Diff line change
Expand Up @@ -189,6 +189,7 @@ export async function loadDashboard(): Promise<void> {
// Load the savings-over-time widget independently -- failure shouldn't
// block the rest of the dashboard (e.g. analytics not configured).
void loadSavingsTrendChart();
void loadYtdSparkline();

// Auto-refresh the recommendations cache if the last collection
// is older than 24h (or never ran). The dashboard's KPI tiles +
Expand Down Expand Up @@ -1062,11 +1063,6 @@ export async function loadSavingsTrendChart(): Promise<void> {

const points = data.data_points ?? [];

// YTD Savings KPI tile sparkline (issue #340 T6) — uses the same
// cumulative_savings series the main chart renders. Skips silently
// when the tile isn't in the DOM (e.g. a different layout is mounted).
attachSparkline('ytd', points.map((p: SavingsDataPoint) => p.cumulative_savings || 0));

// Determine the left edge of the x-axis.
// For 'all': anchor to the earliest purchase so the line fills the
// chart rather than clustering at the right end. Fall back to now-365d
Expand Down Expand Up @@ -1107,7 +1103,6 @@ export async function loadSavingsTrendChart(): Promise<void> {
empty.classList.remove('hidden');
}
if (savingsTrendChart) { savingsTrendChart.destroy(); savingsTrendChart = null; }
attachSparkline('ytd', []);
return;
}
canvas.classList.remove('hidden');
Expand Down Expand Up @@ -1170,7 +1165,6 @@ export async function loadSavingsTrendChart(): Promise<void> {
// the dashboard — hide the widget and fall back to a neutral message.
console.warn('Savings trend chart unavailable:', err);
if (savingsTrendChart) { savingsTrendChart.destroy(); savingsTrendChart = null; }
attachSparkline('ytd', []);
canvas.classList.add('hidden');
if (empty) {
empty.textContent = 'Savings history is not available yet.';
Expand All @@ -1179,6 +1173,41 @@ export async function loadSavingsTrendChart(): Promise<void> {
}
}

let ytdSparklineGeneration = 0;

/**
* Sparkline for the YTD Savings tile. It used to reuse the trend chart's
* points, whose window follows the 7d/30d/90d/All toggle, so the line under
* "YTD" changed shape with a control that does not change the number. It now
* plots its own series from 1 January (UTC, the same year start the backend
* uses for ytd_savings) to now, scoped by the same provider/account chips.
*/
export async function loadYtdSparkline(): Promise<void> {
if (!document.querySelector('.kpi-tile-spark[data-spark-key="ytd"]')) return;
const generation = ++ytdSparklineGeneration;
const now = new Date();
const yearStartMs = Date.UTC(now.getUTCFullYear(), 0, 1);
const days = (now.getTime() - yearStartMs) / 86400_000;
const interval: 'hourly' | 'daily' | 'weekly' = days <= 7 ? 'hourly' : days <= 90 ? 'daily' : 'weekly';
const accountIDs = state.getCurrentAccountIDs();
const provider = state.getCurrentProvider();
try {
const data = await api.getSavingsAnalytics({
start: new Date(yearStartMs).toISOString(),
end: now.toISOString(),
interval,
...(provider ? { provider } : {}),
...(accountIDs.length > 0 ? { account_ids: accountIDs } : {}),
});
if (generation !== ytdSparklineGeneration) return;
attachSparkline('ytd', (data.data_points ?? []).map((p: SavingsDataPoint) => p.cumulative_savings || 0));
} catch (err) {
if (generation !== ytdSparklineGeneration) return;
console.warn('YTD sparkline unavailable:', err);
attachSparkline('ytd', []);
}
}

// Wire up the range-toggle buttons. Called once during dashboard setup.
export function setupSavingsTrendHandlers(): void {
document.querySelectorAll<HTMLButtonElement>('.trend-range').forEach((btn) => {
Expand Down
3 changes: 2 additions & 1 deletion frontend/src/inventory.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import { showSkeletonRows, teardownSkeleton } from './lib/skeleton';
import { formatCurrency, formatDate, amortizedMonthly } from './utils';
import * as state from './state';
import { switchInventorySubTab } from './navigation';
import { ownValue } from './lib/own-property';

type InventorySubSection = 'active-commitments' | 'coverage' | 'trends' | 'ri-exchange';

Expand Down Expand Up @@ -435,7 +436,7 @@ function buildProviderSection(
const header = document.createElement('div');
header.className = 'section-header';

const providerLabel = PROVIDER_DISPLAY_NAMES[section.provider] ?? section.provider.toUpperCase();
const providerLabel = ownValue(PROVIDER_DISPLAY_NAMES, section.provider) ?? section.provider.toUpperCase();

const title = document.createElement('h3');
title.textContent = providerLabel;
Expand Down
11 changes: 11 additions & 0 deletions frontend/src/lib/own-property.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
/**
* Own-property lookup for display/ordering tables keyed by API strings.
*
* `TABLE[key]` walks the prototype chain, so a value such as "constructor"
* or "toString" resolves to an Object.prototype member instead of falling
* through to the caller's `??` default. The same shape reached production via
* tab routing (cloud-commitments-cli#1855) before #1854 fixed it there.
*/
export function ownValue<T>(table: Readonly<Record<string, T>>, key: string): T | undefined {
return Object.prototype.hasOwnProperty.call(table, key) ? table[key] : undefined;
}
11 changes: 6 additions & 5 deletions frontend/src/recommendations.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ function canActOnRecommendations(): boolean {
}
import { parseNumericFilter, applyColumnFilters as applyColumnFiltersLib } from './lib/column-filters';
import { fetchOverridesForAccounts } from './lib/overrides';
import { ownValue } from './lib/own-property';
// Re-export the shared primitives so existing consumers that import from
// recommendations.ts keep working without import-path churn (issue #166).
export { parseNumericFilter } from './lib/column-filters';
Expand Down Expand Up @@ -1334,8 +1335,8 @@ function sortVariantsInCell(variants: LocalRecommendation[]): LocalRecommendatio
return variants.slice().sort((a, b) => {
const termDiff = a.term - b.term;
if (termDiff !== 0) return termDiff;
const pa = PAYMENT_ORDER[a.payment ?? ''] ?? 99;
const pb = PAYMENT_ORDER[b.payment ?? ''] ?? 99;
const pa = ownValue(PAYMENT_ORDER, a.payment ?? '') ?? 99;
const pb = ownValue(PAYMENT_ORDER, b.payment ?? '') ?? 99;
return pa - pb;
});
}
Expand Down Expand Up @@ -1453,7 +1454,7 @@ function cellScoreFor(
// canonical order. Stringified so the existing string-column comparator
// path handles it.
const first = sortVariantsInCell(variants)[0];
return String(PAYMENT_ORDER[first?.payment ?? ''] ?? 99);
return String(ownValue(PAYMENT_ORDER, first?.payment ?? '') ?? 99);
}

// All other numeric columns (savings, count, on_demand_monthly): preserve
Expand Down Expand Up @@ -3098,7 +3099,7 @@ const PAYMENT_DISPLAY_LABELS: Record<string, string> = {

function formatPayment(payment: string | undefined): string {
if (!payment) return '\u2014';
return PAYMENT_DISPLAY_LABELS[payment] ?? payment;
return ownValue(PAYMENT_DISPLAY_LABELS, payment) ?? payment;
}

// renderUsageSparkline returns an inline SVG polyline representing the
Expand Down Expand Up @@ -3239,7 +3240,7 @@ function buildVariantRowMarkup(
rec.region,
accountName,
formatTerm(rec.term),
PAYMENT_DISPLAY_LABELS[rec.payment ?? ''] ?? '',
ownValue(PAYMENT_DISPLAY_LABELS, rec.payment ?? '') ?? '',
].filter(part => part).join(' '),
);
const checkboxCell = showCheckboxes
Expand Down
25 changes: 25 additions & 0 deletions frontend/tests-e2e/crosstab-storage-clear.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
import { test, expect } from '@playwright/test';
import { mockApi } from './fixtures/recs';

// #317: localStorage.clear() in another tab fires a storage event with
// key === null. The cross-tab sign-out listener used to ignore that shape, so
// this tab kept rendering a signed-in view after every key was wiped.
test('localStorage.clear() in another tab signs this tab out', async ({ page, context }) => {
await mockApi(page);
await page.route('**/api/info', route => route.fulfill({ json: { version: 'test', admin_exists: true } }));
// Sign in with a one-off write rather than an init script, which would
// re-seed the token on the reload this test expects.
await page.goto('/');
await page.evaluate(() => { localStorage.setItem('authToken', 'smoke-token'); localStorage.setItem('csrfToken', 'smoke-csrf'); });
await page.reload();
await expect(page.getByRole('button', { name: 'Logout' })).toBeVisible();
await expect(page.getByRole('heading', { name: 'CUDly Login' })).toHaveCount(0);

const other = await context.newPage();
await mockApi(other);
await other.goto('/');
await other.evaluate(() => localStorage.clear());
await other.close();

await expect(page.getByRole('heading', { name: 'CUDly Login' })).toBeVisible();
});
Loading