From d951229ea2b711f4c7af59c9cf3a241e17807383 Mon Sep 17 00:00:00 2001 From: Astrid Gealer Date: Mon, 26 Oct 2026 10:36:57 +0000 Subject: [PATCH] fix(frontend): don't offer Exchange for reshape rows with no target The reshape analyzer returns target_instance_type "" and target_count 0 when it has no exchange target for an RI (a completely unused RI, or, since the go bump in #1246, a target capped at the units the account owns). The table rendered those rows as "0x " under Suggested and kept a live Exchange button that opened the modal with a target count of 0. Such rows now say "No exchange target" (the Reason column already explains why) and get a disabled Exchange button with no quote action. A no-target row that still carries cross-family alternatives keeps a live button, since the modal can preselect from them, and opens it with a count of 1 instead of 0. Co-Authored-By: Claude Opus 5.5 --- .../__tests__/riexchange-no-target.test.ts | 146 ++++++++++++++++++ frontend/src/riexchange.ts | 44 +++++- 2 files changed, 185 insertions(+), 5 deletions(-) create mode 100644 frontend/src/__tests__/riexchange-no-target.test.ts diff --git a/frontend/src/__tests__/riexchange-no-target.test.ts b/frontend/src/__tests__/riexchange-no-target.test.ts new file mode 100644 index 000000000..2ae515edc --- /dev/null +++ b/frontend/src/__tests__/riexchange-no-target.test.ts @@ -0,0 +1,146 @@ +/** + * Reshape rows with no exchange target (issue #1410). + * + * The analyzer returns target_instance_type "" and target_count 0 when it + * has no target for an RI (a completely unused RI, or a target capped at the + * units the account owns). Those rows rendered "0x " in Suggested and kept a + * live Exchange button that opened the modal with a 0 count. + */ + +jest.mock('../api', () => ({ + listConvertibleRIs: jest.fn(), + getRIUtilization: jest.fn().mockResolvedValue([]), + getReshapeRecommendations: jest.fn(), + getRIExchangeHistory: jest.fn().mockResolvedValue([]), + listTargetOfferings: jest.fn().mockResolvedValue([]), +})); + +jest.mock('../navigation', () => ({ + switchTab: jest.fn(), + switchSettingsSubTab: jest.fn(), +})); + +jest.mock('../state', () => ({ + getCurrentUser: jest.fn(() => ({ id: 'u', email: 'u@example.com', groups: ['00000000-0000-5000-8000-000000000001'] })), + getCurrentProvider: jest.fn(() => 'aws'), + getCurrentAccountIDs: jest.fn(() => []), + getRiExchangeColumnFilters: jest.fn(() => ({})), + setRiExchangeColumnFilter: jest.fn(), + clearAllRiExchangeColumnFilters: jest.fn(), + getActiveRiColumnFilters: jest.fn(() => ({})), + setActiveRiColumnFilter: jest.fn(), + clearAllActiveRiColumnFilters: jest.fn(), +})); + +import * as api from '../api'; +import { loadReshapeRecommendations } from '../riexchange'; + +const withTarget = { + source_ri_id: 'ri-target', + source_instance_type: 'm5.xlarge', + source_count: 1, + target_instance_type: 'm5.large', + target_count: 2, + utilization_percent: 50, + normalized_used: 4, + normalized_purchased: 8, + reason: 'Suggest exchanging 1x m5.xlarge for 2x m5.large.', +}; + +const noTarget = { + source_ri_id: 'ri-unused', + source_instance_type: 'm5.xlarge', + source_count: 1, + target_instance_type: '', + target_count: 0, + utilization_percent: 0, + normalized_used: 0, + normalized_purchased: 8, + reason: 'RI is completely unused (0% utilization, 8.0 normalized units wasted).', +}; + +const alternative = { instance_type: 'm7g.large', offering_id: 'off-m7g', effective_monthly_cost: 30 }; + +function setupDom(): HTMLDivElement { + const recs = document.createElement('div'); + recs.id = 'ri-exchange-recommendations-list'; + const modal = document.createElement('div'); + modal.id = 'ri-exchange-modal'; + modal.className = 'modal hidden'; + const content = document.createElement('div'); + content.className = 'modal-content'; + modal.appendChild(content); + document.body.replaceChildren(recs, modal); + return recs; +} + +async function renderRows(recommendations: unknown[]): Promise { + const recs = setupDom(); + (api.getReshapeRecommendations as jest.Mock).mockResolvedValueOnce({ + recommendations, + recs_staleness: '', + recs_collected_at: null, + }); + await loadReshapeRecommendations(); + return Array.from(recs.querySelectorAll('tbody tr')); +} + +// Columns: Source RI, Current, Suggested, Alternatives, Utilization, +// Normalized Units, Reason, Actions. +const SUGGESTED = 2; +const REASON = 6; +const ACTIONS = 7; + +function cell(row: HTMLTableRowElement, i: number): HTMLTableCellElement { + return row.querySelectorAll('td')[i]!; +} + +describe('reshape rows with no exchange target (#1410)', () => { + afterEach(() => { + document.body.replaceChildren(); + (api.getReshapeRecommendations as jest.Mock).mockReset(); + }); + + it('says there is no target instead of "0x " and shows the reason', async () => { + const [row] = await renderRows([noTarget]); + + expect(cell(row!, SUGGESTED).textContent!.trim()).toBe('No exchange target'); + expect(cell(row!, SUGGESTED).textContent).not.toContain('0x'); + expect(cell(row!, REASON).textContent).toBe(noTarget.reason); + }); + + it('disables the Exchange button and attaches no quote action', async () => { + const [row] = await renderRows([noTarget]); + + const btn = cell(row!, ACTIONS).querySelector('button')!; + expect(btn.disabled).toBe(true); + expect(btn.dataset['action']).toBeUndefined(); + expect(btn.title).toBe('No exchange target available for this RI'); + + btn.click(); + expect(document.getElementById('ri-exchange-modal')!.classList.contains('hidden')).toBe(true); + }); + + it('keeps the Exchange button live for a row with a target', async () => { + const [row] = await renderRows([withTarget]); + + expect(cell(row!, SUGGESTED).textContent!.trim()).toBe('2x m5.large'); + const btn = cell(row!, ACTIONS).querySelector('button')!; + expect(btn.disabled).toBe(false); + expect(btn.dataset['action']).toBe('fill-quote'); + }); + + it('keeps Exchange live when alternatives exist, opening the modal with a count of 1', async () => { + const [row] = await renderRows([{ ...noTarget, alternative_targets: [alternative] }]); + + expect(cell(row!, SUGGESTED).textContent!.trim()).toBe('No exchange target'); + const btn = cell(row!, ACTIONS).querySelector('button')!; + expect(btn.disabled).toBe(false); + btn.click(); + + const modal = document.getElementById('ri-exchange-modal')!; + expect(modal.classList.contains('hidden')).toBe(false); + const count = modal.querySelector('.modal-exchange-count')!; + expect(count.value).toBe('1'); + }); +}); diff --git a/frontend/src/riexchange.ts b/frontend/src/riexchange.ts index 4f154fb1d..8e9ce2c86 100644 --- a/frontend/src/riexchange.ts +++ b/frontend/src/riexchange.ts @@ -999,14 +999,12 @@ function renderRecommendations(container: HTMLElement): void { ${escapeHtml(rec.source_ri_id)} ${rec.source_count}x ${escapeHtml(rec.source_instance_type)} - ${rec.target_count}x ${escapeHtml(rec.target_instance_type)} + ${renderSuggestedCell(rec)} ${altCell} ${utilPct === null ? '—' : `${utilPct.toFixed(1)}%`} ${rec.normalized_used.toFixed(1)} / ${rec.normalized_purchased.toFixed(1)} ${escapeHtml(rec.reason)} - ${canExchange ? ` - - ` : ''} + ${canExchange ? `${renderReshapeExchangeButton(rec, idx)}` : ''} `; }).join('')} @@ -1068,8 +1066,44 @@ function renderAlternativesCell(alternatives: OfferingOption[] | undefined): str .join(' '); } +// hasReshapeTarget reports whether a reshape recommendation names a primary +// exchange target. The analyzer emits target_instance_type "" and +// target_count 0 when it has none, for example for a completely unused RI +// or one whose target would exceed the units the account owns (#1410). +function hasReshapeTarget(rec: ReshapeRecommendation): boolean { + return Boolean(rec.target_instance_type) && rec.target_count > 0; +} + +// canExchangeReshape reports whether the row's Exchange button has anything +// to offer: a primary target, or cross-family alternatives the modal's picker +// can preselect from. +function canExchangeReshape(rec: ReshapeRecommendation): boolean { + return hasReshapeTarget(rec) || (rec.alternative_targets?.length ?? 0) > 0; +} + +function renderSuggestedCell(rec: ReshapeRecommendation): string { + if (!hasReshapeTarget(rec)) { + return 'No exchange target'; + } + return `${rec.target_count}x ${escapeHtml(rec.target_instance_type)}`; +} + +// renderReshapeExchangeButton renders the row's Exchange action. A row with +// nothing to exchange into gets a disabled button with no data-action, so no +// click handler is attached and the reason is still discoverable on hover. +function renderReshapeExchangeButton(rec: ReshapeRecommendation, idx: number): string { + if (!canExchangeReshape(rec)) { + const why = 'No exchange target available for this RI'; + return ``; + } + return ``; +} + function fillQuoteFromRecommendation(rec: ReshapeRecommendation): void { - openExchangeModal(rec.source_ri_id, rec.target_count, rec.target_instance_type, rec.alternative_targets); + // With no primary target the modal opens on an empty target row for the + // alternatives; a count of 0 would only fail validation. + const count = hasReshapeTarget(rec) ? rec.target_count : 1; + openExchangeModal(rec.source_ri_id, count, rec.target_instance_type, rec.alternative_targets); } export function fillQuoteFromRI(riId: string, count: number): void {