From 7aaaf3b2d8ec82d3532f55eeba9b6b5c69989484 Mon Sep 17 00:00:00 2001 From: Astrid Gealer Date: Fri, 23 Oct 2026 22:53:40 +0000 Subject: [PATCH] fix(frontend): show totals in the bulk approval confirm The Execute now and Run now confirms already state what is bought and how much (#781), but the bulk (fan-out) "Send N bulk purchases for approval?" confirm still said only how many requests and emails would go out. It now uses the same commitment summary as the single-purchase confirm: the combined upfront on approval, monthly and total over term across every bucket, one line per commitment with account and region, and a confirm button that names the upfront amount. The rec-to-summary-line mapping is shared by both paths. Refs #707 Co-Authored-By: Claude Opus 5.5 --- .../purchase-execution-toast.test.ts | 2 + .../__tests__/purchase-modal-submit.test.ts | 52 +++++++++++++++++++ frontend/src/app.ts | 46 ++++++++++------ .../tests-e2e/bulk-approval-confirm.spec.ts | 39 ++++++++++++++ 4 files changed, 123 insertions(+), 16 deletions(-) create mode 100644 frontend/tests-e2e/bulk-approval-confirm.spec.ts diff --git a/frontend/src/__tests__/purchase-execution-toast.test.ts b/frontend/src/__tests__/purchase-execution-toast.test.ts index 6125d7a31..3cadbd73b 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 confirm summaries (single and fan-out, #707) name each line's account. + 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..a2352b20c 100644 --- a/frontend/src/__tests__/purchase-modal-submit.test.ts +++ b/frontend/src/__tests__/purchase-modal-submit.test.ts @@ -1345,6 +1345,58 @@ describe('Issue #1904: fan-out modal skips incompatible buckets', () => { ]); }); + test('bulk approval confirm states the totals across all buckets (#707)', async () => { + const rows: LocalRecommendation[] = [ + { + id: 'ec2-1-all', provider: 'aws', cloud_account_id: 'a1', service: 'ec2', + region: 'us-east-1', resource_type: 'm5.large', term: 1, payment: 'all-upfront', + count: 4, upfront_cost: 4000, monthly_cost: 0, savings: 1000, + }, + { + id: 'ec2-1-partial', provider: 'aws', cloud_account_id: 'a1', service: 'ec2', + region: 'us-east-1', resource_type: 'm5.large', term: 1, payment: 'partial-upfront', + count: 4, upfront_cost: 2000, monthly_cost: 100, savings: 900, + }, + { + id: 'rds-3-all', provider: 'aws', cloud_account_id: 'a1', service: 'rds', + region: 'us-east-1', resource_type: 'db.r5.large', term: 3, payment: 'all-upfront', + count: 2, upfront_cost: 6000, monthly_cost: 0, savings: 1000, + }, + ]; + (api.getConfig as jest.Mock).mockResolvedValue({ global: { default_payment: 'all-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-1-all', 'rds-3-all'])); + + await loadRecommendations(); + (document.getElementById('bulk-purchase-btn') as HTMLButtonElement).click(); + await flush(); + const ec2Section = Array.from(document.querySelectorAll('.fanout-bucket')) + .find((section) => section.textContent?.includes('ec2'))!; + const paymentSelect = ec2Section.querySelector('.fanout-bucket-payment')!; + paymentSelect.value = 'partial-upfront'; + paymentSelect.dispatchEvent(new Event('change')); + + (confirmDialog as jest.Mock).mockClear(); + (document.getElementById('execute-purchase-btn') as HTMLButtonElement).click(); + await flush(); + + expect(confirmDialog).toHaveBeenCalledTimes(1); + const opts = (confirmDialog as jest.Mock).mock.calls[0]![0] as { title: string; body: HTMLElement; confirmLabel: string }; + expect(opts.title).toBe('Send 2 bulk purchases for approval?'); + const text = opts.body.textContent ?? ''; + // ec2 partial: $2,000 upfront + $100/mo for 12 months; rds all-upfront: $6,000. + expect(text).toContain(`Upfront on approval${formatCurrency(8000, '$', 2)}`); + expect(text).toContain(`Monthly${formatCurrency(100, '$', 2)}`); + expect(text).toContain(`Total over term${formatCurrency(9200, '$', 2)}`); + expect(text).toContain('4 × ec2 m5.large'); + expect(text).toContain('Partial upfront'); + expect(text).toContain('2 × rds db.r5.large'); + expect(text).not.toContain('Charged today'); + expect(opts.confirmLabel).toContain(`(${formatCurrency(8000, '$', 2)} upfront on approval)`); + }); + test('fan-out resolves a saved payment override to its priced sibling on open', async () => { const rows: LocalRecommendation[] = [ { diff --git a/frontend/src/app.ts b/frontend/src/app.ts index c73b9658e..2260238fe 100644 --- a/frontend/src/app.ts +++ b/frontend/src/app.ts @@ -18,7 +18,8 @@ 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 type { LocalRecommendation } from './types'; +import { buildCommitmentSummary, commitmentTotals, executeButtonLabel, executeConfirmLabel, withIntro, type CommitmentLine } from './commitmentSummary'; import { handlePurchaseDeeplink } from './purchases-deeplink'; import { handleArcheraDeeplink, openArcheraOfferModal } from './archera'; import { closeModal } from './modal'; @@ -432,18 +433,7 @@ 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) => toCommitmentLine(r, r.payment)); const totals = commitmentTotals(summaryLines); const summary = buildCommitmentSummary( summaryLines, @@ -566,6 +556,22 @@ export async function handleExecutePurchase(): Promise { } } +/** One confirm-summary line for a rec bought with the given payment option. */ +function toCommitmentLine(r: LocalRecommendation, payment: string): CommitmentLine { + return { + service: r.service, + provider: r.provider, + region: r.region, + resourceType: r.resource_type, + count: r.count, + term: r.term, + payment, + upfront: r.upfront_cost, + monthly: r.monthly_cost ?? null, + ...(r.cloud_account_id ? { account: getAccountName(r.cloud_account_id) } : {}), + }; +} + /** * fanOutBucketLabel renders a short human identifier for a fan-out bucket so a * partial-failure toast can name which buckets created pending executions @@ -598,11 +604,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. + // charges fire when each approver clicks the link in their email. The + // confirm states the combined amounts across every bucket (#707). A + // submittable bucket's recs already carry the bucket's payment option, so + // their upfront and monthly costs are the ones that will be approved. + const summaryLines = buckets.flatMap((b) => b.recs.map((r) => toCommitmentLine(r, b.payment))); + const totals = commitmentTotals(summaryLines); 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.`, - confirmLabel: 'Send all for approval', + 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. Totals below are across all requests.`, + buildCommitmentSummary(summaryLines, { upfrontLabel: 'Upfront on approval' }), + ), + confirmLabel: executeConfirmLabel(false, totals.upfront), destructive: false, }); if (!ok) { diff --git a/frontend/tests-e2e/bulk-approval-confirm.spec.ts b/frontend/tests-e2e/bulk-approval-confirm.spec.ts new file mode 100644 index 000000000..395997d89 --- /dev/null +++ b/frontend/tests-e2e/bulk-approval-confirm.spec.ts @@ -0,0 +1,39 @@ +import { test, expect } from '@playwright/test'; +import { mockApi, RECS, seedAuth } from './fixtures/recs'; + +const usd = (n: number): string => `$${n.toLocaleString('en-US', { minimumFractionDigits: 2, maximumFractionDigits: 2 })}`; + +// Issue #707: the bulk (fan-out) approval confirm stated only the number of +// requests. It now shows the combined upfront, monthly and term totals and +// one line per commitment, like the single-purchase confirm. +test('bulk approval confirm states the totals across every bucket', async ({ page }) => { + const api = await mockApi(page); + await seedAuth(page); + await page.route('**/api/accounts/*/service-overrides', route => route.fulfill({ json: [] })); + await page.goto('/opportunities'); + const ids = ['r01', 'r08', 'r12']; + for (const id of ids) { + await page.locator(`tr.recommendation-row[data-rec-id="${id}"]`) + .getByRole('checkbox', { name: /^Select / }).check(); + } + await page.locator('#bulk-purchase-btn').click(); + await expect(page.locator('#fanout-summary')).toBeVisible(); + await page.locator('#execute-purchase-btn').click(); + + const picked = RECS.filter(rec => ids.includes(rec.id)); + const upfront = picked.reduce((sum, rec) => sum + rec.upfront_cost, 0); + const monthly = picked.reduce((sum, rec) => sum + rec.monthly_cost, 0); + const termTotal = picked.reduce((sum, rec) => sum + rec.upfront_cost + rec.monthly_cost * 12 * rec.term, 0); + + const dialog = page.getByRole('dialog', { name: 'Send 3 bulk purchases for approval?' }); + await expect(dialog).toBeVisible(); + await expect(dialog.locator('.commitment-summary-band')).toContainText(`Upfront on approval${usd(upfront)}`); + await expect(dialog.locator('.commitment-summary-band')).toContainText(`Monthly${usd(monthly)}`); + await expect(dialog.locator('.commitment-summary-band')).toContainText(`Total over term${usd(termTotal)}`); + await expect(dialog.locator('.commitment-summary-lines li')).toHaveCount(3); + await expect(dialog.getByRole('button', { name: `Send for Approval (${usd(upfront)} upfront on approval)` })).toBeVisible(); + + await dialog.getByRole('button', { name: 'Cancel' }).click(); + await expect(dialog).toHaveCount(0); + expect(api.calls.filter(call => call.method === 'POST' && call.url.includes('/purchases/execute'))).toEqual([]); +});