diff --git a/frontend/src/__tests__/history-retry-button.test.ts b/frontend/src/__tests__/history-retry-button.test.ts index 52a11691e..1a9d361c6 100644 --- a/frontend/src/__tests__/history-retry-button.test.ts +++ b/frontend/src/__tests__/history-retry-button.test.ts @@ -16,9 +16,12 @@ * 6. retry_execution_id set → no Retry button (act on the descendant). * 7. lineage links: "↻ Retried as #abc" for predecessors, "↻ Retry #n" for retries. * 8. Click + decline confirmDialog → no API call. - * 9. Click + accept → retryPurchase + reload + toast. + * 9. Click + accept → retryPurchase + reload + success toast (email_sent true). * 10. retryPurchase rejects → toast.error + button re-enabled. * 11. Over-threshold click sends force=true. + * 12. email_sent absent but status=pending → success toast. + * 13. email_sent false → warning toast (approval email failed). + * 14. email_sent false + status pending → warning toast (explicit failure overrides status). */ import { loadHistory } from '../history'; @@ -214,7 +217,13 @@ describe('History inline Retry button (issue #47)', () => { test('threshold-reached row renders override button + sends force=true on accept', async () => { (getCurrentUser as jest.Mock).mockReturnValue(ADMIN_USER); (confirmDialog as jest.Mock).mockResolvedValue(true); - (api.retryPurchase as jest.Mock).mockResolvedValue({ execution_id: 'new', original_execution: 'r-1' }); + (api.retryPurchase as jest.Mock).mockResolvedValue({ + execution_id: 'new', + original_execution: 'r-1', + status: 'pending', + retry_attempt_n: 6, + email_sent: true, + }); (api.getHistory as jest.Mock).mockResolvedValue({ summary: {}, purchases: [ @@ -286,10 +295,16 @@ describe('History inline Retry button (issue #47)', () => { expect(api.retryPurchase).not.toHaveBeenCalled(); }); - test('accepted confirm posts retry + reloads + toasts', async () => { + test('accepted confirm posts retry + reloads + success toast when email_sent is true', async () => { (getCurrentUser as jest.Mock).mockReturnValue(ADMIN_USER); (confirmDialog as jest.Mock).mockResolvedValue(true); - (api.retryPurchase as jest.Mock).mockResolvedValue({ execution_id: 'new', original_execution: 'r-1' }); + (api.retryPurchase as jest.Mock).mockResolvedValue({ + execution_id: 'new', + original_execution: 'r-1', + status: 'pending', + retry_attempt_n: 1, + email_sent: true, + }); (api.getHistory as jest.Mock).mockResolvedValue({ summary: {}, purchases: [makeRow({ purchase_id: 'r-1', created_by_user_id: ADMIN_USER.id })], @@ -303,7 +318,97 @@ describe('History inline Retry button (issue #47)', () => { expect(api.retryPurchase).toHaveBeenCalledWith('r-1', undefined); expect(api.getHistory).toHaveBeenCalledTimes(2); - expect(showToast).toHaveBeenCalledWith(expect.objectContaining({ kind: 'success' })); + expect(showToast).toHaveBeenCalledWith( + expect.objectContaining({ kind: 'success', message: 'Purchase request sent for approval' }), + ); + }); + + test('retry success toast uses approval wording when status is pending but email_sent absent', async () => { + (getCurrentUser as jest.Mock).mockReturnValue(ADMIN_USER); + (confirmDialog as jest.Mock).mockResolvedValue(true); + // Backend may omit email_sent; status==='pending' is sufficient. + (api.retryPurchase as jest.Mock).mockResolvedValue({ + execution_id: 'new', + original_execution: 'r-1', + status: 'pending', + retry_attempt_n: 1, + }); + (api.getHistory as jest.Mock).mockResolvedValue({ + summary: {}, + purchases: [makeRow({ purchase_id: 'r-1', created_by_user_id: ADMIN_USER.id })], + }); + + await loadHistory(); + const btn = document.querySelector('.history-retry-btn'); + btn?.click(); + await new Promise((r) => setTimeout(r, 10)); + + expect(showToast).toHaveBeenCalledWith( + expect.objectContaining({ kind: 'success', message: 'Purchase request sent for approval' }), + ); + }); + + test('retry shows warning toast when email_sent is false', async () => { + (getCurrentUser as jest.Mock).mockReturnValue(ADMIN_USER); + (confirmDialog as jest.Mock).mockResolvedValue(true); + (api.retryPurchase as jest.Mock).mockResolvedValue({ + execution_id: 'new', + original_execution: 'r-1', + status: 'failed', + retry_attempt_n: 1, + email_sent: false, + }); + (api.getHistory as jest.Mock).mockResolvedValue({ + summary: {}, + purchases: [makeRow({ purchase_id: 'r-1', created_by_user_id: ADMIN_USER.id })], + }); + console.error = jest.fn(); + + await loadHistory(); + const btn = document.querySelector('.history-retry-btn'); + btn?.click(); + await new Promise((r) => setTimeout(r, 10)); + + expect(showToast).toHaveBeenCalledWith( + expect.objectContaining({ + kind: 'warning', + message: expect.stringContaining('approval email failed'), + }), + ); + }); + + test('email_sent false overrides pending status - shows warning not success toast', async () => { + // This is the case CR round 2 identified: email_sent===false must win over + // status==='pending'. The prior logic short-circuited on the status check + // and could produce a false success toast on conflicting payloads. + (getCurrentUser as jest.Mock).mockReturnValue(ADMIN_USER); + (confirmDialog as jest.Mock).mockResolvedValue(true); + (api.retryPurchase as jest.Mock).mockResolvedValue({ + execution_id: 'new', + original_execution: 'r-1', + status: 'pending', + retry_attempt_n: 1, + email_sent: false, + }); + (api.getHistory as jest.Mock).mockResolvedValue({ + summary: {}, + purchases: [makeRow({ purchase_id: 'r-1', created_by_user_id: ADMIN_USER.id })], + }); + + await loadHistory(); + const btn = document.querySelector('.history-retry-btn'); + btn?.click(); + await new Promise((r) => setTimeout(r, 10)); + + expect(showToast).toHaveBeenCalledWith( + expect.objectContaining({ + kind: 'warning', + message: expect.stringContaining('approval email failed'), + }), + ); + expect(showToast).not.toHaveBeenCalledWith( + expect.objectContaining({ kind: 'success' }), + ); }); test('retry API failure surfaces toast and re-enables the button', async () => { diff --git a/frontend/src/history.ts b/frontend/src/history.ts index 9287bde6b..f1cf7e506 100644 --- a/frontend/src/history.ts +++ b/frontend/src/history.ts @@ -786,8 +786,9 @@ function wireRowActionHandlers(container: HTMLElement): void { }); if (!ok) return; btn.disabled = true; + let retryResult: Awaited>; try { - await api.retryPurchase(id, overThreshold ? { force: true } : undefined); + retryResult = await api.retryPurchase(id, overThreshold ? { force: true } : undefined); } catch (retryError) { console.error('Failed to retry purchase:', retryError); // Surface structured retry hints from the backend (issue #47): @@ -809,10 +810,22 @@ function wireRowActionHandlers(container: HTMLElement): void { btn.disabled = false; return; } - // Retry POST succeeded — surface success regardless of whether - // the refresh works. The reload error path mirrors the cancel - // flow above. - showToast({ message: 'Retry execution created', kind: 'success', timeout: 5_000 }); + // Gate the toast on the approval email outcome reported by the backend. + // email_sent===false is an explicit failure signal that overrides any + // status-based inference; show a warning even when status==='pending'. + // email_sent===true or a pending/notified status (with email_sent absent) + // means the approval request is in the queue. + const emailExplicitlyFailed = retryResult.email_sent === false; + const emailOk = !emailExplicitlyFailed && ( + retryResult.email_sent === true + || retryResult.status === 'pending' + || retryResult.status === 'notified' + ); + if (emailOk) { + showToast({ message: 'Purchase request sent for approval', kind: 'success', timeout: 5_000 }); + } else { + showToast({ message: 'Retry created but approval email failed - check your notification settings', kind: 'warning', timeout: 8_000 }); + } try { await loadHistory(); } catch (reloadError) {