diff --git a/frontend/src/__tests__/riexchange-audit-321.test.ts b/frontend/src/__tests__/riexchange-audit-321.test.ts new file mode 100644 index 000000000..2e737ebfc --- /dev/null +++ b/frontend/src/__tests__/riexchange-audit-321.test.ts @@ -0,0 +1,120 @@ +/** + * Issue #321 (2026-09-02 audit, A12-067 and A12-069): + * - target-picker option labels are set via textContent, so they must not + * be pre-escaped (that rendered "&" literally); + * - a failed exchange's error reason is shown in the history Status cell. + */ + +jest.mock('../api', () => ({ + listConvertibleRIs: jest.fn().mockResolvedValue([]), + getRIUtilization: jest.fn().mockResolvedValue([]), + getReshapeRecommendations: jest.fn().mockResolvedValue({ recommendations: [], recs_staleness: '', recs_collected_at: null }), + getRIExchangeHistory: jest.fn(), + listTargetOfferings: jest.fn(), +})); + +jest.mock('../navigation', () => ({ + switchTab: jest.fn(), + switchSettingsSubTab: jest.fn(), +})); + +jest.mock('../state', () => ({ + getCurrentUser: jest.fn(() => null), + getCurrentProvider: jest.fn(() => 'aws'), + getCurrentAccountIDs: jest.fn(() => []), + getRiExchangeColumnFilters: jest.fn(() => ({})), + setRiExchangeColumnFilter: jest.fn(), + clearAllRiExchangeColumnFilters: jest.fn(), + getActiveRiColumnFilters: jest.fn(() => ({})), + setActiveRiColumnFilter: jest.fn(), + clearAllActiveRiColumnFilters: jest.fn(), +})); + +import * as api from '../api'; +import { loadRIExchange, openExchangeModal } from '../riexchange'; + +const flush = () => new Promise((resolve) => setTimeout(resolve, 0)); + +function historyRecord(overrides: Record) { + return { + id: 'h1', account_id: 'a', exchange_id: '', region: 'us-east-1', source_ri_ids: ['ri-1'], + source_instance_type: 'm5.large', source_count: 2, target_offering_id: 'o', target_instance_type: 'm5.xlarge', + target_count: 1, payment_due: '0.00', status: 'failed', mode: 'manual', + created_at: '2026-01-01T00:00:00Z', updated_at: '2026-01-01T00:00:00Z', + ...overrides, + }; +} + +describe('RI exchange target picker labels (A12-067)', () => { + let modal: HTMLDivElement; + + beforeEach(() => { + modal = document.createElement('div'); + modal.id = 'ri-exchange-modal'; + modal.className = 'modal hidden'; + const content = document.createElement('div'); + content.className = 'modal-content'; + modal.appendChild(content); + document.body.replaceChildren(modal); + }); + + function optionTexts(): string[] { + const picker = modal.querySelector('.modal-exchange-target-select'); + return picker ? Array.from(picker.options).map((o) => o.textContent ?? '') : []; + } + + test('AWS offering labels keep &, < and quotes as typed', async () => { + (api.listTargetOfferings as jest.Mock).mockResolvedValueOnce([ + { offering_id: 'off-1', instance_type: 'm5.large', offering_type: 'Partial & "Light" ' }, + ]); + openExchangeModal('ri-abc', 2); + await flush(); + + expect(optionTexts()).toContain('m5.large -- Partial & "Light" '); + expect(optionTexts().join('\n')).not.toContain('&'); + }); + + test('CE alternative labels are not pre-escaped', async () => { + (api.listTargetOfferings as jest.Mock).mockResolvedValueOnce([]); + openExchangeModal('ri-abc', 2, undefined, [ + { instance_type: 'x2&.large', offering_id: 'off-alt', effective_monthly_cost: 12 }, + ]); + await flush(); + + const label = optionTexts().find((t) => t.startsWith('x2')); + expect(label).toMatch(/^x2&\.large -- /); + expect(label).not.toContain('&'); + }); +}); + +describe('RI exchange history failure reason (A12-069)', () => { + beforeEach(() => { + const ids = ['ri-exchange-instances-list', 'ri-exchange-recommendations-list', 'ri-exchange-history-list']; + document.body.replaceChildren(...ids.map((id) => Object.assign(document.createElement('div'), { id }))); + }); + + function statusCell(): HTMLTableCellElement { + const row = document.querySelector('#ri-exchange-history-list tbody tr')!; + return row.querySelectorAll('td')[4]!; + } + + test('shows the error reason, as text, under a failed badge', async () => { + (api.getRIExchangeHistory as jest.Mock).mockResolvedValue([ + historyRecord({ error: 'AcceptReservedInstancesExchangeQuote: InvalidParameter' }), + ]); + await loadRIExchange(); + + const cell = statusCell(); + expect(cell.querySelector('.status-badge')?.textContent).toBe('failed'); + const detail = cell.querySelector('.status-detail'); + expect(detail?.textContent).toBe('AcceptReservedInstancesExchangeQuote: InvalidParameter'); + expect(detail?.querySelector('b')).toBeNull(); + }); + + test('renders no detail line when the record has no error', async () => { + (api.getRIExchangeHistory as jest.Mock).mockResolvedValue([historyRecord({ status: 'completed' })]); + await loadRIExchange(); + + expect(statusCell().querySelector('.status-detail')).toBeNull(); + }); +}); diff --git a/frontend/src/riexchange.ts b/frontend/src/riexchange.ts index ca007092e..fcbc2e90d 100644 --- a/frontend/src/riexchange.ts +++ b/frontend/src/riexchange.ts @@ -1139,8 +1139,8 @@ export function openExchangeModal(riId: string, count: number, suggestedTargetTy const opt = document.createElement('option'); opt.value = o.offering_id; // Display: "m5.large -- No Upfront" (no HTML injection: all fields from API) - const label = escapeHtml(o.instance_type) + (o.offering_type ? ' -- ' + escapeHtml(o.offering_type) : ''); - opt.textContent = label; + // textContent escapes on its own; pre-escaping would show "&". + opt.textContent = o.instance_type + (o.offering_type ? ' -- ' + o.offering_type : ''); if (initialOfferingId && o.offering_id === initialOfferingId) { opt.selected = true; } @@ -1157,7 +1157,7 @@ export function openExchangeModal(riId: string, count: number, suggestedTargetTy const opt = document.createElement('option'); opt.value = alt.offering_id; const costStr = formatOfferingCost(alt); - opt.textContent = escapeHtml(alt.instance_type) + ' -- ' + costStr + '/mo'; + opt.textContent = alt.instance_type + ' -- ' + costStr + '/mo'; if (initialOfferingId && alt.offering_id === initialOfferingId) { opt.selected = true; } @@ -1990,6 +1990,11 @@ function renderExchangeHistory(container: HTMLElement, records: RIExchangeHistor const exchangeIdCell = rec.exchange_id ? '' + escapeHtml(rec.exchange_id) + '' : '—'; + // A failed exchange carries the AWS reason in rec.error; show it under the + // badge so the operator can see why without reading backend logs. + const errorLine = rec.error + ? '
' + escapeHtml(rec.error) + '
' + : ''; const approveBtn = canApproveRIExchangeRow(rec) ? '' : ''; @@ -2000,7 +2005,7 @@ function renderExchangeHistory(container: HTMLElement, records: RIExchangeHistor // payment_due is USD (the execute cap is max_payment_due_usd); an // empty amount renders as a dash, not a lone "$" (issue #284). + '' + (rec.payment_due ? '$' + escapeHtml(rec.payment_due) : '—') + '' - + '' + escapeHtml(rec.status) + '' + + '' + escapeHtml(rec.status) + '' + errorLine + '' + '' + exchangeIdCell + '' + '' + approveBtn + '' + ''; diff --git a/frontend/src/styles/components.css b/frontend/src/styles/components.css index 7359b1c44..e65a0835a 100644 --- a/frontend/src/styles/components.css +++ b/frontend/src/styles/components.css @@ -1452,15 +1452,24 @@ tr.rec-variant-row:hover td:not(.checkbox-col) { /* Subline rendered under the Pending badge in the Purchase History Status * column. Surfaces the approver's email address (the recipient of the - * approval email) so the user knows which inbox to check. + * approval email) so the user knows which inbox to check. .status-detail + * reuses it for the failure reason under an RI exchange history badge. */ -.history-approver { +.history-approver, +.status-detail { margin-top: 0.35rem; font-size: 0.78rem; color: var(--cudly-text-muted); line-height: 1.25; } +/* Cap the failure reason so a long AWS error wraps instead of squeezing the + * other history columns. */ +.status-detail { + max-width: 18rem; + overflow-wrap: anywhere; +} + .history-approver strong { color: var(--cudly-text); font-weight: 600;