From 121e54918fe3652a61fee44651060190a55d0fac Mon Sep 17 00:00:00 2001 From: Astrid Gealer Date: Fri, 23 Oct 2026 22:13:17 +0000 Subject: [PATCH] fix(frontend): show the monthly charge and term total on every purchase screen The bulk Review Purchase modal only stated "Total upfront", which reads $0 for no-upfront and partial-upfront buckets that bill monthly for one or three years. Its confirm dialog stated no amounts at all, the Execute Now warning said "charge $0.00 upfront" for a monthly-billed purchase, and the approval dialog showed the upfront and the savings but not the monthly cost, so an approver could not judge the commitment. commitmentSummary.ts already computed upfront, monthly and total over term for the Execute now / Run now confirms. Add commitmentLine (rec to line), formatCommitmentTotals (the three totals as text, never a partial monthly sum) and buildCommitmentBand, and use them in: - the bulk modal header (Monthly recurring, Total over term) and each bucket's totals line (monthly recurring), - the bulk confirm dialog (the same stat band and line list as the single-purchase confirm), - the single purchase modal (a band under the table for the checked rows) and its Execute Now warning, - the approval dialog header (Monthly cost, Total over term), laid out four stats per row so the eight stats do not leave an orphan. Refs #711 Co-Authored-By: Claude Opus 5.5 --- .../src/__tests__/approval-details.test.ts | 45 ++++++++--- .../src/__tests__/commitmentSummary.test.ts | 49 ++++++++++- .../purchase-execution-toast.test.ts | 2 + .../__tests__/purchase-modal-submit.test.ts | 74 +++++++++++++++++ frontend/src/app.ts | 30 +++---- frontend/src/approval-details.ts | 13 ++- frontend/src/commitmentSummary.ts | 81 ++++++++++++++++--- frontend/src/recommendations.ts | 48 +++++++++-- frontend/src/styles/components.css | 4 +- .../tests-e2e/execute-now-warning.spec.ts | 10 ++- 10 files changed, 305 insertions(+), 51 deletions(-) diff --git a/frontend/src/__tests__/approval-details.test.ts b/frontend/src/__tests__/approval-details.test.ts index 67e1f10cb..7812f9e56 100644 --- a/frontend/src/__tests__/approval-details.test.ts +++ b/frontend/src/__tests__/approval-details.test.ts @@ -76,7 +76,7 @@ describe('renderApprovalDetailsBody', () => { const body = renderApprovalDetailsBody(details, accountsById); const stats = body.querySelectorAll('.approval-details-stat'); - expect(stats.length).toBe(6); + expect(stats.length).toBe(8); const text = body.textContent || ''; expect(text).toContain('Upfront'); expect(text).toContain('$1,200'); @@ -89,6 +89,27 @@ describe('renderApprovalDetailsBody', () => { expect(text).toContain('Accounts'); }); + it('states the monthly recurring cost and the total over the term (issue #711)', () => { + const recs = [ + makeRec({ id: 'r1', term: 3, payment: 'no-upfront', upfront_cost: 0, monthly_cost: 200 }), + makeRec({ id: 'r2', term: 1, payment: 'partial-upfront', upfront_cost: 600, monthly_cost: 50 }), + ]; + const body = renderApprovalDetailsBody(makeDetails(recs), new Map()); + const stats = Array.from(body.querySelectorAll('.approval-details-stat')).map((s) => s.textContent); + expect(stats.slice(0, 3)).toEqual(['Upfront$600', 'Monthly cost$250', 'Total over term$8,400']); + }); + + it('says the monthly cost is not reported instead of summing a partial monthly figure', () => { + const recs = [ + makeRec({ id: 'r1', payment: 'no-upfront', upfront_cost: 0, monthly_cost: null }), + makeRec({ id: 'r2', monthly_cost: 50 }), + ]; + const body = renderApprovalDetailsBody(makeDetails(recs), new Map()); + const stats = Array.from(body.querySelectorAll('.approval-details-stat')).map((s) => s.textContent); + expect(stats[1]).toBe('Monthly costNot reported (monthly fees apply)'); + expect(stats[2]).toBe('Total over termNot available'); + }); + it('renders the per-rec table with all 13 columns (Monthly cost added by issue #1112)', () => { const rec = makeRec({}); const body = renderApprovalDetailsBody(makeDetails([rec]), new Map()); @@ -184,8 +205,8 @@ describe('renderApprovalDetailsBody', () => { const details = makeDetails(recs, { estimated_savings: 0.3 }); const body = renderApprovalDetailsBody(details, new Map()); const stats = body.querySelectorAll('.approval-details-stat'); - // Stat index 2 is Annual savings. - expect(stats[2]?.getAttribute('title')).toBeNull(); + // Stat index 4 is Annual savings. + expect(stats[4]?.getAttribute('title')).toBeNull(); }); it('annual-savings tooltip DOES fire when backend and per-row sum genuinely disagree', () => { @@ -195,7 +216,7 @@ describe('renderApprovalDetailsBody', () => { const details = makeDetails(recs, { estimated_savings: 5 }); const body = renderApprovalDetailsBody(details, new Map()); const stats = body.querySelectorAll('.approval-details-stat'); - expect(stats[2]?.getAttribute('title')).toBe('$120 from per-row sum'); + expect(stats[4]?.getAttribute('title')).toBe('$120 from per-row sum'); }); it('counts distinct providers and accounts in the header', () => { @@ -206,10 +227,10 @@ describe('renderApprovalDetailsBody', () => { ]; const body = renderApprovalDetailsBody(makeDetails(recs), new Map()); const stats = body.querySelectorAll('.approval-details-stat'); - // Stat index 4 = Providers, index 5 = Accounts (post-Commitments). - expect(stats[3]?.querySelector('.approval-details-stat-value')?.textContent).toBe('3'); // Commitments - expect(stats[4]?.querySelector('.approval-details-stat-value')?.textContent).toBe('3'); // Providers - expect(stats[5]?.querySelector('.approval-details-stat-value')?.textContent).toBe('2'); // Accounts + // Stat index 6 = Providers, index 7 = Accounts (post-Commitments). + expect(stats[5]?.querySelector('.approval-details-stat-value')?.textContent).toBe('3'); // Commitments + expect(stats[6]?.querySelector('.approval-details-stat-value')?.textContent).toBe('3'); // Providers + expect(stats[7]?.querySelector('.approval-details-stat-value')?.textContent).toBe('2'); // Accounts }); // Regression for CR pass-1 (PR #611): host-account rows must be @@ -219,8 +240,8 @@ describe('renderApprovalDetailsBody', () => { delete rec.cloud_account_id; const body = renderApprovalDetailsBody(makeDetails([rec]), new Map(), '909626172446'); const stats = body.querySelectorAll('.approval-details-stat'); - // Stat index 5 = Accounts. - expect(stats[5]?.querySelector('.approval-details-stat-value')?.textContent).toBe('1'); + // Stat index 7 = Accounts. + expect(stats[7]?.querySelector('.approval-details-stat-value')?.textContent).toBe('1'); }); it('counts "Accounts: 2" for a purchase with one host row and one named-account row', () => { @@ -229,7 +250,7 @@ describe('renderApprovalDetailsBody', () => { const namedRec = makeRec({ id: 'r2', provider: 'aws', cloud_account_id: 'acct-uuid-1' }); const body = renderApprovalDetailsBody(makeDetails([hostRec, namedRec]), new Map(), '909626172446'); const stats = body.querySelectorAll('.approval-details-stat'); - expect(stats[5]?.querySelector('.approval-details-stat-value')?.textContent).toBe('2'); + expect(stats[7]?.querySelector('.approval-details-stat-value')?.textContent).toBe('2'); }); it('does NOT count an orphan row (cloud_account_id nil AND provider !== aws) in the Accounts total', () => { @@ -237,7 +258,7 @@ describe('renderApprovalDetailsBody', () => { delete orphanRec.cloud_account_id; const body = renderApprovalDetailsBody(makeDetails([orphanRec]), new Map(), '909626172446'); const stats = body.querySelectorAll('.approval-details-stat'); - expect(stats[5]?.querySelector('.approval-details-stat-value')?.textContent).toBe('0'); + expect(stats[7]?.querySelector('.approval-details-stat-value')?.textContent).toBe('0'); }); it('escapes user-controlled string values in the table', () => { diff --git a/frontend/src/__tests__/commitmentSummary.test.ts b/frontend/src/__tests__/commitmentSummary.test.ts index 03aa4ded6..e4c2dfe2e 100644 --- a/frontend/src/__tests__/commitmentSummary.test.ts +++ b/frontend/src/__tests__/commitmentSummary.test.ts @@ -1,4 +1,7 @@ -import { buildCommitmentSummary, commitmentTotals, paymentLabel, withIntro, type CommitmentLine } from '../commitmentSummary'; +import { + billsMonthly, buildCommitmentBand, buildCommitmentSummary, commitmentLine, commitmentTotals, formatCommitmentTotals, + paymentLabel, withIntro, type CommitmentLine, +} from '../commitmentSummary'; const line = (over: Partial = {}): CommitmentLine => ({ service: 'ec2', @@ -86,3 +89,47 @@ describe('buildCommitmentSummary', () => { expect(body.querySelector('.commitment-summary')).not.toBeNull(); }); }); + +describe('commitmentLine (issue #711)', () => { + const rec = { + service: 'rds', provider: 'aws', region: 'us-east-1', resource_type: 'db.r5.large', + count: 2, term: 3, payment: 'partial-upfront', upfront_cost: 900, monthly_cost: 40, + }; + + it('maps a priced recommendation onto a line', () => { + expect(commitmentLine(rec)).toEqual({ + service: 'rds', provider: 'aws', region: 'us-east-1', resourceType: 'db.r5.large', + count: 2, term: 3, payment: 'partial-upfront', upfront: 900, monthly: 40, + }); + }); + + it('takes a bucket payment and account, and keeps an absent monthly cost as null', () => { + const l = commitmentLine({ ...rec, monthly_cost: undefined }, { payment: 'no-upfront', account: 'prod' }); + expect(l.payment).toBe('no-upfront'); + expect(l.account).toBe('prod'); + expect(l.monthly).toBeNull(); + }); +}); + +describe('formatCommitmentTotals and buildCommitmentBand (issue #711)', () => { + it('formats the three totals at the requested precision', () => { + expect(formatCommitmentTotals([line({ monthly: 100.4, term: 1 })], 0)) + .toEqual({ upfront: '$0', monthly: '$100', termTotal: '$1,205' }); + }); + + it('never prints a partial monthly sum when one line is unreported', () => { + expect(formatCommitmentTotals([line(), line({ monthly: null })])) + .toEqual({ upfront: '$0.00', monthly: 'Not reported (monthly fees apply)', termTotal: 'Not available' }); + }); + + it('billsMonthly is false only when every line is all upfront', () => { + expect(billsMonthly([line({ payment: 'all-upfront' })])).toBe(false); + expect(billsMonthly([line({ payment: 'all-upfront' }), line()])).toBe(true); + }); + + it('builds the band alone, without the line list', () => { + const band = buildCommitmentBand([line({ upfront: 50, monthly: 10, term: 1 })], { upfrontLabel: 'Upfront' }); + expect(band.textContent).toBe('Upfront$50.00Monthly$10.00Total over term$170.00'); + expect(band.querySelector('li')).toBeNull(); + }); +}); diff --git a/frontend/src/__tests__/purchase-execution-toast.test.ts b/frontend/src/__tests__/purchase-execution-toast.test.ts index 6125d7a31..f8c4c6b26 100644 --- a/frontend/src/__tests__/purchase-execution-toast.test.ts +++ b/frontend/src/__tests__/purchase-execution-toast.test.ts @@ -55,6 +55,8 @@ jest.mock('../recommendations', () => ({ fanOutBucketAccountLabel: jest.fn( (b: { recs: Array<{ cloud_account_id?: string }> }) => b.recs[0]?.cloud_account_id || 'Unattributed', ), + // The fan-out confirm names each line's account (issue #711). + getAccountName: jest.fn((id: string) => id), })); jest.mock('../plans', () => ({ diff --git a/frontend/src/__tests__/purchase-modal-submit.test.ts b/frontend/src/__tests__/purchase-modal-submit.test.ts index d9d6ccaee..bb6268fa9 100644 --- a/frontend/src/__tests__/purchase-modal-submit.test.ts +++ b/frontend/src/__tests__/purchase-modal-submit.test.ts @@ -2007,3 +2007,77 @@ describe('Issue #333: bulk purchase buckets never span more than one cloud accou expect(unattributedBucket.recs).toHaveLength(1); }); }); + +describe('Issue #711: purchase screens state the monthly charge and the term total', () => { + // v-1-no: 1 year, no upfront, $800 a month, so the term costs $9,600. + const noUpfront = (): LocalRecommendation => buildRows()[3]!; + + test('the single purchase modal shows upfront, monthly and total over term for the checked rows', async () => { + await openPurchaseModal([noUpfront()]); + const band = document.getElementById('purchase-modal-commitment-band')!; + expect(band.textContent).toBe('Upfront$0.00Monthly$800.00Total over term$9,600.00'); + + (document.querySelector('.purchase-modal-row-include') as HTMLInputElement).click(); + expect(band.textContent).toBe('Upfront$0.00Monthly$0.00Total over term$0.00'); + }); + + test('the Execute Now warning names the monthly charge of a no-upfront purchase', async () => { + await openPurchaseModal([noUpfront()]); + (document.getElementById('execute-mode-direct') as HTMLInputElement).click(); + expect(document.querySelector('.direct-execute-warning')!.textContent).toBe( + 'Warning: This will charge $0.00 upfront immediately, then $800.00 a month for the term ($9,600.00 in total). This bypasses the approval step.', + ); + }); + + test('the Execute Now warning says monthly fees apply when the provider did not report them', async () => { + await openPurchaseModal([{ ...noUpfront(), monthly_cost: null }]); + (document.getElementById('execute-mode-direct') as HTMLInputElement).click(); + expect(document.querySelector('.direct-execute-warning')!.textContent).toContain( + 'upfront immediately, plus monthly fees the provider did not report.', + ); + }); + + test('the bulk summary, bucket totals and confirm state the monthly charge and term total', async () => { + const rows: LocalRecommendation[] = [ + { + id: 'ec2-no', provider: 'aws', cloud_account_id: 'a1', service: 'ec2', + region: 'us-east-1', resource_type: 'm5.large', term: 1, payment: 'no-upfront', + count: 1, upfront_cost: 0, monthly_cost: 100, savings: 50, + }, + { + id: 'rds-no', provider: 'aws', cloud_account_id: 'a1', service: 'rds', + region: 'us-east-1', resource_type: 'db.r5.large', term: 1, payment: 'no-upfront', + count: 1, upfront_cost: 0, monthly_cost: 200, savings: 80, + }, + ]; + (api.getConfig as jest.Mock).mockResolvedValue({ global: { default_payment: 'no-upfront' } }); + (api.getRecommendations as jest.Mock).mockResolvedValue({ summary: {}, recommendations: rows, regions: [] }); + (state.getRecommendations as jest.Mock).mockReturnValue(rows); + (state.getVisibleRecommendations as jest.Mock).mockReturnValue(rows); + (state.getSelectedRecommendationIDs as jest.Mock).mockReturnValue(new Set(['ec2-no', 'rds-no'])); + + await loadRecommendations(); + (document.getElementById('bulk-purchase-btn') as HTMLButtonElement).click(); + await flush(); + + const line = (label: string): string | null | undefined => Array.from(document.querySelectorAll('#fanout-summary p')) + .find((p) => p.textContent?.startsWith(label))?.querySelector('strong')?.textContent; + expect(line('Total upfront')).toBe('$0'); + expect(line('Monthly recurring')).toBe('$300'); + expect(line('Total over term')).toBe('$3,600'); + const bucketTotals = Array.from(document.querySelectorAll('.fanout-bucket-totals')).map((p) => p.textContent); + expect(bucketTotals).toEqual(expect.arrayContaining([ + expect.stringContaining('$0 upfront · $100/mo recurring'), + expect.stringContaining('$0 upfront · $200/mo recurring'), + ])); + + (confirmDialog as jest.Mock).mockClear(); + (document.getElementById('execute-purchase-btn') as HTMLButtonElement).click(); + await flush(); + const opts = (confirmDialog as jest.Mock).mock.calls[0]![0] as { body: HTMLElement }; + const text = opts.body.textContent ?? ''; + expect(text).toContain('Upfront on approval$0.00'); + expect(text).toContain('Monthly$300.00'); + expect(text).toContain('Total over term$3,600.00'); + }); +}); diff --git a/frontend/src/app.ts b/frontend/src/app.ts index c73b9658e..16525e231 100644 --- a/frontend/src/app.ts +++ b/frontend/src/app.ts @@ -18,7 +18,7 @@ import { setupRIExchangeHandlers, saveAutomationSettings } from './riexchange'; import { showToast } from './toast'; import { formatPaymentAdjustmentNotice } from './commitmentOptions'; import { confirmDialog } from './confirmDialog'; -import { buildCommitmentSummary, commitmentTotals, executeButtonLabel, executeConfirmLabel, withIntro } from './commitmentSummary'; +import { buildCommitmentSummary, commitmentLine, commitmentTotals, executeButtonLabel, executeConfirmLabel, withIntro } from './commitmentSummary'; import { handlePurchaseDeeplink } from './purchases-deeplink'; import { handleArcheraDeeplink, openArcheraOfferModal } from './archera'; import { closeModal } from './modal'; @@ -432,18 +432,10 @@ export async function handleExecutePurchase(): Promise { // - Approval path: low-friction, non-destructive. // - Direct-execute path: red destructive dialog with cost callout and // immediate-charge callout; no cancellation promise (issue #251). - const summaryLines = recsWithPayment.map((r) => ({ - service: r.service, - provider: r.provider, - region: r.region, - resourceType: r.resource_type, - count: r.count, - term: r.term, - payment: r.payment, - upfront: r.upfront_cost, - monthly: r.monthly_cost ?? null, - ...(r.cloud_account_id ? { account: getAccountName(r.cloud_account_id) } : {}), - })); + const summaryLines = recsWithPayment.map((r) => commitmentLine( + r, + r.cloud_account_id ? { account: getAccountName(r.cloud_account_id) } : {}, + )); const totals = commitmentTotals(summaryLines); const summary = buildCommitmentSummary( summaryLines, @@ -599,9 +591,19 @@ async function handleFanOutExecute(buckets: FanOutBucket[]): Promise { // Same approval-required default as the single-purchase path: each // bucket POSTs a request that triggers an approval email; the actual // charges fire when each approver clicks the link in their email. + // State the monthly charge and the term total too (issue #711): for + // no-upfront and partial-upfront buckets the upfront alone reads $0 + // while the account is billed monthly for one or three years. + const summary = buildCommitmentSummary( + buckets.flatMap((b) => b.recs.map((r) => commitmentLine( + r, + { payment: b.payment, ...(r.cloud_account_id ? { account: getAccountName(r.cloud_account_id) } : {}) }, + ))), + { upfrontLabel: 'Upfront on approval' }, + ); const ok = await confirmDialog({ title: `Send ${buckets.length} bulk purchase${buckets.length === 1 ? '' : 's'} for approval?`, - body: `This will submit ${buckets.length} separate purchase request${buckets.length === 1 ? '' : 's'} and email ${buckets.length} approval request${buckets.length === 1 ? '' : 's'}. Each must be approved individually before its commitments are charged.`, + body: withIntro(`This will submit ${buckets.length} separate purchase request${buckets.length === 1 ? '' : 's'} and email ${buckets.length} approval request${buckets.length === 1 ? '' : 's'}. Each must be approved individually before its commitments are charged.`, summary), confirmLabel: 'Send all for approval', destructive: false, }); diff --git a/frontend/src/approval-details.ts b/frontend/src/approval-details.ts index b9604ce38..1f6e97bb4 100644 --- a/frontend/src/approval-details.ts +++ b/frontend/src/approval-details.ts @@ -7,9 +7,9 @@ * * This module fetches `/api/purchases/{id}` + `/api/accounts` and builds * an HTMLElement carrying: - * - An aggregate header (total upfront, total monthly + annual - * savings, recommendation count, distinct providers, distinct - * accounts touched). + * - An aggregate header (total upfront, monthly recurring cost, total + * over the term, total monthly + annual savings, recommendation + * count, distinct providers, distinct accounts touched). * - A per-recommendation table: account / provider / service / * resource type / engine / region / count / term + payment / * upfront / monthly savings / effective savings % when the rec @@ -28,6 +28,7 @@ import type { CloudAccount } from './api/accounts'; import type { PurchaseDetails, Recommendation } from './api/types'; import { escapeHtml, formatCurrency, formatTerm, amortizedMonthly } from './utils'; import { getAmortizeUpfront } from './state'; +import { commitmentLine, formatCommitmentTotals } from './commitmentSummary'; /** * accountsById maps the internal CloudAccount UUID (the value carried @@ -107,7 +108,13 @@ function renderApprovalDetailsHeader(details: PurchaseDetails, recs: Recommendat // sub-cent level, well below the precision formatCurrency renders. const annualMismatch = Math.abs(totalAnnual - monthly * 12) > 0.005; + // Issue #711: an approver must see the recurring charge and what the + // whole term costs, not only the upfront (which is $0 for no-upfront). + const termText = formatCommitmentTotals(recs.map((r) => commitmentLine(r)), 0); + header.appendChild(headerStat('Upfront', formatCurrency(upfront))); + header.appendChild(headerStat('Monthly cost', termText.monthly)); + header.appendChild(headerStat('Total over term', termText.termTotal)); header.appendChild(headerStat('Monthly savings', formatCurrency(monthly))); header.appendChild(headerStat('Annual savings', formatCurrency(monthly * 12), undefined, annualMismatch ? `${formatCurrency(totalAnnual)} from per-row sum` : undefined)); header.appendChild(headerStat('Commitments', String(recs.length))); diff --git a/frontend/src/commitmentSummary.ts b/frontend/src/commitmentSummary.ts index b01059c8a..019d85da4 100644 --- a/frontend/src/commitmentSummary.ts +++ b/frontend/src/commitmentSummary.ts @@ -33,6 +33,38 @@ export interface CommitmentTotals { const MAX_LISTED_LINES = 5; +/** The priced-recommendation fields a commitment line is built from. */ +export interface PricedCommitment { + service: string; + provider?: string; + region?: string; + resource_type: string; + count: number; + term: number; + payment?: string; + upfront_cost: number; + monthly_cost?: number | null; +} + +/** + * Map a priced recommendation onto a commitment line. `payment` overrides + * the rec's own (the bulk modal prices a whole bucket at one payment). + */ +export function commitmentLine(r: PricedCommitment, opts: { payment?: string; account?: string } = {}): CommitmentLine { + return { + service: r.service, + ...(r.provider ? { provider: r.provider } : {}), + ...(r.region ? { region: r.region } : {}), + resourceType: r.resource_type, + count: r.count, + term: r.term, + payment: opts.payment ?? r.payment ?? '', + upfront: r.upfront_cost, + monthly: r.monthly_cost ?? null, + ...(opts.account ? { account: opts.account } : {}), + }; +} + export function commitmentTotals(lines: readonly CommitmentLine[]): CommitmentTotals { let upfront = 0; let monthly: number | null = 0; @@ -88,11 +120,34 @@ function stat(label: string, value: string): HTMLElement { return el; } +/** True when any line is partial or no upfront, so it bills monthly. */ +export function billsMonthly(lines: readonly CommitmentLine[]): boolean { + return lines.some(l => paymentLabel(l.payment) !== 'All upfront'); +} + // A partial or no-upfront commitment bills monthly; saying only "not // reported" would read as "no monthly fee". function monthlyUnknownText(lines: readonly CommitmentLine[]): string { - const bills = lines.some(l => paymentLabel(l.payment) !== 'All upfront'); - return bills ? 'Not reported (monthly fees apply)' : 'Not reported'; + return billsMonthly(lines) ? 'Not reported (monthly fees apply)' : 'Not reported'; +} + +export interface CommitmentTotalsText { + upfront: string; + monthly: string; + termTotal: string; +} + +/** + * The three totals as display strings, so every screen words an unknown + * monthly cost the same way instead of printing a partial sum or $0. + */ +export function formatCommitmentTotals(lines: readonly CommitmentLine[], digits = 2): CommitmentTotalsText { + const totals = commitmentTotals(lines); + return { + upfront: formatCurrency(totals.upfront, '$', digits), + monthly: totals.monthly === null ? monthlyUnknownText(lines) : formatCurrency(totals.monthly, '$', digits), + termTotal: totals.termTotal === null ? 'Not available' : formatCurrency(totals.termTotal, '$', digits), + }; } function describe(l: CommitmentLine): string { @@ -107,19 +162,23 @@ export interface SummaryOptions { upfrontLabel?: string; } -export function buildCommitmentSummary(lines: readonly CommitmentLine[], opts: SummaryOptions = {}): HTMLElement { - const totals = commitmentTotals(lines); - const root = document.createElement('div'); - root.className = 'commitment-summary'; - +/** Just the Upfront / Monthly / Total over term band, for screens that already list the lines. */ +export function buildCommitmentBand(lines: readonly CommitmentLine[], opts: SummaryOptions = {}): HTMLElement { + const text = formatCommitmentTotals(lines); const band = document.createElement('div'); band.className = 'commitment-summary-band'; band.append( - stat(opts.upfrontLabel ?? 'Charged today', formatCurrency(totals.upfront, '$', 2)), - stat('Monthly', totals.monthly === null ? monthlyUnknownText(lines) : formatCurrency(totals.monthly, '$', 2)), - stat('Total over term', totals.termTotal === null ? 'Not available' : formatCurrency(totals.termTotal, '$', 2)), + stat(opts.upfrontLabel ?? 'Charged today', text.upfront), + stat('Monthly', text.monthly), + stat('Total over term', text.termTotal), ); - root.appendChild(band); + return band; +} + +export function buildCommitmentSummary(lines: readonly CommitmentLine[], opts: SummaryOptions = {}): HTMLElement { + const root = document.createElement('div'); + root.className = 'commitment-summary'; + root.appendChild(buildCommitmentBand(lines, opts)); const list = document.createElement('ul'); list.className = 'commitment-summary-lines'; diff --git a/frontend/src/recommendations.ts b/frontend/src/recommendations.ts index 307ff2d7a..3497bbe23 100644 --- a/frontend/src/recommendations.ts +++ b/frontend/src/recommendations.ts @@ -19,7 +19,7 @@ import { type Payment as CompatPayment, type Provider as CompatProvider, } from './lib/purchase-compatibility'; -import { executeButtonLabel } from './commitmentSummary'; +import { billsMonthly, buildCommitmentBand, commitmentLine, commitmentTotals, executeButtonLabel, formatCommitmentTotals, type CommitmentLine } from './commitmentSummary'; import type { AccountServiceOverride } from './api/accounts'; import type { RecommendationsResponse, LocalRecommendation, RecommendationsSummary, GlobalConfig } from './types'; import { openModal } from './modal'; @@ -4540,6 +4540,12 @@ function renderFanOutSummary(summary: HTMLElement, buckets: FanOutBucket[]): voi const fanOutPeriod = state.getCostPeriod(); summary.appendChild(totalLine('Total commitments', String(totals.totalCount))); summary.appendChild(totalLine('Total upfront', formatCurrency(totals.totalUpfront))); + // Issue #711: upfront alone reads $0 for no-upfront buckets that bill + // monthly for one or three years, so state the recurring charge and the + // total over the term next to it. + const termText = formatCommitmentTotals(submittable.flatMap(bucketCommitmentLines), CURRENCY_DEFAULT_DIGITS); + summary.appendChild(totalLine('Monthly recurring', termText.monthly)); + summary.appendChild(totalLine('Total over term', termText.termTotal)); summary.appendChild(totalLine(`Total savings ${periodSuffix(fanOutPeriod)}`, formatCostForPeriod(totals.totalSavings, fanOutPeriod), 'savings')); const executeBtn = document.getElementById('execute-purchase-btn') as HTMLButtonElement | null; @@ -4563,6 +4569,10 @@ function refreshFanOutSummary(): void { renderFanOutSummary(summary, currentFanOutBuckets); } +function bucketCommitmentLines(b: FanOutBucket): CommitmentLine[] { + return b.recs.map((r) => commitmentLine(r, { payment: b.payment })); +} + function computeFanOutTotals(buckets: FanOutBucket[]): { totalCount: number; totalUpfront: number; totalSavings: number } { let totalCount = 0; let totalUpfront = 0; @@ -4703,7 +4713,9 @@ function renderFanOutBucketSection(b: FanOutBucket): HTMLElement { const bPeriod = state.getCostPeriod(); const totals = document.createElement('p'); totals.className = 'fanout-bucket-totals'; - totals.textContent = `${bucketTotal.count} commitments · ${formatCurrency(bucketTotal.upfront)} upfront · ${formatCostForPeriod(bucketTotal.savings, bPeriod)} savings ${periodSuffix(bPeriod)}`; + const bucketMonthly = commitmentTotals(bucketCommitmentLines(b)).monthly; + const monthlyPart = bucketMonthly === null ? 'monthly cost not reported' : `${formatCurrency(bucketMonthly)}/mo recurring`; + totals.textContent = `${bucketTotal.count} commitments · ${formatCurrency(bucketTotal.upfront)} upfront · ${monthlyPart} · ${formatCostForPeriod(bucketTotal.savings, bPeriod)} savings ${periodSuffix(bPeriod)}`; section.appendChild(totals); // Issue #249: for mixed-SP buckets with 2+ distinct plan types, render @@ -5102,22 +5114,36 @@ function resolvePerRecPaymentSeed( // follow both a checkbox toggle (updatePurchaseModalTotals, pre-existing // gap) and a Term/Payment re-price, not only the radio's own change event. // No-ops when the callout isn't in the DOM (no direct-execute permission). +function checkedPurchaseLines(): CommitmentLine[] { + const lines: CommitmentLine[] = []; + for (const idx of checkedPurchaseIndices) { + const r = currentPurchaseRecommendations[idx]; + if (r) lines.push(commitmentLine(r)); + } + return lines; +} + function renderDirectExecuteWarning(): void { const directWarning = document.querySelector('#purchase-details .direct-execute-warning'); if (!directWarning) return; directWarning.hidden = currentExecuteMode !== 'direct'; if (currentExecuteMode !== 'direct') return; - let totalUpfront = 0; - for (const idx of checkedPurchaseIndices) { - const r = currentPurchaseRecommendations[idx]; - if (r) totalUpfront += r.upfront_cost; + const lines = checkedPurchaseLines(); + const totals = commitmentTotals(lines); + const shown = formatCommitmentTotals(lines); + // Issue #711: a no-upfront purchase would otherwise read "charge $0.00". + let recurring = ''; + if (totals.monthly === null) { + recurring = billsMonthly(lines) ? ', plus monthly fees the provider did not report' : ''; + } else if (totals.monthly > 0) { + recurring = `, then ${shown.monthly} a month for the term (${shown.termTotal} in total)`; } while (directWarning.firstChild) directWarning.removeChild(directWarning.firstChild); const icon = document.createElement('strong'); icon.textContent = 'Warning: '; directWarning.appendChild(icon); const text = document.createTextNode( - `This will charge ${formatCurrency(totalUpfront, '$', 2)} upfront immediately. ` + + `This will charge ${shown.upfront} upfront immediately${recurring}. ` + 'This bypasses the approval step.', ); directWarning.appendChild(text); @@ -5335,6 +5361,11 @@ export async function openPurchaseModal(recommendations: LocalRecommendation[], tfoot.appendChild(totalsRow); commitsSection.appendChild(table); + // Upfront / Monthly / Total over term for the checked rows (issue #711); + // refilled by updatePurchaseModalTotals. + const band = document.createElement('div'); + band.id = 'purchase-modal-commitment-band'; + commitsSection.appendChild(band); container.appendChild(commitsSection); // Wire the select-all checkbox (must happen after tbody rows are in DOM @@ -5531,6 +5562,9 @@ function updatePurchaseModalTotals(selectAllCb: HTMLInputElement): void { selectAllCb.indeterminate = true; } + const band = document.getElementById('purchase-modal-commitment-band'); + if (band) band.replaceChildren(buildCommitmentBand(checkedPurchaseLines(), { upfrontLabel: 'Upfront' })); + // Issue #1903: the direct-execute warning's dollar total is derived from // checkedPurchaseIndices too, so it must refresh on every checkbox toggle, // not only when the radio itself changes. diff --git a/frontend/src/styles/components.css b/frontend/src/styles/components.css index 5d76ec88e..1bc1ee39f 100644 --- a/frontend/src/styles/components.css +++ b/frontend/src/styles/components.css @@ -1119,7 +1119,9 @@ tr.rec-variant-row:hover td:not(.checkbox-col) { .approval-details-header { display: grid; - grid-template-columns: repeat(auto-fit, minmax(120px, 1fr)); + /* At most four stats per row, so the eight stats sit in two even rows + instead of seven plus an orphan; fewer per row on narrow screens. */ + grid-template-columns: repeat(auto-fit, minmax(max(120px, calc(25% - 1rem)), 1fr)); gap: 0.5rem 1rem; padding: 0.75rem 1rem; background: var(--cudly-surface-2, #f6f8fa); diff --git a/frontend/tests-e2e/execute-now-warning.spec.ts b/frontend/tests-e2e/execute-now-warning.spec.ts index 537a977b2..c9a89dd55 100644 --- a/frontend/tests-e2e/execute-now-warning.spec.ts +++ b/frontend/tests-e2e/execute-now-warning.spec.ts @@ -28,14 +28,17 @@ for (const sample of [ await expect(warning).toBeHidden(); await page.locator('#execute-mode-direct').check(); await expect(warning).toBeVisible(); - await expect(warning).toHaveText(`Warning: This will charge $${row.upfront_cost.toLocaleString('en-US', { minimumFractionDigits: 2, maximumFractionDigits: 2 })} upfront immediately. This bypasses the approval step.`); + // A monthly-billed commitment also states its recurring charge (issue #711). + const recurring = row.monthly_cost ? `, then $${row.monthly_cost.toFixed(2)} a month for the term ($${(row.monthly_cost * 12).toLocaleString('en-US', { minimumFractionDigits: 2 })} in total)` : ''; + await expect(warning).toHaveText(`Warning: This will charge $${row.upfront_cost.toLocaleString('en-US', { minimumFractionDigits: 2, maximumFractionDigits: 2 })} upfront immediately${recurring}. This bypasses the approval step.`); + await expect(page.locator('#purchase-modal-commitment-band')).toContainText('Total over term'); await expect(warning).not.toContainText(/AWS|cancell|24 hours/); await expect(page.locator('#execute-purchase-btn')).toHaveText('Execute Purchase Now'); await page.getByRole('checkbox', { name: 'Include row 1' }).uncheck(); await expect(warning).toHaveText('Warning: This will charge $0.00 upfront immediately. This bypasses the approval step.'); await expect(page.locator('#execute-purchase-btn')).toBeDisabled(); await page.getByRole('checkbox', { name: 'Include row 1' }).check(); - await expect(warning).toContainText(`$${row.upfront_cost.toLocaleString('en-US', { minimumFractionDigits: 2, maximumFractionDigits: 2 })} upfront immediately.`); + await expect(warning).toContainText(`$${row.upfront_cost.toLocaleString('en-US', { minimumFractionDigits: 2, maximumFractionDigits: 2 })} upfront immediately${recurring}.`); await expect(page.locator('#execute-purchase-btn')).toBeEnabled(); if (sample.provider === 'gcp') { await page.setViewportSize({ width: 390, height: 844 }); @@ -69,6 +72,9 @@ test('mixed providers use fan-out without cancellation reassurance', async ({ pa } await page.locator('#bulk-purchase-btn').click(); await expect(page.locator('#fanout-summary')).toBeVisible(); + // r01 ($350/mo, 1y, $2,400 upfront) + r12 ($200/mo, 1y, $1,500 upfront) (issue #711). + await expect(page.locator('#fanout-summary')).toContainText('Monthly recurring: $550'); + await expect(page.locator('#fanout-summary')).toContainText('Total over term: $10,500'); await expect(page.locator('#purchase-details')).not.toContainText(/cancellation|24 hours/); await expect(page.locator('#execute-mode-direct')).toHaveCount(0); expect(api.calls.filter(call => call.method === 'POST' && call.url.includes('/purchases/execute'))).toEqual([]);