diff --git a/frontend/src/__tests__/crosstab-session.test.ts b/frontend/src/__tests__/crosstab-session.test.ts index ab35a0bc..b30c113a 100644 --- a/frontend/src/__tests__/crosstab-session.test.ts +++ b/frontend/src/__tests__/crosstab-session.test.ts @@ -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(); }); diff --git a/frontend/src/__tests__/dashboard.test.ts b/frontend/src/__tests__/dashboard.test.ts index d10548a5..b1f5be5c 100644 --- a/frontend/src/__tests__/dashboard.test.ts +++ b/frontend/src/__tests__/dashboard.test.ts @@ -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 @@ -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 () => { @@ -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(); + }); +}); diff --git a/frontend/src/__tests__/inventory.test.ts b/frontend/src/__tests__/inventory.test.ts index 05b8bc63..27a5ad70 100644 --- a/frontend/src/__tests__/inventory.test.ts +++ b/frontend/src/__tests__/inventory.test.ts @@ -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: [ diff --git a/frontend/src/__tests__/own-property.test.ts b/frontend/src/__tests__/own-property.test.ts new file mode 100644 index 00000000..93bedcdd --- /dev/null +++ b/frontend/src/__tests__/own-property.test.ts @@ -0,0 +1,17 @@ +import { ownValue } from '../lib/own-property'; + +describe('ownValue (#317)', () => { + const table: Record = { 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(); + }, + ); +}); diff --git a/frontend/src/api/client.ts b/frontend/src/api/client.ts index 33b85827..beaad851 100644 --- a/frontend/src/api/client.ts +++ b/frontend/src/api/client.ts @@ -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 = ''; diff --git a/frontend/src/dashboard.ts b/frontend/src/dashboard.ts index c915341b..0cf8bca0 100644 --- a/frontend/src/dashboard.ts +++ b/frontend/src/dashboard.ts @@ -189,6 +189,7 @@ export async function loadDashboard(): Promise { // 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 + @@ -1062,11 +1063,6 @@ export async function loadSavingsTrendChart(): Promise { 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 @@ -1107,7 +1103,6 @@ export async function loadSavingsTrendChart(): Promise { empty.classList.remove('hidden'); } if (savingsTrendChart) { savingsTrendChart.destroy(); savingsTrendChart = null; } - attachSparkline('ytd', []); return; } canvas.classList.remove('hidden'); @@ -1170,7 +1165,6 @@ export async function loadSavingsTrendChart(): Promise { // 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.'; @@ -1179,6 +1173,41 @@ export async function loadSavingsTrendChart(): Promise { } } +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 { + 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('.trend-range').forEach((btn) => { diff --git a/frontend/src/inventory.ts b/frontend/src/inventory.ts index 0f98fe93..b6c5bca3 100644 --- a/frontend/src/inventory.ts +++ b/frontend/src/inventory.ts @@ -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'; @@ -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; diff --git a/frontend/src/lib/own-property.ts b/frontend/src/lib/own-property.ts new file mode 100644 index 00000000..62ce4101 --- /dev/null +++ b/frontend/src/lib/own-property.ts @@ -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(table: Readonly>, key: string): T | undefined { + return Object.prototype.hasOwnProperty.call(table, key) ? table[key] : undefined; +} diff --git a/frontend/src/recommendations.ts b/frontend/src/recommendations.ts index 26be313d..e6bd280c 100644 --- a/frontend/src/recommendations.ts +++ b/frontend/src/recommendations.ts @@ -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'; @@ -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; }); } @@ -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 @@ -3098,7 +3099,7 @@ const PAYMENT_DISPLAY_LABELS: Record = { 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 @@ -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 diff --git a/frontend/tests-e2e/crosstab-storage-clear.spec.ts b/frontend/tests-e2e/crosstab-storage-clear.spec.ts new file mode 100644 index 00000000..3b5585d0 --- /dev/null +++ b/frontend/tests-e2e/crosstab-storage-clear.spec.ts @@ -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(); +});