diff --git a/frontend/src/__tests__/riexchange.test.ts b/frontend/src/__tests__/riexchange.test.ts index 023eabae..83b17691 100644 --- a/frontend/src/__tests__/riexchange.test.ts +++ b/frontend/src/__tests__/riexchange.test.ts @@ -210,6 +210,9 @@ describe('openExchangeModal', () => { const executeBtn = Array.from(modal.querySelectorAll('button')).find((b) => b.textContent === 'Execute Exchange'); expect(executeBtn?.classList.contains('hidden')).toBe(false); executeBtn?.click(); + expect(mockExecute).not.toHaveBeenCalled(); + expect(document.querySelector('.modal-confirm-body')?.textContent).toContain('USD 12.50'); + document.querySelector('.modal-confirm-actions .btn-destructive')?.click(); await new Promise((resolve) => setTimeout(resolve, 0)); expect(mockExecute).toHaveBeenCalledTimes(1); diff --git a/frontend/src/riexchange.ts b/frontend/src/riexchange.ts index d9130b9b..12674fd9 100644 --- a/frontend/src/riexchange.ts +++ b/frontend/src/riexchange.ts @@ -36,6 +36,7 @@ import { getCurrentUser } from './state'; let currentRIs: ConvertibleRI[] = []; let currentUtilization: Map = new Map(); let currentRecommendations: ReshapeRecommendation[] = []; +const exchangePaymentPattern = /^\d+(?:\.\d+)?$/; // Generation counter to prevent stale utilization data from overwriting fresh data let utilizationGeneration = 0; @@ -1438,6 +1439,9 @@ export function openExchangeModal(riId: string, count: number, suggestedTargetTy }; let modalQuote: ExchangeQuoteSummary | null = null; let modalQuoteReq: QuoteReqShape | null = null; + let quoteRevision = 0; + let sessionOpen = true; + let closeTimer: ReturnType | undefined; // Build header const h3 = document.createElement('h3'); @@ -1658,6 +1662,7 @@ export function openExchangeModal(riId: string, count: number, suggestedTargetTy addTargetBtn.addEventListener('click', () => { addTargetRow(); updateRunningTotal(); + invalidateQuote(); }); addTargetBtnRow.appendChild(addTargetBtn); content.appendChild(addTargetBtnRow); @@ -1722,7 +1727,11 @@ export function openExchangeModal(riId: string, count: number, suggestedTargetTy content.appendChild(btnRow); // Show modal - openModal(modal); + openModal(modal, { onClose: () => { + sessionOpen = false; + invalidateQuote(); + clearTimeout(closeTimer); + } }); cancelBtn.addEventListener('click', () => { closeModal(modal); @@ -1732,6 +1741,7 @@ export function openExchangeModal(riId: string, count: number, suggestedTargetTy // target set changes (picker, count, remove-row), so Execute can never // submit a request that no longer matches what the form displays. function invalidateQuote(): void { + quoteRevision++; modalQuote = null; modalQuoteReq = null; executeBtn.classList.add('hidden'); @@ -1800,17 +1810,21 @@ export function openExchangeModal(riId: string, count: number, suggestedTargetTy return; } + invalidateQuote(); + const revision = quoteRevision; setResultText(resultContainer, 'Getting exchange quote...', 'loading'); - executeBtn.classList.add('hidden'); quoteBtn.disabled = true; const quoteReq = buildQuoteReq(targets); try { - modalQuote = await api.getExchangeQuote(quoteReq); + const quote = await api.getExchangeQuote(quoteReq); + if (!sessionOpen || revision !== quoteRevision) return; + modalQuote = quote; modalQuoteReq = quoteReq; renderModalQuoteResult(resultContainer, modalQuote); if (modalQuote.IsValidExchange) executeBtn.classList.remove('hidden'); } catch (error) { + if (!sessionOpen || revision !== quoteRevision) return; const err = error as Error; setResultText(resultContainer, 'Quote failed: ' + err.message, 'error'); } finally { @@ -1819,39 +1833,53 @@ export function openExchangeModal(riId: string, count: number, suggestedTargetTy } async function submitModalExecute(): Promise { - if (!modalQuote || !modalQuoteReq) return; - - setResultText(resultContainer, 'Executing exchange...', 'loading'); + if (executeBtn.disabled || !sessionOpen || !modalQuote?.IsValidExchange || !modalQuoteReq) return; + const quote = modalQuote; + const request = modalQuoteReq; + if (!exchangePaymentPattern.test(quote.PaymentDueRaw) || !quote.CurrencyCode?.trim()) { + setResultText(resultContainer, 'Cannot confirm exchange without a quoted payment and currency. Get a new quote.', 'error'); + return; + } + const targets = request.targets ?? [{ offering_id: request.target_offering_id, count: request.target_count }]; executeBtn.disabled = true; - + quoteBtn.disabled = true; + let confirmed = false; try { + confirmed = await confirmDialog({ + title: 'Execute RI Exchange', + body: `Quoted payment: ${quote.CurrencyCode} ${quote.PaymentDueRaw}. ` + + `Source RIs: ${request.ri_ids.join(', ')}. Region: ${quote.Region}. ` + + `Targets: ${targets.map(target => `${target.count} × ${target.offering_id}`).join(', ')}. ` + + 'This exchange executes immediately and cannot be reversed.', + confirmLabel: 'Execute Exchange', + destructive: true, + }); + if (!confirmed) return; + setResultText(resultContainer, 'Executing exchange...', 'loading'); const result = await api.executeExchange({ - ri_ids: modalQuoteReq.ri_ids, - targets: modalQuoteReq.targets, - target_offering_id: modalQuoteReq.target_offering_id, - target_count: modalQuoteReq.target_count, - max_payment_due_usd: modalQuote.PaymentDueRaw, - // The backend rejects execute with no region (issue #238): a - // missing region there previously fell through with a 400 on - // every UI-initiated exchange. Reuse the region the quote - // response resolved so quote and execute stay pinned together. - region: modalQuote.Region, + ...request, + max_payment_due_usd: quote.PaymentDueRaw, + region: quote.Region, }); - + void loadConvertibleRIs(); + void loadExchangeHistory(); + if (!sessionOpen) return; setResultText(resultContainer, 'Exchange completed. ID: ' + result.exchange_id, 'success-message'); executeBtn.classList.add('hidden'); modalQuote = null; modalQuoteReq = null; - setTimeout(() => { + closeTimer = setTimeout(() => { closeModal(modal); - void loadConvertibleRIs(); - void loadExchangeHistory(); }, 2000); } catch (error) { + if (!sessionOpen) return; const err = error as Error; setResultText(resultContainer, 'Exchange failed: ' + err.message, 'error'); + } finally { executeBtn.disabled = false; + quoteBtn.disabled = false; + if (!confirmed) executeBtn.focus(); } } } @@ -2260,24 +2288,37 @@ function renderExchangeHistory(container: HTMLElement, records: RIExchangeHistor // Wire Approve button click handlers container.querySelectorAll('.riexchange-approve-btn[data-approve-id]').forEach(btn => { - btn.addEventListener('click', () => handleRIExchangeApproveClick(btn)); + const record = records.find(rec => rec.id === btn.dataset.approveId); + if (record) btn.addEventListener('click', () => handleRIExchangeApproveClick(btn, record)); }); } -async function handleRIExchangeApproveClick(btn: HTMLButtonElement): Promise { - const id = btn.dataset.approveId; - if (!id) return; +async function handleRIExchangeApproveClick(btn: HTMLButtonElement, record: RIExchangeHistoryRecord): Promise { + if (btn.disabled) return; + if (!exchangePaymentPattern.test(record.payment_due)) { + showToast({ kind: 'error', message: 'Cannot approve exchange without a previously quoted payment.' }); + return; + } + btn.disabled = true; const confirmed = await confirmDialog({ title: 'Approve RI Exchange', - body: 'Approve this pending RI exchange? The exchange will execute immediately.', + body: `Previously quoted payment: USD ${record.payment_due}. ` + + `Source RIs: ${record.source_ri_ids.join(', ')}. Region: ${record.region}. ` + + `Target: ${record.target_count} × ${record.target_offering_id} (${record.target_instance_type}). ` + + 'Approving executes immediately using a fresh quote, within the configured per-exchange ' + + 'and remaining daily spending limits. This exchange cannot be reversed.', confirmLabel: 'Approve', + destructive: true, }); - if (!confirmed) return; + if (!confirmed) { + btn.disabled = false; + btn.focus(); + return; + } - btn.disabled = true; try { - await api.approveRIExchange(id); + await api.approveRIExchange(record.id); showToast({ kind: 'success', message: 'RI exchange approved and executing.' }); } catch (err) { const msg = err instanceof Error ? err.message : String(err); diff --git a/frontend/tests-e2e/ri-exchange-confirmation.spec.ts b/frontend/tests-e2e/ri-exchange-confirmation.spec.ts new file mode 100644 index 00000000..8560d6b1 --- /dev/null +++ b/frontend/tests-e2e/ri-exchange-confirmation.spec.ts @@ -0,0 +1,294 @@ +import { expect, test, type Page } from '@playwright/test'; +import { mockApi, seedAuth } from './fixtures/recs'; + +const ri = '11111111-1111-1111-1111-111111111111'; +const otherRI = '55555555-5555-5555-5555-555555555555'; +const offering = '22222222-2222-2222-2222-222222222222'; +const secondOffering = '44444444-4444-4444-4444-444444444444'; +const quotedPayment = '123.456789012345678901'; +const browserErrors = new WeakMap(); + +test.afterEach(({ page }) => { expect(browserErrors.get(page)).toEqual([]); }); + +async function setup(page: Page) { + await seedAuth(page); + await mockApi(page); + const history = { + id: '33333333-3333-3333-3333-333333333333', account_id: 'acct-001', + source_ri_ids: [ri], source_instance_type: 'm5.large', source_count: 2, + target_offering_id: offering, target_instance_type: 'm5.xlarge', target_count: 1, + payment_due: '123.456789', status: 'pending', mode: 'manual', region: 'us-east-1', + created_at: new Date().toISOString(), updated_at: new Date().toISOString(), + }; + const quote = { + IsValidExchange: true, CurrencyCode: 'USD', PaymentDueRaw: quotedPayment, + Region: 'us-east-1', SourceHourlyPriceRaw: '0.10', TargetHourlyPriceRaw: '0.08', + }; + const requests: { path: string; body: unknown }[] = []; + let exchanged = false; + const control = { quoteDelay: Promise.resolve(), executeDelay: Promise.resolve(), failQuote: false, failExecute: false, failApprove: false, quoteCalls: 0 }; + const errors: string[] = []; + browserErrors.set(page, errors); + page.on('pageerror', error => errors.push(error.message)); + page.on('requestfailed', request => errors.push(`${request.method()} ${request.url()}: ${request.failure()?.errorText}`)); + page.on('console', message => { + if (message.type() === 'error' && !message.text().includes('the server responded with a status of 500')) errors.push(message.text()); + }); + page.on('response', response => { + if (response.status() < 400) return; + const path = new URL(response.url()).pathname; + const expected = response.status() === 500 && ( + (path === '/api/ri-exchange/quote' && control.failQuote) + || (path === '/api/ri-exchange/execute' && control.failExecute) + || (path === `/api/ri-exchange/approve/${history.id}` && control.failApprove) + ); + if (!expected) errors.push(`${response.status()} ${path}`); + }); + await page.route('**/api/auth/me/permissions', route => route.fulfill({ json: { + permissions: [{ action: 'admin', resource: '*' }, { action: 'execute', resource: 'ri-exchange' }], + } })); + await page.route('**/api/ri-exchange/**', async route => { + const path = new URL(route.request().url()).pathname; + const end = path.split('/').pop()!; + if (route.request().method() === 'POST') { + if (end === 'quote') { + control.quoteCalls++; + await control.quoteDelay; + await route.fulfill({ status: control.failQuote ? 500 : 200, json: control.failQuote ? { error: 'quote refused' } : quote }); + return; + } + requests.push({ path, body: route.request().postDataJSON() }); + const failed = end === 'execute' ? control.failExecute : control.failApprove; + if (end === 'execute') await control.executeDelay; + if (!failed) { + history.status = 'completed'; + exchanged = true; + } + await route.fulfill({ status: failed ? 500 : 200, json: failed ? { error: 'exchange refused' } : { exchange_id: 'synthetic-exchange', quote } }); + return; + } + const responses: Record = { + instances: { instances: [ri, otherRI].filter(id => id !== ri || !exchanged).map(id => ({ reserved_instance_id: id, instance_type: 'm5.large', instance_count: 2, + region: 'us-east-1', state: 'active', offering_class: 'convertible', offering_type: 'No Upfront', + product_description: 'Linux/UNIX', start: '2026-01-01T00:00:00Z', end: '2027-01-01T00:00:00Z' })) }, + utilization: { utilization: [] }, 'reshape-recommendations': { recommendations: [] }, history: { records: [history] }, + 'target-offerings': { offerings: [offering, secondOffering].map(offering_id => ({ offering_id, instance_type: 'm5.xlarge', offering_type: 'No Upfront' })) }, + }; + await route.fulfill({ json: responses[end] ?? {} }); + }); + await page.goto('/inventory/ri-exchange'); + return { history, quote, requests, control, errors }; +} + +async function openQuote(page: Page) { + await page.locator(`[data-action="quote-ri"][data-ri-id="${ri}"]`).click(); + await page.locator('.modal-exchange-target-select').selectOption(offering); + await page.getByRole('button', { name: 'Get Quote', exact: true }).click(); + await expect(page.locator('#modal-exchange-result')).toContainText('Valid Exchange'); +} + +const execute = (page: Page) => page.locator('#ri-exchange-modal').getByRole('button', { name: 'Execute Exchange', exact: true }); +const dialog = (page: Page) => page.locator('.modal-confirm-backdrop'); +const confirm = (page: Page) => dialog(page).locator('.btn-destructive'); + +for (const multiple of [false, true]) { + test(`Execute confirms exact ${multiple ? 'multiple' : 'single'} targets and raw payment`, async ({ page }) => { + const state = await setup(page); + await openQuote(page); + if (multiple) { + await page.getByRole('button', { name: '+ Add target', exact: true }).click(); + await expect(execute(page)).toBeHidden(); + await page.locator('.modal-exchange-target-select').nth(1).selectOption(secondOffering); + await page.locator('.modal-exchange-count').nth(1).fill('3'); + await page.getByRole('button', { name: 'Get Quote', exact: true }).click(); + } + await execute(page).click(); + await expect(dialog(page)).toContainText(`USD ${quotedPayment}`); + await expect(dialog(page)).toContainText(`2 × ${offering}`); + await expect(dialog(page)).toContainText(ri); + await expect(dialog(page)).toContainText('us-east-1'); + if (multiple) await expect(dialog(page)).toContainText(`3 × ${secondOffering}`); + expect(state.requests).toHaveLength(0); + await expect(confirm(page)).toBeFocused(); + await page.keyboard.press('Tab'); + await expect(dialog(page).getByRole('button', { name: 'Cancel', exact: true })).toBeFocused(); + await confirm(page).click(); + await expect(page.locator('#modal-exchange-result')).toContainText('Exchange completed. ID: synthetic-exchange'); + expect(state.requests).toEqual([{ path: '/api/ri-exchange/execute', body: { + ri_ids: [ri], ...(multiple ? { targets: [{ offering_id: offering, count: 2 }, { offering_id: secondOffering, count: 3 }] } : { target_offering_id: offering, target_count: 2 }), + max_payment_due_usd: quotedPayment, region: 'us-east-1', + } }]); + expect(state.errors).toEqual([]); + }); +} + +for (const action of ['Execute', 'Approve']) { + for (const dismissal of ['Cancel', 'Escape', 'Close', 'backdrop']) { + test(`${action} ${dismissal} refuses mutation and permits a later confirmation`, async ({ page }) => { + const state = await setup(page); + if (action === 'Execute') await openQuote(page); + const trigger = action === 'Execute' ? execute(page) : page.locator('.riexchange-approve-btn'); + await trigger.click(); + await expect(dialog(page)).toHaveCount(1); + if (action === 'Approve') { + await expect(dialog(page)).toContainText('Previously quoted payment: USD 123.456789'); + await expect(dialog(page)).toContainText('fresh quote'); + await expect(dialog(page)).toContainText(`1 × ${offering}`); + } + if (dismissal === 'Escape') await page.keyboard.press('Escape'); + else if (dismissal === 'backdrop') await dialog(page).click({ position: { x: 2, y: 2 } }); + else await dialog(page).getByRole('button', { name: dismissal, exact: true }).click(); + await expect(dialog(page)).toHaveCount(0); + expect(state.requests).toHaveLength(0); + await expect(trigger).toBeEnabled(); + await expect(trigger).toBeFocused(); + await trigger.click(); + await page.keyboard.press('Enter'); + await expect.poll(() => state.requests.length).toBe(1); + if (action === 'Approve') { + expect(state.requests[0]).toEqual({ path: `/api/ri-exchange/approve/${state.history.id}`, body: null }); + await expect(page.locator('#ri-exchange-history-list')).toContainText('completed'); + } + expect(state.errors).toEqual([]); + }); + } + test(`${action} failure requires a fresh confirmation before retry`, async ({ page }) => { + const state = await setup(page); + if (action === 'Execute') await openQuote(page); + state.control.failExecute = state.control.failApprove = true; + const trigger = action === 'Execute' ? execute(page) : page.locator('.riexchange-approve-btn'); + await trigger.click(); + await confirm(page).click(); + await expect(trigger).toBeEnabled(); + await expect(page.getByText(action === 'Execute' ? /Exchange failed:/ : /Failed to approve exchange:/)).toBeVisible(); + state.control.failExecute = state.control.failApprove = false; + await trigger.click(); + expect(state.requests).toHaveLength(1); + await confirm(page).click(); + await expect.poll(() => state.requests.length).toBe(2); + expect(state.errors).toEqual([]); + }); +} + +for (const edit of ['count', 'revert', 'offering', 'add', 'remove']) { + test(`late quote cannot restore authorization after ${edit}`, async ({ page }) => { + const state = await setup(page); + await openQuote(page); + if (edit === 'remove') await page.getByRole('button', { name: '+ Add target', exact: true }).click(); + if (edit === 'remove') await page.locator('.modal-exchange-target-select').nth(1).selectOption(secondOffering); + let release!: () => void; + state.control.quoteDelay = new Promise(resolve => { release = resolve; }); + await page.getByRole('button', { name: 'Get Quote', exact: true }).click(); + await expect.poll(() => state.control.quoteCalls).toBe(2); + if (edit === 'count' || edit === 'revert') await page.locator('.modal-exchange-count').first().fill('3'); + if (edit === 'revert') await page.locator('.modal-exchange-count').first().fill('2'); + if (edit === 'offering') await page.locator('.modal-exchange-target-select').first().selectOption(secondOffering); + if (edit === 'add') await page.getByRole('button', { name: '+ Add target', exact: true }).click(); + if (edit === 'remove') await page.getByRole('button', { name: 'Remove target' }).last().click(); + release(); + await expect(page.getByRole('button', { name: 'Get Quote', exact: true })).toBeEnabled(); + await expect(execute(page)).toBeHidden(); + expect(state.requests).toHaveLength(0); + if (edit === 'add') await page.locator('.modal-exchange-target-select').nth(1).selectOption(secondOffering); + await page.getByRole('button', { name: 'Get Quote', exact: true }).click(); + await execute(page).click(); + await confirm(page).click(); + await expect.poll(() => state.requests.length).toBe(1); + }); +} + +for (const reopenBeforeResponse of [false, true]) { + test(`old execution refreshes inventory and history without closing a reopened modal (pending response: ${reopenBeforeResponse})`, async ({ page }) => { + const state = await setup(page); + await openQuote(page); + let release!: () => void; + state.control.executeDelay = new Promise(resolve => { release = resolve; }); + await execute(page).click(); + await confirm(page).dblclick(); + await expect.poll(() => state.requests.length).toBe(1); + await expect(execute(page)).toBeDisabled(); + if (!reopenBeforeResponse) { + release(); + await expect(page.locator('#modal-exchange-result')).toContainText('Exchange completed'); + } + await page.locator('#ri-exchange-modal').getByRole('button', { name: 'Cancel', exact: true }).click(); + await page.locator(`[data-action="quote-ri"][data-ri-id="${otherRI}"]`).click(); + if (reopenBeforeResponse) release(); + await expect(page.locator('#ri-exchange-history-list')).toContainText('completed'); + await expect(page.locator(`[data-action="quote-ri"][data-ri-id="${ri}"]`)).toHaveCount(0); + await page.waitForTimeout(2200); + await expect(page.locator('#ri-exchange-modal')).toBeVisible(); + expect(state.requests).toHaveLength(1); + expect(state.errors).toEqual([]); + }); +} + +test('quote errors and close during quote leave a new session usable', async ({ page }) => { + const state = await setup(page); + state.control.failQuote = true; + await page.locator(`[data-action="quote-ri"][data-ri-id="${ri}"]`).click(); + await page.locator('.modal-exchange-target-select').selectOption(offering); + await page.getByRole('button', { name: 'Get Quote', exact: true }).click(); + await expect(page.locator('#modal-exchange-result')).toContainText('Quote failed:'); + state.control.failQuote = false; + let release!: () => void; + state.control.quoteDelay = new Promise(resolve => { release = resolve; }); + await page.getByRole('button', { name: 'Get Quote', exact: true }).click(); + await expect.poll(() => state.control.quoteCalls).toBe(2); + await page.locator('#ri-exchange-modal').getByRole('button', { name: 'Cancel', exact: true }).click(); + await page.locator(`[data-action="quote-ri"][data-ri-id="${ri}"]`).click(); + release(); + await expect(execute(page)).toBeHidden(); + await page.locator('.modal-exchange-target-select').selectOption(offering); + await page.getByRole('button', { name: 'Get Quote', exact: true }).click(); + await execute(page).click(); + await confirm(page).click(); + await expect.poll(() => state.requests.length).toBe(1); +}); + +test('zero remains explicit and absent money or currency cannot authorize', async ({ page }) => { + const state = await setup(page); + state.quote.PaymentDueRaw = ''; + await openQuote(page); + await execute(page).click(); + await expect(page.locator('#modal-exchange-result')).toContainText('Cannot confirm'); + state.quote.PaymentDueRaw = 'NaN'; + await page.getByRole('button', { name: 'Get Quote', exact: true }).click(); + await execute(page).click(); + await expect(page.locator('#modal-exchange-result')).toContainText('Cannot confirm'); + state.quote.PaymentDueRaw = '0'; + state.quote.CurrencyCode = ''; + await page.getByRole('button', { name: 'Get Quote', exact: true }).click(); + await execute(page).click(); + await expect(page.locator('#modal-exchange-result')).toContainText('Cannot confirm'); + expect(state.requests).toHaveLength(0); + state.quote.CurrencyCode = 'USD'; + await page.getByRole('button', { name: 'Get Quote', exact: true }).click(); + await execute(page).click(); + await expect(dialog(page)).toContainText('USD 0'); + await confirm(page).click(); + await expect.poll(() => state.requests.length).toBe(1); +}); + +test('approval handles missing and zero record amounts without inventing money', async ({ page }) => { + const state = await setup(page); + state.history.payment_due = ''; + await page.locator('#ri-exchange-refresh-btn').click(); + await page.locator('.riexchange-approve-btn').click(); + await expect(page.getByText('Cannot approve exchange without a previously quoted payment.')).toBeVisible(); + expect(state.requests).toHaveLength(0); + state.history.payment_due = 'NaN'; + await page.locator('#ri-exchange-refresh-btn').click(); + await page.locator('.riexchange-approve-btn').click(); + await expect(dialog(page)).toHaveCount(0); + expect(state.requests).toHaveLength(0); + state.history.payment_due = '0.000000'; + state.history.target_instance_type = ''; + await page.locator('#ri-exchange-refresh-btn').click(); + await page.locator('.riexchange-approve-btn').click(); + await expect(dialog(page)).toContainText('Previously quoted payment: USD 0.000000'); + await expect(dialog(page)).toContainText(state.history.target_instance_type); + await expect(dialog(page).locator('img')).toHaveCount(0); + await confirm(page).click(); + await expect.poll(() => state.requests.length).toBe(1); +});