diff --git a/frontend/src/__tests__/purchase-execution-toast.test.ts b/frontend/src/__tests__/purchase-execution-toast.test.ts new file mode 100644 index 00000000..d00f600e --- /dev/null +++ b/frontend/src/__tests__/purchase-execution-toast.test.ts @@ -0,0 +1,412 @@ +/** + * Tests for handleExecutePurchase / handleFanOutExecute toast messages. + * + * CR pass on PR #294 / issue #288: + * Finding 1 — success toasts must name the approval recipient when the + * backend returns approval_recipient. + * Finding 2 — fulfilled executePurchase responses with email_sent === false + * or status === 'failed' must count as failures, not successes. + */ + +// ── mocks (must precede imports) ───────────────────────────────────────────── + +jest.mock('../api', () => ({ + initAuth: jest.fn(), + isAuthenticated: jest.fn(), + getCurrentUser: jest.fn(), + executePurchase: jest.fn(), +})); + +jest.mock('../state', () => ({ + setCurrentUser: jest.fn(), + setCurrentProvider: jest.fn(), +})); + +jest.mock('../auth', () => ({ + showLoginModal: jest.fn(), + updateUserUI: jest.fn(), +})); + +jest.mock('../dashboard', () => ({ + loadDashboard: jest.fn().mockResolvedValue(undefined), + setupDashboardHandlers: jest.fn(), +})); + +jest.mock('../navigation', () => ({ + switchTab: jest.fn(), + applyTabFromPath: jest.fn().mockReturnValue('dashboard'), + initRouter: jest.fn(), + switchSettingsSubTab: jest.fn(), + getSettingsSubTabFromPath: jest.fn().mockReturnValue('general'), +})); + +jest.mock('../recommendations', () => ({ + setupRecommendationsHandlers: jest.fn(), + getPurchaseModalRecommendations: jest.fn(), + clearPurchaseModalRecommendations: jest.fn(), + getFanOutBuckets: jest.fn(), + clearFanOutBuckets: jest.fn(), +})); + +jest.mock('../plans', () => ({ + savePlan: jest.fn(), + setupPlanHandlers: jest.fn(), + closePlanModal: jest.fn(), + openNewPlanModal: jest.fn(), + closePurchaseModal: jest.fn(), +})); + +jest.mock('../settings', () => ({ + saveGlobalSettings: jest.fn(), + setupSettingsHandlers: jest.fn(), + resetSettings: jest.fn(), +})); + +jest.mock('../riexchange', () => ({ + setupRIExchangeHandlers: jest.fn(), + saveAutomationSettings: jest.fn(), +})); + +jest.mock('../users', () => ({ + setupUserHandlers: jest.fn(), +})); + +jest.mock('../apikeys', () => ({ + initApiKeys: jest.fn(), +})); + +jest.mock('../history', () => ({ + loadHistory: jest.fn(), +})); + +jest.mock('../modules/savings-history', () => ({ + initSavingsHistory: jest.fn(), +})); + +jest.mock('../purchases-deeplink', () => ({ + handlePurchaseDeeplink: jest.fn(), +})); + +jest.mock('../modal', () => ({ + closeModal: jest.fn(), +})); + +// confirmDialog defaults to true (user confirms); individual tests can override +jest.mock('../confirmDialog', () => ({ + confirmDialog: jest.fn().mockResolvedValue(true), +})); + +// ── imports ─────────────────────────────────────────────────────────────────── + +import { setupEventListeners } from '../app'; +import * as api from '../api'; +import * as recs from '../recommendations'; +import * as plans from '../plans'; + +// ── helpers ─────────────────────────────────────────────────────────────────── + +function buildMinimalRec() { + return { + id: 'rec-1', + provider: 'aws' as const, + service: 'ec2', + region: 'us-east-1', + resource_type: 't3.medium', + count: 1, + term: 1, + payment: 'all-upfront', + upfront_cost: 100, + monthly_cost: 10, + savings: 20, + selected: true, + purchased: false, + }; +} + +/** + * Mount a minimal DOM using safe DOM API methods and wire up event listeners. + * Returns the execute-purchase button so tests can click it. + */ +function setup(): HTMLButtonElement { + const modal = document.createElement('div'); + modal.id = 'purchase-modal'; + document.body.appendChild(modal); + + const btn = document.createElement('button'); + btn.id = 'execute-purchase-btn'; + btn.textContent = 'Send for Approval'; + document.body.appendChild(btn); + + const capacityInput = document.createElement('input'); + capacityInput.id = 'bulk-purchase-capacity'; + capacityInput.value = '100'; + document.body.appendChild(capacityInput); + + setupEventListeners(); + return btn; +} + +/** Return the text content of the most-recently rendered toast message node. */ +function lastToastMessage(): string | null { + const container = document.getElementById('toast-container'); + if (!container) return null; + const toasts = container.querySelectorAll('.toast'); + if (toasts.length === 0) return null; + const last = toasts[toasts.length - 1]; + return last?.querySelector('.toast-message')?.textContent ?? last?.textContent ?? null; +} + +/** Return the kind class of the most recently rendered toast (success/error/warning). */ +function lastToastKind(): string | null { + const container = document.getElementById('toast-container'); + if (!container) return null; + const toasts = container.querySelectorAll('.toast'); + if (toasts.length === 0) return null; + const last = toasts[toasts.length - 1]; + for (const cls of Array.from(last?.classList ?? [])) { + const m = cls.match(/^toast--(.+)$/); + if (m) return m[1] ?? null; + } + return null; +} + +// ── tests ───────────────────────────────────────────────────────────────────── + +describe('handleExecutePurchase — single-record path', () => { + beforeEach(() => { + jest.clearAllMocks(); + // Default: no fan-out buckets, one single-record rec + (recs.getFanOutBuckets as jest.Mock).mockReturnValue([]); + (recs.getPurchaseModalRecommendations as jest.Mock).mockReturnValue([buildMinimalRec()]); + (plans.closePurchaseModal as jest.Mock).mockImplementation(() => undefined); + }); + + afterEach(() => { + document.body.textContent = ''; + }); + + test('Finding 1 — with approval_recipient: toast shows recipient name', async () => { + (api.executePurchase as jest.Mock).mockResolvedValue({ + execution_id: 'exec-1', + status: 'queued', + email_sent: true, + approval_recipient: 'approver@example.com', + }); + + const btn = setup(); + btn.click(); + await new Promise((r) => setTimeout(r, 0)); + + expect(lastToastMessage()).toContain('approver@example.com'); + expect(lastToastMessage()).toContain('Approval request sent to'); + expect(lastToastKind()).toBe('success'); + }); + + test('Finding 1 — without approval_recipient: toast falls back to generic copy', async () => { + (api.executePurchase as jest.Mock).mockResolvedValue({ + execution_id: 'exec-2', + status: 'queued', + email_sent: true, + // approval_recipient intentionally absent + }); + + const btn = setup(); + btn.click(); + await new Promise((r) => setTimeout(r, 0)); + + const msg = lastToastMessage(); + expect(msg).toContain('check your email'); + expect(lastToastKind()).toBe('success'); + }); + + test('email_sent === false: warning toast, not success', async () => { + (api.executePurchase as jest.Mock).mockResolvedValue({ + execution_id: 'exec-3', + status: 'queued', + email_sent: false, + email_reason: 'no notification email configured', + }); + + const btn = setup(); + btn.click(); + await new Promise((r) => setTimeout(r, 0)); + + expect(lastToastKind()).toBe('warning'); + expect(lastToastMessage()).toContain('no notification email configured'); + }); +}); + +describe('handleFanOutExecute — fan-out path', () => { + function buildBucket(id: string, capacityPercent = 100) { + return { + key: `key-${id}`, + label: `Bucket ${id}`, + recs: [buildMinimalRec()], + payment: 'all-upfront', + capacityPercent, + }; + } + + beforeEach(() => { + jest.clearAllMocks(); + (recs.getPurchaseModalRecommendations as jest.Mock).mockReturnValue([]); + (plans.closePurchaseModal as jest.Mock).mockImplementation(() => undefined); + }); + + afterEach(() => { + document.body.textContent = ''; + }); + + test('Finding 2 — email_sent === false counts as failure, email_reason in toast, recipient excluded', async () => { + (recs.getFanOutBuckets as jest.Mock).mockReturnValue([ + buildBucket('a'), + buildBucket('b'), + ]); + + (api.executePurchase as jest.Mock) + .mockResolvedValueOnce({ + execution_id: 'exec-a', + status: 'queued', + email_sent: true, + approval_recipient: 'alice@example.com', + }) + .mockResolvedValueOnce({ + execution_id: 'exec-b', + status: 'queued', + email_sent: false, + email_reason: 'SMTP timeout', + approval_recipient: 'bob@example.com', + }); + + const btn = setup(); + btn.click(); + await new Promise((r) => setTimeout(r, 0)); + + const msg = lastToastMessage(); + // 1 of 2 succeeded; 1 failed (business-level email failure) + expect(msg).toContain('1 of 2'); + expect(msg).toContain('failed'); + // The email_reason from the submission failure must appear in the message + expect(msg).toContain('SMTP timeout'); + // bob's email_sent was false — must NOT appear as approved recipient + expect(msg).not.toContain('bob@example.com'); + // Toast kind must reflect partial failure + expect(lastToastKind()).toBe('warning'); + }); + + test('Finding 2 — status === "failed" also counts as failure, recipient excluded', async () => { + (recs.getFanOutBuckets as jest.Mock).mockReturnValue([ + buildBucket('a'), + buildBucket('b'), + ]); + + (api.executePurchase as jest.Mock) + .mockResolvedValueOnce({ + execution_id: 'exec-a', + status: 'queued', + email_sent: true, + approval_recipient: 'alice@example.com', + }) + .mockResolvedValueOnce({ + execution_id: 'exec-b', + status: 'failed', + email_sent: undefined, + email_reason: 'backend error', + approval_recipient: 'bob@example.com', + }); + + const btn = setup(); + btn.click(); + await new Promise((r) => setTimeout(r, 0)); + + const msg = lastToastMessage(); + expect(msg).toContain('1 of 2'); + expect(msg).toContain('failed'); + expect(msg).toContain('backend error'); + expect(msg).not.toContain('bob@example.com'); + expect(lastToastKind()).toBe('warning'); + }); + + test('Finding 1 fan-out — all succeed: success toast lists unique recipients', async () => { + (recs.getFanOutBuckets as jest.Mock).mockReturnValue([ + buildBucket('a'), + buildBucket('b'), + ]); + + (api.executePurchase as jest.Mock) + .mockResolvedValueOnce({ + execution_id: 'exec-a', + status: 'queued', + email_sent: true, + approval_recipient: 'alice@example.com', + }) + .mockResolvedValueOnce({ + execution_id: 'exec-b', + status: 'queued', + email_sent: true, + approval_recipient: 'bob@example.com', + }); + + const btn = setup(); + btn.click(); + await new Promise((r) => setTimeout(r, 0)); + + const msg = lastToastMessage(); + expect(lastToastKind()).toBe('success'); + expect(msg).toContain('alice@example.com'); + expect(msg).toContain('bob@example.com'); + expect(msg).toContain('sent for approval'); + }); + + test('Finding 1 fan-out — deduplicates same recipient across buckets', async () => { + (recs.getFanOutBuckets as jest.Mock).mockReturnValue([ + buildBucket('a'), + buildBucket('b'), + ]); + + (api.executePurchase as jest.Mock) + .mockResolvedValueOnce({ + execution_id: 'exec-a', + status: 'queued', + email_sent: true, + approval_recipient: 'shared@example.com', + }) + .mockResolvedValueOnce({ + execution_id: 'exec-b', + status: 'queued', + email_sent: true, + approval_recipient: 'shared@example.com', + }); + + const btn = setup(); + btn.click(); + await new Promise((r) => setTimeout(r, 0)); + + const msg = lastToastMessage() ?? ''; + expect(lastToastKind()).toBe('success'); + // Address should appear exactly once after deduplication + const occurrences = (msg.match(/shared@example\.com/g) ?? []).length; + expect(occurrences).toBe(1); + }); + + test('Finding 2 — all email failures: error toast with reason', async () => { + (recs.getFanOutBuckets as jest.Mock).mockReturnValue([ + buildBucket('a'), + ]); + + (api.executePurchase as jest.Mock).mockResolvedValueOnce({ + execution_id: 'exec-a', + status: 'queued', + email_sent: false, + email_reason: 'no SMTP config', + }); + + const btn = setup(); + btn.click(); + await new Promise((r) => setTimeout(r, 0)); + + const msg = lastToastMessage(); + expect(lastToastKind()).toBe('error'); + expect(msg).toContain('no SMTP config'); + }); +}); diff --git a/frontend/src/__tests__/recommendations.test.ts b/frontend/src/__tests__/recommendations.test.ts index 1abe0edf..e908a960 100644 --- a/frontend/src/__tests__/recommendations.test.ts +++ b/frontend/src/__tests__/recommendations.test.ts @@ -79,6 +79,10 @@ describe('Recommendations Module', () => { `; @@ -851,6 +855,51 @@ describe('Recommendations Module', () => { await expect(openPurchaseModal([])).resolves.not.toThrow(); }); + + // Issue #288: the primary action does NOT execute the purchase — it + // sends an approval-request email. Pin the post-fix label + body + // wording so a regression that reverts to the misleading "Execute + // Purchase" framing fails this suite. + describe('approval-required messaging (issue #288)', () => { + const baseRec = { + id: 'rec-288', + provider: 'aws' as const, + service: 'ec2', + resource_type: 't3.medium', + region: 'us-east-1', + count: 5, + term: 1, + savings: 100, + upfront_cost: 500, + }; + + test('primary button reads "Send for Approval", not "Execute Purchase"', async () => { + await openPurchaseModal([baseRec]); + + const btn = document.getElementById('execute-purchase-btn'); + expect(btn?.textContent).toBe('Send for Approval'); + // Belt-and-braces: ensure no element on the rendered modal still + // carries the pre-#288 text — guards against a future template + // re-introducing the misleading wording somewhere new. + const modal = document.getElementById('purchase-modal'); + expect(modal?.textContent).not.toContain('Execute Purchase'); + }); + + test('modal body carries the approval-required explanation', async () => { + await openPurchaseModal([baseRec]); + + const details = document.getElementById('purchase-details'); + expect(details?.textContent).toContain('will email an approval request'); + }); + + test('approval-required note renders with its dedicated class', async () => { + await openPurchaseModal([baseRec]); + + const note = document.querySelector('#purchase-details .approval-required-note'); + expect(note).not.toBeNull(); + expect(note?.textContent).toMatch(/approval request/i); + }); + }); }); describe('refreshRecommendations', () => { diff --git a/frontend/src/api/purchases.ts b/frontend/src/api/purchases.ts index 4474c4e8..d875314a 100644 --- a/frontend/src/api/purchases.ts +++ b/frontend/src/api/purchases.ts @@ -61,6 +61,10 @@ export interface RetryPurchaseResult { retry_attempt_n: number; email_sent?: boolean; email_reason?: string; + // Resolved To address that received the approval email; surfaced so + // the post-submit toast can name the approver per CR pass on PR #294. + // Absent when recipient resolution itself failed (no approvers configured). + approval_recipient?: string; } export async function retryPurchase( diff --git a/frontend/src/api/types.ts b/frontend/src/api/types.ts index 2907bcaa..b98d88f3 100644 --- a/frontend/src/api/types.ts +++ b/frontend/src/api/types.ts @@ -202,6 +202,11 @@ export interface PurchaseResult { // History instead of waiting for an inbox. email_sent?: boolean; email_reason?: string; + // Resolved To address that received the approval email. Surfaced so the + // post-submit toast can name the approver ("Approval request sent to + // alice@acme.com") per the CR pass on PR #294 / issue #288. Absent when + // recipient resolution itself failed (no approvers configured). + approval_recipient?: string; results?: Array<{ recommendation_id: string; status: string; diff --git a/frontend/src/app.ts b/frontend/src/app.ts index 2d40f8c8..402fcffc 100644 --- a/frontend/src/app.ts +++ b/frontend/src/app.ts @@ -275,11 +275,17 @@ async function handleExecutePurchase(): Promise { return; } + // Default approval-required path: clicking sends an approval request to + // the configured approver(s) — it does NOT spend money. The actual + // upfront charge fires only after an approver clicks the email link. + // Issue #289 will introduce a session-permission branch where holders + // of `execute-any:purchases` can opt into direct execution; until that + // lands, every user is on this approval path. const ok = await confirmDialog({ - title: `Execute ${localRecs.length} purchase${localRecs.length === 1 ? '' : 's'}?`, - body: 'This will spend real money on cloud commitments. Make sure the selection + terms + payment options are what you intend.', - confirmLabel: 'Execute purchases', - destructive: true, + title: `Send ${localRecs.length} purchase${localRecs.length === 1 ? '' : 's'} for approval?`, + body: 'This will email an approval request to the configured approver. Cloud commitments are charged only after the approver clicks the link in that email.', + confirmLabel: 'Send for approval', + destructive: false, }); if (!ok) return; @@ -319,7 +325,7 @@ async function handleExecutePurchase(): Promise { const executeBtn = document.getElementById('execute-purchase-btn') as HTMLButtonElement | null; if (executeBtn) { executeBtn.disabled = true; - executeBtn.textContent = 'Executing...'; + executeBtn.textContent = 'Sending...'; } try { @@ -339,16 +345,28 @@ async function handleExecutePurchase(): Promise { timeout: null, }); } else { - showToast({ message: 'Purchase submitted — check your email to approve.', kind: 'success', timeout: 10_000 }); + // Name the approver in the success toast so the user can confirm WHO + // received the request (CR pass on PR #294 / issue #288). The backend + // surfaces `approval_recipient` from `resolveApprovalRecipients`; the + // field may be absent on older deploys or when only the global notify + // mailbox is configured — fall back to the generic line in that case. + const recipient = result.approval_recipient; + showToast({ + message: recipient + ? `Approval request sent to ${recipient}.` + : 'Purchase submitted — check your email to approve.', + kind: 'success', + timeout: 10_000, + }); } await loadDashboard(); } catch (error) { const err = error as Error; - showToast({ message: `Failed to execute purchase: ${err.message}`, kind: 'error' }); + showToast({ message: `Failed to send purchase for approval: ${err.message}`, kind: 'error' }); } finally { if (executeBtn) { executeBtn.disabled = false; - executeBtn.textContent = 'Execute Purchase'; + executeBtn.textContent = 'Send for Approval'; } } } @@ -363,18 +381,21 @@ async function handleExecutePurchase(): Promise { * confirmDialog. */ 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. const ok = await confirmDialog({ - title: `Execute ${buckets.length} bulk purchase${buckets.length === 1 ? '' : 's'}?`, - body: `This will submit ${buckets.length} separate purchase execution${buckets.length === 1 ? '' : 's'} and send ${buckets.length} approval email${buckets.length === 1 ? '' : 's'}. Each must be approved individually.`, - confirmLabel: 'Execute all', - destructive: true, + 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', + destructive: false, }); if (!ok) return; const executeBtn = document.getElementById('execute-purchase-btn') as HTMLButtonElement | null; if (executeBtn) { executeBtn.disabled = true; - executeBtn.textContent = `Executing 0/${buckets.length}…`; + executeBtn.textContent = `Sending 0/${buckets.length}…`; } // Fire all POSTs in parallel via allSettled so one failure doesn't @@ -402,22 +423,57 @@ async function handleFanOutExecute(buckets: FanOutBucket[]): Promise { ); const results = await Promise.allSettled(promises); - const succeeded = results.filter((r) => r.status === 'fulfilled').length; + // Reclassify business-level email failures: a fulfilled POST that returns + // email_sent === false or status === 'failed' is not a true success — + // the approval email never went out (CR pass on PR #294 Finding 2). + const fulfilled = results.filter( + (r): r is PromiseFulfilledResult => r.status === 'fulfilled', + ); + const submissionFailures = fulfilled.filter( + (r) => r.value.email_sent === false || r.value.status === 'failed', + ); + const succeeded = fulfilled.length - submissionFailures.length; const failed = results.length - succeeded; closePurchaseModal(); clearFanOutBuckets(); clearPurchaseModalRecommendations(); if (failed === 0) { + // Collect the unique approval-recipient set from truly-succeeded responses + // only (email_sent !== false and status !== 'failed') so the toast doesn't + // name a recipient whose email never arrived. Multi-bucket purchases can + // route to different approvers; dedupe so the toast is compact. + const recipients = new Set(); + for (const r of fulfilled) { + if ( + r.value.email_sent !== false && + r.value.status !== 'failed' && + r.value.approval_recipient + ) { + recipients.add(r.value.approval_recipient); + } + } + const noun = succeeded === 1 ? 'purchase' : 'purchases'; + let message: string; + if (recipients.size === 0) { + message = `${succeeded} ${noun} submitted — check your email to approve each.`; + } else if (recipients.size === 1) { + message = `${succeeded} ${noun} sent for approval to ${[...recipients][0]}.`; + } else { + message = `${succeeded} ${noun} sent for approval to ${recipients.size} recipients (${[...recipients].sort().join(', ')}).`; + } showToast({ - message: `${succeeded} purchase${succeeded === 1 ? '' : 's'} submitted — check your email to approve each.`, + message, kind: 'success', timeout: 15_000, }); } else { - const failureMsgs = results - .filter((r): r is PromiseRejectedResult => r.status === 'rejected') - .map((r) => (r.reason instanceof Error ? r.reason.message : String(r.reason))) + const failureMsgs = [ + ...results + .filter((r): r is PromiseRejectedResult => r.status === 'rejected') + .map((r) => (r.reason instanceof Error ? r.reason.message : String(r.reason))), + ...submissionFailures.map((r) => r.value.email_reason || 'approval email did not send'), + ] .slice(0, 3) .join('; '); showToast({ @@ -430,7 +486,7 @@ async function handleFanOutExecute(buckets: FanOutBucket[]): Promise { if (executeBtn) { executeBtn.disabled = false; - executeBtn.textContent = 'Execute Purchase'; + executeBtn.textContent = 'Send for Approval'; } } diff --git a/frontend/src/index.html b/frontend/src/index.html index 0074ea2f..9c4fd893 100644 --- a/frontend/src/index.html +++ b/frontend/src/index.html @@ -809,7 +809,7 @@

Configure Purchase

diff --git a/frontend/src/recommendations.ts b/frontend/src/recommendations.ts index 46669137..b4248968 100644 --- a/frontend/src/recommendations.ts +++ b/frontend/src/recommendations.ts @@ -2081,7 +2081,7 @@ function handleBulkPurchaseClick(recommendations: LocalRecommendation[]): void { } // Single-bucket happy path: open the preview modal + submit via the - // existing execute-purchase flow. The modal's "Execute Purchase" button + // existing approval-request flow. The modal's "Send for Approval" button // (wired in app.ts) picks up the recs via getPurchaseModalRecommendations. // openPurchaseModal is async (issue #111 (iii): per-rec override // prefetch); fire-and-forget — the modal is the user's surface. @@ -2125,7 +2125,7 @@ export interface FanOutBucket { paymentSource: 'override' | 'toolbar'; } -// Fan-out modal state. app.ts's Execute Purchase click reads these +// Fan-out modal state. app.ts's Send-for-Approval click reads these // via getFanOutBuckets() to fire one POST per bucket. Cleared when // the modal closes. let currentFanOutBuckets: FanOutBucket[] | null = null; @@ -2742,6 +2742,20 @@ export async function openPurchaseModal(recommendations: LocalRecommendation[]): upfrontLine.appendChild(upfrontStrong); summary.appendChild(upfrontLine); + // Approval-required note: clicking the modal's primary button does NOT + // execute the purchase — it sends an approval-request email to the + // configured approver(s). The actual upfront charges fire only when an + // approver clicks the link in that email. Issue #288 closed the + // earlier "Execute Purchase" button-label gap that implied immediate + // execution; #289 will introduce a session-permission branch where + // holders of `execute-any:purchases` can opt into direct execution and + // this note will become conditional on the resolved auth path. + const approvalNote = document.createElement('p'); + approvalNote.className = 'approval-required-note'; + approvalNote.textContent = + 'Submitting will email an approval request to the configured approver — commitments are charged only after the approver clicks the link in that email.'; + summary.appendChild(approvalNote); + container.appendChild(summary); // Commitments table with per-row Term and Payment selects. diff --git a/internal/api/handler_purchases.go b/internal/api/handler_purchases.go index 6709c91d..df2eba1c 100644 --- a/internal/api/handler_purchases.go +++ b/internal/api/handler_purchases.go @@ -564,7 +564,7 @@ func (h *Handler) retryPurchase(ctx context.Context, req *events.LambdaFunctionU // `failed` so the user sees the reason in History; the linkage on // the original row is unaffected (it points at the failed-again // successor, which is exactly the audit trail we want). - emailSent, emailReason := h.sendPurchaseApprovalEmail(ctx, req, newExecution, failedExec.Recommendations, totalUpfront, totalSavings) + emailSent, emailReason, recipient := h.sendPurchaseApprovalEmail(ctx, req, newExecution, failedExec.Recommendations, totalUpfront, totalSavings) status := h.finalizePurchaseStatus(ctx, newExecution, emailSent, emailReason) resp := map[string]any{ @@ -580,6 +580,9 @@ func (h *Handler) retryPurchase(ctx context.Context, req *events.LambdaFunctionU if emailReason != "" { resp["email_reason"] = emailReason } + if recipient != "" { + resp["approval_recipient"] = recipient + } return resp, nil } @@ -1092,7 +1095,7 @@ func (h *Handler) executePurchase(ctx context.Context, req *events.LambdaFunctio // best-effort and never blocks the response body; the returned // email_sent / email_reason fields let the UI tell the user whether they // should wait for an inbox or cancel/retry manually. - emailSent, emailReason := h.sendPurchaseApprovalEmail(ctx, req, execution, execReq.Recommendations, totalUpfront, totalSavings) + emailSent, emailReason, recipient := h.sendPurchaseApprovalEmail(ctx, req, execution, execReq.Recommendations, totalUpfront, totalSavings) status := h.finalizePurchaseStatus(ctx, execution, emailSent, emailReason) message := "Purchase execution created and pending approval" @@ -1111,24 +1114,33 @@ func (h *Handler) executePurchase(ctx context.Context, req *events.LambdaFunctio if emailReason != "" { resp["email_reason"] = emailReason } + if recipient != "" { + resp["approval_recipient"] = recipient + } return resp, nil } // sendPurchaseApprovalEmail sends an approval-request email for a newly created // execution and returns a structured outcome: -// - (true, "") on successful send -// - (false, "") on any preflight gate or send error +// - (true, "", recipient) on successful send +// - (false, "", "") on any preflight gate or send error +// - (false, "", recipient) when send failed AFTER recipient resolution +// (so the response can still surface who would have been notified) +// +// `recipient` is the resolved To address per `resolveApprovalRecipients` — +// surfaced in the response so the post-submit toast can name the approver +// per CR pass on PR #294 / issue #288. // // Errors are also logged at Errorf level so they show up in CloudWatch, but // the reason string is what the API response surfaces to the UI. -func (h *Handler) sendPurchaseApprovalEmail(ctx context.Context, req *events.LambdaFunctionURLRequest, execution *config.PurchaseExecution, recs []config.RecommendationRecord, totalUpfront, totalSavings float64) (bool, string) { +func (h *Handler) sendPurchaseApprovalEmail(ctx context.Context, req *events.LambdaFunctionURLRequest, execution *config.PurchaseExecution, recs []config.RecommendationRecord, totalUpfront, totalSavings float64) (bool, string, string) { if h.emailNotifier == nil { - return false, "email notifier not configured for this deployment" + return false, "email notifier not configured for this deployment", "" } globalCfg, err := h.config.GetGlobalConfig(ctx) if err != nil { logging.Errorf("Failed to load global config for approval email: %v", err) - return false, fmt.Sprintf("failed to load settings: %v", err) + return false, fmt.Sprintf("failed to load settings: %v", err), "" } globalNotify := "" if globalCfg.NotificationEmail != nil { @@ -1137,10 +1149,10 @@ func (h *Handler) sendPurchaseApprovalEmail(ctx context.Context, req *events.Lam to, cc, approvers, err := h.resolveApprovalRecipients(ctx, recs, globalNotify) if err != nil { logging.Errorf("Failed to resolve approval recipients: %v", err) - return false, fmt.Sprintf("failed to resolve recipients: %v", err) + return false, fmt.Sprintf("failed to resolve recipients: %v", err), "" } if to == "" { - return false, "no notification email set in Settings → General and no account contact emails configured" + return false, "no notification email set in Settings → General and no account contact emails configured", "" } summaries := make([]email.RecommendationSummary, 0, len(recs)) for _, rec := range recs { @@ -1168,14 +1180,14 @@ func (h *Handler) sendPurchaseApprovalEmail(ctx context.Context, req *events.Lam logging.Errorf("Failed to send purchase approval email: %v", err) switch { case errors.Is(err, email.ErrNoRecipient): - return false, "no notification email set in Settings → General" + return false, "no notification email set in Settings → General", "" case errors.Is(err, email.ErrNoFromEmail): - return false, "FROM_EMAIL not configured for this deployment" + return false, "FROM_EMAIL not configured for this deployment", to default: - return false, fmt.Sprintf("send failed: %v", err) + return false, fmt.Sprintf("send failed: %v", err), to } } - return true, "" + return true, "", to } // resolveDashboardURL returns the absolute base URL to embed in email