diff --git a/frontend/src/__tests__/history-marketplace-sell-button.test.ts b/frontend/src/__tests__/history-marketplace-sell-button.test.ts index 6730be160..9870779aa 100644 --- a/frontend/src/__tests__/history-marketplace-sell-button.test.ts +++ b/frontend/src/__tests__/history-marketplace-sell-button.test.ts @@ -77,6 +77,7 @@ jest.mock('../state', () => ({ import * as api from '../api'; import { getCurrentUser } from '../state'; import { confirmDialog } from '../confirmDialog'; +import { formatCurrency } from '../utils'; // Administrators group GUID -- mirrors ADMINISTRATORS_GROUP_ID in // frontend/src/permissions.ts. Without this, isAdmin() returns false and @@ -452,6 +453,36 @@ describe('Marketplace consent modal residual proration (issue #808 follow-up)', expect(text).not.toContain('$585'); }); + // Issue #318: list price, AWS fee and net proceeds were each rounded to whole + // dollars on their own, so a $142.50 listing showed $143 = $17 + $125. Run + // the real formatter and check that the three lines reconcile to the cent. + test('issue #318: list price, fee and net proceeds show cents and add up', async () => { + const realFormat = jest.requireActual('../utils').formatCurrency; + (formatCurrency as jest.Mock).mockImplementation(realFormat); + try { + const PARTWAY = new Date(Date.now() - 2.5 * 30.4375 * 24 * 60 * 60 * 1000).toISOString(); + (api.getHistory as jest.Mock).mockResolvedValue({ + summary: {}, + purchases: [makeRow({ + purchase_id: 'ri-cents', offering_class: 'standard', term: 1, timestamp: PARTWAY, + count: 1, upfront_cost: 200, monthly_cost: 0, + })], + }); + await loadHistory(); + document.querySelector('.history-marketplace-sell-btn[data-marketplace-sell-id="ri-cents"]')!.click(); + await Promise.resolve(); + await Promise.resolve(); + + const arg = (confirmDialog as jest.Mock).mock.calls[0][0] as { body: HTMLElement }; + const values = Array.from(arg.body.querySelectorAll('dd')).map(dd => dd.textContent); + // remaining 9 of 12 months: residual 150, list 142.50, net 142.50 * 0.88 = 125.40. + expect(values).toEqual(expect.arrayContaining(['$142.50', '$17.10', '$125.40'])); + expect(values).not.toContain('$143'); + } finally { + (formatCurrency as jest.Mock).mockImplementation((val) => `$${val || 0}`); + } + }); + test('shows default price as unavailable for a no-upfront RI instead of a fabricated price', async () => { const JUST_NOW = new Date(Date.now() - 24 * 60 * 60 * 1000).toISOString(); (api.getHistory as jest.Mock).mockResolvedValue({ diff --git a/frontend/src/__tests__/history.test.ts b/frontend/src/__tests__/history.test.ts index f2b8ebf97..564e4ba3e 100644 --- a/frontend/src/__tests__/history.test.ts +++ b/frontend/src/__tests__/history.test.ts @@ -33,6 +33,7 @@ jest.mock('../utils', () => ({ if (!str) return ''; return str.replace(/&/g, '&').replace(//g, '>').replace(/"/g, '"').replace(/'/g, '''); }), + amortizedMonthly: jest.requireActual('../utils').amortizedMonthly, populateAccountFilter: jest.fn(() => Promise.resolve()) })); @@ -105,6 +106,30 @@ describe('History Module', () => { expect(queue?.querySelectorAll('td')[8]?.textContent).toBe('--'); }); + // Issue #318: with amortize on, the upfront is already in Monthly Cost, so + // the Upfront cell is muted and labeled to keep it from reading as additive. + test.each([false, true])('upfront folded into monthly with amortization %s is labeled only when on', async (amortize) => { + (getAmortizeUpfront as jest.Mock).mockReturnValue(amortize); + const purchase = { + timestamp: '2026-09-29T00:00:00Z', provider: 'aws', service: 'ec2', resource_type: 'm5.large', + region: 'us-east-1', count: 1, term: 1, upfront_cost: 1200, monthly_cost: 100, estimated_savings: 300, + }; + (api.getHistory as jest.Mock).mockResolvedValue({ summary: {}, purchases: [ + { ...purchase, purchase_id: 'complete', status: 'completed' }, + { ...purchase, purchase_id: 'pending', status: 'pending' }, + ] }); + await loadHistory(); + const historyCells = document.querySelectorAll('#history-list tr[data-execution-id="complete"] td'); + const queueCells = document.querySelectorAll('#purchases-approval-queue tr[data-execution-id="pending"] td'); + expect(historyCells[9]?.textContent).toBe(amortize ? '$200' : '$100'); + expect(queueCells[7]?.textContent).toBe(amortize ? '$200' : '$100'); + for (const upfront of [historyCells[8], queueCells[8]]) { + expect(upfront?.classList.contains('upfront-in-monthly')).toBe(amortize); + expect(upfront?.textContent).toBe(amortize ? '$1200(in monthly)' : '$1200'); + expect(upfront?.getAttribute('title')).toBe(amortize ? 'Included in the amortized monthly cost' : null); + } + }); + describe('initHistoryDateRange', () => { test('sets default date range to 7 days', () => { initHistoryDateRange(); diff --git a/frontend/src/__tests__/settings.test.ts b/frontend/src/__tests__/settings.test.ts index 0736b4e97..46d34f3be 100644 --- a/frontend/src/__tests__/settings.test.ts +++ b/frontend/src/__tests__/settings.test.ts @@ -1020,6 +1020,31 @@ describe('Settings Module', () => { expect(isUnsavedChanges()).toBe(false); }); + test.each(['12.7', '-40', '500'])('an SP card coverage of %s aborts the save before any PUT (#318)', async (value) => { + await loadWithCoverage50(); + mockShowToast.mockClear(); + (document.getElementById('aws-savings-plans-compute-coverage') as HTMLInputElement).value = value; + + await saveGlobalSettings({ preventDefault: jest.fn() } as unknown as Event); + + expect(api.updateConfig).not.toHaveBeenCalled(); + expect(api.updateServiceConfig).not.toHaveBeenCalled(); + expect(mockShowToast).toHaveBeenCalledWith(expect.objectContaining({ + kind: 'error', + message: expect.stringContaining('whole number between 0 and 100 (savings-plans-compute)'), + })); + expect(isUnsavedChanges()).toBe(true); + }); + + test.each([['0', 0], ['100', 100]])('an SP card coverage of %s is within range and saved (#318)', async (value, sent) => { + await loadWithCoverage50(); + (document.getElementById('aws-savings-plans-compute-coverage') as HTMLInputElement).value = value; + + await saveGlobalSettings({ preventDefault: jest.fn() } as unknown as Event); + + expect(coverageSentFor('savings-plans-compute')).toBe(sent); + }); + test('a saved service payment edit leaves nothing unsaved (#576)', async () => { await loadWithCoverage50(); (document.getElementById('aws-ec2-payment') as HTMLSelectElement).value = 'partial-upfront'; diff --git a/frontend/src/history.ts b/frontend/src/history.ts index b4c183d44..5051f8ad3 100644 --- a/frontend/src/history.ts +++ b/frontend/src/history.ts @@ -562,6 +562,16 @@ function providerCell(p: HistoryPurchase): string { return `${safeProvider.toUpperCase()}`; } +// upfrontCell renders the Upfront Cost cell. With amortize on, the upfront is +// already spread into the Monthly Cost column, so the cell is muted and +// labeled to stop Upfront + 12 x Monthly being read as additive (#318). +function upfrontCell(p: HistoryPurchase, amortize: boolean): string { + const amount = formatCurrency(p.upfront_cost); + const folded = amortize && p.monthly_cost != null && p.upfront_cost != null && p.upfront_cost > 0 && p.term > 0; + if (!folded) return `${amount}`; + return `${amount}
(in monthly)
`; +} + // canCancelPendingRow returns true when the current session is permitted // to cancel the given pending/notified history row via the session-authed // Cancel button (issue #46). UX gate only — the backend @@ -1215,6 +1225,7 @@ function renderHistoryList(purchases: HistoryPurchase[]): void { const colFilters = state.getPurchaseHistoryColumnFilters(); const visible = applyPurchaseHistoryColumnFilters(statusFiltered, colFilters); + const amortize = state.getAmortizeUpfront(); const tableRows = visible.map(p => { const statusCell = (() => { const badge = statusBadgeHTML(normalizeStatus(p)); @@ -1229,7 +1240,6 @@ function renderHistoryList(purchases: HistoryPurchase[]): void { })(); const execIdAttr = p.purchase_id ? ` data-execution-id="${escapeHtmlAttr(p.purchase_id)}"` : ''; const planCellContent = renderActionCell(p); - const amortize = state.getAmortizeUpfront(); const rawMonthly = p.monthly_cost != null ? p.monthly_cost : null; const displayMonthly = (rawMonthly != null && amortize) ? (p.upfront_cost == null ? null : amortizedMonthly(rawMonthly, p.upfront_cost, p.term)) @@ -1247,7 +1257,7 @@ function renderHistoryList(purchases: HistoryPurchase[]): void { ${escapeHtml(p.region)} ${p.count} ${formatTerm(p.term)} - ${formatCurrency(p.upfront_cost)} + ${upfrontCell(p, amortize)} ${monthlyCostCell} ${formatCurrency(p.estimated_savings)} ${planCellContent} @@ -1255,7 +1265,6 @@ function renderHistoryList(purchases: HistoryPurchase[]): void { `; }).join(''); - const amortize = state.getAmortizeUpfront(); const monthlyColHeader = amortize ? 'Monthly Cost (amortized)' : 'Monthly Cost'; const fbtn = (col: PurchaseHistoryColumnId): string => renderHistoryFilterButton( col, PURCHASE_HISTORY_LABELS[col], colFilters[col] != null, @@ -1582,11 +1591,17 @@ function wireRowActionHandlers(container: HTMLElement): void { addRow('Resource type', purchase.resource_type || '-'); addRow('Remaining term', remainingMonths === 1 ? '1 month' : `${remainingMonths} months`); if (listPricePerUnit > 0) { + // Show cents, and derive the fee from the two rounded totals, so + // the three lines always reconcile (fee + net = list price). The + // dashboard's whole-dollar default rounded each line on its own, + // e.g. $101 = $12 + $88 for a $100.50 listing (#318). + const listCents = Math.round(listPriceTotal * 100); + const netCents = Math.round(netProceedsTotal * 100); addRow('Default list price', count > 1 - ? `${formatCurrency(listPricePerUnit)}/unit (${formatCurrency(listPriceTotal)} total for ${count} units)` - : formatCurrency(listPriceTotal)); - addRow(`AWS fee (${AWS_MARKETPLACE_FEE_PERCENT}%)`, formatCurrency(listPriceTotal * (AWS_MARKETPLACE_FEE_PERCENT / 100))); - addRow('Estimated net proceeds', formatCurrency(netProceedsTotal)); + ? `${formatCurrency(listPricePerUnit, '$', 2)}/unit (${formatCurrency(listCents / 100, '$', 2)} total for ${count} units)` + : formatCurrency(listCents / 100, '$', 2)); + addRow(`AWS fee (${AWS_MARKETPLACE_FEE_PERCENT}%)`, formatCurrency((listCents - netCents) / 100, '$', 2)); + addRow('Estimated net proceeds', formatCurrency(netCents / 100, '$', 2)); } else { // No default price can be computed (no upfront cost or unknown // term) -- listing will be rejected server-side unless a custom @@ -1837,6 +1852,7 @@ export function renderApprovalQueue(purchases: HistoryPurchase[]): void { const colFilters = state.getApprovalQueueColumnFilters(); const visible = applyApprovalQueueColumnFilters(pending, colFilters); + const amortize = state.getAmortizeUpfront(); const rows = visible.map(p => { const actions = renderPendingActionButtons(p); const actionsCell = actions || '-'; @@ -1852,7 +1868,6 @@ export function renderApprovalQueue(purchases: HistoryPurchase[]): void { : '-'; const termCell = p.term ? escapeHtml(formatTerm(p.term)) : '-'; const paymentCell = p.payment ? escapeHtml(p.payment) : '-'; - const amortize = state.getAmortizeUpfront(); const rawMonthly = p.monthly_cost != null ? p.monthly_cost : null; const displayMonthly = (rawMonthly != null && amortize) ? (p.upfront_cost == null ? null : amortizedMonthly(rawMonthly, p.upfront_cost, p.term)) @@ -1871,7 +1886,7 @@ export function renderApprovalQueue(purchases: HistoryPurchase[]): void { ${termCell} ${paymentCell} ${monthlyCostCell} - ${formatCurrency(p.upfront_cost)} + ${upfrontCell(p, amortize)} ${formatCurrency(p.estimated_savings)} ${createdBy} ${actionsCell} @@ -1879,7 +1894,6 @@ export function renderApprovalQueue(purchases: HistoryPurchase[]): void { `; }).join(''); - const amortize = state.getAmortizeUpfront(); const monthlyColHeader = amortize ? 'Monthly Cost (amortized)' : 'Monthly Cost'; // monthlyColHeader is a hardcoded constant string (no user data), so // interpolating it directly into the template is safe. diff --git a/frontend/src/settings.ts b/frontend/src/settings.ts index efbed45af..e0c9c6f3f 100644 --- a/frontend/src/settings.ts +++ b/frontend/src/settings.ts @@ -3435,6 +3435,26 @@ async function renderAccountsOverview(): Promise { }); } +/** + * Names of the per-Savings-Plan coverage cards whose edited value is not a + * whole number between 0 and 100. Untouched cards and empty inputs are not + * checked: the save keeps their persisted value, as it always has. + */ +function invalidEditedCardCoverages(): string[] { + const bad: string[] = []; + for (const field of SERVICE_FIELDS) { + if (!('coverageId' in field) || !field.coverageId) continue; + const input = byId(field.coverageId); + const raw = input?.value ?? ''; + if (raw.trim() === '' || getFieldValue(field.coverageId) === savedSnapshot[field.coverageId]) continue; + const parsed = Number(raw); + if (Number.isInteger(parsed) && parsed >= 0 && parsed <= 100) continue; + const title = input?.closest('.service-default-card')?.querySelector('h5')?.textContent?.trim(); + bad.push(title || field.service); + } + return bad; +} + /** * Save global settings */ @@ -3519,6 +3539,17 @@ export async function saveGlobalSettings(e: Event): Promise { saveInFlight = false; return; } + // The per-Savings-Plan coverage cards sit outside the settings
, so + // native min/max validation never runs for them and the inline hint only + // paints a message. Apply the global field's whole-number 0..100 rule here, + // before any PUT, so a fractional value like 12.7 is not persisted. + const badCoverageCards = invalidEditedCardCoverages(); + if (badCoverageCards.length > 0) { + showToast({ message: `Target coverage must be a whole number between 0 and 100 (${badCoverageCards.join(', ')})`, kind: 'error' }); + if (saveBtn) saveBtn.disabled = false; + saveInFlight = false; + return; + } const rawOfferingClass = byId('setting-ec2-offering-class')?.value ?? 'convertible'; const offeringClass: 'convertible' | 'standard' = (rawOfferingClass === 'standard') ? 'standard' : 'convertible'; diff --git a/frontend/src/styles/components.css b/frontend/src/styles/components.css index 5d76ec88e..60a922e45 100644 --- a/frontend/src/styles/components.css +++ b/frontend/src/styles/components.css @@ -1440,6 +1440,18 @@ tr.rec-variant-row:hover td:not(.checkbox-col) { font-weight: 600; } +/* Upfront Cost cell when the amortize toggle folds the upfront into the + * Monthly Cost column (#318): muted so the two are not read as additive. + */ +.upfront-in-monthly { + color: var(--cudly-text-muted); +} + +.upfront-in-monthly-note { + font-size: 0.78rem; + line-height: 1.25; +} + /* Purchase detail status badges */ .status-badge.completed { background: var(--cudly-success-bg); diff --git a/frontend/tests-e2e/amortized-upfront.spec.ts b/frontend/tests-e2e/amortized-upfront.spec.ts new file mode 100644 index 000000000..4309e7263 --- /dev/null +++ b/frontend/tests-e2e/amortized-upfront.spec.ts @@ -0,0 +1,32 @@ +import { test, expect } from '@playwright/test'; +import { mockApi, seedAuth } from './fixtures/recs'; + +// Issue #318: with amortize on, the upfront is already spread into Monthly +// Cost, so the Upfront cell is muted and labeled instead of reading as a +// separate lump sum on top of the amortized monthly figure. +test('amortize toggle labels the upfront as included in the monthly cost', async ({ page }) => { + await seedAuth(page); + await mockApi(page); + const purchase = { + purchase_id: '11111111-1111-1111-1111-111111111111', timestamp: new Date().toISOString(), + provider: 'aws', service: 'ec2', resource_type: 'm5.large', region: 'us-east-1', + count: 1, term: 1, upfront_cost: 1200, monthly_cost: 100, estimated_savings: 300, + offering_class: 'convertible', status: 'completed', account_id: 'account-1', + }; + await page.route('**/api/history**', route => route.fulfill({ json: { summary: {}, purchases: [purchase] } })); + + await page.goto('/purchases/history'); + const row = page.locator('#history-list tbody tr').first(); + const upfront = row.locator('td').nth(8); + await expect(upfront).toHaveText('$1,200'); + await expect(upfront).not.toHaveClass(/upfront-in-monthly/); + + await page.locator('#history-amortize-checkbox').check(); + await expect(row.locator('td').nth(9)).toHaveText('$200'); + await expect(upfront).toHaveClass(/upfront-in-monthly/); + await expect(upfront).toHaveAttribute('title', 'Included in the amortized monthly cost'); + await expect(upfront).toContainText('(in monthly)'); + + await page.locator('#history-amortize-checkbox').uncheck(); + await expect(upfront).toHaveText('$1,200'); +});