From 0a3e8d7c004e0c2f4ba1cfe48f91ce715d0c6e88 Mon Sep 17 00:00:00 2001 From: Astrid Gealer Date: Sat, 24 Oct 2026 02:12:45 +0000 Subject: [PATCH 1/6] feat(frontend): add the Azure RI exchange API client and request helpers The backend already serves POST /api/ri-exchange/azure-instances/ compatible-offerings and /exchange, but the frontend had no client for them. Add typed request/response shapes that mirror internal/api/handler_ri_exchange.go (nullable amounts stay nullable), the two API calls, and pure helpers the exchange UI builds on: the billing subscription a set of source reservations must be exchanged under, form and guardrail validation matching the backend's checks, and user-facing text for the 422 guardrail refusals and other failures. Refs #81 Co-Authored-By: Claude Opus 5.5 --- frontend/src/__tests__/azure-exchange.test.ts | 112 ++++++++++++++ frontend/src/api/index.ts | 10 ++ frontend/src/api/riexchange.ts | 28 ++++ frontend/src/api/types.ts | 62 ++++++++ frontend/src/lib/azure-exchange.ts | 138 ++++++++++++++++++ 5 files changed, 350 insertions(+) create mode 100644 frontend/src/__tests__/azure-exchange.test.ts create mode 100644 frontend/src/lib/azure-exchange.ts diff --git a/frontend/src/__tests__/azure-exchange.test.ts b/frontend/src/__tests__/azure-exchange.test.ts new file mode 100644 index 000000000..ddb7edf9f --- /dev/null +++ b/frontend/src/__tests__/azure-exchange.test.ts @@ -0,0 +1,112 @@ +/** + * Pure helpers for the Azure RI exchange flow (issue #81). + */ +import { + azureTermLabel, describeAzureExchangeError, formatAzureAmount, resolveExchangeSubscription, + subscriptionFromBillingScope, validateAzureExchangeForm, validateAzureGuardrails, +} from '../lib/azure-exchange'; +import type { ExchangeableAzureRI } from '../api/types'; + +const SUB = '11111111-2222-3333-4444-555555555555'; +const ri = (id: string, scope?: string): ExchangeableAzureRI => ({ + reservation_order_id: `o-${id}`, reservation_id: id, sku: 'Standard_D2s_v3', quantity: 2, instance_flexibility: 'On', billing_scope_id: scope, +}); + +describe('subscriptionFromBillingScope', () => { + it.each([ + [`/subscriptions/${SUB}`, SUB], + [`/SUBSCRIPTIONS/${SUB}/`, SUB], + [` /subscriptions/${SUB} `, SUB], + [`/subscriptions/${SUB}/resourceGroups/rg`, null], + ['/providers/Microsoft.Billing/billingAccounts/1', null], + ['', null], + [undefined, null], + ])('%p -> %p', (scope, expected) => { + expect(subscriptionFromBillingScope(scope)).toBe(expected); + }); +}); + +describe('resolveExchangeSubscription', () => { + it('returns the shared billing subscription', () => { + expect(resolveExchangeSubscription([ri('a', `/subscriptions/${SUB}`), ri('b', `/subscriptions/${SUB.toUpperCase()}`)])) + .toEqual({ subscriptionId: SUB }); + }); + it('refuses reservations billed to different subscriptions', () => { + const r = resolveExchangeSubscription([ri('a', `/subscriptions/${SUB}`), ri('b', '/subscriptions/other')]); + expect('error' in r && r.error).toMatch(/different subscriptions/); + }); + it('refuses a reservation without a reported billing scope', () => { + const r = resolveExchangeSubscription([ri('a')]); + expect('error' in r && r.error).toMatch(/did not report which subscription pays for a/); + }); + it('refuses an empty or oversized selection', () => { + expect('error' in resolveExchangeSubscription([])).toBe(true); + const many = Array.from({ length: 51 }, (_, i) => ri(String(i), `/subscriptions/${SUB}`)); + expect('error' in resolveExchangeSubscription(many)).toBe(true); + }); +}); + +describe('validateAzureExchangeForm', () => { + const target = { sku: 'Standard_D4s_v5', location: 'eastus', term: 'P3Y', quantity: 1 }; + const source = { reservation_id: 'a', quantity: 2 }; + it('accepts a complete form', () => { + expect(validateAzureExchangeForm([source], [2], [target])).toBeNull(); + }); + it.each([ + [[{ ...source, quantity: 3 }], [target], /Source 1: quantity must be a whole number from 1 to 2/], + [[{ ...source, quantity: 0 }], [target], /Source 1/], + [[{ ...source, quantity: 1.5 }], [target], /Source 1/], + [[source], [], /at least one target/], + [[source], [{ ...target, sku: ' ' }], /Target 1: enter a VM size/], + [[source], [{ ...target, location: '' }], /Target 1: enter a region/], + [[source], [{ ...target, term: 'P2Y' }], /Target 1: pick a term/], + [[source], [target, { ...target, quantity: 0 }], /Target 2: quantity/], + ])('rejects %#', (sources, targets, msg) => { + expect(validateAzureExchangeForm(sources, [2], targets)).toMatch(msg); + }); +}); + +describe('validateAzureGuardrails', () => { + it.each([['1250.00', 'USD'], ['0', 'eur'], [' 10 ', ' GBP ']])('accepts %p %p', (max, cur) => { + expect(validateAzureGuardrails(max, cur)).toBeNull(); + }); + it.each([['', 'USD'], ['-5', 'USD'], ['1/3', 'USD'], ['1e3', 'USD'], ['10', ''], ['10', 'US'], ['10', 'US$']])('rejects %p %p', (max, cur) => { + expect(validateAzureGuardrails(max, cur)).not.toBeNull(); + }); +}); + +describe('formatting', () => { + it('never shows a missing amount as zero', () => { + expect(formatAzureAmount(null, 'USD')).toBe('not reported'); + expect(formatAzureAmount(undefined, 'USD')).toBe('not reported'); + expect(formatAzureAmount(0, 'USD')).toBe('USD 0.00'); + expect(formatAzureAmount(-12.5, 'EUR')).toBe('EUR -12.50'); + }); + it('labels terms', () => { + expect(azureTermLabel('P3Y')).toBe('3 years'); + expect(azureTermLabel('P7Y')).toBe('P7Y'); + expect(azureTermLabel(undefined)).toBe('—'); + expect(azureTermLabel('toString')).toBe('toString'); + }); +}); + +describe('describeAzureExchangeError', () => { + const e = (status: number, message: string) => Object.assign(new Error(message), { status }); + it.each([ + ['quoted net payable 120.00 USD exceeds max_payment_due 100.00 USD', /safety check: quoted net payable .* Raise the maximum payment/], + ['quoted currency "EUR" does not match requested currency "USD"', /Set the currency to the one Azure quoted/], + ['Azure did not return a net payable amount; refusing to execute', /Azure gave no price/], + ['Azure rejected this exchange: expired reservation', /meet Azure's exchange policy/], + ])('explains the 422 guardrail %p', (msg, expected) => { + expect(describeAzureExchangeError(e(422, msg), 'execute')).toMatch(expected); + }); + it('explains permission, scope and request errors', () => { + expect(describeAzureExchangeError(e(403, 'direct RI exchange execution is disabled while 4-eyes approval mode is on; use the approval flow'), 'execute')) + .toBe('Exchange not executed: you are not allowed to do this. direct RI exchange execution is disabled while 4-eyes approval mode is on; use the approval flow'); + expect(describeAzureExchangeError(e(404, 'not found'), 'preview')).toMatch(/^Could not preview the exchange: no Azure account/); + expect(describeAzureExchangeError(e(400, 'targets[0].sku is required'), 'preview')).toMatch(/rejected\. targets\[0\]\.sku is required/); + expect(describeAzureExchangeError(e(500, 'exchange execution failed and may already have been submitted to Azure'), 'execute')) + .toMatch(/may already have been submitted/); + expect(describeAzureExchangeError(undefined, 'preview')).toBe('Could not preview the exchange: Unknown error'); + }); +}); diff --git a/frontend/src/api/index.ts b/frontend/src/api/index.ts index 562b6f5ef..8e83fbbcc 100644 --- a/frontend/src/api/index.ts +++ b/frontend/src/api/index.ts @@ -50,6 +50,14 @@ export type { SavingsAnalyticsFilters, ConvertibleRI, ExchangeableAzureRI, + AzureExchangeSource, + AzureExchangeTarget, + AzureCompatibleOfferingsRequest, + AzureExecuteExchangeRequest, + AzureCompatibleOffering, + AzureExchangePreview, + AzureCompatibleOfferingsResponse, + AzureExecuteExchangeResponse, RIUtilization, ReshapeRecommendation, OfferingOption, @@ -197,6 +205,8 @@ export type { ReshapeRecommendationsResponse, ExcludedRI } from './riexchange'; export { listConvertibleRIs, listExchangeableAzureRIs, + getAzureCompatibleOfferings, + executeAzureExchange, getRIUtilization, getReshapeRecommendations, getExchangeQuote, diff --git a/frontend/src/api/riexchange.ts b/frontend/src/api/riexchange.ts index b7ee73d11..d6765c346 100644 --- a/frontend/src/api/riexchange.ts +++ b/frontend/src/api/riexchange.ts @@ -6,6 +6,10 @@ import { apiRequest } from './client'; import type { ConvertibleRI, ExchangeableAzureRI, + AzureCompatibleOfferingsRequest, + AzureCompatibleOfferingsResponse, + AzureExecuteExchangeRequest, + AzureExecuteExchangeResponse, RIUtilization, ReshapeRecommendation, ExchangeQuoteRequest, @@ -43,6 +47,30 @@ export async function listExchangeableAzureRIs(subscriptionID?: string): Promise return resp.reservations ?? []; } +/** + * Price a proposed Azure RI exchange without committing anything (issue #81): + * returns the compatible offerings plus the net payable / refunds / purchases + * preview and any Azure policy errors. + */ +export async function getAzureCompatibleOfferings(req: AzureCompatibleOfferingsRequest): Promise { + return apiRequest('/ri-exchange/azure-instances/compatible-offerings', { + method: 'POST', + body: JSON.stringify(req), + }); +} + +/** + * Execute an Azure RI exchange (issue #81). The backend re-quotes and refuses + * with a 422 when the fresh quote has policy errors, no net payable, another + * currency, or a net payable above max_payment_due. + */ +export async function executeAzureExchange(req: AzureExecuteExchangeRequest): Promise { + return apiRequest('/ri-exchange/azure-instances/exchange', { + method: 'POST', + body: JSON.stringify(req), + }); +} + /** * Get per-RI utilization from Cost Explorer */ diff --git a/frontend/src/api/types.ts b/frontend/src/api/types.ts index f3ad8bc6f..1af97e71a 100644 --- a/frontend/src/api/types.ts +++ b/frontend/src/api/types.ts @@ -736,6 +736,68 @@ export interface ExchangeableAzureRI { expiry_date?: string; instance_flexibility: string; display_name?: string; + /** ARM scope billed for this reservation, e.g. "/subscriptions/{id}" (issue #81). */ + billing_scope_id?: string; +} + +// Azure RI exchange requests (issue #81). Mirror AzureExchangeSourceBody / +// AzureExchangeTargetBody in internal/api/handler_ri_exchange.go. term is +// the ISO 8601 reservation term ("P1Y", "P3Y", "P5Y"). +export interface AzureExchangeSource { + reservation_id: string; + quantity: number; +} + +export interface AzureExchangeTarget { + sku: string; + location: string; + term: string; + quantity: number; +} + +export interface AzureCompatibleOfferingsRequest { + subscription_id: string; + sources: AzureExchangeSource[]; + targets: AzureExchangeTarget[]; +} + +/** Mandatory guardrails: the backend refuses a fresh quote above the cap or in another currency. */ +export interface AzureExecuteExchangeRequest extends AzureCompatibleOfferingsRequest { + max_payment_due: string; + currency: string; +} + +// Amounts are null when Azure did not report them; never treat that as 0. +export interface AzureCompatibleOffering { + sku: string; + location: string; + term: string; + quantity: number; + billing_currency_total: number | null; + currency_code?: string; +} + +export interface AzureExchangePreview { + session_id: string; + net_payable: number | null; + net_payable_currency?: string; + refunds_total: number | null; + purchases_total: number | null; + policy_errors?: string[]; +} + +export interface AzureCompatibleOfferingsResponse { + offerings: AzureCompatibleOffering[] | null; + preview: AzureExchangePreview | null; +} + +export interface AzureExecuteExchangeResponse { + session_id: string; + status: string; + net_payable: number | null; + net_payable_currency?: string; + refunds_total: number | null; + purchases_total: number | null; } export interface RIUtilization { diff --git a/frontend/src/lib/azure-exchange.ts b/frontend/src/lib/azure-exchange.ts new file mode 100644 index 000000000..ae192a21d --- /dev/null +++ b/frontend/src/lib/azure-exchange.ts @@ -0,0 +1,138 @@ +// Pure helpers for the Azure RI exchange flow (issue #81): request shaping, +// input validation and user-facing error text for +// POST /api/ri-exchange/azure-instances/{compatible-offerings,exchange}. + +import type { ExchangeableAzureRI } from '../api/types'; + +/** Terms the backend accepts (armreservations.PossibleReservationTermValues). */ +export const AZURE_EXCHANGE_TERMS: readonly string[] = ['P1Y', 'P3Y', 'P5Y']; + +const TERM_LABELS: Readonly> = { P1Y: '1 year', P3Y: '3 years', P5Y: '5 years' }; + +/** "P3Y" -> "3 years"; unknown terms are shown as Azure sent them. */ +export function azureTermLabel(term: string | undefined): string { + if (!term) return '—'; + return Object.prototype.hasOwnProperty.call(TERM_LABELS, term) ? TERM_LABELS[term]! : term; +} + +/** Backend cap on sources and on targets per request (maxAzureExchangeItems). */ +export const MAX_AZURE_EXCHANGE_ITEMS = 50; + +/** A non-negative plain decimal; the backend parses max_payment_due as an exact rational. */ +export const AZURE_MAX_PAYMENT_PATTERN = /^\d+(?:\.\d+)?$/; + +/** ISO 4217 currency code. */ +export const AZURE_CURRENCY_PATTERN = /^[A-Za-z]{3}$/; + +/** The subscription GUID in a billing scope such as "/subscriptions/{id}". */ +export function subscriptionFromBillingScope(scope: string | undefined): string | null { + const m = /^\/subscriptions\/([^/\s]+)\/?$/i.exec((scope ?? '').trim()); + return m ? m[1]! : null; +} + +/** + * The subscription an exchange of these reservations must be requested + * under. The backend only lets a subscription hand back reservations it is + * billed for, so every source must report the same billing subscription. + */ +export function resolveExchangeSubscription(sources: readonly ExchangeableAzureRI[]): { subscriptionId: string } | { error: string } { + if (sources.length === 0) return { error: 'Select at least one reservation to exchange.' }; + if (sources.length > MAX_AZURE_EXCHANGE_ITEMS) { + return { error: `Select at most ${MAX_AZURE_EXCHANGE_ITEMS} reservations per exchange.` }; + } + let subscriptionId: string | null = null; + for (const r of sources) { + const sub = subscriptionFromBillingScope(r.billing_scope_id); + if (!sub) { + return { error: `Azure did not report which subscription pays for ${r.display_name || r.reservation_id}, so it cannot be exchanged here.` }; + } + if (subscriptionId !== null && sub.toLowerCase() !== subscriptionId.toLowerCase()) { + return { error: 'The selected reservations are billed to different subscriptions. Exchange them separately, one subscription at a time.' }; + } + subscriptionId ??= sub; + } + return { subscriptionId: subscriptionId! }; +} + +export interface AzureExchangeSourceInput { + reservation_id: string; + quantity: number; +} + +export interface AzureExchangeTargetInput { + sku: string; + location: string; + term: string; + quantity: number; +} + +/** Validates the form values the way the backend does, so the user sees the problem before a round trip. */ +export function validateAzureExchangeForm(sources: readonly AzureExchangeSourceInput[], sourceMax: readonly number[], targets: readonly AzureExchangeTargetInput[]): string | null { + for (let i = 0; i < sources.length; i++) { + const q = sources[i]!.quantity; + if (!Number.isInteger(q) || q < 1 || q > (sourceMax[i] ?? q)) { + return `Source ${i + 1}: quantity must be a whole number from 1 to ${sourceMax[i]}.`; + } + } + if (targets.length === 0) return 'Add at least one target.'; + if (targets.length > MAX_AZURE_EXCHANGE_ITEMS) return `Add at most ${MAX_AZURE_EXCHANGE_ITEMS} targets.`; + for (let i = 0; i < targets.length; i++) { + const t = targets[i]!; + if (!t.sku.trim()) return `Target ${i + 1}: enter a VM size (SKU).`; + if (!t.location.trim()) return `Target ${i + 1}: enter a region.`; + if (!AZURE_EXCHANGE_TERMS.includes(t.term)) return `Target ${i + 1}: pick a term.`; + if (!Number.isInteger(t.quantity) || t.quantity < 1) return `Target ${i + 1}: quantity must be a whole number of at least 1.`; + } + return null; +} + +/** Validates the mandatory execute guardrails. */ +export function validateAzureGuardrails(maxPaymentDue: string, currency: string): string | null { + if (!AZURE_MAX_PAYMENT_PATTERN.test(maxPaymentDue.trim())) { + return 'Enter the maximum you are willing to pay as a plain number, for example 1250.00.'; + } + if (!AZURE_CURRENCY_PATTERN.test(currency.trim())) return 'Enter the three-letter currency code, for example USD.'; + return null; +} + +/** Formats a nullable amount; a missing amount is shown as such, never as 0. */ +export function formatAzureAmount(amount: number | null | undefined, currency: string | undefined): string { + if (amount === null || amount === undefined || !Number.isFinite(amount)) return 'not reported'; + const code = currency ? `${currency} ` : ''; + return `${code}${amount.toFixed(2)}`; +} + +interface ApiErrorLike { + message?: string; + status?: number; +} + +// Hints for the 422 guardrail refusals checkAzureExchangeMoneyGuardrails +// returns, keyed on its fixed message text. +const GUARDRAIL_HINTS: ReadonlyArray<[RegExp, string]> = [ + [/exceeds max_payment_due/, 'Raise the maximum payment or change the targets, then preview again.'], + [/does not match requested currency/, 'Set the currency to the one Azure quoted.'], + [/did not return a net payable/, 'Azure gave no price for this exchange. Try again later.'], + [/^Azure rejected this exchange/, 'Change the sources or targets so they meet Azure\'s exchange policy.'], +]; + +/** User-facing text for a failed preview or execute call. */ +export function describeAzureExchangeError(error: unknown, phase: 'preview' | 'execute'): string { + const err = (error ?? {}) as ApiErrorLike; + const msg = err.message || 'Unknown error'; + const what = phase === 'preview' ? 'Could not preview the exchange' : 'Exchange not executed'; + switch (err.status) { + case 422: { + const hint = GUARDRAIL_HINTS.find(([re]) => re.test(msg))?.[1]; + return `Exchange blocked by a safety check: ${msg}.${hint ? ` ${hint}` : ''}`; + } + case 403: + return `${what}: you are not allowed to do this. ${msg}`; + case 404: + return `${what}: no Azure account you can access is registered for this subscription.`; + case 400: + return `${what}: the request was rejected. ${msg}`; + default: + return `${what}: ${msg}`; + } +} From 8d25988df91f69561d7e65922c1772974add9594 Mon Sep 17 00:00:00 2001 From: Astrid Gealer Date: Sat, 24 Oct 2026 02:12:45 +0000 Subject: [PATCH 2/6] feat(frontend): add the Azure RI exchange preview and execute modal The modal takes the selected source reservations, lets the user set how many of each to hand back and which SKU/region/term/quantity to buy, and previews the exchange through compatible-offerings: net payable, refunds and purchases totals, the compatible offerings and any Azure policy errors. Execute is offered only for a preview with no policy errors and a reported net payable, only to holders of execute:ri-exchange, and only after the user types a maximum payment due and currency (never prefilled from the quote), matching the backend's mandatory guardrail. Any change to sources or targets discards the preview. Execution goes through a destructive confirm and its outcome, including 422 guardrail refusals, is shown below the preview. Refs #81 Co-Authored-By: Claude Opus 5.5 --- frontend/src/riexchange-azure-modal.ts | 355 +++++++++++++++++++++++++ 1 file changed, 355 insertions(+) create mode 100644 frontend/src/riexchange-azure-modal.ts diff --git a/frontend/src/riexchange-azure-modal.ts b/frontend/src/riexchange-azure-modal.ts new file mode 100644 index 000000000..b385e704b --- /dev/null +++ b/frontend/src/riexchange-azure-modal.ts @@ -0,0 +1,355 @@ +/** + * Azure RI exchange modal (issue #81), loaded on demand from + * riexchange-azure.ts so it stays out of the main bundle until used: pick quantities from the selected + * source reservations, describe the target SKU / region / term / quantity, + * preview the priced exchange (compatible offerings, net payable, refunds, + * purchases, policy errors), then execute behind an explicit + * max_payment_due + currency guardrail. + * + * Every Azure-derived string goes through textContent, never innerHTML. + */ + +import * as api from './api'; +import type { AzureCompatibleOfferingsResponse, AzureExecuteExchangeRequest, ExchangeableAzureRI } from './api'; +import { openModal, closeModal } from './modal'; +import { confirmDialog } from './confirmDialog'; +import { isRIExchanger } from './permissions'; +import { + AZURE_EXCHANGE_TERMS, MAX_AZURE_EXCHANGE_ITEMS, azureTermLabel, describeAzureExchangeError, formatAzureAmount, + resolveExchangeSubscription, validateAzureExchangeForm, validateAzureGuardrails, + type AzureExchangeSourceInput, type AzureExchangeTargetInput, +} from './lib/azure-exchange'; + +function el(tag: K, className?: string, text?: string): HTMLElementTagNameMap[K] { + const node = document.createElement(tag); + if (className) node.className = className; + if (text !== undefined) node.textContent = text; + return node; +} + +function labeled(label: string, control: HTMLElement): HTMLLabelElement { + const l = el('label', undefined, `${label} `); + l.appendChild(control); + return l; +} + +function numberInput(className: string, value: number, max?: number): HTMLInputElement { + const input = el('input', className); + input.type = 'number'; + input.min = '1'; + if (max !== undefined) input.max = String(max); + input.step = '1'; + input.value = String(value); + return input; +} + +function textInput(className: string, value: string, placeholder: string): HTMLInputElement { + const input = el('input', className); + input.type = 'text'; + input.value = value; + input.placeholder = placeholder; + return input; +} + +function setMessage(container: HTMLElement, message: string, cls: string): void { + container.textContent = ''; + container.appendChild(el('p', cls, message)); +} + +interface TargetRow { + row: HTMLElement; + sku: HTMLInputElement; + location: HTMLInputElement; + term: HTMLSelectElement; + quantity: HTMLInputElement; +} + +function renderPreview(container: HTMLElement, resp: AzureCompatibleOfferingsResponse): void { + container.textContent = ''; + const preview = resp.preview; + const policyErrors = preview?.policy_errors ?? []; + const blocked = !preview || policyErrors.length > 0 || preview.net_payable === null; + const box = el('div', `quote-summary azure-exchange-preview ${blocked ? 'quote-invalid' : 'quote-valid'}`); + box.appendChild(el('h4', undefined, blocked ? 'Azure will not accept this exchange as it is' : 'Exchange preview')); + if (!preview) { + box.appendChild(el('p', 'error', 'Azure returned no preview for this exchange.')); + } else { + const currency = preview.net_payable_currency; + const details = el('div', 'quote-details'); + for (const [label, value] of [ + ['Net payable', formatAzureAmount(preview.net_payable, currency)], + ['Refunds total', formatAzureAmount(preview.refunds_total, currency)], + ['Purchases total', formatAzureAmount(preview.purchases_total, currency)], + ] as const) { + const row = el('div', 'quote-row'); + row.append(el('span', undefined, `${label}:`), el('strong', undefined, value)); + details.appendChild(row); + } + box.appendChild(details); + if (preview.net_payable !== null && preview.net_payable < 0) { + box.appendChild(el('p', 'help-text', 'A negative net payable is a refund to you.')); + } + if (preview.net_payable === null) { + box.appendChild(el('p', 'error', 'Azure did not report a net payable amount, so this exchange cannot be executed.')); + } + if (policyErrors.length > 0) { + box.appendChild(el('strong', undefined, 'Azure policy errors:')); + const ul = el('ul', 'azure-exchange-policy-errors'); + for (const msg of policyErrors) ul.appendChild(el('li', undefined, msg)); + box.appendChild(ul); + } + } + const offerings = resp.offerings ?? []; + if (offerings.length > 0) { + const table = el('table', 'azure-exchange-offerings'); + const head = table.createTHead().insertRow(); + for (const h of ['SKU', 'Region', 'Term', 'Quantity', 'Total']) head.appendChild(el('th', undefined, h)); + const body = table.createTBody(); + for (const o of offerings) { + const tr = body.insertRow(); + for (const v of [o.sku, o.location, azureTermLabel(o.term), String(o.quantity), formatAzureAmount(o.billing_currency_total, o.currency_code)]) { + tr.appendChild(el('td', undefined, v)); + } + } + box.appendChild(el('h5', undefined, 'Compatible offerings')); + box.appendChild(table); + } + container.appendChild(box); +} + +/** + * Opens the exchange modal for the selected reservations. onExecuted runs + * after a successful exchange so the caller can reload the list. + */ +export function openAzureExchangeModal(sources: readonly ExchangeableAzureRI[], onExecuted: () => void): void { + const modal = document.getElementById('ri-exchange-modal'); + const content = modal?.querySelector('.modal-content'); + if (!modal || !content) return; + + let sessionOpen = true; + let revision = 0; + let lastPreview: AzureCompatibleOfferingsResponse | null = null; + let lastRequest: { subscription_id: string; sources: AzureExchangeSourceInput[]; targets: AzureExchangeTargetInput[] } | null = null; + + // A fresh root per open: the modal element is shared with the AWS flow, so + // listeners must not be attached to the long-lived .modal-content. + content.textContent = ''; + const root = el('div', 'azure-exchange'); + content.appendChild(root); + root.appendChild(el('h3', undefined, 'Azure Reservation Exchange')); + const resultBox = el('div', 'azure-exchange-result'); + resultBox.id = 'azure-exchange-result'; + + const subscription = resolveExchangeSubscription(sources); + if ('error' in subscription) { + setMessage(resultBox, subscription.error, 'error'); + const close = el('button', 'btn', 'Close'); + close.type = 'button'; + close.addEventListener('click', () => closeModal(modal)); + const btns = el('div', 'modal-buttons'); + btns.appendChild(close); + root.append(resultBox, btns); + openModal(modal); + return; + } + const subscriptionId = subscription.subscriptionId; + root.appendChild(el('p', 'help-text', `Subscription ${subscriptionId}. Azure prices the exchange first; nothing changes until you execute it.`)); + + // Sources: how many of each selected reservation to hand back. + root.appendChild(el('h4', undefined, 'Return')); + const sourceQty: HTMLInputElement[] = []; + for (const r of sources) { + const row = el('div', 'setting-row azure-exchange-source-row'); + row.appendChild(el('span', undefined, `${r.display_name || r.reservation_id}: ${r.sku}, ${r.region || 'region not reported'}, ${azureTermLabel(r.term)}`)); + const qty = numberInput('azure-exchange-source-qty', r.quantity, r.quantity); + row.appendChild(labeled('Quantity', qty)); + row.appendChild(el('span', 'help-text', `of ${r.quantity}`)); + sourceQty.push(qty); + root.appendChild(row); + } + + // Targets: what to buy instead, seeded from the first source. + root.appendChild(el('h4', undefined, 'Buy instead')); + const targetsBox = el('div', 'azure-exchange-targets'); + root.appendChild(targetsBox); + const targetRows: TargetRow[] = []; + const first = sources[0]!; + const addTarget = (): void => { + if (targetRows.length >= MAX_AZURE_EXCHANGE_ITEMS) return; + const row = el('div', 'setting-row azure-exchange-target-row'); + const sku = textInput('azure-exchange-target-sku', first.sku, 'Standard_D4s_v5'); + const location = textInput('azure-exchange-target-location', first.region ?? '', 'eastus'); + const term = el('select', 'azure-exchange-target-term'); + for (const t of AZURE_EXCHANGE_TERMS) { + const opt = el('option', undefined, azureTermLabel(t)); + opt.value = t; + term.appendChild(opt); + } + term.value = first.term && AZURE_EXCHANGE_TERMS.includes(first.term) ? first.term : 'P1Y'; + const quantity = numberInput('azure-exchange-target-qty', 1); + const remove = el('button', 'btn exchange-remove-target', '×'); + remove.type = 'button'; + remove.setAttribute('aria-label', 'Remove target'); + const entry: TargetRow = { row, sku, location, term, quantity }; + remove.addEventListener('click', () => { + if (targetRows.length <= 1) return; + targetRows.splice(targetRows.indexOf(entry), 1); + row.remove(); + invalidate(); + }); + row.append(labeled('VM size', sku), labeled('Region', location), labeled('Term', term), labeled('Quantity', quantity), remove); + targetsBox.appendChild(row); + targetRows.push(entry); + }; + addTarget(); + const addBtn = el('button', 'btn', '+ Add target'); + addBtn.type = 'button'; + addBtn.addEventListener('click', () => { addTarget(); invalidate(); }); + const addRow = el('div', 'setting-row'); + addRow.appendChild(addBtn); + root.appendChild(addRow); + + root.appendChild(resultBox); + + // Guardrails: required before execute, never prefilled from the quote. + const guard = el('fieldset', 'azure-exchange-guardrails hidden'); + guard.appendChild(el('legend', undefined, 'Spending limit (required to execute)')); + const maxInput = textInput('azure-exchange-max-payment', '', 'e.g. 1250.00'); + maxInput.id = 'azure-exchange-max-payment'; + maxInput.inputMode = 'decimal'; + const currencyInput = textInput('azure-exchange-currency', '', 'e.g. USD'); + currencyInput.id = 'azure-exchange-currency'; + currencyInput.maxLength = 3; + guard.append( + el('p', 'help-text', 'Azure prices the exchange again right before executing it. It is refused if that price is above this limit or in another currency.'), + labeled('Maximum payment due', maxInput), + labeled('Currency', currencyInput), + ); + const canExecute = isRIExchanger(); + if (!canExecute) guard.appendChild(el('p', 'help-text', 'Executing an exchange needs the execute:ri-exchange permission (RI Exchanger group).')); + root.appendChild(guard); + // Execute progress and outcome go below the guardrails so the preview the + // user is acting on stays visible. + const statusBox = el('div', 'azure-exchange-status'); + statusBox.id = 'azure-exchange-status'; + root.appendChild(statusBox); + + const btns = el('div', 'modal-buttons'); + const previewBtn = el('button', 'btn primary', 'Preview exchange'); + previewBtn.type = 'button'; + const executeBtn = el('button', 'btn primary hidden', 'Execute exchange'); + executeBtn.type = 'button'; + const cancelBtn = el('button', 'btn', 'Cancel'); + cancelBtn.type = 'button'; + btns.append(previewBtn, executeBtn, cancelBtn); + root.appendChild(btns); + + function readForm(): { sources: AzureExchangeSourceInput[]; targets: AzureExchangeTargetInput[] } { + return { + sources: sources.map((r, i) => ({ reservation_id: r.reservation_id, quantity: Number(sourceQty[i]!.value) })), + targets: targetRows.map(t => ({ + sku: t.sku.value.trim(), location: t.location.value.trim(), term: t.term.value, quantity: Number(t.quantity.value), + })), + }; + } + + function invalidate(): void { + revision++; + lastPreview = null; + lastRequest = null; + executeBtn.classList.add('hidden'); + guard.classList.add('hidden'); + resultBox.textContent = ''; + statusBox.textContent = ''; + } + // Any change to sources or targets discards the preview, so Execute can + // never send something other than what was priced. The guardrail inputs + // don't affect the price and are excluded. + const onEdit = (e: Event): void => { if (!guard.contains(e.target as Node)) invalidate(); }; + root.addEventListener('input', onEdit); + root.addEventListener('change', onEdit); + + async function preview(): Promise { + if (previewBtn.disabled) return; + const form = readForm(); + const invalid = validateAzureExchangeForm(form.sources, sources.map(r => r.quantity), form.targets); + if (invalid) { + setMessage(resultBox, invalid, 'error'); + return; + } + invalidate(); + const rev = revision; + setMessage(resultBox, 'Asking Azure to price the exchange...', 'loading'); + previewBtn.disabled = true; + const request = { subscription_id: subscriptionId, ...form }; + try { + const resp = await api.getAzureCompatibleOfferings(request); + if (!sessionOpen || rev !== revision) return; + renderPreview(resultBox, resp); + const p = resp.preview; + if (p && p.net_payable !== null && (p.policy_errors ?? []).length === 0) { + lastPreview = resp; + lastRequest = request; + maxInput.placeholder = `quoted ${formatAzureAmount(p.net_payable, undefined)}`; + currencyInput.placeholder = p.net_payable_currency ? `quoted ${p.net_payable_currency}` : 'e.g. USD'; + guard.classList.remove('hidden'); + if (canExecute) executeBtn.classList.remove('hidden'); + } + } catch (error) { + if (!sessionOpen || rev !== revision) return; + setMessage(resultBox, describeAzureExchangeError(error, 'preview'), 'error'); + } finally { + previewBtn.disabled = false; + } + } + + async function execute(): Promise { + if (executeBtn.disabled || !lastPreview?.preview || !lastRequest) return; + const guardError = validateAzureGuardrails(maxInput.value, currencyInput.value); + const status = el('p', 'error azure-exchange-guard-error'); + guard.querySelector('.azure-exchange-guard-error')?.remove(); + if (guardError) { + status.textContent = guardError; + guard.appendChild(status); + return; + } + const req: AzureExecuteExchangeRequest = { + ...lastRequest, max_payment_due: maxInput.value.trim(), currency: currencyInput.value.trim().toUpperCase(), + }; + const quoted = lastPreview.preview; + const body = el('div'); + body.append( + el('p', undefined, `Return: ${req.sources.map((s, i) => `${s.quantity} × ${sources[i]!.sku} (${sources[i]!.display_name || s.reservation_id})`).join(', ')}`), + el('p', undefined, `Buy: ${req.targets.map(t => `${t.quantity} × ${t.sku} in ${t.location}, ${azureTermLabel(t.term)}`).join(', ')}`), + el('p', undefined, `Quoted net payable: ${formatAzureAmount(quoted.net_payable, quoted.net_payable_currency)}`), + el('p', undefined, `Refused if Azure's fresh price is above ${req.currency} ${req.max_payment_due}.`), + el('p', undefined, 'This exchange executes immediately and cannot be reversed.'), + ); + executeBtn.disabled = true; + previewBtn.disabled = true; + try { + const ok = await confirmDialog({ title: 'Execute Azure reservation exchange', body, confirmLabel: 'Execute exchange', destructive: true, initialFocus: 'cancel' }); + if (!ok || !sessionOpen) return; + setMessage(statusBox, 'Executing exchange...', 'loading'); + const result = await api.executeAzureExchange(req); + onExecuted(); + if (!sessionOpen) return; + setMessage(statusBox, `Exchange ${result.status || 'submitted'}. Net payable ${formatAzureAmount(result.net_payable, result.net_payable_currency)}; refunds ${formatAzureAmount(result.refunds_total, result.net_payable_currency)}; purchases ${formatAzureAmount(result.purchases_total, result.net_payable_currency)}. Session ${result.session_id}.`, 'success-message'); + executeBtn.classList.add('hidden'); + guard.classList.add('hidden'); + lastPreview = null; + lastRequest = null; + } catch (error) { + if (!sessionOpen) return; + setMessage(statusBox, describeAzureExchangeError(error, 'execute'), 'error'); + } finally { + executeBtn.disabled = false; + previewBtn.disabled = false; + } + } + + previewBtn.addEventListener('click', () => { void preview(); }); + executeBtn.addEventListener('click', () => { void execute(); }); + cancelBtn.addEventListener('click', () => closeModal(modal)); + openModal(modal, { onClose: () => { sessionOpen = false; revision++; } }); +} From ad57119dc39388be2d63507f3831804c19c5142a Mon Sep 17 00:00:00 2001 From: Astrid Gealer Date: Sat, 24 Oct 2026 02:12:45 +0000 Subject: [PATCH 3/6] feat(frontend): wire the Azure exchangeable-reservations list to the exchange modal The Azure reservations table was read-only. For users who can preview purchases it now has a selection column and an "Exchange selected" action that opens the exchange modal for the ticked reservations and reloads the tab after an exchange. The modal module is loaded on demand so the main bundle stays under webpack's 500 KiB size hint. Refs #81 Co-Authored-By: Claude Opus 5.5 --- .../src/__tests__/riexchange-azure.test.ts | 238 ++++++++++++++++++ frontend/src/__tests__/riexchange.test.ts | 16 ++ frontend/src/riexchange-azure.ts | 58 +++++ frontend/src/riexchange.ts | 19 +- frontend/src/styles/components.css | 58 +++++ 5 files changed, 383 insertions(+), 6 deletions(-) create mode 100644 frontend/src/__tests__/riexchange-azure.test.ts create mode 100644 frontend/src/riexchange-azure.ts diff --git a/frontend/src/__tests__/riexchange-azure.test.ts b/frontend/src/__tests__/riexchange-azure.test.ts new file mode 100644 index 000000000..e6fd2e789 --- /dev/null +++ b/frontend/src/__tests__/riexchange-azure.test.ts @@ -0,0 +1,238 @@ +/** + * Azure RI exchange modal and table action (issue #81). + */ +jest.mock('../api', () => ({ + getAzureCompatibleOfferings: jest.fn(), + executeAzureExchange: jest.fn(), +})); +jest.mock('../permissions', () => ({ canAccess: jest.fn(), isRIExchanger: jest.fn() })); +jest.mock('../confirmDialog', () => ({ confirmDialog: jest.fn() })); + +import * as api from '../api'; +import { canAccess, isRIExchanger } from '../permissions'; +import { confirmDialog } from '../confirmDialog'; +import { attachAzureExchangeToolbar, buildAzureSelectCell, canPreviewAzureExchange } from '../riexchange-azure'; +import { openAzureExchangeModal } from '../riexchange-azure-modal'; +import type { ExchangeableAzureRI } from '../api/types'; + +const SUB = '11111111-2222-3333-4444-555555555555'; +const reservations: ExchangeableAzureRI[] = [ + { reservation_order_id: 'o1', reservation_id: '/r/one', sku: 'Standard_D2s_v3', quantity: 4, region: 'eastus', term: 'P3Y', instance_flexibility: 'On', display_name: 'web', billing_scope_id: `/subscriptions/${SUB}` }, + { reservation_order_id: 'o2', reservation_id: '/r/two', sku: 'Standard_E2s_v3', quantity: 1, region: 'westeurope', term: 'P1Y', instance_flexibility: 'On', billing_scope_id: `/subscriptions/${SUB}` }, + { reservation_order_id: 'o3', reservation_id: '/r/three', sku: 'Standard_F2s_v2', quantity: 1, region: 'eastus', term: 'P1Y', instance_flexibility: 'On', billing_scope_id: '/subscriptions/other' }, +]; + +const okPreview = { + offerings: [{ sku: 'Standard_D4s_v5', location: 'eastus', term: 'P3Y', quantity: 2, billing_currency_total: 900.5, currency_code: 'USD' }], + preview: { session_id: 's1', net_payable: 120.25, net_payable_currency: 'USD', refunds_total: 780.25, purchases_total: 900.5 }, +}; + +const flush = () => new Promise(r => setTimeout(r, 0)); +const $ = (sel: string) => document.querySelector(sel) as T; +const resultText = () => $('#azure-exchange-result').textContent ?? ''; +const statusText = () => $('#azure-exchange-status').textContent ?? ''; + +function setValue(input: HTMLInputElement | HTMLSelectElement, value: string): void { + input.value = value; + input.dispatchEvent(new Event('input', { bubbles: true })); +} + +function grant(...perms: string[]): void { + (canAccess as jest.Mock).mockImplementation((action: string, resource: string) => perms.includes(`${action}:${resource}`)); + (isRIExchanger as jest.Mock).mockReturnValue(perms.includes('execute:ri-exchange')); +} + +async function openAndPreview(sources = [reservations[0]!]): Promise { + openAzureExchangeModal(sources, onExecuted); + setValue($('.azure-exchange-target-sku'), 'Standard_D4s_v5'); + setValue($('.azure-exchange-target-qty'), '2'); + $('.modal-buttons .btn.primary').click(); + await flush(); +} + +const onExecuted = jest.fn(); +const executeBtn = () => Array.from(document.querySelectorAll('.modal-buttons button')).find(b => b.textContent === 'Execute exchange')!; + +beforeEach(() => { + jest.clearAllMocks(); + document.body.innerHTML = ''; + grant('view:purchases', 'execute:ri-exchange'); + (api.getAzureCompatibleOfferings as jest.Mock).mockResolvedValue(okPreview); + (confirmDialog as jest.Mock).mockResolvedValue(true); +}); + +describe('Azure exchange table action', () => { + function mountTable(): HTMLTableElement { + const container = document.createElement('div'); + const table = document.createElement('table'); + const body = table.createTBody(); + reservations.forEach((r, i) => body.insertRow().appendChild(buildAzureSelectCell(r, i))); + container.appendChild(table); + document.body.appendChild(container); + attachAzureExchangeToolbar(container, table, reservations, onExecuted); + return table; + } + + it('is offered to users who can preview purchases', () => { + expect(canPreviewAzureExchange()).toBe(true); + grant(); + expect(canPreviewAzureExchange()).toBe(false); + }); + + it('enables "Exchange selected" once rows are ticked and opens the modal for them', async () => { + const table = mountTable(); + const btn = $('#azure-exchange-selected-btn'); + expect(btn.disabled).toBe(true); + const boxes = table.querySelectorAll('input.azure-ri-select'); + boxes[0]!.checked = true; + boxes[1]!.checked = true; + table.dispatchEvent(new Event('change')); + expect(btn.textContent).toBe('Exchange selected (2)'); + btn.click(); + // The modal module is loaded on demand. + for (let i = 0; i < 5 && $('#ri-exchange-modal').classList.contains('hidden'); i++) await flush(); + expect($('#ri-exchange-modal').classList.contains('hidden')).toBe(false); + expect(document.querySelectorAll('.azure-exchange-source-row')).toHaveLength(2); + expect($('.azure-exchange').textContent).toContain(`Subscription ${SUB}`); + }); +}); + +describe('openAzureExchangeModal', () => { + it('refuses reservations billed to different subscriptions', () => { + openAzureExchangeModal([reservations[0]!, reservations[2]!], onExecuted); + expect(resultText()).toMatch(/billed to different subscriptions/); + expect(document.querySelector('.azure-exchange-target-row')).toBeNull(); + }); + + it('seeds the target from the source and previews the priced exchange', async () => { + openAzureExchangeModal([reservations[0]!], onExecuted); + expect($('.azure-exchange-target-location').value).toBe('eastus'); + expect($('.azure-exchange-target-term').value).toBe('P3Y'); + setValue($('.azure-exchange-source-qty'), '3'); + setValue($('.azure-exchange-target-sku'), 'Standard_D4s_v5'); + $('.modal-buttons .btn.primary').click(); + await flush(); + expect(api.getAzureCompatibleOfferings).toHaveBeenCalledWith({ + subscription_id: SUB, + sources: [{ reservation_id: '/r/one', quantity: 3 }], + targets: [{ sku: 'Standard_D4s_v5', location: 'eastus', term: 'P3Y', quantity: 1 }], + }); + const text = resultText(); + expect(text).toContain('Net payable:USD 120.25'); + expect(text).toContain('Refunds total:USD 780.25'); + expect(text).toContain('Purchases total:USD 900.50'); + expect(text).toContain('Standard_D4s_v5'); + expect(executeBtn().classList.contains('hidden')).toBe(false); + // Guardrails are visible but never prefilled from the quote. + expect($('#azure-exchange-max-payment').value).toBe(''); + expect($('#azure-exchange-currency').value).toBe(''); + }); + + it('validates the form before calling the API', async () => { + openAzureExchangeModal([reservations[0]!], onExecuted); + setValue($('.azure-exchange-source-qty'), '9'); + $('.modal-buttons .btn.primary').click(); + await flush(); + expect(api.getAzureCompatibleOfferings).not.toHaveBeenCalled(); + expect(resultText()).toMatch(/Source 1: quantity must be a whole number from 1 to 4/); + }); + + it.each([ + ['policy errors', { ...okPreview, preview: { ...okPreview.preview, policy_errors: ['Reservation expired', 'Term not allowed'] } }, /Reservation expired.*Term not allowed/s], + ['no net payable', { ...okPreview, preview: { ...okPreview.preview, net_payable: null } }, /did not report a net payable/], + ['no preview', { offerings: null, preview: null }, /no preview/], + ])('does not offer Execute when Azure returns %s', async (_name, resp, msg) => { + (api.getAzureCompatibleOfferings as jest.Mock).mockResolvedValue(resp); + await openAndPreview(); + expect(resultText()).toMatch(msg); + expect(executeBtn().classList.contains('hidden')).toBe(true); + }); + + it('shows a preview failure as a user-facing message', async () => { + (api.getAzureCompatibleOfferings as jest.Mock).mockRejectedValue(Object.assign(new Error('sources[0].reservation_id is not a reservation billed to subscription'), { status: 403 })); + await openAndPreview(); + expect(resultText()).toMatch(/^Could not preview the exchange: you are not allowed to do this\./); + }); + + it('discards the preview when a target changes', async () => { + await openAndPreview(); + expect(executeBtn().classList.contains('hidden')).toBe(false); + setValue($('.azure-exchange-target-location'), 'westus2'); + expect(executeBtn().classList.contains('hidden')).toBe(true); + expect(resultText()).toBe(''); + }); + + it('requires the spending limit and currency before executing', async () => { + await openAndPreview(); + executeBtn().click(); + await flush(); + expect(confirmDialog).not.toHaveBeenCalled(); + expect(api.executeAzureExchange).not.toHaveBeenCalled(); + expect($('.azure-exchange-guard-error').textContent).toMatch(/maximum you are willing to pay/); + }); + + it('executes with the explicit cap and currency after confirmation', async () => { + (api.executeAzureExchange as jest.Mock).mockResolvedValue({ session_id: 's2', status: 'Succeeded', net_payable: 118, net_payable_currency: 'USD', refunds_total: 780.25, purchases_total: 898.25 }); + await openAndPreview(); + setValue($('#azure-exchange-max-payment'), '150'); + setValue($('#azure-exchange-currency'), 'usd'); + expect(executeBtn().classList.contains('hidden')).toBe(false); + executeBtn().click(); + await flush(); + await flush(); + const confirmBody = (confirmDialog as jest.Mock).mock.calls[0]![0].body as HTMLElement; + expect(confirmBody.textContent).toContain('Refused if Azure\'s fresh price is above USD 150'); + expect(api.executeAzureExchange).toHaveBeenCalledWith({ + subscription_id: SUB, + sources: [{ reservation_id: '/r/one', quantity: 4 }], + targets: [{ sku: 'Standard_D4s_v5', location: 'eastus', term: 'P3Y', quantity: 2 }], + max_payment_due: '150', + currency: 'USD', + }); + expect(onExecuted).toHaveBeenCalled(); + expect(statusText()).toMatch(/^Exchange Succeeded\. Net payable USD 118\.00/); + // The preview the user acted on stays on screen. + expect(resultText()).toContain('Net payable:USD 120.25'); + }); + + it('does nothing when the confirmation is cancelled', async () => { + (confirmDialog as jest.Mock).mockResolvedValue(false); + await openAndPreview(); + setValue($('#azure-exchange-max-payment'), '150'); + setValue($('#azure-exchange-currency'), 'USD'); + executeBtn().click(); + await flush(); + expect(api.executeAzureExchange).not.toHaveBeenCalled(); + }); + + it('explains a 422 guardrail refusal', async () => { + (api.executeAzureExchange as jest.Mock).mockRejectedValue(Object.assign(new Error('quoted net payable 160.00 USD exceeds max_payment_due 150.00 USD'), { status: 422 })); + await openAndPreview(); + setValue($('#azure-exchange-max-payment'), '150'); + setValue($('#azure-exchange-currency'), 'USD'); + executeBtn().click(); + await flush(); + await flush(); + expect(statusText()).toBe('Exchange blocked by a safety check: quoted net payable 160.00 USD exceeds max_payment_due 150.00 USD. Raise the maximum payment or change the targets, then preview again.'); + expect(onExecuted).not.toHaveBeenCalled(); + }); + + it('previews but hides Execute without execute:ri-exchange', async () => { + grant('view:purchases'); + await openAndPreview(); + expect(resultText()).toContain('Net payable'); + expect(executeBtn().classList.contains('hidden')).toBe(true); + expect($('.azure-exchange-guardrails').textContent).toContain('needs the execute:ri-exchange permission'); + }); + + it('adds and removes target rows', () => { + openAzureExchangeModal([reservations[0]!], onExecuted); + const add = Array.from(document.querySelectorAll('button')).find(b => b.textContent === '+ Add target')!; + add.click(); + expect(document.querySelectorAll('.azure-exchange-target-row')).toHaveLength(2); + document.querySelector('.exchange-remove-target')!.click(); + expect(document.querySelectorAll('.azure-exchange-target-row')).toHaveLength(1); + document.querySelector('.exchange-remove-target')!.click(); + expect(document.querySelectorAll('.azure-exchange-target-row')).toHaveLength(1); + }); +}); diff --git a/frontend/src/__tests__/riexchange.test.ts b/frontend/src/__tests__/riexchange.test.ts index e9f42d41c..c4640711a 100644 --- a/frontend/src/__tests__/riexchange.test.ts +++ b/frontend/src/__tests__/riexchange.test.ts @@ -1229,6 +1229,22 @@ describe('RI Exchange global filter scoping (issue #871)', () => { expect(recsEl.textContent).toContain('not available for Azure'); }); + it('lets a user who can preview purchases select Azure reservations for an exchange (issue #81)', async () => { + stateMod().getCurrentProvider.mockReturnValue('azure'); + // An Administrators-group member (admin:* covers view:purchases). + (jest.requireMock('../state') as { getCurrentUser: jest.Mock }).getCurrentUser + .mockReturnValue({ id: 'u', email: 'u@example.com', groups: ['00000000-0000-5000-8000-000000000001'] }); + (api.listExchangeableAzureRIs as jest.Mock).mockResolvedValue([ + { reservation_order_id: 'o1', reservation_id: 'r1', sku: 'Standard_D2s_v3', quantity: 2, region: 'eastus', term: 'P1Y', instance_flexibility: 'On', billing_scope_id: '/subscriptions/s1' }, + { reservation_order_id: 'o2', reservation_id: 'r2', sku: 'Standard_E2s_v3', quantity: 1, region: 'eastus', term: 'P1Y', instance_flexibility: 'On', billing_scope_id: '/subscriptions/s1' }, + ]); + await loadRIExchange(); + expect(instancesEl.querySelectorAll('tbody input.azure-ri-select')).toHaveLength(2); + expect(instancesEl.querySelectorAll('thead th')).toHaveLength(7); + const btn = instancesEl.querySelector('#azure-exchange-selected-btn')!; + expect(btn.disabled).toBe(true); + }); + it('shows a provider-aware empty state for Azure with no reservations', async () => { stateMod().getCurrentProvider.mockReturnValue('azure'); stateMod().getCurrentAccountIDs.mockReturnValue(['uuid-az']); diff --git a/frontend/src/riexchange-azure.ts b/frontend/src/riexchange-azure.ts new file mode 100644 index 000000000..a39c4c75b --- /dev/null +++ b/frontend/src/riexchange-azure.ts @@ -0,0 +1,58 @@ +/** + * Azure RI exchange entry points on the exchangeable-reservations table + * (issue #81): a selection column and an "Exchange selected" action. The + * exchange modal itself (riexchange-azure-modal.ts) is loaded on demand. + */ + +import type { ExchangeableAzureRI } from './api'; +import { canAccess } from './permissions'; + +/** Previewing needs view:purchases; executing needs execute:ri-exchange (carved out of admin:*). */ +export function canPreviewAzureExchange(): boolean { + return canAccess('view', 'purchases'); +} + +function el(tag: K, className?: string, text?: string): HTMLElementTagNameMap[K] { + const node = document.createElement(tag); + if (className) node.className = className; + if (text !== undefined) node.textContent = text; + return node; +} + +/** Checkbox cell that selects one reservation for an exchange. */ +export function buildAzureSelectCell(r: ExchangeableAzureRI, index: number): HTMLTableCellElement { + const td = el('td'); + const box = el('input', 'azure-ri-select'); + box.type = 'checkbox'; + box.dataset['index'] = String(index); + box.setAttribute('aria-label', `Select ${r.display_name || r.reservation_id} for exchange`); + td.appendChild(box); + return td; +} + +/** + * Adds the "Exchange selected" action above the reservations table. The + * button counts the checked rows and opens the exchange modal for them. + */ +export function attachAzureExchangeToolbar(container: HTMLElement, table: HTMLTableElement, reservations: readonly ExchangeableAzureRI[], onExecuted: () => void): void { + const bar = el('div', 'setting-row azure-exchange-toolbar'); + const btn = el('button', 'btn btn-small', 'Exchange selected'); + btn.type = 'button'; + btn.id = 'azure-exchange-selected-btn'; + const selected = (): ExchangeableAzureRI[] => Array.from(table.querySelectorAll('input.azure-ri-select:checked')) + .map(b => reservations[Number(b.dataset['index'])]) + .filter((r): r is ExchangeableAzureRI => r !== undefined); + const sync = (): void => { + const n = selected().length; + btn.disabled = n === 0; + btn.textContent = n === 0 ? 'Exchange selected' : `Exchange selected (${n})`; + }; + table.addEventListener('change', sync); + btn.addEventListener('click', () => { + const picked = selected(); + void import('./riexchange-azure-modal').then(m => m.openAzureExchangeModal(picked, onExecuted)); + }); + bar.append(btn, el('span', 'help-text', 'Tick the reservations to hand back, then preview what Azure offers for them.')); + sync(); + container.insertBefore(bar, table); +} diff --git a/frontend/src/riexchange.ts b/frontend/src/riexchange.ts index 4b0855f7b..fab32b494 100644 --- a/frontend/src/riexchange.ts +++ b/frontend/src/riexchange.ts @@ -32,6 +32,7 @@ import { canAccess } from './permissions'; import { applyReadOnlySettings, isPermissionDeniedError } from './settings'; import { showToast } from './toast'; import { getCurrentUser } from './state'; +import { attachAzureExchangeToolbar, buildAzureSelectCell, canPreviewAzureExchange } from './riexchange-azure'; // Module state let currentRIs: ConvertibleRI[] = []; @@ -461,8 +462,9 @@ function renderRIsTable(container: HTMLElement, accountID?: string): void { * entirely via DOM construction / textContent — never innerHTML — so any * Azure-derived field (SKU, display name, region) cannot inject markup. * - * Azure reservations are listed read-only here; the quote/execute modal is - * AWS-specific (offering-UUID flow), so no per-row Exchange button is shown. + * Users who can preview an exchange get a selection column and an + * "Exchange selected" action that opens the Azure exchange modal + * (riexchange-azure.ts, issue #81). */ function renderAzureRIsTable( container: HTMLElement, @@ -479,10 +481,12 @@ function renderAzureRIsTable( container.textContent = ''; const table = document.createElement('table'); + const canExchange = canPreviewAzureExchange(); const thead = document.createElement('thead'); const headerRow = document.createElement('tr'); - for (const label of ['Reservation', 'SKU', 'Quantity', 'Region', 'Term', 'Expiry']) { + const labels = ['Reservation', 'SKU', 'Quantity', 'Region', 'Term', 'Expiry']; + for (const label of canExchange ? ['', ...labels] : labels) { const th = document.createElement('th'); th.textContent = label; headerRow.appendChild(th); @@ -491,11 +495,14 @@ function renderAzureRIsTable( table.appendChild(thead); const tbody = document.createElement('tbody'); - for (const r of reservations) { - tbody.appendChild(buildAzureRIRow(r)); - } + reservations.forEach((r, i) => { + const tr = buildAzureRIRow(r); + if (canExchange) tr.prepend(buildAzureSelectCell(r, i)); + tbody.appendChild(tr); + }); table.appendChild(tbody); container.appendChild(table); + if (canExchange) attachAzureExchangeToolbar(container, table, reservations, () => { void loadRIExchange(); }); } function buildAzureRIRow(r: ExchangeableAzureRI): HTMLTableRowElement { diff --git a/frontend/src/styles/components.css b/frontend/src/styles/components.css index 5d76ec88e..c278f40ae 100644 --- a/frontend/src/styles/components.css +++ b/frontend/src/styles/components.css @@ -1271,6 +1271,64 @@ tr.rec-variant-row:hover td:not(.checkbox-col) { font-size: 0.9rem; } +/* Azure RI exchange modal (issue #81). */ +.setting-row.azure-exchange-source-row, +.setting-row.azure-exchange-target-row { + justify-content: flex-start; + gap: 0.75rem; + padding: 0.5rem 0; +} +.setting-row.azure-exchange-target-row { + display: grid; + grid-template-columns: 2fr 1.5fr 1fr 0.8fr auto; + align-items: end; +} +.azure-exchange .azure-exchange-target-row label, +.azure-exchange .azure-exchange-source-row label { + display: flex; + flex-direction: column; + gap: 0.25rem; + font-size: var(--cudly-fs-sm); +} +.azure-exchange-source-row input[type="number"] { + width: 6em; +} +.azure-exchange-preview .quote-row { + display: flex; + justify-content: space-between; + max-width: 22rem; +} +.azure-exchange-preview.quote-invalid h4, +.azure-exchange-policy-errors { + color: var(--cudly-error-fg); +} +.azure-exchange-guardrails { + display: grid; + grid-template-columns: 1fr 8rem; + gap: 0.5rem 1rem; + margin: 1rem 0; + padding: 0.75rem 1rem; + border: 1px solid var(--cudly-border-strong); + border-radius: 6px; +} +.azure-exchange-guardrails.hidden { + display: none; +} +.azure-exchange-guardrails legend, +.azure-exchange-guardrails > p { + grid-column: 1 / -1; +} +.azure-exchange .azure-exchange-guardrails label { + display: flex; + flex-direction: column; + gap: 0.25rem; +} +.setting-row.azure-exchange-toolbar { + justify-content: flex-start; + gap: 0.75rem; + padding: 0.5rem 0; +} + /* Info icon tooltip */ /* Issue #909: Opportunities lookback selector toolbar. */ .recommendations-toolbar { From 4ac6e996122320c265f3e5b5791c00a5c00645d8 Mon Sep 17 00:00:00 2001 From: Astrid Gealer Date: Sat, 24 Oct 2026 02:12:45 +0000 Subject: [PATCH 4/6] test(frontend): cover the Azure RI exchange flow end to end Playwright spec with mocked Azure endpoints: select a reservation, preview, get refused without a spending limit, execute with an explicit cap and currency, and see a 422 guardrail refusal explained rather than reported as a generic error. Refs #81 Co-Authored-By: Claude Opus 5.5 --- frontend/tests-e2e/azure-ri-exchange.spec.ts | 87 ++++++++++++++++++++ 1 file changed, 87 insertions(+) create mode 100644 frontend/tests-e2e/azure-ri-exchange.spec.ts diff --git a/frontend/tests-e2e/azure-ri-exchange.spec.ts b/frontend/tests-e2e/azure-ri-exchange.spec.ts new file mode 100644 index 000000000..b5f667007 --- /dev/null +++ b/frontend/tests-e2e/azure-ri-exchange.spec.ts @@ -0,0 +1,87 @@ +import { expect, test, type Page } from '@playwright/test'; +import { mockApi, seedAuth } from './fixtures/recs'; + +// Azure RI exchange: select reservations, preview, then execute behind the +// max_payment_due + currency guardrail (issue #81). All Azure calls are mocked. +const SUB = '11111111-2222-3333-4444-555555555555'; +const reservations = [ + { reservation_order_id: 'o1', reservation_id: '/providers/Microsoft.Capacity/reservationOrders/o1/reservations/r1', sku: 'Standard_D2s_v3', quantity: 4, + region: 'eastus', term: 'P3Y', expiry_date: '2028-01-01T00:00:00Z', instance_flexibility: 'On', display_name: 'web-tier', billing_scope_id: `/subscriptions/${SUB}` }, + { reservation_order_id: 'o2', reservation_id: '/providers/Microsoft.Capacity/reservationOrders/o2/reservations/r2', sku: 'Standard_E4s_v3', quantity: 2, + region: 'eastus', term: 'P1Y', expiry_date: '2027-03-01T00:00:00Z', instance_flexibility: 'On', display_name: 'batch', billing_scope_id: `/subscriptions/${SUB}` }, +]; +const preview = { + offerings: [{ sku: 'Standard_D4s_v5', location: 'eastus', term: 'P3Y', quantity: 2, billing_currency_total: 900.5, currency_code: 'USD' }], + preview: { session_id: 's1', net_payable: 120.25, net_payable_currency: 'USD', refunds_total: 780.25, purchases_total: 900.5 }, +}; + +async function setup(page: Page, executeStatus = 200) { + await seedAuth(page); + await mockApi(page); + await page.route('**/api/auth/me/permissions', route => route.fulfill({ json: { + permissions: [{ action: 'admin', resource: '*' }, { action: 'execute', resource: 'ri-exchange' }], is_admin: true, + } })); + const posts: { path: string; body: unknown }[] = []; + await page.route('**/api/ri-exchange/**', async route => { + const path = new URL(route.request().url()).pathname; + if (route.request().method() === 'POST') { + posts.push({ path, body: route.request().postDataJSON() }); + if (path.endsWith('/compatible-offerings')) return route.fulfill({ json: preview }); + if (executeStatus !== 200) { + return route.fulfill({ status: executeStatus, json: { error: 'quoted net payable 160.00 USD exceeds max_payment_due 150.00 USD' } }); + } + return route.fulfill({ json: { session_id: 's2', status: 'Succeeded', net_payable: 118, net_payable_currency: 'USD', refunds_total: 780.25, purchases_total: 898.25 } }); + } + if (path.endsWith('/azure-instances')) return route.fulfill({ json: { reservations } }); + if (path.endsWith('/history')) return route.fulfill({ json: { records: [] } }); + return route.fulfill({ json: {} }); + }); + await page.goto('/inventory/ri-exchange?provider=azure'); + await expect(page.locator('#ri-exchange-instances-list')).toContainText('web-tier'); + return posts; +} + +async function previewFirst(page: Page) { + await page.getByRole('checkbox', { name: 'Select web-tier for exchange' }).check(); + await page.locator('#azure-exchange-selected-btn').click(); + const modal = page.locator('#ri-exchange-modal'); + await modal.locator('.azure-exchange-target-sku').fill('Standard_D4s_v5'); + await modal.locator('.azure-exchange-target-qty').fill('2'); + await modal.getByRole('button', { name: 'Preview exchange' }).click(); + await expect(modal.locator('#azure-exchange-result')).toContainText('Net payable:USD 120.25'); + return modal; +} + +test('previews and executes an Azure exchange with an explicit spending limit', async ({ page }) => { + const posts = await setup(page); + const modal = await previewFirst(page); + expect(posts[0]).toEqual({ path: '/api/ri-exchange/azure-instances/compatible-offerings', body: { + subscription_id: SUB, + sources: [{ reservation_id: reservations[0]!.reservation_id, quantity: 4 }], + targets: [{ sku: 'Standard_D4s_v5', location: 'eastus', term: 'P3Y', quantity: 2 }], + } }); + + await modal.getByRole('button', { name: 'Execute exchange' }).click(); + await expect(modal.locator('.azure-exchange-guard-error')).toContainText('maximum you are willing to pay'); + await modal.locator('#azure-exchange-max-payment').fill('150'); + await modal.locator('#azure-exchange-currency').fill('usd'); + await modal.getByRole('button', { name: 'Execute exchange' }).click(); + const confirm = page.locator('.confirm-dialog, [role="alertdialog"], [role="dialog"]').filter({ hasText: 'cannot be reversed' }).last(); + await confirm.getByRole('button', { name: 'Execute exchange' }).click(); + await expect(modal.locator('#azure-exchange-status')).toContainText('Exchange Succeeded. Net payable USD 118.00'); + expect(posts[1]!.path).toBe('/api/ri-exchange/azure-instances/exchange'); + expect(posts[1]!.body).toMatchObject({ subscription_id: SUB, max_payment_due: '150', currency: 'USD' }); +}); + +test('explains a guardrail refusal instead of a generic error', async ({ page }) => { + await setup(page, 422); + const modal = await previewFirst(page); + await modal.locator('#azure-exchange-max-payment').fill('150'); + await modal.locator('#azure-exchange-currency').fill('USD'); + await modal.getByRole('button', { name: 'Execute exchange' }).click(); + const confirm = page.locator('.confirm-dialog, [role="alertdialog"], [role="dialog"]').filter({ hasText: 'cannot be reversed' }).last(); + await confirm.getByRole('button', { name: 'Execute exchange' }).click(); + await expect(modal.locator('#azure-exchange-status')).toHaveText( + 'Exchange blocked by a safety check: quoted net payable 160.00 USD exceeds max_payment_due 150.00 USD. Raise the maximum payment or change the targets, then preview again.', + ); +}); From a6b14b3b483d90ea35bd648a108b0b32917707df Mon Sep 17 00:00:00 2001 From: Astrid Gealer Date: Sat, 24 Oct 2026 13:35:47 +0000 Subject: [PATCH 5/6] fix(frontend): report an unknown Azure exchange outcome instead of "not executed" A 5xx, gateway timeout, client timeout or network error on execute does not prove the exchange was refused. The backend runs ExecuteExchange on a detached context and audits those failures as outcome unknown (directExchangeFailureStatus), so labelling them "Exchange not executed" was wrong. It also left Execute enabled on the same preview and skipped the list refresh, which invited a retry of an irreversible exchange. Only a 4xx is now reported as a refusal. Any other failure says the exchange may have been submitted and asks the user to check the history or the Azure portal. It also refreshes the list and history and retires the preview, so a retry needs a fresh preview. Also show a toast when the exchange dialog chunk fails to load, and give the selection column's header a visually hidden accessible name. Refs #81 Co-Authored-By: Claude Opus 5.5 --- frontend/src/__tests__/azure-exchange.test.ts | 27 +++++++++++- .../src/__tests__/riexchange-azure.test.ts | 44 +++++++++++++++++++ frontend/src/__tests__/riexchange.test.ts | 3 ++ frontend/src/lib/azure-exchange.ts | 16 +++++++ frontend/src/riexchange-azure-modal.ts | 24 +++++++--- frontend/src/riexchange-azure.ts | 12 ++++- frontend/src/riexchange.ts | 5 ++- 7 files changed, 120 insertions(+), 11 deletions(-) diff --git a/frontend/src/__tests__/azure-exchange.test.ts b/frontend/src/__tests__/azure-exchange.test.ts index ddb7edf9f..a3b51d42e 100644 --- a/frontend/src/__tests__/azure-exchange.test.ts +++ b/frontend/src/__tests__/azure-exchange.test.ts @@ -105,8 +105,31 @@ describe('describeAzureExchangeError', () => { .toBe('Exchange not executed: you are not allowed to do this. direct RI exchange execution is disabled while 4-eyes approval mode is on; use the approval flow'); expect(describeAzureExchangeError(e(404, 'not found'), 'preview')).toMatch(/^Could not preview the exchange: no Azure account/); expect(describeAzureExchangeError(e(400, 'targets[0].sku is required'), 'preview')).toMatch(/rejected\. targets\[0\]\.sku is required/); - expect(describeAzureExchangeError(e(500, 'exchange execution failed and may already have been submitted to Azure'), 'execute')) - .toMatch(/may already have been submitted/); expect(describeAzureExchangeError(undefined, 'preview')).toBe('Could not preview the exchange: Unknown error'); }); + + // The backend records any non-4xx execute failure as outcome unknown + // (directExchangeFailureStatus): Azure may already have the exchange. + const unknownOutcome = (detail: string) => + `The exchange may have been submitted to Azure. Check the RI exchange history (or the Azure portal) before trying again. Details: ${detail}`; + it.each([ + ['a 500', e(500, 'exchange execution failed and may already have been submitted to Azure; verify the reservation state in the Azure portal before retrying'), + 'exchange execution failed and may already have been submitted to Azure; verify the reservation state in the Azure portal before retrying'], + ['a 504 gateway timeout', e(504, 'HTTP 504'), 'HTTP 504'], + ['a client timeout (status 0)', e(0, 'Request to /ri-exchange/azure-instances/exchange timed out after 90000ms'), + 'Request to /ri-exchange/azure-instances/exchange timed out after 90000ms'], + ['a network error (no status)', new TypeError('Failed to fetch'), 'Failed to fetch'], + ])('reports %s on execute as an unknown outcome, never "not executed"', (_label, error, detail) => { + expect(describeAzureExchangeError(error, 'execute')).toBe(unknownOutcome(detail)); + }); + it('keeps 4xx execute refusals as definite', () => { + const guardrail = e(422, 'quoted net payable 120.00 USD exceeds max_payment_due 100.00 USD'); + expect(describeAzureExchangeError(guardrail, 'execute')).toBe( + 'Exchange blocked by a safety check: quoted net payable 120.00 USD exceeds max_payment_due 100.00 USD. Raise the maximum payment or change the targets, then preview again.'); + const conflict = e(409, 'an identical exchange is already being submitted'); + expect(describeAzureExchangeError(conflict, 'execute')).toBe('Exchange not executed: an identical exchange is already being submitted'); + }); + it('keeps preview failures as "could not preview" whatever the status', () => { + expect(describeAzureExchangeError(e(504, 'HTTP 504'), 'preview')).toBe('Could not preview the exchange: HTTP 504'); + }); }); diff --git a/frontend/src/__tests__/riexchange-azure.test.ts b/frontend/src/__tests__/riexchange-azure.test.ts index e6fd2e789..bc897fcb2 100644 --- a/frontend/src/__tests__/riexchange-azure.test.ts +++ b/frontend/src/__tests__/riexchange-azure.test.ts @@ -7,6 +7,7 @@ jest.mock('../api', () => ({ })); jest.mock('../permissions', () => ({ canAccess: jest.fn(), isRIExchanger: jest.fn() })); jest.mock('../confirmDialog', () => ({ confirmDialog: jest.fn() })); +jest.mock('../toast', () => ({ showToast: jest.fn() })); import * as api from '../api'; import { canAccess, isRIExchanger } from '../permissions'; @@ -95,6 +96,27 @@ describe('Azure exchange table action', () => { expect(document.querySelectorAll('.azure-exchange-source-row')).toHaveLength(2); expect($('.azure-exchange').textContent).toContain(`Subscription ${SUB}`); }); + + it('shows a toast when the exchange dialog fails to load', async () => { + let toast: jest.Mock | undefined; + jest.isolateModules(() => { + jest.doMock('../riexchange-azure-modal', () => { throw new Error('ChunkLoadError'); }); + toast = (jest.requireMock('../toast') as { showToast: jest.Mock }).showToast; + const isolated = jest.requireActual('../riexchange-azure') as typeof import('../riexchange-azure'); + const container = document.createElement('div'); + const table = document.createElement('table'); + table.createTBody().insertRow().appendChild(isolated.buildAzureSelectCell(reservations[0]!, 0)); + container.appendChild(table); + document.body.appendChild(container); + isolated.attachAzureExchangeToolbar(container, table, reservations, onExecuted); + table.querySelector('input.azure-ri-select')!.checked = true; + table.dispatchEvent(new Event('change')); + }); + $('#azure-exchange-selected-btn').click(); + for (let i = 0; i < 5 && toast!.mock.calls.length === 0; i++) await flush(); + jest.dontMock('../riexchange-azure-modal'); + expect(toast).toHaveBeenCalledWith({ message: 'Could not load the exchange dialog. Reload the page and try again.', kind: 'error' }); + }); }); describe('openAzureExchangeModal', () => { @@ -217,6 +239,28 @@ describe('openAzureExchangeModal', () => { expect(onExecuted).not.toHaveBeenCalled(); }); + it.each([ + ['a 504 gateway timeout', Object.assign(new Error('HTTP 504'), { status: 504 }), 'HTTP 504'], + ['a client timeout', Object.assign(new Error('Request to /ri-exchange/azure-instances/exchange timed out after 90000ms'), { status: 0 }), + 'Request to /ri-exchange/azure-instances/exchange timed out after 90000ms'], + ])('treats %s on execute as an unknown outcome: refreshes and requires a new preview', async (_label, error, detail) => { + (api.executeAzureExchange as jest.Mock).mockRejectedValue(error); + await openAndPreview(); + setValue($('#azure-exchange-max-payment'), '150'); + setValue($('#azure-exchange-currency'), 'USD'); + executeBtn().click(); + await flush(); + await flush(); + expect(statusText()).toBe(`The exchange may have been submitted to Azure. Check the RI exchange history (or the Azure portal) before trying again. Details: ${detail}`); + expect(onExecuted).toHaveBeenCalledTimes(1); + expect(executeBtn().classList.contains('hidden')).toBe(true); + expect($('.azure-exchange-guardrails').classList.contains('hidden')).toBe(true); + // A stray click on the now-hidden button cannot resubmit the old preview. + executeBtn().click(); + await flush(); + expect(api.executeAzureExchange).toHaveBeenCalledTimes(1); + }); + it('previews but hides Execute without execute:ri-exchange', async () => { grant('view:purchases'); await openAndPreview(); diff --git a/frontend/src/__tests__/riexchange.test.ts b/frontend/src/__tests__/riexchange.test.ts index c4640711a..56edd1e04 100644 --- a/frontend/src/__tests__/riexchange.test.ts +++ b/frontend/src/__tests__/riexchange.test.ts @@ -1241,6 +1241,9 @@ describe('RI Exchange global filter scoping (issue #871)', () => { await loadRIExchange(); expect(instancesEl.querySelectorAll('tbody input.azure-ri-select')).toHaveLength(2); expect(instancesEl.querySelectorAll('thead th')).toHaveLength(7); + // The checkbox column's header has an accessible name, visually hidden. + const selectHeader = instancesEl.querySelector('thead th')!; + expect(selectHeader.querySelector('.sr-only')?.textContent).toBe('Select for exchange'); const btn = instancesEl.querySelector('#azure-exchange-selected-btn')!; expect(btn.disabled).toBe(true); }); diff --git a/frontend/src/lib/azure-exchange.ts b/frontend/src/lib/azure-exchange.ts index ae192a21d..1f10ce8f3 100644 --- a/frontend/src/lib/azure-exchange.ts +++ b/frontend/src/lib/azure-exchange.ts @@ -116,10 +116,26 @@ const GUARDRAIL_HINTS: ReadonlyArray<[RegExp, string]> = [ [/^Azure rejected this exchange/, 'Change the sources or targets so they meet Azure\'s exchange policy.'], ]; +/** + * Whether a failed execute call leaves the exchange's outcome unknown. Only a + * 4xx is a definite refusal: every gate, the guardrails, the idempotency claim + * and Azure's own client-fault rejections answer 4xx. A 5xx, a gateway or + * client timeout (status 0) or a network error may arrive after the exchange + * was submitted, which is why the backend audits those as unknown + * (directExchangeFailureStatus). + */ +export function isAzureExchangeOutcomeUnknown(error: unknown): boolean { + const status = (error as ApiErrorLike | null | undefined)?.status; + return !(typeof status === 'number' && status >= 400 && status < 500); +} + /** User-facing text for a failed preview or execute call. */ export function describeAzureExchangeError(error: unknown, phase: 'preview' | 'execute'): string { const err = (error ?? {}) as ApiErrorLike; const msg = err.message || 'Unknown error'; + if (phase === 'execute' && isAzureExchangeOutcomeUnknown(error)) { + return `The exchange may have been submitted to Azure. Check the RI exchange history (or the Azure portal) before trying again. Details: ${msg}`; + } const what = phase === 'preview' ? 'Could not preview the exchange' : 'Exchange not executed'; switch (err.status) { case 422: { diff --git a/frontend/src/riexchange-azure-modal.ts b/frontend/src/riexchange-azure-modal.ts index b385e704b..0330878bf 100644 --- a/frontend/src/riexchange-azure-modal.ts +++ b/frontend/src/riexchange-azure-modal.ts @@ -17,7 +17,7 @@ import { isRIExchanger } from './permissions'; import { AZURE_EXCHANGE_TERMS, MAX_AZURE_EXCHANGE_ITEMS, azureTermLabel, describeAzureExchangeError, formatAzureAmount, resolveExchangeSubscription, validateAzureExchangeForm, validateAzureGuardrails, - type AzureExchangeSourceInput, type AzureExchangeTargetInput, + isAzureExchangeOutcomeUnknown, type AzureExchangeSourceInput, type AzureExchangeTargetInput, } from './lib/azure-exchange'; function el(tag: K, className?: string, text?: string): HTMLElementTagNameMap[K] { @@ -119,7 +119,8 @@ function renderPreview(container: HTMLElement, resp: AzureCompatibleOfferingsRes /** * Opens the exchange modal for the selected reservations. onExecuted runs - * after a successful exchange so the caller can reload the list. + * after an exchange that succeeded or may have, so the caller can reload the + * list and history. */ export function openAzureExchangeModal(sources: readonly ExchangeableAzureRI[], onExecuted: () => void): void { const modal = document.getElementById('ri-exchange-modal'); @@ -303,6 +304,15 @@ export function openAzureExchangeModal(sources: readonly ExchangeableAzureRI[], } } + // Hides Execute and drops the preview it would send, keeping the outcome + // message and the preview on screen. + function retirePreview(): void { + executeBtn.classList.add('hidden'); + guard.classList.add('hidden'); + lastPreview = null; + lastRequest = null; + } + async function execute(): Promise { if (executeBtn.disabled || !lastPreview?.preview || !lastRequest) return; const guardError = validateAzureGuardrails(maxInput.value, currencyInput.value); @@ -335,13 +345,15 @@ export function openAzureExchangeModal(sources: readonly ExchangeableAzureRI[], onExecuted(); if (!sessionOpen) return; setMessage(statusBox, `Exchange ${result.status || 'submitted'}. Net payable ${formatAzureAmount(result.net_payable, result.net_payable_currency)}; refunds ${formatAzureAmount(result.refunds_total, result.net_payable_currency)}; purchases ${formatAzureAmount(result.purchases_total, result.net_payable_currency)}. Session ${result.session_id}.`, 'success-message'); - executeBtn.classList.add('hidden'); - guard.classList.add('hidden'); - lastPreview = null; - lastRequest = null; + retirePreview(); } catch (error) { + // An unknown outcome may still commit at Azure: refresh the list and + // history, and retire this preview so a retry needs a fresh one. + const unknown = isAzureExchangeOutcomeUnknown(error); + if (unknown) onExecuted(); if (!sessionOpen) return; setMessage(statusBox, describeAzureExchangeError(error, 'execute'), 'error'); + if (unknown) retirePreview(); } finally { executeBtn.disabled = false; previewBtn.disabled = false; diff --git a/frontend/src/riexchange-azure.ts b/frontend/src/riexchange-azure.ts index a39c4c75b..5eaa728c7 100644 --- a/frontend/src/riexchange-azure.ts +++ b/frontend/src/riexchange-azure.ts @@ -6,6 +6,7 @@ import type { ExchangeableAzureRI } from './api'; import { canAccess } from './permissions'; +import { showToast } from './toast'; /** Previewing needs view:purchases; executing needs execute:ri-exchange (carved out of admin:*). */ export function canPreviewAzureExchange(): boolean { @@ -19,6 +20,13 @@ function el(tag: K, className?: string, t return node; } +/** Header for the selection column: an accessible name, visually hidden. */ +export function buildAzureSelectHeader(): HTMLTableCellElement { + const th = el('th'); + th.appendChild(el('span', 'sr-only', 'Select for exchange')); + return th; +} + /** Checkbox cell that selects one reservation for an exchange. */ export function buildAzureSelectCell(r: ExchangeableAzureRI, index: number): HTMLTableCellElement { const td = el('td'); @@ -50,7 +58,9 @@ export function attachAzureExchangeToolbar(container: HTMLElement, table: HTMLTa table.addEventListener('change', sync); btn.addEventListener('click', () => { const picked = selected(); - void import('./riexchange-azure-modal').then(m => m.openAzureExchangeModal(picked, onExecuted)); + import('./riexchange-azure-modal') + .then(m => m.openAzureExchangeModal(picked, onExecuted)) + .catch(() => showToast({ message: 'Could not load the exchange dialog. Reload the page and try again.', kind: 'error' })); }); bar.append(btn, el('span', 'help-text', 'Tick the reservations to hand back, then preview what Azure offers for them.')); sync(); diff --git a/frontend/src/riexchange.ts b/frontend/src/riexchange.ts index fab32b494..e90a53547 100644 --- a/frontend/src/riexchange.ts +++ b/frontend/src/riexchange.ts @@ -32,7 +32,7 @@ import { canAccess } from './permissions'; import { applyReadOnlySettings, isPermissionDeniedError } from './settings'; import { showToast } from './toast'; import { getCurrentUser } from './state'; -import { attachAzureExchangeToolbar, buildAzureSelectCell, canPreviewAzureExchange } from './riexchange-azure'; +import { attachAzureExchangeToolbar, buildAzureSelectCell, buildAzureSelectHeader, canPreviewAzureExchange } from './riexchange-azure'; // Module state let currentRIs: ConvertibleRI[] = []; @@ -486,7 +486,8 @@ function renderAzureRIsTable( const thead = document.createElement('thead'); const headerRow = document.createElement('tr'); const labels = ['Reservation', 'SKU', 'Quantity', 'Region', 'Term', 'Expiry']; - for (const label of canExchange ? ['', ...labels] : labels) { + if (canExchange) headerRow.appendChild(buildAzureSelectHeader()); + for (const label of labels) { const th = document.createElement('th'); th.textContent = label; headerRow.appendChild(th); From 8ffadd673c88a743c2243fe3d41ac0915f24570f Mon Sep 17 00:00:00 2001 From: Astrid Gealer Date: Sat, 24 Oct 2026 13:50:29 +0000 Subject: [PATCH 6/6] fix(frontend): point an unknown Azure exchange outcome at the Azure portal The unknown-outcome text told the user to check the RI exchange history, but that history reads only ri_exchange_history, while direct Azure executions are audited in ri_exchange_direct_executions, which nothing serves. The history could never show the exchange, inviting the user to conclude it did not happen and execute again. Point only at the Azure portal, as the backend's own 500 text does, and say the history here does not list Azure exchanges. Closing the modal while execute was in flight dropped the outcome; it is now shown as a toast (a sticky warning when the outcome is unknown). Execute used the default 90s client timeout against a 4-minute backend execution budget, so slow successful exchanges were reported as unknown; it now gets a 5-minute timeout. Refs #81 Co-Authored-By: Claude Opus 5.5 --- .../__tests__/api-riexchange-azure.test.ts | 30 +++++++++ frontend/src/__tests__/azure-exchange.test.ts | 2 +- .../src/__tests__/riexchange-azure.test.ts | 63 ++++++++++++++++++- frontend/src/api/riexchange.ts | 9 +++ frontend/src/lib/azure-exchange.ts | 5 +- frontend/src/riexchange-azure-modal.ts | 19 ++++-- 6 files changed, 121 insertions(+), 7 deletions(-) create mode 100644 frontend/src/__tests__/api-riexchange-azure.test.ts diff --git a/frontend/src/__tests__/api-riexchange-azure.test.ts b/frontend/src/__tests__/api-riexchange-azure.test.ts new file mode 100644 index 000000000..d716ca180 --- /dev/null +++ b/frontend/src/__tests__/api-riexchange-azure.test.ts @@ -0,0 +1,30 @@ +/** + * Request options for the Azure RI exchange API calls (issue #81). + */ +import { apiRequest } from '../api/client'; +import { executeAzureExchange, getAzureCompatibleOfferings } from '../api/riexchange'; + +jest.mock('../api/client', () => ({ apiRequest: jest.fn() })); + +const req = { + subscription_id: 'sub', + sources: [{ reservation_id: '/r/one', quantity: 1 }], + targets: [{ sku: 'Standard_D4s_v5', location: 'eastus', term: 'P3Y', quantity: 1 }], +}; + +beforeEach(() => (apiRequest as jest.Mock).mockReset().mockResolvedValue({})); + +it('gives execute a client timeout longer than the backend 4-minute execution budget', async () => { + await executeAzureExchange({ ...req, max_payment_due: '150', currency: 'USD' }); + expect(apiRequest).toHaveBeenCalledWith('/ri-exchange/azure-instances/exchange', { + method: 'POST', + body: JSON.stringify({ ...req, max_payment_due: '150', currency: 'USD' }), + timeoutMs: 5 * 60_000, + }); +}); + +it('keeps the default timeout for preview', async () => { + await getAzureCompatibleOfferings(req); + const opts = (apiRequest as jest.Mock).mock.calls[0]![1]; + expect(opts).not.toHaveProperty('timeoutMs'); +}); diff --git a/frontend/src/__tests__/azure-exchange.test.ts b/frontend/src/__tests__/azure-exchange.test.ts index a3b51d42e..79a88b59e 100644 --- a/frontend/src/__tests__/azure-exchange.test.ts +++ b/frontend/src/__tests__/azure-exchange.test.ts @@ -111,7 +111,7 @@ describe('describeAzureExchangeError', () => { // The backend records any non-4xx execute failure as outcome unknown // (directExchangeFailureStatus): Azure may already have the exchange. const unknownOutcome = (detail: string) => - `The exchange may have been submitted to Azure. Check the RI exchange history (or the Azure portal) before trying again. Details: ${detail}`; + `The exchange may have been submitted to Azure. Verify the reservation state in the Azure portal (the reservation order's history) before trying again; the RI exchange history here does not list Azure exchanges. Details: ${detail}`; it.each([ ['a 500', e(500, 'exchange execution failed and may already have been submitted to Azure; verify the reservation state in the Azure portal before retrying'), 'exchange execution failed and may already have been submitted to Azure; verify the reservation state in the Azure portal before retrying'], diff --git a/frontend/src/__tests__/riexchange-azure.test.ts b/frontend/src/__tests__/riexchange-azure.test.ts index bc897fcb2..c9fdf2887 100644 --- a/frontend/src/__tests__/riexchange-azure.test.ts +++ b/frontend/src/__tests__/riexchange-azure.test.ts @@ -12,6 +12,7 @@ jest.mock('../toast', () => ({ showToast: jest.fn() })); import * as api from '../api'; import { canAccess, isRIExchanger } from '../permissions'; import { confirmDialog } from '../confirmDialog'; +import { showToast } from '../toast'; import { attachAzureExchangeToolbar, buildAzureSelectCell, canPreviewAzureExchange } from '../riexchange-azure'; import { openAzureExchangeModal } from '../riexchange-azure-modal'; import type { ExchangeableAzureRI } from '../api/types'; @@ -251,7 +252,7 @@ describe('openAzureExchangeModal', () => { executeBtn().click(); await flush(); await flush(); - expect(statusText()).toBe(`The exchange may have been submitted to Azure. Check the RI exchange history (or the Azure portal) before trying again. Details: ${detail}`); + expect(statusText()).toBe(`The exchange may have been submitted to Azure. Verify the reservation state in the Azure portal (the reservation order's history) before trying again; the RI exchange history here does not list Azure exchanges. Details: ${detail}`); expect(onExecuted).toHaveBeenCalledTimes(1); expect(executeBtn().classList.contains('hidden')).toBe(true); expect($('.azure-exchange-guardrails').classList.contains('hidden')).toBe(true); @@ -261,6 +262,66 @@ describe('openAzureExchangeModal', () => { expect(api.executeAzureExchange).toHaveBeenCalledTimes(1); }); + describe('when the modal is closed while execute is in flight', () => { + // Starts execute, closes the modal while the request is pending, then + // settles the request. + async function executeThenClose(settle: (resolve: (v: unknown) => void, reject: (e: unknown) => void) => void): Promise { + let resolve!: (v: unknown) => void; + let reject!: (e: unknown) => void; + (api.executeAzureExchange as jest.Mock).mockReturnValue(new Promise((res, rej) => { resolve = res; reject = rej; })); + await openAndPreview(); + setValue($('#azure-exchange-max-payment'), '150'); + setValue($('#azure-exchange-currency'), 'USD'); + executeBtn().click(); + await flush(); + expect(api.executeAzureExchange).toHaveBeenCalledTimes(1); + Array.from(document.querySelectorAll('.modal-buttons button')).find(b => b.textContent === 'Cancel')!.click(); + expect($('#ri-exchange-modal').classList.contains('hidden')).toBe(true); + expect(showToast).not.toHaveBeenCalled(); + settle(resolve, reject); + await flush(); + await flush(); + } + + it('shows a success as a toast', async () => { + await executeThenClose(ok => ok({ session_id: 's2', status: 'Succeeded', net_payable: 118, net_payable_currency: 'USD', refunds_total: 780.25, purchases_total: 898.25 })); + expect(onExecuted).toHaveBeenCalledTimes(1); + expect(showToast).toHaveBeenCalledTimes(1); + expect(showToast).toHaveBeenCalledWith({ message: expect.stringMatching(/^Exchange Succeeded\. Net payable USD 118\.00.*Session s2\.$/), kind: 'success' }); + }); + + it('shows a 4xx refusal as an error toast', async () => { + await executeThenClose((_ok, fail) => fail(Object.assign(new Error('quoted net payable 160.00 USD exceeds max_payment_due 150.00 USD'), { status: 422 }))); + expect(onExecuted).not.toHaveBeenCalled(); + expect(showToast).toHaveBeenCalledWith({ + message: 'Exchange blocked by a safety check: quoted net payable 160.00 USD exceeds max_payment_due 150.00 USD. Raise the maximum payment or change the targets, then preview again.', + kind: 'error', + }); + }); + + it('shows an unknown outcome as a sticky warning toast', async () => { + await executeThenClose((_ok, fail) => fail(Object.assign(new Error('HTTP 504'), { status: 504 }))); + expect(onExecuted).toHaveBeenCalledTimes(1); + expect(showToast).toHaveBeenCalledWith({ + message: expect.stringMatching(/^The exchange may have been submitted to Azure\. Verify the reservation state in the Azure portal.*Details: HTTP 504$/), + kind: 'warning', + timeout: null, + }); + }); + }); + + it('does not toast the outcome while the modal is open', async () => { + (api.executeAzureExchange as jest.Mock).mockRejectedValue(Object.assign(new Error('HTTP 504'), { status: 504 })); + await openAndPreview(); + setValue($('#azure-exchange-max-payment'), '150'); + setValue($('#azure-exchange-currency'), 'USD'); + executeBtn().click(); + await flush(); + await flush(); + expect(statusText()).toMatch(/^The exchange may have been submitted/); + expect(showToast).not.toHaveBeenCalled(); + }); + it('previews but hides Execute without execute:ri-exchange', async () => { grant('view:purchases'); await openAndPreview(); diff --git a/frontend/src/api/riexchange.ts b/frontend/src/api/riexchange.ts index d6765c346..197814ff3 100644 --- a/frontend/src/api/riexchange.ts +++ b/frontend/src/api/riexchange.ts @@ -59,6 +59,14 @@ export async function getAzureCompatibleOfferings(req: AzureCompatibleOfferingsR }); } +/** + * Client timeout for executeAzureExchange. The backend runs the submit and + * its long-running-operation poll on a detached 4-minute budget + * (riExchangeExecutionTimeout), so the default 90s timeout would report many + * slow but successful exchanges as an unknown outcome. + */ +const AZURE_EXCHANGE_EXECUTE_TIMEOUT_MS = 5 * 60_000; + /** * Execute an Azure RI exchange (issue #81). The backend re-quotes and refuses * with a 422 when the fresh quote has policy errors, no net payable, another @@ -68,6 +76,7 @@ export async function executeAzureExchange(req: AzureExecuteExchangeRequest): Pr return apiRequest('/ri-exchange/azure-instances/exchange', { method: 'POST', body: JSON.stringify(req), + timeoutMs: AZURE_EXCHANGE_EXECUTE_TIMEOUT_MS, }); } diff --git a/frontend/src/lib/azure-exchange.ts b/frontend/src/lib/azure-exchange.ts index 1f10ce8f3..fbef03bbe 100644 --- a/frontend/src/lib/azure-exchange.ts +++ b/frontend/src/lib/azure-exchange.ts @@ -134,7 +134,10 @@ export function describeAzureExchangeError(error: unknown, phase: 'preview' | 'e const err = (error ?? {}) as ApiErrorLike; const msg = err.message || 'Unknown error'; if (phase === 'execute' && isAzureExchangeOutcomeUnknown(error)) { - return `The exchange may have been submitted to Azure. Check the RI exchange history (or the Azure portal) before trying again. Details: ${msg}`; + // Point only at the Azure portal, as the backend's own 500 text does: + // direct Azure executions are audited in ri_exchange_direct_executions, + // which nothing serves, so the tab's history can never show them. + return `The exchange may have been submitted to Azure. Verify the reservation state in the Azure portal (the reservation order's history) before trying again; the RI exchange history here does not list Azure exchanges. Details: ${msg}`; } const what = phase === 'preview' ? 'Could not preview the exchange' : 'Exchange not executed'; switch (err.status) { diff --git a/frontend/src/riexchange-azure-modal.ts b/frontend/src/riexchange-azure-modal.ts index 0330878bf..0d46ea780 100644 --- a/frontend/src/riexchange-azure-modal.ts +++ b/frontend/src/riexchange-azure-modal.ts @@ -13,6 +13,7 @@ import * as api from './api'; import type { AzureCompatibleOfferingsResponse, AzureExecuteExchangeRequest, ExchangeableAzureRI } from './api'; import { openModal, closeModal } from './modal'; import { confirmDialog } from './confirmDialog'; +import { showToast } from './toast'; import { isRIExchanger } from './permissions'; import { AZURE_EXCHANGE_TERMS, MAX_AZURE_EXCHANGE_ITEMS, azureTermLabel, describeAzureExchangeError, formatAzureAmount, @@ -343,16 +344,26 @@ export function openAzureExchangeModal(sources: readonly ExchangeableAzureRI[], setMessage(statusBox, 'Executing exchange...', 'loading'); const result = await api.executeAzureExchange(req); onExecuted(); - if (!sessionOpen) return; - setMessage(statusBox, `Exchange ${result.status || 'submitted'}. Net payable ${formatAzureAmount(result.net_payable, result.net_payable_currency)}; refunds ${formatAzureAmount(result.refunds_total, result.net_payable_currency)}; purchases ${formatAzureAmount(result.purchases_total, result.net_payable_currency)}. Session ${result.session_id}.`, 'success-message'); + const done = `Exchange ${result.status || 'submitted'}. Net payable ${formatAzureAmount(result.net_payable, result.net_payable_currency)}; refunds ${formatAzureAmount(result.refunds_total, result.net_payable_currency)}; purchases ${formatAzureAmount(result.purchases_total, result.net_payable_currency)}. Session ${result.session_id}.`; + // Closed mid-execute: the outcome still has to reach the user. + if (!sessionOpen) { + showToast({ message: done, kind: 'success' }); + return; + } + setMessage(statusBox, done, 'success-message'); retirePreview(); } catch (error) { // An unknown outcome may still commit at Azure: refresh the list and // history, and retire this preview so a retry needs a fresh one. const unknown = isAzureExchangeOutcomeUnknown(error); if (unknown) onExecuted(); - if (!sessionOpen) return; - setMessage(statusBox, describeAzureExchangeError(error, 'execute'), 'error'); + const text = describeAzureExchangeError(error, 'execute'); + if (!sessionOpen) { + // An unknown outcome stays until dismissed: it may mean money moved. + showToast(unknown ? { message: text, kind: 'warning', timeout: null } : { message: text, kind: 'error' }); + return; + } + setMessage(statusBox, text, 'error'); if (unknown) retirePreview(); } finally { executeBtn.disabled = false;