diff --git a/frontend/src/__tests__/riexchange.test.ts b/frontend/src/__tests__/riexchange.test.ts index 8ec4b2ef3..fc42efb2d 100644 --- a/frontend/src/__tests__/riexchange.test.ts +++ b/frontend/src/__tests__/riexchange.test.ts @@ -5,6 +5,7 @@ // Mock the api module defensively (riexchange.ts imports it) jest.mock('../api', () => ({ listConvertibleRIs: jest.fn(), + listAccountsMinimal: jest.fn(), listExchangeableAzureRIs: jest.fn(), getRIUtilization: jest.fn(), getReshapeRecommendations: jest.fn(), @@ -1024,6 +1025,11 @@ describe('RI Exchange global filter scoping (issue #871)', () => { (api.listConvertibleRIs as jest.Mock).mockResolvedValue([]); (api.listExchangeableAzureRIs as jest.Mock).mockResolvedValue([]); + (api.listAccountsMinimal as jest.Mock).mockResolvedValue([ + { id: 'uuid-1', name: 'Prod', external_id: '123456789012', provider: 'aws' }, + { id: 'uuid-2', name: 'Other', external_id: '210987654321', provider: 'aws' }, + { id: 'uuid-az', name: 'Sub', external_id: 'aaaaaaaa-0000-0000-0000-bbbbbbbbbbbb', provider: 'azure' }, + ]); (api.getRIUtilization as jest.Mock).mockResolvedValue([]); (api.getReshapeRecommendations as jest.Mock).mockResolvedValue({ recommendations: [], recs_staleness: '', recs_collected_at: null }); (api.getRIExchangeHistory as jest.Mock).mockResolvedValue([]); @@ -1047,11 +1053,128 @@ describe('RI Exchange global filter scoping (issue #871)', () => { }); it('forwards the single selected account to the AWS list endpoint', async () => { - stateMod().getCurrentAccountIDs.mockReturnValue(['123456789012']); + stateMod().getCurrentAccountIDs.mockReturnValue(['uuid-1']); await loadRIExchange(); expect(api.listConvertibleRIs).toHaveBeenCalledWith('123456789012'); }); + it('forwards the Azure subscription GUID (external_id), not the account UUID', async () => { + stateMod().getCurrentProvider.mockReturnValue('azure'); + stateMod().getCurrentAccountIDs.mockReturnValue(['uuid-az']); + await loadRIExchange(); + expect(api.listExchangeableAzureRIs).toHaveBeenCalledWith('aaaaaaaa-0000-0000-0000-bbbbbbbbbbbb'); + }); + + it('shows an error and calls no list endpoint when the selected account is unknown', async () => { + stateMod().getCurrentAccountIDs.mockReturnValue(['uuid-missing']); + await loadRIExchange(); + expect(api.listConvertibleRIs).not.toHaveBeenCalled(); + expect(instancesEl.textContent).toContain('uuid-missing'); + expect(instancesEl.querySelector('.error')).not.toBeNull(); + }); + + it('a pending utilization fetch from a prior scope cannot overwrite the resolve error', async () => { + const sampleRI = { + reserved_instance_id: 'ri-old', instance_type: 'm5.large', availability_zone: 'us-east-1a', + instance_count: 1, start: '2026-01-01T00:00:00Z', end: '2027-01-01T00:00:00Z', + offering_type: 'Convertible', fixed_price: 0, usage_price: 0, state: 'active', normalization_factor: 4, + }; + let releaseUtil: (v: unknown) => void = () => undefined; + (api.getRIUtilization as jest.Mock).mockReturnValueOnce(new Promise(r => { releaseUtil = r; })); + (api.listConvertibleRIs as jest.Mock).mockResolvedValue([sampleRI]); + stateMod().getCurrentAccountIDs.mockReturnValue(['uuid-1']); + await loadRIExchange(); + stateMod().getCurrentAccountIDs.mockReturnValue(['uuid-missing']); + await loadRIExchange(); + releaseUtil([]); + await new Promise(r => setTimeout(r, 0)); + expect(instancesEl.querySelector('.error')).not.toBeNull(); + expect(instancesEl.textContent).not.toContain('ri-old'); + expect(recsEl.textContent).toContain('Select a valid account'); + expect(historyEl.textContent).toContain('Select a valid account'); + }); + + it('drops a superseded failing resolve instead of rendering its error', async () => { + let rejectLookup: (e: Error) => void = () => undefined; + (api.listAccountsMinimal as jest.Mock).mockReturnValueOnce(new Promise((_, rej) => { rejectLookup = rej; })); + stateMod().getCurrentAccountIDs.mockReturnValue(['uuid-1']); + const first = loadRIExchange(); + stateMod().getCurrentAccountIDs.mockReturnValue([]); + await loadRIExchange(); + rejectLookup(new Error('boom')); + await first; + expect(instancesEl.querySelector('.error')).toBeNull(); + expect(instancesEl.textContent).not.toContain('boom'); + }); + + it('shows the request failure and makes no list call when listAccountsMinimal rejects', async () => { + (api.listAccountsMinimal as jest.Mock).mockRejectedValueOnce(new Error('HTTP 500')); + stateMod().getCurrentAccountIDs.mockReturnValue(['uuid-1']); + await loadRIExchange(); + expect(api.listConvertibleRIs).not.toHaveBeenCalled(); + expect(instancesEl.textContent).toContain('HTTP 500'); + }); + + it('fails loudly when the selected account has no external_id', async () => { + (api.listAccountsMinimal as jest.Mock).mockResolvedValueOnce([ + { id: 'uuid-1', name: 'Prod', external_id: '', provider: 'aws' }, + ]); + stateMod().getCurrentAccountIDs.mockReturnValue(['uuid-1']); + await loadRIExchange(); + expect(api.listConvertibleRIs).not.toHaveBeenCalled(); + expect(instancesEl.querySelector('.error')).not.toBeNull(); + }); + + it('in-flight loaders from a superseded load cannot overwrite a later resolve error', async () => { + const pending = () => { + let resolve: (v: T) => void = () => undefined; + const promise = new Promise(r => { resolve = r; }); + return { promise, resolve }; + }; + const ris = pending(); + const recs = pending(); + const hist = pending(); + (api.listConvertibleRIs as jest.Mock).mockReturnValueOnce(ris.promise); + (api.getReshapeRecommendations as jest.Mock).mockReturnValueOnce(recs.promise); + (api.getRIExchangeHistory as jest.Mock).mockReturnValueOnce(hist.promise); + const loadA = loadRIExchange(); + await new Promise(r => setTimeout(r, 0)); + stateMod().getCurrentAccountIDs.mockReturnValue(['uuid-missing']); + await loadRIExchange(); + ris.resolve([{ + reserved_instance_id: 'ri-old', instance_type: 'm5.large', availability_zone: 'us-east-1a', + instance_count: 1, start: '2026-01-01T00:00:00Z', end: '2027-01-01T00:00:00Z', + offering_type: 'Convertible', fixed_price: 0, usage_price: 0, state: 'active', normalization_factor: 4, + }]); + recs.resolve({ recommendations: [], recs_staleness: '', recs_collected_at: null }); + hist.resolve([]); + await loadA; + expect(instancesEl.querySelector('.error')).not.toBeNull(); + expect(instancesEl.textContent).not.toContain('ri-old'); + expect(recsEl.textContent).toContain('Select a valid account'); + expect(historyEl.textContent).toContain('Select a valid account'); + }); + + it('forwards undefined and skips the account lookup when no account is selected', async () => { + await loadRIExchange(); + expect(api.listAccountsMinimal).not.toHaveBeenCalled(); + expect(api.listConvertibleRIs).toHaveBeenCalledWith(undefined); + }); + + it('drops a superseded load when the account chip changes mid-lookup', async () => { + let release: (v: unknown) => void = () => undefined; + (api.listAccountsMinimal as jest.Mock).mockReturnValueOnce(new Promise(r => { release = r; })); + stateMod().getCurrentAccountIDs.mockReturnValue(['uuid-1']); + const first = loadRIExchange(); + stateMod().getCurrentAccountIDs.mockReturnValue(['uuid-2']); + const second = loadRIExchange(); + await second; + release([{ id: 'uuid-1', name: 'Prod', external_id: '123456789012', provider: 'aws' }]); + await first; + expect(api.listConvertibleRIs).toHaveBeenCalledTimes(1); + expect(api.listConvertibleRIs).toHaveBeenCalledWith('210987654321'); + }); + it('does not forward an account id when more than one account is selected', async () => { stateMod().getCurrentAccountIDs.mockReturnValue(['111111111111', '222222222222']); await loadRIExchange(); @@ -1059,7 +1182,7 @@ describe('RI Exchange global filter scoping (issue #871)', () => { }); it('renders a scoped empty-state naming the AWS account', async () => { - stateMod().getCurrentAccountIDs.mockReturnValue(['123456789012']); + stateMod().getCurrentAccountIDs.mockReturnValue(['uuid-1']); (api.listConvertibleRIs as jest.Mock).mockResolvedValue([]); await loadRIExchange(); expect(instancesEl.textContent).toContain('123456789012'); @@ -1081,11 +1204,11 @@ describe('RI Exchange global filter scoping (issue #871)', () => { it('shows a provider-aware empty state for Azure with no reservations', async () => { stateMod().getCurrentProvider.mockReturnValue('azure'); - stateMod().getCurrentAccountIDs.mockReturnValue(['sub-abc']); + stateMod().getCurrentAccountIDs.mockReturnValue(['uuid-az']); (api.listExchangeableAzureRIs as jest.Mock).mockResolvedValue([]); await loadRIExchange(); - expect(api.listExchangeableAzureRIs).toHaveBeenCalledWith('sub-abc'); - expect(instancesEl.textContent).toContain('No exchangeable reservations for Azure subscription sub-abc'); + expect(api.listExchangeableAzureRIs).toHaveBeenCalledWith('aaaaaaaa-0000-0000-0000-bbbbbbbbbbbb'); + expect(instancesEl.textContent).toContain('No exchangeable reservations for Azure subscription aaaaaaaa-0000-0000-0000-bbbbbbbbbbbb'); }); it('shows the not-available empty state for GCP and calls no list endpoint', async () => { diff --git a/frontend/src/riexchange.ts b/frontend/src/riexchange.ts index 109e48478..c4ad1f52f 100644 --- a/frontend/src/riexchange.ts +++ b/frontend/src/riexchange.ts @@ -41,6 +41,10 @@ const exchangePaymentPattern = /^\d+(?:\.\d+)?$/; // Generation counter to prevent stale utilization data from overwriting fresh data let utilizationGeneration = 0; +// Generation counter so a superseded loadRIExchange (chip changed while the +// account lookup was in flight) never issues its list call. +let scopeGeneration = 0; + // The AWS account the convertible-RI list is currently scoped to (the // single-select account chip value, or undefined for all-accounts). Tracked // at module scope so the asynchronous utilization re-render preserves the @@ -58,13 +62,18 @@ function providerLabel(provider: string): string { // resolveScope reads the Main Header global Provider/Account chips so every // RI Exchange load is scoped consistently with the other Inventory sub-tabs -// (issue #871). The account chip is single-select; an account_id is forwarded -// only when exactly one account is active. -function resolveScope(): { provider: Provider | ''; accountID?: string } { +// (issue #871). The account chip holds a CloudAccount UUID, but the RI +// endpoints expect the provider account number / subscription GUID +// (external_id), so the single selected account is translated here. +async function resolveScope(): Promise<{ provider: Provider | ''; accountID?: string }> { const provider = state.getCurrentProvider(); const accountIDs = state.getCurrentAccountIDs(); - const accountID = accountIDs.length === 1 ? accountIDs[0] : undefined; - return { provider, accountID }; + if (accountIDs.length !== 1) return { provider }; + const accounts = await api.listAccountsMinimal(); + const account = accounts.find(a => a.id === accountIDs[0]); + if (!account) throw new Error(`Selected account ${accountIDs[0]} was not found`); + if (!account.external_id) throw new Error(`Selected account ${accountIDs[0]} has no provider account ID`); + return { provider, accountID: account.external_id }; } // Mode label mapping — single source of truth @@ -89,7 +98,31 @@ void MODE_VALUES; * for GCP" empty state and no list endpoint is called. */ export async function loadRIExchange(): Promise { - const { provider, accountID } = resolveScope(); + const generation = ++scopeGeneration; + let scope: Awaited>; + try { + scope = await resolveScope(); + } catch (error) { + if (generation !== scopeGeneration) return; + utilizationGeneration++; + currentRIs = []; + currentUtilization = new Map(); + currentRecommendations = []; + currentRIAccountID = undefined; + const message = 'Select a valid account to load this section.'; + for (const id of ['ri-exchange-recommendations-list', 'ri-exchange-history-list']) { + const el = document.getElementById(id); + if (el) renderEmptyState(el, message); + } + renderReshapeStalenessBanner('', null); + const container = document.getElementById('ri-exchange-instances-list'); + if (container) { + container.innerHTML = `

Failed to resolve the selected account: ${escapeHtml((error as Error).message)}

`; + } + return; + } + if (generation !== scopeGeneration) return; + const { provider, accountID } = scope; if (provider === 'gcp') { renderGCPEmptyStates(); @@ -98,8 +131,8 @@ export async function loadRIExchange(): Promise { if (provider === 'azure') { await Promise.all([ - loadExchangeableAzureRIs(accountID), - loadExchangeHistory(), + loadExchangeableAzureRIs(accountID, generation), + loadExchangeHistory(generation), ]); renderReshapeNotApplicable('azure'); return; @@ -107,9 +140,9 @@ export async function loadRIExchange(): Promise { // AWS (default / empty provider): full convertible-RI flow. await Promise.all([ - loadConvertibleRIs(accountID), - loadReshapeRecommendations(), - loadExchangeHistory(), + loadConvertibleRIs(accountID, generation), + loadReshapeRecommendations(generation), + loadExchangeHistory(generation), ]); } @@ -252,7 +285,7 @@ function closeExchangeModalIfOpen(): void { // Convertible RIs table // ────────────────────────────────────────────── -async function loadConvertibleRIs(accountID?: string): Promise { +async function loadConvertibleRIs(accountID?: string, generation: number = scopeGeneration): Promise { const container = document.getElementById('ri-exchange-instances-list'); if (!container) return; @@ -268,12 +301,15 @@ async function loadConvertibleRIs(accountID?: string): Promise { try { // issue #871: scope to the selected account so the page honours the // Main Header global filter. - currentRIs = await api.listConvertibleRIs(accountID); + const ris = await api.listConvertibleRIs(accountID); + if (generation !== scopeGeneration) return; + currentRIs = ris; renderRIsTable(container, accountID); // Load utilization asynchronously (Cost Explorer is slow) utilizationGeneration++; void loadUtilization(utilizationGeneration); } catch (error) { + if (generation !== scopeGeneration) return; teardownSkeleton(container); const err = error as Error; container.innerHTML = `

Failed to load convertible RIs: ${escapeHtml(err.message)}

`; @@ -287,7 +323,7 @@ async function loadConvertibleRIs(accountID?: string): Promise { * The optional accountID is the selected subscription chip; it scopes the * capacity-provider registration check on the backend. */ -async function loadExchangeableAzureRIs(accountID?: string): Promise { +async function loadExchangeableAzureRIs(accountID: string | undefined, generation: number): Promise { const container = document.getElementById('ri-exchange-instances-list'); if (!container) return; @@ -304,8 +340,10 @@ async function loadExchangeableAzureRIs(accountID?: string): Promise { try { const reservations = await api.listExchangeableAzureRIs(accountID); + if (generation !== scopeGeneration) return; renderAzureRIsTable(container, reservations, accountID); } catch (error) { + if (generation !== scopeGeneration) return; teardownSkeleton(container); const err = error as Error; container.innerHTML = `

Failed to load Azure reservations: ${escapeHtml(err.message)}

`; @@ -479,7 +517,7 @@ function appendAzureCell(tr: HTMLTableRowElement, text: string): void { // Reshape recommendations // ────────────────────────────────────────────── -export async function loadReshapeRecommendations(): Promise { +export async function loadReshapeRecommendations(generation: number = scopeGeneration): Promise { const container = document.getElementById('ri-exchange-recommendations-list'); if (!container) return; @@ -491,10 +529,12 @@ export async function loadReshapeRecommendations(): Promise { try { const resp: ReshapeRecommendationsResponse = await api.getReshapeRecommendations(); + if (generation !== scopeGeneration) return; currentRecommendations = resp.recommendations ?? []; renderRecommendations(container); renderReshapeStalenessBanner(resp.recs_staleness, resp.recs_collected_at); } catch (error) { + if (generation !== scopeGeneration) return; teardownSkeleton(container); const err = error as Error; container.innerHTML = `

Failed to load recommendations: ${escapeHtml(err.message)}

`; @@ -2252,7 +2292,7 @@ function canApproveRIExchangeRow(rec: RIExchangeHistoryRecord): boolean { return canAccess('approve-own', 'purchases') && rec.created_by_user_id === user.id; } -async function loadExchangeHistory(): Promise { +async function loadExchangeHistory(generation: number = scopeGeneration): Promise { const container = document.getElementById('ri-exchange-history-list'); if (!container) return; @@ -2264,8 +2304,10 @@ async function loadExchangeHistory(): Promise { try { const records = await api.getRIExchangeHistory(); + if (generation !== scopeGeneration) return; renderExchangeHistory(container, records); } catch (error) { + if (generation !== scopeGeneration) return; const err = error as Error; container.textContent = ''; const errorP = document.createElement('p');