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([]);