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
133 changes: 128 additions & 5 deletions frontend/src/__tests__/riexchange.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand Down Expand Up @@ -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([]);
Expand All @@ -1047,19 +1053,136 @@ 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 = <T,>() => {
let resolve: (v: T) => void = () => undefined;
const promise = new Promise<T>(r => { resolve = r; });
return { promise, resolve };
};
const ris = pending<unknown[]>();
const recs = pending<unknown>();
const hist = pending<unknown[]>();
(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();
expect(api.listConvertibleRIs).toHaveBeenCalledWith(undefined);
});

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');
Expand All @@ -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 () => {
Expand Down
74 changes: 58 additions & 16 deletions frontend/src/riexchange.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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 };
Comment thread
coderabbitai[bot] marked this conversation as resolved.
}

// Mode label mapping β€” single source of truth
Expand All @@ -89,7 +98,31 @@ void MODE_VALUES;
* for GCP" empty state and no list endpoint is called.
*/
export async function loadRIExchange(): Promise<void> {
const { provider, accountID } = resolveScope();
const generation = ++scopeGeneration;
let scope: Awaited<ReturnType<typeof resolveScope>>;
try {
scope = await resolveScope();
} catch (error) {
if (generation !== scopeGeneration) return;
utilizationGeneration++;
Comment thread
coderabbitai[bot] marked this conversation as resolved.
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 = `<p class="error">Failed to resolve the selected account: ${escapeHtml((error as Error).message)}</p>`;
}
return;
}
if (generation !== scopeGeneration) return;
const { provider, accountID } = scope;

if (provider === 'gcp') {
renderGCPEmptyStates();
Expand All @@ -98,18 +131,18 @@ export async function loadRIExchange(): Promise<void> {

if (provider === 'azure') {
await Promise.all([
loadExchangeableAzureRIs(accountID),
loadExchangeHistory(),
loadExchangeableAzureRIs(accountID, generation),
loadExchangeHistory(generation),
]);
renderReshapeNotApplicable('azure');
return;
}

// AWS (default / empty provider): full convertible-RI flow.
await Promise.all([
loadConvertibleRIs(accountID),
loadReshapeRecommendations(),
loadExchangeHistory(),
loadConvertibleRIs(accountID, generation),
loadReshapeRecommendations(generation),
loadExchangeHistory(generation),
]);
}

Expand Down Expand Up @@ -252,7 +285,7 @@ function closeExchangeModalIfOpen(): void {
// Convertible RIs table
// ──────────────────────────────────────────────

async function loadConvertibleRIs(accountID?: string): Promise<void> {
async function loadConvertibleRIs(accountID?: string, generation: number = scopeGeneration): Promise<void> {
const container = document.getElementById('ri-exchange-instances-list');
if (!container) return;

Expand All @@ -268,12 +301,15 @@ async function loadConvertibleRIs(accountID?: string): Promise<void> {
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 = `<p class="error">Failed to load convertible RIs: ${escapeHtml(err.message)}</p>`;
Expand All @@ -287,7 +323,7 @@ async function loadConvertibleRIs(accountID?: string): Promise<void> {
* The optional accountID is the selected subscription chip; it scopes the
* capacity-provider registration check on the backend.
*/
async function loadExchangeableAzureRIs(accountID?: string): Promise<void> {
async function loadExchangeableAzureRIs(accountID: string | undefined, generation: number): Promise<void> {
const container = document.getElementById('ri-exchange-instances-list');
if (!container) return;

Expand All @@ -304,8 +340,10 @@ async function loadExchangeableAzureRIs(accountID?: string): Promise<void> {

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 = `<p class="error">Failed to load Azure reservations: ${escapeHtml(err.message)}</p>`;
Expand Down Expand Up @@ -479,7 +517,7 @@ function appendAzureCell(tr: HTMLTableRowElement, text: string): void {
// Reshape recommendations
// ──────────────────────────────────────────────

export async function loadReshapeRecommendations(): Promise<void> {
export async function loadReshapeRecommendations(generation: number = scopeGeneration): Promise<void> {
const container = document.getElementById('ri-exchange-recommendations-list');
if (!container) return;

Expand All @@ -491,10 +529,12 @@ export async function loadReshapeRecommendations(): Promise<void> {

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 = `<p class="error">Failed to load recommendations: ${escapeHtml(err.message)}</p>`;
Expand Down Expand Up @@ -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<void> {
async function loadExchangeHistory(generation: number = scopeGeneration): Promise<void> {
const container = document.getElementById('ri-exchange-history-list');
if (!container) return;

Expand All @@ -2264,8 +2304,10 @@ async function loadExchangeHistory(): Promise<void> {

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');
Expand Down
Loading