diff --git a/frontend/src/__tests__/purchase-execution-toast.test.ts b/frontend/src/__tests__/purchase-execution-toast.test.ts index 6c2317f7e..6f190e301 100644 --- a/frontend/src/__tests__/purchase-execution-toast.test.ts +++ b/frontend/src/__tests__/purchase-execution-toast.test.ts @@ -388,6 +388,61 @@ describe('handleExecutePurchase — single-record path', () => { }); }); +// Issue #735: toast must show notification_email (approval_recipient from API), +// not a hardcoded or per-account contact_email. The backend now sets +// approval_recipient = notification_email when configured; the toast must use +// whatever the API returns without a hardcoded fallback. +describe('issue #735 — toast uses API approval_recipient, not hardcoded email', () => { + beforeEach(() => { + jest.clearAllMocks(); + (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('toast shows the notification_email returned by the API as approval_recipient', async () => { + const notificationEmail = 'admin@company.example'; + (api.executePurchase as jest.Mock).mockResolvedValue({ + execution_id: 'exec-735', + status: 'queued', + email_sent: true, + approval_recipient: notificationEmail, + }); + + const btn = setup(); + btn.click(); + await new Promise((r) => setTimeout(r, 0)); + + expect(lastToastMessage()).toContain(notificationEmail); + expect(lastToastMessage()).toContain('Approval request sent to'); + expect(lastToastKind()).toBe('success'); + }); + + test('toast does not contain a different address when API returns notification_email', async () => { + const notificationEmail = 'admin@company.example'; + const contactEmail = 'contact@acct.example'; + // Simulate backend now returning notification_email (not contact_email). + (api.executePurchase as jest.Mock).mockResolvedValue({ + execution_id: 'exec-735b', + status: 'queued', + email_sent: true, + approval_recipient: notificationEmail, + }); + + const btn = setup(); + btn.click(); + await new Promise((r) => setTimeout(r, 0)); + + expect(lastToastMessage()).toContain(notificationEmail); + expect(lastToastMessage()).not.toContain(contactEmail); + expect(lastToastKind()).toBe('success'); + }); +}); + describe('handleFanOutExecute — fan-out path', () => { function buildBucket(id: string, capacityPercent = 100) { return { diff --git a/internal/api/coverage_gaps_test.go b/internal/api/coverage_gaps_test.go index 4f5ad3256..999de28dc 100644 --- a/internal/api/coverage_gaps_test.go +++ b/internal/api/coverage_gaps_test.go @@ -768,6 +768,105 @@ func TestHandler_sendPurchaseApprovalEmail_NoNotificationEmail(t *testing.T) { ) } +// --------------------------------------------------------------------------- +// sendPurchaseApprovalEmail — approval_recipient uses notification_email (issue #735) +// --------------------------------------------------------------------------- + +// recordingEmailNotifier captures the NotificationData passed to +// SendPurchaseApprovalRequest so tests can assert on recipient fields. +type recordingEmailNotifier struct { + stubEmailNotifier + captured email.NotificationData +} + +func (r *recordingEmailNotifier) SendPurchaseApprovalRequest(_ context.Context, data email.NotificationData) error { + r.captured = data + return nil +} + +// TestHandler_sendPurchaseApprovalEmail_ResponseRecipientUsesNotificationEmail +// is the regression test for issue #735: when both a per-account contact_email +// and a global notification_email are configured, the approval_recipient field +// returned by sendPurchaseApprovalEmail must equal notification_email (the +// value the History UI shows via resolvePendingApproverEmail), not contact_email. +// The actual email To address remains the contact_email — the fix only affects +// the approval_recipient used in the post-submit toast. +func TestHandler_sendPurchaseApprovalEmail_ResponseRecipientUsesNotificationEmail(t *testing.T) { + ctx := context.Background() + notificationEmail := "admin@example.com" + contactEmail := "contact@acct.example.com" + accountID := "acct-1" + + mockStore := new(MockConfigStore) + mockStore.On("GetGlobalConfig", ctx).Return(&config.GlobalConfig{ + NotificationEmail: ¬ificationEmail, + }, nil) + mockStore.GetCloudAccountFn = func(_ context.Context, id string) (*config.CloudAccount, error) { + return &config.CloudAccount{ID: id, ContactEmail: contactEmail}, nil + } + + notifier := &recordingEmailNotifier{} + h := &Handler{ + config: mockStore, + emailNotifier: notifier, + } + + exec := &config.PurchaseExecution{ + ExecutionID: "11111111-1111-1111-1111-111111111111", + ApprovalToken: "tok", + Recommendations: []config.RecommendationRecord{ + {ID: "r1", CloudAccountID: &accountID}, + }, + } + emailSent, _, responseRecipient := h.sendPurchaseApprovalEmail(ctx, nil, exec, exec.Recommendations, 0, 0) + + require.True(t, emailSent, "email send must succeed") + // The response recipient surfaced in the toast must be the notification_email + // (consistent with History), not the per-account contact_email. + assert.Equal(t, notificationEmail, responseRecipient, + "approval_recipient must equal notification_email so the toast matches History (issue #735)") + // The actual email To must still be the contact_email (security model unchanged). + assert.Equal(t, contactEmail, notifier.captured.RecipientEmail, + "actual email To address must be the per-account contact_email") +} + +// TestHandler_sendPurchaseApprovalEmail_ResponseRecipientFallsBackToContactEmail +// covers the case where no notification_email is set: responseRecipient falls +// back to the per-account contact_email (the actual To address). +func TestHandler_sendPurchaseApprovalEmail_ResponseRecipientFallsBackToContactEmail(t *testing.T) { + ctx := context.Background() + contactEmail := "contact@acct.example.com" + accountID := "acct-2" + + mockStore := new(MockConfigStore) + mockStore.On("GetGlobalConfig", ctx).Return(&config.GlobalConfig{ + // NotificationEmail intentionally not set. + }, nil) + mockStore.GetCloudAccountFn = func(_ context.Context, id string) (*config.CloudAccount, error) { + return &config.CloudAccount{ID: id, ContactEmail: contactEmail}, nil + } + + notifier := &recordingEmailNotifier{} + h := &Handler{ + config: mockStore, + emailNotifier: notifier, + } + + exec := &config.PurchaseExecution{ + ExecutionID: "22222222-2222-2222-2222-222222222222", + ApprovalToken: "tok2", + Recommendations: []config.RecommendationRecord{ + {ID: "r1", CloudAccountID: &accountID}, + }, + } + emailSent, _, responseRecipient := h.sendPurchaseApprovalEmail(ctx, nil, exec, exec.Recommendations, 0, 0) + + require.True(t, emailSent, "email send must succeed") + // Without a notification_email, the response recipient falls back to contact_email. + assert.Equal(t, contactEmail, responseRecipient, + "when notification_email is absent, approval_recipient falls back to contact_email") +} + // --------------------------------------------------------------------------- // Helper types for tests above // --------------------------------------------------------------------------- diff --git a/internal/api/handler_purchases.go b/internal/api/handler_purchases.go index c00644a6f..943662a21 100644 --- a/internal/api/handler_purchases.go +++ b/internal/api/handler_purchases.go @@ -1501,6 +1501,20 @@ func archeraEducationURL(dashboardBase string) string { return dashboardBase + "/archera-insurance" } +// approvalResponseRecipient returns the email address to surface in the +// approval_recipient API response field (and therefore in the post-submit toast). +// It returns globalNotify when set (after trimming whitespace), matching the +// address the History handler shows for pending rows (resolvePendingApproverEmail +// also returns globalNotify first). Falls back to to (the per-account +// contact_email) when globalNotify is empty or whitespace-only. +// Extracted to keep sendPurchaseApprovalEmail under the cyclomatic-complexity ceiling. +func approvalResponseRecipient(globalNotify, to string) string { + if trimmed := strings.TrimSpace(globalNotify); trimmed != "" { + return trimmed + } + return to +} + // sendPurchaseApprovalEmail sends an approval-request email for a newly created // execution and returns a structured outcome: // - (true, "", recipient) on successful send @@ -1508,9 +1522,13 @@ func archeraEducationURL(dashboardBase string) string { // - (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. +// `recipient` is the address surfaced in the post-submit toast. It is the +// Admin notification email (Settings -> General) when configured, matching the +// value the History UI shows for pending rows via resolvePendingApproverEmail. +// When no notification email is set, it falls back to the per-account +// contact_email (the actual To address). This fixes issue #735 where the toast +// named the per-account contact_email instead of the Admin notification email, +// creating a discrepancy with the History "awaiting approval from X" display. // // 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. @@ -1535,6 +1553,12 @@ func (h *Handler) sendPurchaseApprovalEmail(ctx context.Context, req *events.Lam if to == "" { return false, "no notification email set in Settings → General and no account contact emails configured", "" } + // responseRecipient is the email address surfaced in the UI toast (approval_recipient + // API field). It matches what the History handler shows for pending rows via + // resolvePendingApproverEmail, which always returns globalNotify when set. Using + // globalNotify here keeps both displays consistent. When globalNotify is empty, + // fall back to to (the per-account contact_email). See issue #735. + responseRecipient := approvalResponseRecipient(globalNotify, to) summaries := make([]email.RecommendationSummary, 0, len(recs)) for _, rec := range recs { summaries = append(summaries, email.RecommendationSummary{ @@ -1565,12 +1589,12 @@ func (h *Handler) sendPurchaseApprovalEmail(ctx context.Context, req *events.Lam case errors.Is(err, email.ErrNoRecipient): return false, "no notification email set in Settings → General", "" case errors.Is(err, email.ErrNoFromEmail): - return false, "FROM_EMAIL not configured for this deployment", to + return false, "FROM_EMAIL not configured for this deployment", responseRecipient default: - return false, fmt.Sprintf("send failed: %v", err), to + return false, fmt.Sprintf("send failed: %v", err), responseRecipient } } - return true, "", to + return true, "", responseRecipient } // resolveDashboardURL returns the absolute base URL to embed in email diff --git a/internal/api/handler_purchases_test.go b/internal/api/handler_purchases_test.go index 111c483b4..c6c976d95 100644 --- a/internal/api/handler_purchases_test.go +++ b/internal/api/handler_purchases_test.go @@ -2572,3 +2572,21 @@ func TestResolveOpsHint(t *testing.T) { }) } } + +// TestHandler_approvalResponseRecipient_TrimsWhitespace verifies that a +// whitespace-only notification_email does not count as set: the contact_email +// fallback must be used instead, and no whitespace must appear in the response. +func TestHandler_approvalResponseRecipient_TrimsWhitespace(t *testing.T) { + result := approvalResponseRecipient(" \t\n ", "contact@example.com") + assert.Equal(t, "contact@example.com", result, + "whitespace-only globalNotify must fall back to contact email") +} + +// TestHandler_approvalResponseRecipient_TrimsNonEmptyValue verifies that when +// notification_email has surrounding whitespace the returned value is trimmed, +// so no stray spaces appear in the toast or email headers. +func TestHandler_approvalResponseRecipient_TrimsNonEmptyValue(t *testing.T) { + result := approvalResponseRecipient(" cristi@example.com ", "contact@example.com") + assert.Equal(t, "cristi@example.com", result, + "globalNotify with surrounding whitespace must be returned trimmed") +}