diff --git a/frontend/src/__tests__/dashboard.test.ts b/frontend/src/__tests__/dashboard.test.ts index f72cd84a7..9a296bec6 100644 --- a/frontend/src/__tests__/dashboard.test.ts +++ b/frontend/src/__tests__/dashboard.test.ts @@ -285,6 +285,58 @@ describe('Dashboard Module', () => { expect(upcomingList?.innerHTML).toContain('Step 1 of 4'); }); + test('shows an inline error with Retry, not the empty state, when the upcoming fetch fails (#713)', async () => { + (api.getDashboardSummary as jest.Mock).mockResolvedValue({ + potential_monthly_savings: 1000, + by_service: {} + }); + (api.getUpcomingPurchases as jest.Mock).mockRejectedValueOnce(new Error('API error: 503 Service Unavailable')); + + await loadDashboard(); + + const upcomingList = document.getElementById('upcoming-list'); + expect(upcomingList?.textContent).not.toContain('No upcoming scheduled purchases'); + expect(upcomingList?.querySelector('.error')?.textContent).toContain('Failed to load upcoming purchases'); + // The summary is unaffected by the upcoming failure. + expect(document.getElementById('summary')?.querySelector('.error')).toBeNull(); + + (api.getUpcomingPurchases as jest.Mock).mockResolvedValueOnce({ + purchases: [{ + execution_id: 'exec-1', plan_id: 'plan-1', plan_name: 'Retry Plan', + scheduled_date: '2025-01-15', provider: 'aws', service: 'ec2', + step_number: 1, total_steps: 2, estimated_savings: 120, + }] + }); + const retry = upcomingList?.querySelector('button[data-action="retry-upcoming"]') as HTMLButtonElement; + expect(retry).not.toBeNull(); + retry.click(); + await new Promise(r => setTimeout(r, 0)); + + expect(api.getUpcomingPurchases).toHaveBeenCalledTimes(2); + expect(api.getDashboardSummary).toHaveBeenCalledTimes(1); + expect(upcomingList?.innerHTML).toContain('Retry Plan'); + expect(upcomingList?.querySelector('.error')).toBeNull(); + }); + + test('upcoming error is announced and Retry keeps keyboard focus in the widget (#713)', async () => { + (api.getDashboardSummary as jest.Mock).mockResolvedValue({ potential_monthly_savings: 1000, by_service: {} }); + (api.getUpcomingPurchases as jest.Mock).mockRejectedValueOnce(new Error('API error: 503')); + await loadDashboard(); + + const upcomingList = document.getElementById('upcoming-list') as HTMLElement; + expect(upcomingList.querySelector('.error')?.getAttribute('role')).toBe('alert'); + + (api.getUpcomingPurchases as jest.Mock).mockResolvedValueOnce({ purchases: [] }); + const retry = upcomingList.querySelector('button[data-action="retry-upcoming"]') as HTMLButtonElement; + retry.focus(); + retry.click(); + // Retry replaces its own button; focus must land on the container, not . + expect(upcomingList.getAttribute('tabindex')).toBe('-1'); + expect(document.activeElement).toBe(upcomingList); + await new Promise(r => setTimeout(r, 0)); + expect(document.activeElement).toBe(upcomingList); + }); + test('shows empty message when no upcoming purchases', async () => { (api.getDashboardSummary as jest.Mock).mockResolvedValue({ potential_monthly_savings: 1000, @@ -1651,6 +1703,101 @@ describe('Dashboard Module', () => { expect(totalLine).toMatch(/\$400/); }); + describe('stable per-service colours and matching legend (#700)', () => { + type ChartCfg = { + data: { labels: string[]; datasets: { label: string; backgroundColor: string[] }[] }; + options: { plugins: { legend: { labels: { generateLabels?: (c: unknown) => { text: string; fillStyle: string }[] } } } }; + }; + const lastCfg = (): ChartCfg => { + const calls = (Chart as unknown as jest.Mock).mock.calls; + return calls[calls.length - 1]?.[1] as ChartCfg; + }; + const colourOf = (cfg: ChartCfg, svc: string): string => { + const ds = cfg.data.datasets.find((d) => d.label === 'Lowest option'); + return ds?.backgroundColor[cfg.data.labels.indexOf(svc)] ?? ''; + }; + + test('a service keeps its colour when its rank changes between renders', () => { + buildDOM(); + renderSavingsByService([rec('ec2', 500), rec('rds', 300), rec('elasticache', 100)]); + const before = lastCfg(); + expect(before.data.labels[0]).toBe('ec2'); + const ec2Before = colourOf(before, 'ec2'); + const rdsBefore = colourOf(before, 'rds'); + const cacheBefore = colourOf(before, 'elasticache'); + + // Data refresh: rds overtakes ec2, elasticache jumps to rank 2. + renderSavingsByService([rec('ec2', 120), rec('rds', 900), rec('elasticache', 400)]); + const after = lastCfg(); + expect(after.data.labels[0]).toBe('rds'); + expect(colourOf(after, 'ec2')).toBe(ec2Before); + expect(colourOf(after, 'rds')).toBe(rdsBefore); + expect(colourOf(after, 'elasticache')).toBe(cacheBefore); + expect(new Set([ec2Before, rdsBefore, cacheBefore]).size).toBe(3); + }); + + test('every known service slug has a distinct colour per family, twins share one', () => { + const { serviceBarColor } = require('../dashboard') as { serviceBarColor: (s: string) => string }; + const families: string[][] = [ + ['compute', 'ec2', 'vm'], + ['relational-db', 'rds', 'sql', 'cloudsql'], + ['cache', 'elasticache', 'redis', 'memorystore'], + ['savingsplans', 'savings-plans', 'savings-plans-compute', 'savings-plans-ec2instance', 'savings-plans-sagemaker', 'savings-plans-database'], + ['search', 'opensearch'], + ['data-warehouse', 'redshift'], + ['memorydb'], + ]; + const colours = families.map((slugs) => { + const set = new Set(slugs.map((x) => serviceBarColor(x))); + expect([...set]).toHaveLength(1); + return [...set][0]; + }); + expect(new Set(colours).size).toBe(families.length); + }); + + test('unknown slugs never reuse a known family colour', () => { + const { serviceBarColor } = require('../dashboard') as { serviceBarColor: (s: string) => string }; + // Fallback colours must not reuse a known family's colour. + const known = new Set(['compute', 'relational-db', 'cache', 'savingsplans', 'search', 'data-warehouse', 'memorydb'].map((x) => serviceBarColor(x))); + for (const unknown of ['alpha', 'zeta-service', 'nosuch', 'x1', 'foo-bar']) { + expect(known.has(serviceBarColor(unknown))).toBe(false); + } + }); + + test('an unknown slug gets a deterministic colour from the name hash', () => { + const { serviceBarColor } = require('../dashboard') as { serviceBarColor: (s: string) => string }; + expect(serviceBarColor('mystery-service')).toBe(serviceBarColor('mystery-service')); + expect(serviceBarColor('Mystery-Service')).toBe(serviceBarColor('mystery-service')); + // The hash must actually vary: a fixed slot for every unknown slug is a bug. + const seen = new Set(['a', 'b', 'c', 'd', 'e', 'f', 'g', 'h'].map((x) => serviceBarColor(`svc-${x}`))); + expect(seen.size).toBeGreaterThan(1); + }); + + test('legend swatches carry a border so the pale Upside swatch stays visible', () => { + buildDOM(); + renderSavingsByService([rec('ec2', 500)]); + const cfg = lastCfg(); + const items = cfg.options.plugins.legend.labels.generateLabels?.({ data: { datasets: cfg.data.datasets }, isDatasetVisible: () => true }) as unknown as { strokeStyle: string; lineWidth: number }[]; + for (const item of items) { + expect(item.lineWidth).toBeGreaterThanOrEqual(1); + expect(item.strokeStyle).toBe('#546e7a'); + } + }); + + test('legend swatches use neutral tones matching the three bar layers, not the first bar colour', () => { + buildDOM(); + renderSavingsByService([rec('ec2', 500), rec('rds', 300)]); + const cfg = lastCfg(); + const gen = cfg.options.plugins.legend.labels.generateLabels; + expect(typeof gen).toBe('function'); + const items = gen?.({ data: { datasets: cfg.data.datasets }, isDatasetVisible: () => true }) ?? []; + expect(items.map((i) => i.text)).toEqual(['Current / Committed', 'Lowest option', 'Upside']); + const barColours = cfg.data.datasets.flatMap((d) => d.backgroundColor); + for (const item of items) expect(barColours).not.toContain(item.fillStyle); + expect(new Set(items.map((i) => i.fillStyle)).size).toBe(3); + }); + }); + test('tooltip shows the committed value for a service with a commitment but no recommendation (#752)', () => { buildDOM(); renderSavingsByService( diff --git a/frontend/src/__tests__/purchase-modal-submit.test.ts b/frontend/src/__tests__/purchase-modal-submit.test.ts index f8b91b880..d9d6ccaee 100644 --- a/frontend/src/__tests__/purchase-modal-submit.test.ts +++ b/frontend/src/__tests__/purchase-modal-submit.test.ts @@ -314,6 +314,38 @@ describe('Issue #251: Execute Now makes no cancellation promise', () => { expect(text).not.toContain('Charged today'); }); + test('confirm button names the action and the upfront amount in both modes (#701)', async () => { + const rec: LocalRecommendation = { ...buildRows()[0]!, provider: 'aws', service: 'ec2', term: 1 }; + (state.getRecommendations as jest.Mock).mockReturnValue([rec]); + const amount = formatCurrency(rec.upfront_cost, '$', 2); + const confirmButtonLabel = (): string => + ((confirmDialog as jest.Mock).mock.calls[0]![0] as { confirmLabel: string }).confirmLabel; + + (confirmDialog as jest.Mock).mockClear(); + await openPurchaseModal([rec]); + const modalBtn = document.getElementById('execute-purchase-btn') as HTMLButtonElement; + const approvalVerb = modalBtn.textContent; + modalBtn.click(); + await flush(); + expect(confirmButtonLabel()).toBe(`${approvalVerb} (${amount} upfront on approval)`); + + (confirmDialog as jest.Mock).mockClear(); + (document.getElementById('close-purchase-modal-btn') as HTMLButtonElement).click(); + await openPurchaseModal([rec]); + (document.getElementById('execute-mode-direct') as HTMLInputElement).click(); + const directVerb = (document.getElementById('execute-purchase-btn') as HTMLButtonElement).textContent; + (document.getElementById('execute-purchase-btn') as HTMLButtonElement).click(); + await flush(); + expect(confirmButtonLabel()).toBe(`${directVerb} (${amount} charged today)`); + }); + + test('the purchase modal title is not the generic "Configure Purchase" (#701)', () => { + // eslint-disable-next-line @typescript-eslint/no-var-requires + const html = (require('fs') as typeof import('fs')).readFileSync(require('path').join(__dirname, '..', 'index.html'), 'utf8'); + const title = /

([^<]*)<\/h2>/.exec(html)?.[1]; + expect(title).toBe('Review Purchase'); + }); + test('direct-execute confirm dialog makes no cancellation promise', async () => { const rec: LocalRecommendation = { ...buildRows()[0]!, provider: 'aws', service: 'ec2', term: 1 }; (state.getRecommendations as jest.Mock).mockReturnValue([rec]); diff --git a/frontend/src/app.ts b/frontend/src/app.ts index fa0509443..c73b9658e 100644 --- a/frontend/src/app.ts +++ b/frontend/src/app.ts @@ -18,7 +18,7 @@ import { setupRIExchangeHandlers, saveAutomationSettings } from './riexchange'; import { showToast } from './toast'; import { formatPaymentAdjustmentNotice } from './commitmentOptions'; import { confirmDialog } from './confirmDialog'; -import { buildCommitmentSummary, withIntro } from './commitmentSummary'; +import { buildCommitmentSummary, commitmentTotals, executeButtonLabel, executeConfirmLabel, withIntro } from './commitmentSummary'; import { handlePurchaseDeeplink } from './purchases-deeplink'; import { handleArcheraDeeplink, openArcheraOfferModal } from './archera'; import { closeModal } from './modal'; @@ -432,8 +432,7 @@ export async function handleExecutePurchase(): Promise { // - Approval path: low-friction, non-destructive. // - Direct-execute path: red destructive dialog with cost callout and // immediate-charge callout; no cancellation promise (issue #251). - const summary = buildCommitmentSummary( - recsWithPayment.map((r) => ({ + const summaryLines = recsWithPayment.map((r) => ({ service: r.service, provider: r.provider, region: r.region, @@ -444,25 +443,28 @@ export async function handleExecutePurchase(): Promise { upfront: r.upfront_cost, monthly: r.monthly_cost ?? null, ...(r.cloud_account_id ? { account: getAccountName(r.cloud_account_id) } : {}), - })), + })); + const totals = commitmentTotals(summaryLines); + const summary = buildCommitmentSummary( + summaryLines, isDirect ? {} : { upfrontLabel: 'Upfront on approval' }, ); const ok = isDirect ? await confirmDialog({ title: `Execute ${localRecs.length} purchase${localRecs.length === 1 ? '' : 's'} now?`, body: withIntro('This charges the amount shown under "Charged today" immediately and bypasses the approval step. The monthly amount, if any, is billed for the whole term.', summary), - confirmLabel: 'Execute Purchase Now', + confirmLabel: executeConfirmLabel(true, totals.upfront), destructive: true, }) : await confirmDialog({ title: `Send ${localRecs.length} purchase${localRecs.length === 1 ? '' : 's'} for approval?`, body: withIntro('This emails an approval request to the configured approver. Nothing is charged until the approver clicks the link in that email.', summary), - confirmLabel: 'Send for approval', + confirmLabel: executeConfirmLabel(false, totals.upfront), destructive: false, }); if (!ok) { - setExecutePurchaseSubmitting(executeBtn, false, getExecuteMode() === 'direct' ? 'Execute Purchase Now' : 'Send for Approval'); + setExecutePurchaseSubmitting(executeBtn, false, executeButtonLabel(getExecuteMode() === 'direct')); return; } @@ -560,7 +562,7 @@ export async function handleExecutePurchase(): Promise { const verb = isDirect ? 'execute' : 'send for approval'; showToast({ message: `Failed to ${verb} purchase: ${err.message}`, kind: 'error' }); } finally { - setExecutePurchaseSubmitting(executeBtn, false, getExecuteMode() === 'direct' ? 'Execute Purchase Now' : 'Send for Approval'); + setExecutePurchaseSubmitting(executeBtn, false, executeButtonLabel(getExecuteMode() === 'direct')); } } @@ -604,14 +606,14 @@ async function handleFanOutExecute(buckets: FanOutBucket[]): Promise { destructive: false, }); if (!ok) { - setExecutePurchaseSubmitting(executeBtn, false, 'Send for Approval'); + setExecutePurchaseSubmitting(executeBtn, false, executeButtonLabel(false)); return; } try { await submitFanOutBuckets(buckets); } finally { - setExecutePurchaseSubmitting(executeBtn, false, 'Send for Approval'); + setExecutePurchaseSubmitting(executeBtn, false, executeButtonLabel(false)); } } diff --git a/frontend/src/commitmentSummary.ts b/frontend/src/commitmentSummary.ts index 84b0762d6..b01059c8a 100644 --- a/frontend/src/commitmentSummary.ts +++ b/frontend/src/commitmentSummary.ts @@ -50,6 +50,20 @@ export function commitmentTotals(lines: readonly CommitmentLine[]): CommitmentTo return { upfront, monthly, termTotal }; } +/** Label of the purchase modal's primary button; one source for every place that sets it. */ +export function executeButtonLabel(direct: boolean): string { + return direct ? 'Execute Purchase Now' : 'Send for Approval'; +} + +/** + * Confirm-dialog button: the same verb as the modal button plus the upfront + * amount, so the highest-stakes click states the money involved. + */ +export function executeConfirmLabel(direct: boolean, upfront: number): string { + const amount = formatCurrency(upfront, '$', 2); + return `${executeButtonLabel(direct)} (${amount} ${direct ? 'charged today' : 'upfront on approval'})`; +} + export function paymentLabel(payment: string): string { switch (payment.replace(/_/g, '-')) { case 'upfront': // Azure spelling of all-upfront diff --git a/frontend/src/dashboard.ts b/frontend/src/dashboard.ts index f719c8634..2a8e61716 100644 --- a/frontend/src/dashboard.ts +++ b/frontend/src/dashboard.ts @@ -31,8 +31,8 @@ let savingsByServiceChart: Chart | null = null; // Maximum number of services to show in the bar chart before truncating. const SAVINGS_BY_SERVICE_MAX = 10; -// Default palette for per-service bars. Cycles if there are more services -// than colours; alpha variant is computed inline so the array stays short. +// Palette for per-service bars. A service's colour is fixed by its name (see +// serviceBarColor), never by its rank, so it survives data refreshes (#700). const SERVICE_BAR_COLORS = [ '#1a73e8', // blue '#34a853', // green @@ -43,9 +43,45 @@ const SERVICE_BAR_COLORS = [ '#ff5722', // deep-orange '#607d8b', // blue-grey '#795548', // brown - '#4caf50', // light-green + '#e91e63', // pink ]; +// One palette slot per service family, keyed on the slugs the app emits +// (purchase mapServiceSlug, plan-service-ids.ts). A canonical slug and its +// legacy twin share a slot, so the same service is one colour whichever +// spelling a provider returns. Slots 0-6 are reserved for these families. +const KNOWN_SERVICE_COLOR_INDEX: Readonly> = { + compute: 0, ec2: 0, vm: 0, + 'relational-db': 1, rds: 1, sql: 1, cloudsql: 1, + cache: 2, elasticache: 2, redis: 2, memorystore: 2, + savingsplans: 3, 'savings-plans': 3, 'savings-plans-compute': 3, + 'savings-plans-ec2instance': 3, 'savings-plans-sagemaker': 3, 'savings-plans-database': 3, + search: 4, opensearch: 4, + 'data-warehouse': 5, redshift: 5, + memorydb: 6, +}; +const KNOWN_SERVICE_SLOTS = 7; + +/** Stable colour for a service name: identical on every render, regardless of rank. */ +export function serviceBarColor(service: string): string { + const key = service.toLowerCase(); + let idx = KNOWN_SERVICE_COLOR_INDEX[key]; + if (idx === undefined) { + // Unknown slugs hash into the spare slots; two unknown slugs may collide. + let h = 0; + for (let i = 0; i < key.length; i++) h = (h * 31 + key.charCodeAt(i)) >>> 0; + idx = KNOWN_SERVICE_SLOTS + (h % (SERVICE_BAR_COLORS.length - KNOWN_SERVICE_SLOTS)); + } + return SERVICE_BAR_COLORS[idx] ?? '#1a73e8'; +} + +// The legend names the three stacked layers, which every service shares, so +// its swatches are neutral tones that mirror the bar shading (dark / solid / +// 35% alpha) instead of the first bar's service colour. +const LEGEND_SWATCH_COLORS = ['#37474f', '#78909c', 'rgba(120,144,156,0.35)']; +// Border keeps the pale Upside swatch at >= 3:1 against the white card. +const LEGEND_SWATCH_BORDER = '#546e7a'; + /** * Parse a #rrggbb hex string into its r/g/b components. Falls back to the * default service bar blue (#1a73e8) for malformed input so the chart never @@ -215,7 +251,12 @@ export async function loadDashboard(): Promise { // 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); - renderUpcomingPurchases(upcomingData?.purchases || []); + if (upcomingResult.status === 'rejected') { + console.error('Failed to load upcoming purchases:', upcomingResult.reason); + renderUpcomingError(); + } else { + renderUpcomingPurchases(upcomingData?.purchases || []); + } // Load the savings-over-time widget independently -- failure shouldn't // block the rest of the dashboard (e.g. analytics not configured). void loadSavingsTrendChart(); @@ -428,6 +469,43 @@ export function canCancelUpcomingPurchase(purchase: UpcomingPurchase): boolean { return canAccess('delete', 'purchases') || canAccess('cancel-own', 'purchases'); } +// A failed fetch must not read as "nothing scheduled" (#713): show an inline +// error with a Retry that refetches only this widget. +function renderUpcomingError(): void { + const container = document.getElementById('upcoming-list'); + if (!container) return; + teardownSkeleton(container); + container.textContent = ''; + const msg = document.createElement('p'); + msg.className = 'error'; + msg.setAttribute('role', 'alert'); + msg.textContent = 'Failed to load upcoming purchases.'; + const retry = document.createElement('button'); + retry.type = 'button'; + retry.className = 'btn btn-secondary'; + retry.dataset['action'] = 'retry-upcoming'; + retry.textContent = 'Retry'; + retry.addEventListener('click', () => { void retryUpcomingPurchases(); }); + container.append(msg, retry); +} + +async function retryUpcomingPurchases(): Promise { + const container = document.getElementById('upcoming-list'); + if (container) { + // The Retry button is about to be replaced; keep focus in the widget. + container.tabIndex = -1; + container.focus(); + showSkeletonBlock(container, '100%', '6rem'); + } + try { + const data = (await api.getUpcomingPurchases()) as { purchases?: UpcomingPurchase[] } | null; + renderUpcomingPurchases(data?.purchases || []); + } catch (error) { + console.error('Failed to load upcoming purchases:', error); + renderUpcomingError(); + } +} + function renderUpcomingPurchases(purchases: UpcomingPurchase[]): void { const container = document.getElementById('upcoming-list'); if (!container) return; @@ -907,8 +985,8 @@ export function renderSavingsByService( const totalSavings = positive.reduce((acc, e) => acc + e.visibleTotal, 0); - // Colour palette: one base colour per service, then derive lighter/darker shades. - const bgColors = visible.map((_, i) => SERVICE_BAR_COLORS[i % SERVICE_BAR_COLORS.length] ?? '#1a73e8'); + // Colour: one stable base colour per service name, then derive lighter/darker shades. + const bgColors = visible.map(e => serviceBarColor(e.svc)); // Current / Committed: darkest shade (30% darker than base). const currentColors = bgColors.map(c => darkenHexColor(c)); @@ -960,7 +1038,17 @@ export function renderSavingsByService( legend: { display: true, position: 'top', - labels: { boxWidth: 12 }, + labels: { + boxWidth: 12, + generateLabels: (chart) => chart.data.datasets.map((ds, i) => ({ + text: ds.label ?? '', + fillStyle: LEGEND_SWATCH_COLORS[i] ?? LEGEND_SWATCH_COLORS[1] ?? '#78909c', + strokeStyle: LEGEND_SWATCH_BORDER, + lineWidth: 1, + hidden: !chart.isDatasetVisible(i), + datasetIndex: i, + })), + }, }, tooltip: { callbacks: { @@ -1001,6 +1089,8 @@ export function renderSavingsByService( x: { stacked: true, title: { display: true, text: 'Service' }, + // Keep every service label on narrow screens (rotate instead of skipping). + ticks: { autoSkip: false, maxRotation: 60 }, }, y: { stacked: true, diff --git a/frontend/src/index.html b/frontend/src/index.html index 444da187b..ebbbdd24c 100644 --- a/frontend/src/index.html +++ b/frontend/src/index.html @@ -995,7 +995,7 @@

Automation Settings