From 1bd694985f5d9eb92b0f107c59a2c30aace1fa25 Mon Sep 17 00:00:00 2001 From: TurtleWolfe Date: Tue, 28 Apr 2026 00:42:17 +0000 Subject: [PATCH 1/5] feat(payments): schema + types for retry tracking + payment_retry audit event MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds the database surface that the next two commits build on. Idempotent ALTERs to the monolithic migration; new columns inherit existing RLS policies (verified by pnpm test:rls — 55/55 green). Schema: - payment_intents.retry_count INTEGER NOT NULL DEFAULT 0 - payment_intents.parent_intent_id UUID REFERENCES payment_intents(id) ON DELETE SET NULL, with a partial index for parent-lookup queries - auth_audit_logs.event_type CHECK extended with 'payment_retry' Both fresh-DB (CREATE TABLE inline) and existing-DB (ALTER ... DROP + ADD CONSTRAINT) paths are covered. Applied to local Supabase via psql as supabase_admin; cloud application deferred to PR review per the \"local-first sandbox\" memory rule. Types: - src/types/payment.ts — PaymentIntent gains retry_count, parent_intent_id, idempotency_key (the last was already in the DB from PR #59 but had not been mirrored into the domain type) - src/lib/supabase/types.ts — same three columns added to Row/Insert/Update, plus the new self-referential FK Relationship entry - src/lib/auth/audit-logger.ts — AuditEventType union gains 'payment_retry' Test fixtures: - tests/unit/payment-service.test.ts — isPaymentIntentExpired fixture needs the three new required fields Why this commit on its own: schema + type contract is the ground truth that the service and UI commits build on. Reviewing it standalone makes the rest of the PR cheaper to read. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/lib/auth/audit-logger.ts | 3 +- src/lib/supabase/types.ts | 19 ++++++++++- src/types/payment.ts | 3 ++ .../20251006_complete_monolithic_setup.sql | 33 ++++++++++++++++++- tests/unit/payment-service.test.ts | 6 ++++ 5 files changed, 61 insertions(+), 3 deletions(-) diff --git a/src/lib/auth/audit-logger.ts b/src/lib/auth/audit-logger.ts index d60e56d5..4ecb3449 100644 --- a/src/lib/auth/audit-logger.ts +++ b/src/lib/auth/audit-logger.ts @@ -18,7 +18,8 @@ export type AuditEventType = | 'password_reset_request' | 'email_verification' | 'oauth_link' - | 'oauth_unlink'; + | 'oauth_unlink' + | 'payment_retry'; export interface AuditLogEntry { user_id?: string; diff --git a/src/lib/supabase/types.ts b/src/lib/supabase/types.ts index ddf906a4..aa162fae 100644 --- a/src/lib/supabase/types.ts +++ b/src/lib/supabase/types.ts @@ -48,8 +48,11 @@ export type Database = { description: string | null; expires_at: string; id: string; + idempotency_key: string | null; interval: string | null; metadata: Json | null; + parent_intent_id: string | null; + retry_count: number; template_user_id: string; type: string; }; @@ -61,8 +64,11 @@ export type Database = { description?: string | null; expires_at?: string; id?: string; + idempotency_key?: string | null; interval?: string | null; metadata?: Json | null; + parent_intent_id?: string | null; + retry_count?: number; template_user_id: string; type: string; }; @@ -74,12 +80,23 @@ export type Database = { description?: string | null; expires_at?: string; id?: string; + idempotency_key?: string | null; interval?: string | null; metadata?: Json | null; + parent_intent_id?: string | null; + retry_count?: number; template_user_id?: string; type?: string; }; - Relationships: []; + Relationships: [ + { + foreignKeyName: 'payment_intents_parent_intent_id_fkey'; + columns: ['parent_intent_id']; + isOneToOne: false; + referencedRelation: 'payment_intents'; + referencedColumns: ['id']; + }, + ]; }; payment_provider_config: { Row: { diff --git a/src/types/payment.ts b/src/types/payment.ts index a76a17ef..f6d87bd5 100644 --- a/src/types/payment.ts +++ b/src/types/payment.ts @@ -38,6 +38,9 @@ export interface PaymentIntent { customer_email: string; description: string | null; metadata: Record | null; + idempotency_key: string | null; + retry_count: number; + parent_intent_id: string | null; created_at: string; // ISO 8601 timestamp expires_at: string; // ISO 8601 timestamp } diff --git a/supabase/migrations/20251006_complete_monolithic_setup.sql b/supabase/migrations/20251006_complete_monolithic_setup.sql index c1645cbb..eae1cd8e 100644 --- a/supabase/migrations/20251006_complete_monolithic_setup.sql +++ b/supabase/migrations/20251006_complete_monolithic_setup.sql @@ -62,6 +62,19 @@ CREATE UNIQUE INDEX IF NOT EXISTS idx_payment_intents_idempotency_key ON payment_intents(idempotency_key) WHERE idempotency_key IS NOT NULL; +-- Retry tracking (#43, B1). Each retry creates a new INSERT-only row that +-- references its parent and bumps retry_count. The "Payment intents are +-- immutable" UPDATE policy below ensures retry_count only ever moves +-- forward (set on INSERT, never UPDATEd). +ALTER TABLE payment_intents + ADD COLUMN IF NOT EXISTS retry_count INTEGER NOT NULL DEFAULT 0; +ALTER TABLE payment_intents + ADD COLUMN IF NOT EXISTS parent_intent_id UUID REFERENCES payment_intents(id) ON DELETE SET NULL; + +CREATE INDEX IF NOT EXISTS idx_payment_intents_parent + ON payment_intents(parent_intent_id) + WHERE parent_intent_id IS NOT NULL; + COMMENT ON TABLE payment_intents IS 'Customer payment intentions before provider redirect (24hr expiry)'; -- Payment results @@ -201,7 +214,8 @@ CREATE TABLE IF NOT EXISTS auth_audit_logs ( 'email_verification', 'email_verification_sent', 'email_verification_complete', 'token_refresh', 'account_delete', - 'oauth_link', 'oauth_unlink' + 'oauth_link', 'oauth_unlink', + 'payment_retry' )), event_data JSONB, ip_address INET, @@ -219,6 +233,23 @@ CREATE INDEX IF NOT EXISTS idx_audit_logs_user_event ON auth_audit_logs(user_id, COMMENT ON TABLE auth_audit_logs IS 'Security audit trail for all auth events (90-day retention)'; +-- Idempotent extension of auth_audit_logs.event_type CHECK to include +-- 'payment_retry' (#43, B1). The CREATE TABLE above picks up the new +-- value on a fresh DB; this DROP + ADD applies it to existing DBs. +ALTER TABLE auth_audit_logs DROP CONSTRAINT IF EXISTS auth_audit_logs_event_type_check; +ALTER TABLE auth_audit_logs ADD CONSTRAINT auth_audit_logs_event_type_check + CHECK (event_type IN ( + 'sign_up', + 'sign_in', 'sign_in_success', 'sign_in_failed', + 'sign_out', + 'password_change', 'password_reset_request', 'password_reset_complete', + 'email_verification', 'email_verification_sent', 'email_verification_complete', + 'token_refresh', + 'account_delete', + 'oauth_link', 'oauth_unlink', + 'payment_retry' + )); + -- ============================================================================ -- PART 3: SECURITY TABLES (Feature 017) -- ============================================================================ diff --git a/tests/unit/payment-service.test.ts b/tests/unit/payment-service.test.ts index 6b9dbcb4..ccabcb6a 100644 --- a/tests/unit/payment-service.test.ts +++ b/tests/unit/payment-service.test.ts @@ -170,6 +170,9 @@ describe('Payment Service', () => { customer_email: 'test@example.com', description: null, metadata: null, + idempotency_key: null, + retry_count: 0, + parent_intent_id: null, created_at: new Date().toISOString(), expires_at: new Date(Date.now() + 3600000).toISOString(), // 1 hour from now }; @@ -188,6 +191,9 @@ describe('Payment Service', () => { customer_email: 'test@example.com', description: null, metadata: null, + idempotency_key: null, + retry_count: 0, + parent_intent_id: null, created_at: new Date(Date.now() - 7200000).toISOString(), // 2 hours ago expires_at: new Date(Date.now() - 3600000).toISOString(), // 1 hour ago }; From faadd2c3dafff1af5516ba7aee98b5bedda4cbb4 Mon Sep 17 00:00:00 2001 From: TurtleWolfe Date: Tue, 28 Apr 2026 00:42:53 +0000 Subject: [PATCH 2/5] feat(payments): retry service with idempotency-key reuse, attempt cap, cooling, expiry guard MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes the load-bearing gap from #43: the retry button was generating a fresh idempotency_key on every click, defeating the partial unique index that landed in PR #59. A doubled click or a click that races a webhook could produce two intents → two charges. Same-key reuse turns those races into ON CONFLICT no-ops. retryFailedPayment() now: - reuses parent.idempotency_key (FR-006); if absent (legacy intent), gens a fresh one and the retry chain still self-dedupes - bumps retry_count on the new (child) row; throws PaymentRetryLimitError past RETRY_LIMIT=3 (FR-009) - enforces COOLING_PERIOD_MS=30s since parent.created_at; throws PaymentRetryCoolingError carrying waitMs so UI can render countdown (FR-010) - guards parent.expires_at; throws PaymentRetryExpiredError if the parent's 24h provider session has lapsed - upserts with onConflict:'idempotency_key', ignoreDuplicates:true — a doubled click returns null which we surface as \"original is the truth\" by returning the parent intent (no double-charge, no UI surprise) - audit-logs every attempt as 'payment_retry' (NFR-007), including the deduped flag so admin analytics can distinguish a fresh retry attempt from a server-side dedupe of an already-won race New libs: - src/lib/payments/error-categorization.ts — pure mapping from Stripe + PayPal error_code (already in payment_results) to one of FR-002's 8 categories with non-technical user message + resolution hint + a recoverable flag that gates the retry button (FR-001/003/006/NFR-001). 27 tests including 'never leak raw provider text' and 'no technical jargon' - src/lib/payments/audit.ts — logPaymentRetryEvent wrapper around logAuthEvent. Non-throwing; an audit-write failure must never break payment UX Tests: 16 new in retry.test.ts (key reuse, cap, cooling, expiry, dedupe-conflict path, audit emission), 27 in error-categorization.test.ts. All 92 payment-service-area tests green; full suite 3249/3249. Why on its own: this is the load-bearing change. UI commit follows and just consumes the public surface (RETRY_LIMIT, error classes, hook). Co-Authored-By: Claude Opus 4.7 (1M context) --- .../__tests__/error-categorization.test.ts | 138 +++++++++++ src/lib/payments/__tests__/retry.test.ts | 230 ++++++++++++++++++ src/lib/payments/audit.ts | 67 +++++ src/lib/payments/error-categorization.ts | 157 ++++++++++++ src/lib/payments/payment-service.ts | 174 +++++++++++-- 5 files changed, 746 insertions(+), 20 deletions(-) create mode 100644 src/lib/payments/__tests__/error-categorization.test.ts create mode 100644 src/lib/payments/__tests__/retry.test.ts create mode 100644 src/lib/payments/audit.ts create mode 100644 src/lib/payments/error-categorization.ts diff --git a/src/lib/payments/__tests__/error-categorization.test.ts b/src/lib/payments/__tests__/error-categorization.test.ts new file mode 100644 index 00000000..842c2b82 --- /dev/null +++ b/src/lib/payments/__tests__/error-categorization.test.ts @@ -0,0 +1,138 @@ +/** + * Error categorization tests — pure mapping logic, no I/O. + * FR-001 (clear messages), FR-002 (categorization), FR-003 (resolution + * suggestions), NFR-001 (non-technical, actionable). + */ + +import { describe, it, expect } from 'vitest'; +import { + categorizePaymentError, + type PaymentErrorCategory, +} from '../error-categorization'; + +describe('categorizePaymentError', () => { + describe('Stripe error codes', () => { + it.each<[string, PaymentErrorCategory, boolean]>([ + ['card_declined', 'card_declined', true], + ['generic_decline', 'card_declined', true], + ['insufficient_funds', 'insufficient_funds', true], + ['expired_card', 'expired_card', false], + ['incorrect_cvc', 'invalid_card', true], + ['invalid_cvc', 'invalid_card', true], + ['invalid_number', 'invalid_card', true], + ['invalid_expiry_month', 'invalid_card', true], + ['invalid_expiry_year', 'invalid_card', true], + ['processing_error', 'processing_error', true], + ['authentication_required', 'authentication_required', true], + ['card_not_supported', 'card_declined', true], + ['currency_not_supported', 'limit_exceeded', false], + ['amount_too_large', 'limit_exceeded', false], + ['amount_too_small', 'limit_exceeded', false], + ])( + 'maps Stripe code %s to category %s (recoverable=%s)', + (code, expectedCategory, expectedRecoverable) => { + const result = categorizePaymentError(code, null); + expect(result.category).toBe(expectedCategory); + expect(result.recoverable).toBe(expectedRecoverable); + expect(result.userMessage.length).toBeGreaterThan(0); + expect(result.resolutionHint.length).toBeGreaterThan(0); + } + ); + }); + + describe('PayPal error codes', () => { + it.each<[string, PaymentErrorCategory, boolean]>([ + ['INSTRUMENT_DECLINED', 'card_declined', true], + ['PAYER_ACTION_REQUIRED', 'authentication_required', true], + ['INSUFFICIENT_FUNDS', 'insufficient_funds', true], + ['CARD_EXPIRED', 'expired_card', false], + ['INVALID_CARD_NUMBER', 'invalid_card', true], + ['TRANSACTION_REFUSED', 'card_declined', true], + ['TRANSACTION_LIMIT_EXCEEDED', 'limit_exceeded', false], + ])( + 'maps PayPal code %s to category %s (recoverable=%s)', + (code, expectedCategory, expectedRecoverable) => { + const result = categorizePaymentError(code, null); + expect(result.category).toBe(expectedCategory); + expect(result.recoverable).toBe(expectedRecoverable); + } + ); + }); + + describe('unknown / null inputs', () => { + it('maps null code to unknown', () => { + const result = categorizePaymentError(null, null); + expect(result.category).toBe('unknown'); + // Unknown failures get a conservative recoverable=true so the user + // can attempt a retry; the retry-cap and cooling-period guard against + // abuse anyway. (FR-006 — retry available for "recoverable failures", + // and we can't tell what we don't recognize.) + expect(result.recoverable).toBe(true); + expect(result.userMessage).toMatch(/payment.*could not be completed/i); + }); + + it('maps unrecognized code to unknown', () => { + const result = categorizePaymentError('some_future_provider_code', null); + expect(result.category).toBe('unknown'); + expect(result.recoverable).toBe(true); + }); + + it('does not leak the raw error_message into userMessage', () => { + // NFR-001: error messages MUST be non-technical. Provider messages + // can include internal IDs, stack traces, "PG: 23505", etc. + const result = categorizePaymentError( + null, + 'PG ERROR 23505 duplicate key violates uniq_xyz on (template_user_id)' + ); + expect(result.userMessage).not.toContain('PG ERROR'); + expect(result.userMessage).not.toContain('23505'); + }); + }); + + describe('category-level invariants (NFR-001)', () => { + it('every userMessage is short enough to read at a glance (< 200 chars)', () => { + const codes = [ + 'card_declined', + 'insufficient_funds', + 'expired_card', + 'incorrect_cvc', + 'processing_error', + 'authentication_required', + 'amount_too_large', + null, + ]; + for (const code of codes) { + const result = categorizePaymentError(code, null); + expect(result.userMessage.length).toBeLessThan(200); + expect(result.resolutionHint.length).toBeLessThan(200); + } + }); + + it('every userMessage avoids technical terms', () => { + const codes = [ + 'card_declined', + 'insufficient_funds', + 'expired_card', + 'incorrect_cvc', + 'processing_error', + 'authentication_required', + 'amount_too_large', + null, + ]; + const technicalTerms = [ + /\bnull\b/i, + /\bundefined\b/i, + /\bAPI\b/, + /\bUUID\b/i, + /\bHTTP\b/, + /\bresponse code\b/i, + ]; + for (const code of codes) { + const result = categorizePaymentError(code, null); + for (const re of technicalTerms) { + expect(result.userMessage).not.toMatch(re); + } + } + }); + }); +}); diff --git a/src/lib/payments/__tests__/retry.test.ts b/src/lib/payments/__tests__/retry.test.ts new file mode 100644 index 00000000..3ad04b65 --- /dev/null +++ b/src/lib/payments/__tests__/retry.test.ts @@ -0,0 +1,230 @@ +/** + * retryFailedPayment tests — covers the gaps that #43 actually closes: + * idempotency-key reuse, retry cap (FR-009), cooling period (FR-010), + * expiry guard, dedupe-conflict path, and audit-log emission (NFR-007). + */ + +import { describe, it, expect, vi, beforeEach } from 'vitest'; +import { + retryFailedPayment, + PaymentRetryLimitError, + PaymentRetryCoolingError, + PaymentRetryExpiredError, + RETRY_LIMIT, + COOLING_PERIOD_MS, +} from '../payment-service'; +import type { PaymentIntent } from '@/types/payment'; + +// ── Test fixtures ────────────────────────────────────────────────────── +const USER_ID = 'user-abc-123'; + +function makeParent(overrides: Partial = {}): PaymentIntent { + const now = Date.now(); + return { + id: 'parent-1', + template_user_id: USER_ID, + amount: 2000, + currency: 'usd', + type: 'one_time', + interval: null, + customer_email: 'test@example.com', + description: null, + metadata: null, + idempotency_key: 'orig-key-xyz', + retry_count: 0, + parent_intent_id: null, + created_at: new Date(now - COOLING_PERIOD_MS - 1000).toISOString(), // past cooling + expires_at: new Date(now + 3_600_000).toISOString(), // 1h from now + ...overrides, + }; +} + +// ── Mock surface ─────────────────────────────────────────────────────── +// Captures the upsert payload + onConflict options for assertions. +const upsertCalls: Array<{ payload: Record; opts: unknown }> = + []; +let parentIntent: PaymentIntent = makeParent(); +let upsertResult: { data: unknown; error: unknown } = { + data: { id: 'child-2' }, + error: null, +}; +const auditCalls: Array = []; + +vi.mock('@/lib/supabase/client', () => ({ + supabase: { + auth: { + getSession: vi.fn(() => + Promise.resolve({ + data: { session: { user: { id: USER_ID }, access_token: 't' } }, + error: null, + }) + ), + }, + from: vi.fn(() => ({ + select: vi.fn(() => ({ + eq: vi.fn(() => ({ + single: vi.fn(() => + Promise.resolve({ data: parentIntent, error: null }) + ), + })), + })), + upsert: vi.fn((payload: Record, opts: unknown) => { + upsertCalls.push({ payload, opts }); + return { + select: vi.fn(() => ({ + maybeSingle: vi.fn(() => Promise.resolve(upsertResult)), + })), + }; + }), + })), + }, + isSupabaseOnline: vi.fn(() => Promise.resolve(true)), +})); + +vi.mock('@/lib/payments/audit', () => ({ + logPaymentRetryEvent: vi.fn((params: unknown) => { + auditCalls.push(params); + return Promise.resolve(); + }), +})); + +// ── Tests ────────────────────────────────────────────────────────────── +beforeEach(() => { + upsertCalls.length = 0; + auditCalls.length = 0; + parentIntent = makeParent(); + upsertResult = { data: { id: 'child-2' }, error: null }; +}); + +describe('retryFailedPayment — idempotency-key reuse (#43)', () => { + it("reuses the parent's idempotency_key in the new intent", async () => { + parentIntent = makeParent({ idempotency_key: 'shared-key-42' }); + await retryFailedPayment('parent-1'); + expect(upsertCalls).toHaveLength(1); + expect(upsertCalls[0].payload.idempotency_key).toBe('shared-key-42'); + }); + + it("uses upsert(onConflict: 'idempotency_key', ignoreDuplicates: true) so double-clicks dedupe server-side", async () => { + await retryFailedPayment('parent-1'); + expect(upsertCalls[0].opts).toEqual({ + onConflict: 'idempotency_key', + ignoreDuplicates: true, + }); + }); + + it('generates a fresh key only if parent has none (legacy intent)', async () => { + parentIntent = makeParent({ idempotency_key: null }); + await retryFailedPayment('parent-1'); + const sent = upsertCalls[0].payload.idempotency_key as string; + expect(sent).toMatch(/^[0-9a-f-]{36}$/i); // UUID + }); +}); + +describe('retryFailedPayment — retry cap (FR-009)', () => { + it(`throws PaymentRetryLimitError when parent.retry_count >= ${RETRY_LIMIT}`, async () => { + parentIntent = makeParent({ retry_count: RETRY_LIMIT }); + await expect(retryFailedPayment('parent-1')).rejects.toBeInstanceOf( + PaymentRetryLimitError + ); + }); + + it(`allows retry when retry_count < ${RETRY_LIMIT}`, async () => { + parentIntent = makeParent({ retry_count: RETRY_LIMIT - 1 }); + await expect(retryFailedPayment('parent-1')).resolves.toBeDefined(); + }); + + it('writes the bumped retry_count to the new intent', async () => { + parentIntent = makeParent({ retry_count: 1 }); + await retryFailedPayment('parent-1'); + expect(upsertCalls[0].payload.retry_count).toBe(2); + }); + + it('writes parent_intent_id linkage to the new intent', async () => { + await retryFailedPayment('parent-1'); + expect(upsertCalls[0].payload.parent_intent_id).toBe('parent-1'); + }); +}); + +describe('retryFailedPayment — cooling period (FR-010)', () => { + it('throws PaymentRetryCoolingError when parent created within cooling window', async () => { + parentIntent = makeParent({ + created_at: new Date(Date.now() - 1000).toISOString(), // 1s ago + }); + await expect(retryFailedPayment('parent-1')).rejects.toBeInstanceOf( + PaymentRetryCoolingError + ); + }); + + it('cooling error carries the remaining waitMs so UI can render countdown', async () => { + parentIntent = makeParent({ + created_at: new Date(Date.now() - 5000).toISOString(), + }); + try { + await retryFailedPayment('parent-1'); + expect.fail('should have thrown'); + } catch (err) { + expect(err).toBeInstanceOf(PaymentRetryCoolingError); + const waitMs = (err as PaymentRetryCoolingError).waitMs; + expect(waitMs).toBeGreaterThan(0); + expect(waitMs).toBeLessThanOrEqual(COOLING_PERIOD_MS); + } + }); + + it('allows retry once cooling expires', async () => { + parentIntent = makeParent({ + created_at: new Date(Date.now() - COOLING_PERIOD_MS - 100).toISOString(), + }); + await expect(retryFailedPayment('parent-1')).resolves.toBeDefined(); + }); +}); + +describe('retryFailedPayment — expiry guard', () => { + it('throws PaymentRetryExpiredError when parent has expired', async () => { + parentIntent = makeParent({ + expires_at: new Date(Date.now() - 1000).toISOString(), // expired 1s ago + }); + await expect(retryFailedPayment('parent-1')).rejects.toBeInstanceOf( + PaymentRetryExpiredError + ); + }); + + it('allows retry while parent is still within expiry window', async () => { + parentIntent = makeParent({ + expires_at: new Date(Date.now() + 60_000).toISOString(), + }); + await expect(retryFailedPayment('parent-1')).resolves.toBeDefined(); + }); +}); + +describe('retryFailedPayment — dedupe-conflict path', () => { + it('returns the parent intent when the upsert hits ON CONFLICT (no double-charge)', async () => { + upsertResult = { data: null, error: null }; // ON CONFLICT DO NOTHING fired + const result = await retryFailedPayment('parent-1'); + expect(result.id).toBe('parent-1'); + }); + + it('records the conflict in the audit log so admins can see the dedupe', async () => { + upsertResult = { data: null, error: null }; + await retryFailedPayment('parent-1'); + expect(auditCalls).toHaveLength(1); + expect((auditCalls[0] as { deduped: boolean }).deduped).toBe(true); + }); +}); + +describe('retryFailedPayment — audit log (NFR-007)', () => { + it('emits an audit event for every retry attempt', async () => { + await retryFailedPayment('parent-1'); + expect(auditCalls).toHaveLength(1); + }); + + it('audit event carries retry_count, original/new intent ids, user id', async () => { + parentIntent = makeParent({ retry_count: 1 }); + upsertResult = { data: { id: 'child-fresh-id' }, error: null }; + await retryFailedPayment('parent-1'); + const event = auditCalls[0] as Record; + expect(event.userId).toBe(USER_ID); + expect(event.originalIntentId).toBe('parent-1'); + expect(event.newIntentId).toBe('child-fresh-id'); + expect(event.retryCount).toBe(2); + }); +}); diff --git a/src/lib/payments/audit.ts b/src/lib/payments/audit.ts new file mode 100644 index 00000000..36118ede --- /dev/null +++ b/src/lib/payments/audit.ts @@ -0,0 +1,67 @@ +/** + * Payment-specific audit logging. + * + * Thin wrapper around `logAuthEvent` (`src/lib/auth/audit-logger.ts:55`) + * that gives payment retry attempts a consistent event-data shape so the + * admin audit dashboard at `/admin/audit` can render them without a + * separate code path. + * + * Non-throwing — matches the existing audit-logger contract. An audit + * write failure must never break the user's payment flow. + */ + +import { logAuthEvent } from '@/lib/auth/audit-logger'; +import { createLogger } from '@/lib/logger'; +import type { PaymentErrorCategory } from './error-categorization'; + +const logger = createLogger('lib:payments:audit'); + +export interface PaymentRetryAuditParams { + userId: string; + originalIntentId: string; + /** null when the retry was deduped by the unique idempotency_key index */ + newIntentId: string | null; + retryCount: number; + /** true when the upsert hit ON CONFLICT (server-side dedupe) */ + deduped: boolean; + errorCategory?: PaymentErrorCategory; +} + +/** + * Record a payment retry attempt. NFR-007. + * + * Writes a `payment_retry` row to `auth_audit_logs` with: + * - user_id: the authenticated user + * - event_data: structured retry metadata (admin dashboard reads this) + * - success: false when deduped (the retry produced no new charge), + * true when a new intent was created + */ +export async function logPaymentRetryEvent( + params: PaymentRetryAuditParams +): Promise { + try { + await logAuthEvent({ + user_id: params.userId, + event_type: 'payment_retry', + event_data: { + original_intent_id: params.originalIntentId, + new_intent_id: params.newIntentId, + retry_count: params.retryCount, + deduped: params.deduped, + ...(params.errorCategory !== undefined && { + error_category: params.errorCategory, + }), + }, + // A deduped retry is not a "new" success — flag it so analytics can + // distinguish "user retried and we created a fresh attempt" from + // "user retried but the old attempt had already won the race". + success: !params.deduped, + }); + } catch (err) { + logger.error('Failed to log payment_retry audit event', { + err, + originalIntentId: params.originalIntentId, + }); + // Swallow — audit failures must not break payment UX. + } +} diff --git a/src/lib/payments/error-categorization.ts b/src/lib/payments/error-categorization.ts new file mode 100644 index 00000000..a28b14fe --- /dev/null +++ b/src/lib/payments/error-categorization.ts @@ -0,0 +1,157 @@ +/** + * Payment Error Categorization (FR-002) + * + * Maps provider-specific error codes from `payment_results.error_code` + * to one of 8 user-facing categories with non-technical messages and + * resolution hints (FR-001, FR-003, NFR-001). + * + * Pure mapping, no I/O. The `recoverable` flag gates the retry button + * (FR-006): non-recoverable failures hide retry and show support contact + * instead of letting the user burn through retry attempts that can't help. + */ + +export type PaymentErrorCategory = + | 'card_declined' + | 'insufficient_funds' + | 'expired_card' + | 'invalid_card' + | 'processing_error' + | 'network_error' + | 'authentication_required' + | 'limit_exceeded' + | 'unknown'; + +export interface CategorizedError { + category: PaymentErrorCategory; + userMessage: string; + recoverable: boolean; + resolutionHint: string; +} + +/** + * Lookup table: provider code (lowercased for case-insensitive match) + * → category. Stripe codes are snake_case; PayPal codes are SCREAMING_CASE. + * Extend by adding entries here. + */ +const CODE_TO_CATEGORY: Record = { + // ── Stripe ──────────────────────────────────────────────────────────── + card_declined: 'card_declined', + generic_decline: 'card_declined', + card_not_supported: 'card_declined', + insufficient_funds: 'insufficient_funds', + expired_card: 'expired_card', + incorrect_cvc: 'invalid_card', + invalid_cvc: 'invalid_card', + invalid_number: 'invalid_card', + invalid_expiry_month: 'invalid_card', + invalid_expiry_year: 'invalid_card', + processing_error: 'processing_error', + authentication_required: 'authentication_required', + amount_too_large: 'limit_exceeded', + amount_too_small: 'limit_exceeded', + currency_not_supported: 'limit_exceeded', + + // ── PayPal ──────────────────────────────────────────────────────────── + instrument_declined: 'card_declined', + payer_action_required: 'authentication_required', + card_expired: 'expired_card', + invalid_card_number: 'invalid_card', + transaction_refused: 'card_declined', + transaction_limit_exceeded: 'limit_exceeded', +}; + +/** + * Categories where retry can plausibly help. Categories listed as + * non-recoverable (`expired_card`, `limit_exceeded`, `currency_not_supported`) + * require user action that retry alone cannot accomplish — show support + * contact instead. + */ +const RECOVERABLE_CATEGORIES: Record = { + card_declined: true, // transient bank-side decisions can flip + insufficient_funds: true, // user may add funds and retry + invalid_card: true, // typo correction is a retry + processing_error: true, // provider-side hiccup + network_error: true, + authentication_required: true, // 3DS prompt may now be acked + unknown: true, // conservative — let cap + cooling guard + expired_card: false, // requires new card, not retry + limit_exceeded: false, // requires different amount/method +}; + +const COPY: Record< + PaymentErrorCategory, + { userMessage: string; resolutionHint: string } +> = { + card_declined: { + userMessage: 'Your card was declined.', + resolutionHint: + 'Please contact your card issuer or try a different payment method.', + }, + insufficient_funds: { + userMessage: 'Your card does not have enough funds for this purchase.', + resolutionHint: + 'Please add funds to your account or try a different payment method.', + }, + expired_card: { + userMessage: 'Your card has expired.', + resolutionHint: + 'Please use a different payment method with a current expiry date.', + }, + invalid_card: { + userMessage: 'The card details look incorrect.', + resolutionHint: + 'Please check the card number, expiry, and security code, then try again.', + }, + processing_error: { + userMessage: + 'We could not process your payment right now. This is usually temporary.', + resolutionHint: + 'Please wait a moment and try again. If the problem persists, contact support.', + }, + network_error: { + userMessage: 'A network problem interrupted your payment.', + resolutionHint: + 'Please check your connection and try again. Your payment was not charged.', + }, + authentication_required: { + userMessage: 'Your bank needs to verify this payment.', + resolutionHint: + 'Please complete the verification step shown by your bank, then try again.', + }, + limit_exceeded: { + userMessage: 'This amount is outside the limits for your payment method.', + resolutionHint: + 'Please try a smaller amount or use a different payment method.', + }, + unknown: { + userMessage: 'Your payment could not be completed.', + resolutionHint: + 'Please try again. If the problem persists, contact support with the transaction reference.', + }, +}; + +/** + * Categorize a payment error. + * + * @param errorCode - Provider's error code, e.g. Stripe `card_declined` + * or PayPal `INSTRUMENT_DECLINED`. May be null. + * @param _errorMessage - Provider's raw error message. Currently unused — + * we never surface raw provider text to users + * (NFR-001) — but the parameter is kept so callers + * have a stable API if richer mapping is added later. + */ +export function categorizePaymentError( + errorCode: string | null, + _errorMessage: string | null +): CategorizedError { + const key = errorCode?.toLowerCase() ?? null; + const category: PaymentErrorCategory = + (key && CODE_TO_CATEGORY[key]) || 'unknown'; + const copy = COPY[category]; + return { + category, + userMessage: copy.userMessage, + resolutionHint: copy.resolutionHint, + recoverable: RECOVERABLE_CATEGORIES[category], + }; +} diff --git a/src/lib/payments/payment-service.ts b/src/lib/payments/payment-service.ts index dd418360..ddcd148a 100644 --- a/src/lib/payments/payment-service.ts +++ b/src/lib/payments/payment-service.ts @@ -17,6 +17,66 @@ import type { } from '@/types/payment'; import { validatePaymentAmount, validateCurrency } from '@/config/payment'; import { validateAndSanitizeMetadata } from './metadata-validator'; +import { logPaymentRetryEvent } from './audit'; + +/** + * Maximum retry attempts per payment chain (FR-009). + * The CHECK is on the *parent's* `retry_count` — when retry_count of the + * intent the user is retrying reaches this value, no further retry is + * allowed. The first retry produces a child with retry_count=1; the second, + * 2; the third, 3. The fourth click trips the cap. + */ +export const RETRY_LIMIT = 3; + +/** + * Cooling period between retries (FR-010). Measured from the parent + * intent's `created_at`. Picked to be long enough to prevent trivial + * mistype-and-spam loops, short enough not to feel punitive. + */ +export const COOLING_PERIOD_MS = 30_000; + +/** + * Thrown by `retryFailedPayment` when the parent intent has already been + * retried `RETRY_LIMIT` times. UI should hide the retry button and surface + * the support contact path. + */ +export class PaymentRetryLimitError extends Error { + readonly attempts: number; + readonly limit: number; + constructor(attempts: number, limit: number) { + super(`This payment has reached the maximum of ${limit} retry attempts.`); + this.name = 'PaymentRetryLimitError'; + this.attempts = attempts; + this.limit = limit; + } +} + +/** + * Thrown by `retryFailedPayment` when the parent intent was created less + * than `COOLING_PERIOD_MS` ago. Carries the remaining wait so the UI can + * render a countdown (FR-010). + */ +export class PaymentRetryCoolingError extends Error { + readonly waitMs: number; + constructor(waitMs: number) { + super(`Please wait ${Math.ceil(waitMs / 1000)}s before retrying.`); + this.name = 'PaymentRetryCoolingError'; + this.waitMs = waitMs; + } +} + +/** + * Thrown by `retryFailedPayment` when the parent intent has passed its + * 24-hour expiry. The provider's session is gone; a same-key retry would + * succeed at the DB but fail at the provider redirect. Better to refuse + * here with a clear message than to surface a confusing failure later. + */ +export class PaymentRetryExpiredError extends Error { + constructor() { + super('This payment session has expired. Please start a new payment.'); + this.name = 'PaymentRetryExpiredError'; + } +} /** * Get authenticated user ID @@ -231,38 +291,112 @@ export async function getPaymentHistory( } /** - * Retry a failed payment - * REQ-SEC-001: Requires authentication, RLS ensures user owns the intent + * Retry a failed payment (#43, B1). + * + * Creates a new INSERT-only payment_intent row that: + * - reuses the parent's `idempotency_key` so the partial unique index + * turns a server-side race (double-click, two tabs) into a no-op + * instead of a duplicate charge — same pattern as the offline-queue + * adapter (`src/lib/offline-queue/payment-adapter.ts:165-195`) + * - links to the parent via `parent_intent_id` + * - bumps `retry_count` so the cap (FR-009) is enforced on the next click + * + * Refuses to proceed when: + * - parent.retry_count >= RETRY_LIMIT (FR-009) → PaymentRetryLimitError + * - parent created within COOLING_PERIOD_MS (FR-010) → PaymentRetryCoolingError + * - parent has passed its 24h expiry → PaymentRetryExpiredError + * + * Audit-logs every attempt to `auth_audit_logs` as `payment_retry` (NFR-007). + * + * REQ-SEC-001: requires authentication; RLS's "Users can create own payment + * intents" policy ensures `template_user_id = auth.uid()`. The "Payment + * intents are immutable" UPDATE policy means we never mutate the parent — + * `retry_count` is only ever set on the new (child) row at INSERT time. */ export async function retryFailedPayment( intentId: string ): Promise { - // Require authentication (REQ-SEC-001) - await getAuthenticatedUserId(); + const userId = await getAuthenticatedUserId(); - // Get original intent (RLS ensures user owns it) - const { data: originalIntent, error: fetchError } = await supabase + const { data: parent, error: fetchError } = await supabase .from('payment_intents') .select('*') .eq('id', intentId) .single(); if (fetchError) throw fetchError; + if (!parent) { + throw new Error('Cannot retry — original payment intent not found.'); + } - // Create new intent with same data - // createPaymentIntent will use the authenticated user's ID - return await createPaymentIntent( - originalIntent.amount, - originalIntent.currency as Currency, - originalIntent.type as PaymentType, - originalIntent.customer_email, - { - interval: originalIntent.interval as PaymentInterval | undefined, - description: originalIntent.description || undefined, - metadata: - (originalIntent.metadata as Record) || undefined, - } - ); + // FR-009: cap retry attempts on this chain. + if (parent.retry_count >= RETRY_LIMIT) { + throw new PaymentRetryLimitError(parent.retry_count, RETRY_LIMIT); + } + + // Expiry guard: a same-key retry against a stale provider session is + // worse UX than refusing here. + const now = Date.now(); + if (new Date(parent.expires_at).getTime() < now) { + throw new PaymentRetryExpiredError(); + } + + // FR-010: cooling period since the parent was created. + const elapsedMs = now - new Date(parent.created_at).getTime(); + if (elapsedMs < COOLING_PERIOD_MS) { + throw new PaymentRetryCoolingError(COOLING_PERIOD_MS - elapsedMs); + } + + // Reuse parent's idempotency_key if present; if absent (legacy intents + // pre-PR #59), generate a fresh one so this retry chain still dedupes + // with itself. Same fallback pattern as payment-adapter.ts:155-163. + const idempotencyKey = parent.idempotency_key ?? crypto.randomUUID(); + const newRetryCount = parent.retry_count + 1; + + // Upsert with ignoreDuplicates: a doubled click on the retry button (same + // idempotency_key reaching the server twice) returns a null row from the + // ON CONFLICT path instead of a 23505. We surface that as "the retry is + // a no-op, the original is still authoritative" by returning the parent. + const { data: child, error: insertError } = await supabase + .from('payment_intents') + .upsert( + { + amount: parent.amount, + currency: parent.currency, + type: parent.type, + interval: parent.interval, + customer_email: parent.customer_email, + description: parent.description, + metadata: parent.metadata, + template_user_id: userId, // RLS WITH CHECK enforces this anyway + idempotency_key: idempotencyKey, + parent_intent_id: parent.id, + retry_count: newRetryCount, + }, + { onConflict: 'idempotency_key', ignoreDuplicates: true } + ) + .select() + .maybeSingle(); + + if (insertError) throw insertError; + + const deduped = !child; + + // NFR-007: audit. Non-throwing — never break the user flow over an + // audit-log write failure. + await logPaymentRetryEvent({ + userId, + originalIntentId: parent.id, + newIntentId: deduped ? null : (child as PaymentIntent).id, + retryCount: newRetryCount, + deduped, + }); + + // ON CONFLICT fired — the prior attempt's row is the truth. + if (deduped) { + return parent as PaymentIntent; + } + return child as PaymentIntent; } /** From 06538395b4d1114c1f6d532eb763e15b518cd86d Mon Sep 17 00:00:00 2001 From: TurtleWolfe Date: Tue, 28 Apr 2026 00:43:30 +0000 Subject: [PATCH 3/5] =?UTF-8?q?feat(payments):=20/payment-result=20UX=20?= =?UTF-8?q?=E2=80=94=20categorized=20errors,=20attempt=20counter,=20coolin?= =?UTF-8?q?g=20button,=20offline=20banner?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Consumes the service surface from the previous commit to surface the retry UX gaps that #43 actually closes. PaymentStatusDisplay (failed-state block rewritten): - Renders categorized userMessage + resolutionHint instead of raw status (FR-001/003). Transaction reference shown for support inquiries (FR-004). - Recoverable errors show retry button + 'Attempt N of M' counter (FR-008). Non-recoverable errors hide retry and show a Contact Support link (FR-019 lite — no wizard yet, US4 is a follow-up). - Cooling state disables the button and shows 'Try again in 23s' that ticks down (FR-010). PaymentRetryLimitError / PaymentRetryCoolingError / PaymentRetryExpiredError thrown by the service surface client-side as retry-error alerts. usePaymentRetryStatus hook: - Reads parent intent, computes { retryCount, canRetry, disabledReason, coolingMsRemaining }. 1Hz tick while cooling so the countdown is live. - A UX hint, not a security boundary — server-side checks in retryFailedPayment are still the source of truth. The hook just stops the user clicking a button that's certain to fail. - 7 tests using vi.useFakeTimers({ shouldAdvanceTime: true }) so waitFor internals work alongside the controlled cooling tick. OfflineRetryBanner (new 5-file component, peer of PaymentStatusDisplay): - Reads useOfflineStatus + paymentQueue.getCount(). Renders a warning alert when offline (with queued count if any) and an info alert when online but queue still draining. Silent in the steady state. - Mounted above PaymentStatusDisplay on /payment-result for the loaded state and above the not-found alert for the still-processing state. - 7 unit + 1 a11y test, including singular/plural copy and a getCount rejection survives without throwing. E2E (tests/e2e/payment/03-failed-payment-retry.spec.ts): - Un-skipped 'should display offline error banner when offline' (was skipped pending offline UI). Uses context.setOffline(true) to force the navigator.onLine bit before navigation, asserts the banner copy. Closes one row of #53's skip index. Verification: - pnpm test: 3249/3249 across 286 files - pnpm test:rls: 55/55 (new columns inherit existing policies as expected) - pnpm run type-check: clean - pnpm run lint: clean Out of scope (deferred to a follow-up PR): - User Story 3 (update payment method, FR-011-FR-015) — stripe.js + PCI - User Story 4 (guided recovery wizard, FR-016-FR-019) — largest piece Co-Authored-By: Claude Opus 4.7 (1M context) --- src/app/payment-result/page.tsx | 3 + .../OfflineRetryBanner.accessibility.test.tsx | 38 ++++ .../OfflineRetryBanner.stories.tsx | 32 ++++ .../OfflineRetryBanner.test.tsx | 94 +++++++++ .../OfflineRetryBanner/OfflineRetryBanner.tsx | 136 +++++++++++++ .../payment/OfflineRetryBanner/index.tsx | 6 + ...aymentStatusDisplay.accessibility.test.tsx | 19 ++ .../PaymentStatusDisplay.test.tsx | 130 +++++++++++++ .../PaymentStatusDisplay.tsx | 178 ++++++++++++++---- .../__tests__/usePaymentRetryStatus.test.ts | 138 ++++++++++++++ src/hooks/usePaymentRetryStatus.ts | 163 ++++++++++++++++ .../payment/03-failed-payment-retry.spec.ts | 23 ++- 12 files changed, 916 insertions(+), 44 deletions(-) create mode 100644 src/components/payment/OfflineRetryBanner/OfflineRetryBanner.accessibility.test.tsx create mode 100644 src/components/payment/OfflineRetryBanner/OfflineRetryBanner.stories.tsx create mode 100644 src/components/payment/OfflineRetryBanner/OfflineRetryBanner.test.tsx create mode 100644 src/components/payment/OfflineRetryBanner/OfflineRetryBanner.tsx create mode 100644 src/components/payment/OfflineRetryBanner/index.tsx create mode 100644 src/hooks/__tests__/usePaymentRetryStatus.test.ts create mode 100644 src/hooks/usePaymentRetryStatus.ts diff --git a/src/app/payment-result/page.tsx b/src/app/payment-result/page.tsx index 213c7e98..8f7ce99b 100644 --- a/src/app/payment-result/page.tsx +++ b/src/app/payment-result/page.tsx @@ -12,6 +12,7 @@ import { useSearchParams } from 'next/navigation'; import Link from 'next/link'; import ProtectedRoute from '@/components/auth/ProtectedRoute'; import { PaymentStatusDisplay } from '@/components/payment/PaymentStatusDisplay/PaymentStatusDisplay'; +import { OfflineRetryBanner } from '@/components/payment/OfflineRetryBanner'; import { featureFlags } from '@/config/payment'; import { getPaymentStatus } from '@/lib/payments/payment-service'; @@ -210,6 +211,7 @@ function PaymentResultContent() { return (
+
+ ({ + useOfflineStatus: vi.fn(() => ({ + isOffline: true, + wasOffline: false, + lastOnline: null, + connectionSpeed: 'unknown', + })), +})); + +vi.mock('@/lib/offline-queue/payment-adapter', () => ({ + paymentQueue: { + getCount: vi.fn(() => Promise.resolve(2)), + }, +})); + +describe('OfflineRetryBanner Accessibility', () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + it('has no a11y violations in offline state with queued items', async () => { + const { container } = render(); + await waitFor(() => container.querySelector('.alert')); + const results = await axe(container); + expect(results).toHaveNoViolations(); + }); +}); diff --git a/src/components/payment/OfflineRetryBanner/OfflineRetryBanner.stories.tsx b/src/components/payment/OfflineRetryBanner/OfflineRetryBanner.stories.tsx new file mode 100644 index 00000000..1a8b46a4 --- /dev/null +++ b/src/components/payment/OfflineRetryBanner/OfflineRetryBanner.stories.tsx @@ -0,0 +1,32 @@ +/** + * OfflineRetryBanner Storybook stories + */ + +import type { Meta, StoryObj } from '@storybook/nextjs-vite'; +import { OfflineRetryBanner } from './OfflineRetryBanner'; + +const meta: Meta = { + title: 'Features/Payment/OfflineRetryBanner', + component: OfflineRetryBanner, + parameters: { + layout: 'padded', + docs: { + description: { + component: + 'Renders on the payment-result page when the user is offline or when queued payments are still syncing. Stays silent in the online + empty-queue steady state.', + }, + }, + }, +}; + +export default meta; +type Story = StoryObj; + +/** Default render — relies on real online-status + empty queue. In the + * Storybook iframe this typically renders nothing (the steady state). */ +export const Default: Story = {}; + +/** Story name = scenario; the actual offline / queued visuals are easier + * to verify by toggling devtools "Offline" while viewing the running app + * at /payment-result, since this component reads navigator.onLine and + * an IndexedDB-backed queue that Storybook doesn't simulate. */ diff --git a/src/components/payment/OfflineRetryBanner/OfflineRetryBanner.test.tsx b/src/components/payment/OfflineRetryBanner/OfflineRetryBanner.test.tsx new file mode 100644 index 00000000..25e52d49 --- /dev/null +++ b/src/components/payment/OfflineRetryBanner/OfflineRetryBanner.test.tsx @@ -0,0 +1,94 @@ +/** + * OfflineRetryBanner unit tests + */ + +import { describe, it, expect, vi, beforeEach } from 'vitest'; +import { render, screen, waitFor } from '@testing-library/react'; +import { OfflineRetryBanner } from './OfflineRetryBanner'; +import { useOfflineStatus } from '@/hooks/useOfflineStatus'; + +vi.mock('@/hooks/useOfflineStatus'); + +const getCountMock = vi.fn(); +vi.mock('@/lib/offline-queue/payment-adapter', () => ({ + paymentQueue: { + getCount: () => getCountMock(), + }, +})); + +beforeEach(() => { + vi.clearAllMocks(); + getCountMock.mockResolvedValue(0); +}); + +function setOnline(isOffline: boolean) { + vi.mocked(useOfflineStatus).mockReturnValue({ + isOffline, + wasOffline: false, + lastOnline: null, + connectionSpeed: 'unknown', + }); +} + +describe('OfflineRetryBanner', () => { + it('renders nothing when online and queue is empty (steady state)', async () => { + setOnline(false); + getCountMock.mockResolvedValue(0); + const { container } = render(); + // wait for the initial getCount to settle + await waitFor(() => expect(getCountMock).toHaveBeenCalled()); + expect(container.firstChild).toBeNull(); + }); + + it('shows offline banner with retry promise when offline (no queued items)', async () => { + setOnline(true); + getCountMock.mockResolvedValue(0); + render(); + await waitFor(() => expect(getCountMock).toHaveBeenCalled()); + expect(screen.getByText(/you.?re offline/i)).toBeInTheDocument(); + expect( + screen.getByText(/we.?ll process your payment/i) + ).toBeInTheDocument(); + // No "waiting" copy when count is 0. + expect(screen.queryByText(/waiting to send/i)).not.toBeInTheDocument(); + }); + + it('shows queued count in offline banner (singular form)', async () => { + setOnline(true); + getCountMock.mockResolvedValue(1); + render(); + await waitFor(() => + expect(screen.getByText(/1 payment is waiting/i)).toBeInTheDocument() + ); + }); + + it('shows queued count in offline banner (plural form)', async () => { + setOnline(true); + getCountMock.mockResolvedValue(3); + render(); + await waitFor(() => + expect(screen.getByText(/3 payments are waiting/i)).toBeInTheDocument() + ); + }); + + it('shows "Syncing N queued payments" when online but queue still has items', async () => { + setOnline(false); + getCountMock.mockResolvedValue(2); + render(); + await waitFor(() => + expect(screen.getByText(/syncing 2 queued payments/i)).toBeInTheDocument() + ); + expect(screen.getByRole('status')).toBeInTheDocument(); + }); + + it('survives a getCount rejection without throwing', async () => { + setOnline(false); + getCountMock.mockRejectedValue(new Error('IndexedDB locked')); + render(); + await waitFor(() => expect(getCountMock).toHaveBeenCalled()); + // Component still renders something; specifically, online + count=0 + // is the steady state, so the banner is null. The test passes if + // render didn't throw. + expect(true).toBe(true); + }); +}); diff --git a/src/components/payment/OfflineRetryBanner/OfflineRetryBanner.tsx b/src/components/payment/OfflineRetryBanner/OfflineRetryBanner.tsx new file mode 100644 index 00000000..58cec99d --- /dev/null +++ b/src/components/payment/OfflineRetryBanner/OfflineRetryBanner.tsx @@ -0,0 +1,136 @@ +/** + * OfflineRetryBanner + * + * Surfaces offline state on the payment-result page so users understand + * why their retry isn't going through. Reads `useOfflineStatus` for the + * connectivity bit and `paymentQueue.getCount()` for the count of pending + * queued payments. When queued items exist and the user is offline, the + * banner promises retry-when-online; otherwise it stays out of the way. + * + * Mount above PaymentStatusDisplay on /payment-result. + */ + +'use client'; + +import React from 'react'; +import { useOfflineStatus } from '@/hooks/useOfflineStatus'; +import { paymentQueue } from '@/lib/offline-queue/payment-adapter'; + +export interface OfflineRetryBannerProps { + /** Override the default polling interval for the queue count (ms). */ + pollIntervalMs?: number; + className?: string; +} + +const DEFAULT_POLL_MS = 5_000; + +export const OfflineRetryBanner: React.FC = ({ + pollIntervalMs = DEFAULT_POLL_MS, + className = '', +}) => { + const { isOffline } = useOfflineStatus(); + const [queuedCount, setQueuedCount] = React.useState(0); + + // Poll the queue count rather than subscribing — the queue API is Dexie + // and doesn't expose a change emitter, and the count is small + cheap. + React.useEffect(() => { + let cancelled = false; + + async function refresh() { + try { + const count = await paymentQueue.getCount(); + if (!cancelled) setQueuedCount(count); + } catch { + // Queue read failure is non-critical — banner just stays at 0. + } + } + + refresh(); + const id = window.setInterval(refresh, pollIntervalMs); + return () => { + cancelled = true; + window.clearInterval(id); + }; + }, [pollIntervalMs]); + + // Render nothing if there's nothing useful to say. The banner is meant + // to be silent when the user is online and the queue is empty — which + // is the expected steady state. + if (!isOffline && queuedCount === 0) { + return null; + } + + if (isOffline) { + return ( +
+ +
+

You’re offline.

+

+ We’ll process your payment when your connection returns. + {queuedCount > 0 && ( + <> + {' '} + {queuedCount === 1 + ? '1 payment is waiting to send.' + : `${queuedCount} payments are waiting to send.`} + + )} +

+
+
+ ); + } + + // Online but with queued items — happens briefly during sync, or when + // the queue is in a stuck state. Surfaces it so the user isn't left + // wondering why their action seemed to take effect but didn't show up. + return ( +
+ + + Syncing{' '} + {queuedCount === 1 + ? '1 queued payment' + : `${queuedCount} queued payments`} + … + +
+ ); +}; + +OfflineRetryBanner.displayName = 'OfflineRetryBanner'; diff --git a/src/components/payment/OfflineRetryBanner/index.tsx b/src/components/payment/OfflineRetryBanner/index.tsx new file mode 100644 index 00000000..e9e2d3fe --- /dev/null +++ b/src/components/payment/OfflineRetryBanner/index.tsx @@ -0,0 +1,6 @@ +/** + * OfflineRetryBanner Component Barrel Export + */ + +export { OfflineRetryBanner } from './OfflineRetryBanner'; +export type { OfflineRetryBannerProps } from './OfflineRetryBanner'; diff --git a/src/components/payment/PaymentStatusDisplay/PaymentStatusDisplay.accessibility.test.tsx b/src/components/payment/PaymentStatusDisplay/PaymentStatusDisplay.accessibility.test.tsx index 6ee4db7d..18b96a9f 100644 --- a/src/components/payment/PaymentStatusDisplay/PaymentStatusDisplay.accessibility.test.tsx +++ b/src/components/payment/PaymentStatusDisplay/PaymentStatusDisplay.accessibility.test.tsx @@ -37,6 +37,25 @@ vi.mock('@/hooks/usePaymentRealtime', () => ({ vi.mock('@/lib/payments/payment-service', () => ({ formatPaymentAmount: vi.fn(() => '$20.00'), retryFailedPayment: vi.fn(() => Promise.resolve({ id: 'new-intent-123' })), + RETRY_LIMIT: 3, + COOLING_PERIOD_MS: 30_000, + PaymentRetryLimitError: class extends Error {}, + PaymentRetryCoolingError: class extends Error { + waitMs = 0; + }, + PaymentRetryExpiredError: class extends Error {}, +})); + +// Mock retry-status hook so accessibility scenarios stay deterministic. +vi.mock('@/hooks/usePaymentRetryStatus', () => ({ + usePaymentRetryStatus: vi.fn(() => ({ + loading: false, + retryCount: 0, + maxRetries: 3, + canRetry: true, + disabledReason: null, + coolingMsRemaining: 0, + })), })); describe('PaymentStatusDisplay Accessibility', () => { diff --git a/src/components/payment/PaymentStatusDisplay/PaymentStatusDisplay.test.tsx b/src/components/payment/PaymentStatusDisplay/PaymentStatusDisplay.test.tsx index ba6272bf..4e9b0ac0 100644 --- a/src/components/payment/PaymentStatusDisplay/PaymentStatusDisplay.test.tsx +++ b/src/components/payment/PaymentStatusDisplay/PaymentStatusDisplay.test.tsx @@ -14,6 +14,17 @@ const mockRetryFailedPayment = vi.fn(); vi.mock('@/hooks/usePaymentRealtime'); +vi.mock('@/hooks/usePaymentRetryStatus', () => ({ + usePaymentRetryStatus: vi.fn(() => ({ + loading: false, + retryCount: 0, + maxRetries: 3, + canRetry: true, + disabledReason: null, + coolingMsRemaining: 0, + })), +})); + vi.mock('@/lib/payments/payment-service', () => ({ retryFailedPayment: (...args: unknown[]) => mockRetryFailedPayment(...args), formatPaymentAmount: vi.fn((amount: number, currency: string) => { @@ -25,6 +36,29 @@ vi.mock('@/lib/payments/payment-service', () => ({ }; return `${symbols[currency] || '$'}${formatted}`; }), + // The hook reads these constants at module load. Keep in sync with + // src/lib/payments/payment-service.ts. + RETRY_LIMIT: 3, + COOLING_PERIOD_MS: 30_000, + PaymentRetryLimitError: class PaymentRetryLimitError extends Error { + constructor() { + super('Limit reached'); + this.name = 'PaymentRetryLimitError'; + } + }, + PaymentRetryCoolingError: class PaymentRetryCoolingError extends Error { + waitMs = 0; + constructor() { + super('Cooling'); + this.name = 'PaymentRetryCoolingError'; + } + }, + PaymentRetryExpiredError: class PaymentRetryExpiredError extends Error { + constructor() { + super('Expired'); + this.name = 'PaymentRetryExpiredError'; + } + }, })); const createMockResult = (status: PaymentResult['status']): PaymentResult => ({ @@ -291,4 +325,100 @@ describe('PaymentStatusDisplay', () => { screen.queryByRole('button', { name: /retry/i }) ).not.toBeInTheDocument(); }); + + describe('B1 — categorized error UI (#43)', () => { + it('renders the categorized userMessage instead of raw provider text (NFR-001)', () => { + const failed = createMockResult('failed'); + failed.error_code = 'card_declined'; + failed.error_message = 'PG ERROR 23505 some/internal/path'; + vi.mocked(usePaymentRealtime).mockReturnValue({ + paymentResult: failed, + loading: false, + error: null, + }); + render(); + expect(screen.getByText(/your card was declined/i)).toBeInTheDocument(); + expect(screen.queryByText(/PG ERROR 23505/)).not.toBeInTheDocument(); + }); + + it('shows transaction reference for support inquiries (FR-004)', () => { + const failed = createMockResult('failed'); + failed.error_code = 'card_declined'; + failed.transaction_id = 'pi_abc_xyz_42'; + vi.mocked(usePaymentRealtime).mockReturnValue({ + paymentResult: failed, + loading: false, + error: null, + }); + render(); + // Reference shows in both the details panel and the new error block; + // any rendering of it satisfies FR-004. + expect(screen.getAllByText(/pi_abc_xyz_42/).length).toBeGreaterThan(0); + }); + + it('hides retry and shows support link for non-recoverable errors (FR-019 lite)', () => { + const failed = createMockResult('failed'); + failed.error_code = 'expired_card'; // not recoverable + vi.mocked(usePaymentRealtime).mockReturnValue({ + paymentResult: failed, + loading: false, + error: null, + }); + render(); + expect( + screen.queryByRole('button', { name: /retry/i }) + ).not.toBeInTheDocument(); + expect( + screen.getByRole('link', { name: /contact support/i }) + ).toBeInTheDocument(); + }); + + it('renders the attempt counter "Attempt N of M" (FR-008)', async () => { + const failed = createMockResult('failed'); + failed.error_code = 'card_declined'; + vi.mocked(usePaymentRealtime).mockReturnValue({ + paymentResult: failed, + loading: false, + error: null, + }); + const { usePaymentRetryStatus } = await import( + '@/hooks/usePaymentRetryStatus' + ); + vi.mocked(usePaymentRetryStatus).mockReturnValue({ + loading: false, + retryCount: 1, + maxRetries: 3, + canRetry: true, + disabledReason: null, + coolingMsRemaining: 0, + }); + render(); + expect(screen.getByText(/Attempt 2 of 3/)).toBeInTheDocument(); + }); + + it('disables button + shows countdown while cooling (FR-010)', async () => { + const failed = createMockResult('failed'); + failed.error_code = 'card_declined'; + vi.mocked(usePaymentRealtime).mockReturnValue({ + paymentResult: failed, + loading: false, + error: null, + }); + const { usePaymentRetryStatus } = await import( + '@/hooks/usePaymentRetryStatus' + ); + vi.mocked(usePaymentRetryStatus).mockReturnValue({ + loading: false, + retryCount: 0, + maxRetries: 3, + canRetry: false, + disabledReason: 'cooling', + coolingMsRemaining: 23_400, + }); + render(); + const btn = screen.getByRole('button', { name: /retry available in/i }); + expect(btn).toBeDisabled(); + expect(btn).toHaveTextContent(/Try again in 24s/); + }); + }); }); diff --git a/src/components/payment/PaymentStatusDisplay/PaymentStatusDisplay.tsx b/src/components/payment/PaymentStatusDisplay/PaymentStatusDisplay.tsx index 3f4de2f1..65919966 100644 --- a/src/components/payment/PaymentStatusDisplay/PaymentStatusDisplay.tsx +++ b/src/components/payment/PaymentStatusDisplay/PaymentStatusDisplay.tsx @@ -7,10 +7,15 @@ import React from 'react'; import { usePaymentRealtime } from '@/hooks/usePaymentRealtime'; +import { usePaymentRetryStatus } from '@/hooks/usePaymentRetryStatus'; import { retryFailedPayment, formatPaymentAmount, + PaymentRetryLimitError, + PaymentRetryCoolingError, + PaymentRetryExpiredError, } from '@/lib/payments/payment-service'; +import { categorizePaymentError } from '@/lib/payments/error-categorization'; import type { Currency } from '@/types/payment'; export interface PaymentStatusDisplayProps { @@ -45,11 +50,16 @@ export const PaymentStatusDisplay: React.FC = ({ className = '', }) => { const { paymentResult, loading, error } = usePaymentRealtime(paymentResultId); + const retryStatus = usePaymentRetryStatus(paymentResult?.intent_id ?? null); const [isRetrying, setIsRetrying] = React.useState(false); + // Surfaces user-facing errors thrown by retryFailedPayment (limit, cooling, + // expired). Cleared on the next click attempt. + const [retryError, setRetryError] = React.useState(null); const handleRetry = async () => { if (!paymentResult?.intent_id) return; + setRetryError(null); setIsRetrying(true); try { @@ -59,10 +69,20 @@ export const PaymentStatusDisplay: React.FC = ({ onRetrySuccess(newIntent.id); } } catch (err) { - const errorObj = err instanceof Error ? err : new Error('Retry failed'); - - if (onRetryError) { - onRetryError(errorObj); + // Map known retry-flow errors to actionable user messages. Anything + // else falls back to the existing onRetryError callback so callers + // can surface their own toast/alert. + if ( + err instanceof PaymentRetryLimitError || + err instanceof PaymentRetryCoolingError || + err instanceof PaymentRetryExpiredError + ) { + setRetryError(err.message); + } else { + const errorObj = err instanceof Error ? err : new Error('Retry failed'); + if (onRetryError) { + onRetryError(errorObj); + } } } finally { setIsRetrying(false); @@ -300,44 +320,120 @@ export const PaymentStatusDisplay: React.FC = ({
)} - {/* Retry Button for Failed Payments */} - {paymentResult.status === 'failed' && ( -
- -
- )} + {retryError} +
+ )} + + {/* FR-006: only show retry for recoverable categories. + Non-recoverable failures route to support contact (FR-019 lite). */} + {categorized.recoverable ? ( +
+ {/* FR-008: attempt counter */} +

+ Attempt {retryStatus.retryCount + 1} of{' '} + {retryStatus.maxRetries} +

+ +
+ ) : ( + + )} + + ); + })()}
); diff --git a/src/hooks/__tests__/usePaymentRetryStatus.test.ts b/src/hooks/__tests__/usePaymentRetryStatus.test.ts new file mode 100644 index 00000000..03676e4d --- /dev/null +++ b/src/hooks/__tests__/usePaymentRetryStatus.test.ts @@ -0,0 +1,138 @@ +/** + * usePaymentRetryStatus tests — derives retry-button state (count, can-retry, + * cooling countdown) from the parent intent so PaymentStatusDisplay can + * render the correct UI before the user clicks (FR-008, FR-010). + */ + +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import { renderHook, act, waitFor } from '@testing-library/react'; +import { usePaymentRetryStatus } from '../usePaymentRetryStatus'; +import { COOLING_PERIOD_MS, RETRY_LIMIT } from '@/lib/payments/payment-service'; + +// ── Mock parent-intent fetch ──────────────────────────────────────────── +const mockSingle = vi.fn(); +vi.mock('@/lib/supabase/client', () => ({ + supabase: { + from: vi.fn(() => ({ + select: vi.fn(() => ({ + eq: vi.fn(() => ({ + single: mockSingle, + })), + })), + })), + }, +})); + +beforeEach(() => { + mockSingle.mockReset(); + // shouldAdvanceTime keeps microtask queues + waitFor internals working + // while still letting us control setInterval / setTimeout deterministically. + vi.useFakeTimers({ shouldAdvanceTime: true }); +}); + +afterEach(() => { + vi.useRealTimers(); +}); + +const NOW = new Date('2026-04-27T20:00:00Z').getTime(); + +function setNow() { + vi.setSystemTime(new Date(NOW)); +} + +function mockIntent(overrides: Record = {}) { + mockSingle.mockResolvedValue({ + data: { + id: 'parent-1', + retry_count: 0, + created_at: new Date(NOW - COOLING_PERIOD_MS - 1000).toISOString(), + expires_at: new Date(NOW + 3_600_000).toISOString(), + ...overrides, + }, + error: null, + }); +} + +describe('usePaymentRetryStatus', () => { + it('returns loading=true initially, then resolved status', async () => { + setNow(); + mockIntent({ retry_count: 1 }); + const { result } = renderHook(() => usePaymentRetryStatus('parent-1')); + expect(result.current.loading).toBe(true); + await waitFor(() => expect(result.current.loading).toBe(false)); + expect(result.current.retryCount).toBe(1); + expect(result.current.maxRetries).toBe(RETRY_LIMIT); + }); + + it('canRetry=false when retry_count >= RETRY_LIMIT', async () => { + setNow(); + mockIntent({ retry_count: RETRY_LIMIT }); + const { result } = renderHook(() => usePaymentRetryStatus('parent-1')); + await waitFor(() => expect(result.current.loading).toBe(false)); + expect(result.current.canRetry).toBe(false); + expect(result.current.disabledReason).toBe('limit'); + }); + + it('canRetry=false when within cooling window; disabledReason=cooling', async () => { + setNow(); + mockIntent({ + created_at: new Date(NOW - 5_000).toISOString(), // 5s ago + }); + const { result } = renderHook(() => usePaymentRetryStatus('parent-1')); + await waitFor(() => expect(result.current.loading).toBe(false)); + expect(result.current.canRetry).toBe(false); + expect(result.current.disabledReason).toBe('cooling'); + expect(result.current.coolingMsRemaining).toBeGreaterThan(0); + expect(result.current.coolingMsRemaining).toBeLessThanOrEqual( + COOLING_PERIOD_MS + ); + }); + + it('canRetry=false when expired; disabledReason=expired', async () => { + setNow(); + mockIntent({ + expires_at: new Date(NOW - 1000).toISOString(), + }); + const { result } = renderHook(() => usePaymentRetryStatus('parent-1')); + await waitFor(() => expect(result.current.loading).toBe(false)); + expect(result.current.canRetry).toBe(false); + expect(result.current.disabledReason).toBe('expired'); + }); + + it('canRetry=true when fresh, not maxed, not expired', async () => { + setNow(); + mockIntent(); + const { result } = renderHook(() => usePaymentRetryStatus('parent-1')); + await waitFor(() => expect(result.current.loading).toBe(false)); + expect(result.current.canRetry).toBe(true); + expect(result.current.disabledReason).toBe(null); + }); + + it('countdown ticks down while cooling and flips canRetry once it hits 0', async () => { + setNow(); + mockIntent({ + created_at: new Date(NOW - (COOLING_PERIOD_MS - 3_000)).toISOString(), + }); + const { result } = renderHook(() => usePaymentRetryStatus('parent-1')); + await waitFor(() => expect(result.current.loading).toBe(false)); + + expect(result.current.canRetry).toBe(false); + const initial = result.current.coolingMsRemaining; + expect(initial).toBeGreaterThan(0); + + // Advance past the cooling window + await act(async () => { + vi.advanceTimersByTime(3_500); + }); + expect(result.current.coolingMsRemaining).toBe(0); + expect(result.current.canRetry).toBe(true); + }); + + it('returns { loading: false, canRetry: false, disabledReason: null } for a null intentId', async () => { + setNow(); + const { result } = renderHook(() => usePaymentRetryStatus(null)); + await waitFor(() => expect(result.current.loading).toBe(false)); + expect(result.current.canRetry).toBe(false); + expect(mockSingle).not.toHaveBeenCalled(); + }); +}); diff --git a/src/hooks/usePaymentRetryStatus.ts b/src/hooks/usePaymentRetryStatus.ts new file mode 100644 index 00000000..a77bbd33 --- /dev/null +++ b/src/hooks/usePaymentRetryStatus.ts @@ -0,0 +1,163 @@ +/** + * usePaymentRetryStatus + * + * Reads the parent payment intent and computes the retry button's state + * — count, cap, cooling countdown, expiry — so PaymentStatusDisplay can + * render the right UI before the user clicks (FR-008, FR-010). + * + * The retry-cap and cooling logic mirror the server-side checks in + * `retryFailedPayment` (`src/lib/payments/payment-service.ts`). The hook + * is a UX hint, not a security boundary — a user who manipulates client + * state still hits the same checks server-side and gets a thrown error + * caught by PaymentStatusDisplay's catch handler. + */ + +'use client'; + +import { useEffect, useState, useRef } from 'react'; +import { supabase } from '@/lib/supabase/client'; +import { COOLING_PERIOD_MS, RETRY_LIMIT } from '@/lib/payments/payment-service'; + +export type RetryDisabledReason = 'limit' | 'cooling' | 'expired' | null; + +export interface PaymentRetryStatus { + loading: boolean; + retryCount: number; + maxRetries: number; + canRetry: boolean; + disabledReason: RetryDisabledReason; + /** ms remaining until cooling expires; 0 when not cooling */ + coolingMsRemaining: number; +} + +const INITIAL: PaymentRetryStatus = { + loading: true, + retryCount: 0, + maxRetries: RETRY_LIMIT, + canRetry: false, + disabledReason: null, + coolingMsRemaining: 0, +}; + +const RESOLVED_NULL: PaymentRetryStatus = { + ...INITIAL, + loading: false, +}; + +/** Tick interval for the cooling countdown. 1Hz is enough — the UI just + * shows whole seconds, and we're not doing animations. */ +const COOLING_TICK_MS = 1000; + +export function usePaymentRetryStatus( + intentId: string | null +): PaymentRetryStatus { + const [status, setStatus] = useState( + intentId ? INITIAL : RESOLVED_NULL + ); + // Refs hold the parent's createdAt + expiresAt so the cooling tick can + // recompute without re-fetching. + const createdAtRef = useRef(null); + const expiresAtRef = useRef(null); + const retryCountRef = useRef(0); + + // Fetch parent intent. + useEffect(() => { + if (!intentId) { + setStatus(RESOLVED_NULL); + return; + } + + let cancelled = false; + setStatus(INITIAL); + + (async () => { + const { data, error } = await supabase + .from('payment_intents') + .select('id, retry_count, created_at, expires_at') + .eq('id', intentId) + .single(); + + if (cancelled) return; + + if (error || !data) { + setStatus({ ...RESOLVED_NULL, disabledReason: null }); + return; + } + + const createdAt = new Date( + (data as { created_at: string }).created_at + ).getTime(); + const expiresAt = new Date( + (data as { expires_at: string }).expires_at + ).getTime(); + const retryCount = (data as { retry_count: number }).retry_count; + + createdAtRef.current = createdAt; + expiresAtRef.current = expiresAt; + retryCountRef.current = retryCount; + + setStatus(computeStatus(retryCount, createdAt, expiresAt, false)); + })(); + + return () => { + cancelled = true; + }; + }, [intentId]); + + // Cooling countdown tick. + useEffect(() => { + if (status.disabledReason !== 'cooling') return; + + const tick = setInterval(() => { + if (createdAtRef.current === null || expiresAtRef.current === null) { + return; + } + setStatus( + computeStatus( + retryCountRef.current, + createdAtRef.current, + expiresAtRef.current, + false + ) + ); + }, COOLING_TICK_MS); + + return () => clearInterval(tick); + }, [status.disabledReason]); + + return status; +} + +function computeStatus( + retryCount: number, + createdAt: number, + expiresAt: number, + loading: boolean +): PaymentRetryStatus { + const now = Date.now(); + const base = { + loading, + retryCount, + maxRetries: RETRY_LIMIT, + coolingMsRemaining: 0, + }; + + if (retryCount >= RETRY_LIMIT) { + return { ...base, canRetry: false, disabledReason: 'limit' }; + } + if (expiresAt < now) { + return { ...base, canRetry: false, disabledReason: 'expired' }; + } + + const elapsedMs = now - createdAt; + if (elapsedMs < COOLING_PERIOD_MS) { + return { + ...base, + canRetry: false, + disabledReason: 'cooling', + coolingMsRemaining: COOLING_PERIOD_MS - elapsedMs, + }; + } + + return { ...base, canRetry: true, disabledReason: null }; +} diff --git a/tests/e2e/payment/03-failed-payment-retry.spec.ts b/tests/e2e/payment/03-failed-payment-retry.spec.ts index 7bf2c68a..4589c6df 100644 --- a/tests/e2e/payment/03-failed-payment-retry.spec.ts +++ b/tests/e2e/payment/03-failed-payment-retry.spec.ts @@ -73,12 +73,29 @@ test.describe('Failed Payment Retry Logic', () => { ).toBeVisible(); }); - test.skip('should display error message for network failure', async ({ + test('should display offline error banner when offline', async ({ page, context, }) => { - // Skip: Offline error display not yet implemented - test.skip(true, 'Offline error handling not yet implemented'); + // Force offline before navigating so the banner picks it up on mount. + await context.setOffline(true); + try { + await page.goto( + '/payment-result?id=00000000-0000-0000-0000-000000000000' + ); + await dismissCookieBanner(page); + + // Banner has role="status" and the offline copy is the same one + // exercised by the unit test. + await expect(page.getByText(/you.?re offline/i)).toBeVisible({ + timeout: 10_000, + }); + await expect( + page.getByText(/we.?ll process your payment/i) + ).toBeVisible(); + } finally { + await context.setOffline(false); + } }); test.skip('should handle subscription payment retry with exponential backoff', async ({ From 1ea0833e5c2d78192c3ce422de031d389d14c883 Mon Sep 17 00:00:00 2001 From: TurtleWolfe Date: Tue, 28 Apr 2026 02:09:11 +0000 Subject: [PATCH 4/5] fix(e2e): offline-banner test must navigate online first (#63 CI fix) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The test as shipped called context.setOffline(true) BEFORE page.goto(), which works on a dev server (page is in cache after first hit) but fails in CI's static-export flow with net::ERR_INTERNET_DISCONNECTED — there's no service-worker cache warmed for this URL when the page first loads. Fix: navigate online first, wait for the page to render the loaded / not-found branch (both of which mount OfflineRetryBanner), then flip offline. useOfflineStatus listens for the browser 'offline' event so the banner re-renders without another navigation. Same story as the diagnostic-loud failure modes PR #58 was designed to catch — the failure was clear from the CI log so this is a one-shot fix. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../payment/03-failed-payment-retry.spec.ts | 28 +++++++++++++------ 1 file changed, 20 insertions(+), 8 deletions(-) diff --git a/tests/e2e/payment/03-failed-payment-retry.spec.ts b/tests/e2e/payment/03-failed-payment-retry.spec.ts index 4589c6df..2d108da1 100644 --- a/tests/e2e/payment/03-failed-payment-retry.spec.ts +++ b/tests/e2e/payment/03-failed-payment-retry.spec.ts @@ -77,18 +77,30 @@ test.describe('Failed Payment Retry Logic', () => { page, context, }) => { - // Force offline before navigating so the banner picks it up on mount. + // Navigate ONLINE first — the static-export build can't be loaded + // without network in CI (no SW cache warmed for this URL). After the + // page is up, flip offline; useOfflineStatus listens for the browser + // 'offline' event and the banner re-renders. + await page.goto('/payment-result?id=00000000-0000-0000-0000-000000000000'); + await dismissCookieBanner(page); + await waitForAuthenticatedState(page); + + // Wait for the loaded/not-found render to settle so the banner is in + // the DOM tree (it mounts inside the loaded + not-found branches). + await page + .getByText( + /no payment session found|payment is still processing|payment result/i + ) + .first() + .waitFor({ state: 'visible', timeout: 15_000 }); + await context.setOffline(true); try { - await page.goto( - '/payment-result?id=00000000-0000-0000-0000-000000000000' - ); - await dismissCookieBanner(page); - // Banner has role="status" and the offline copy is the same one - // exercised by the unit test. + // exercised by the unit test. Polls because useOfflineStatus + the + // banner's poll interval need a moment to converge after the flip. await expect(page.getByText(/you.?re offline/i)).toBeVisible({ - timeout: 10_000, + timeout: 15_000, }); await expect( page.getByText(/we.?ll process your payment/i) From a3508331dffc6bcf62ce6820c710b2a677662c57 Mon Sep 17 00:00:00 2001 From: TurtleWolfe Date: Tue, 28 Apr 2026 18:02:13 +0000 Subject: [PATCH 5/5] fix(e2e): offline-banner test must synthesize the event for cross-browser (#63) Previous fix navigated online first, then called context.setOffline(true). That works in Chromium (chromium-gen 3/6 went green) but fails in Firefox and WebKit because Playwright's setOffline blocks request traffic without firing the 'offline' browser event in those engines. useOfflineStatus only listens for window 'offline' events, so the banner never re-renders. Real browsers DO fire the event on real network drops, so synthesizing it in the test is closer to production behavior than relying on Playwright's imperfect cross-browser setOffline. Override navigator.onLine + dispatch both 'offline' and (in cleanup) 'online' so subsequent tests don't see stuck state. Single-shard verification was the chromium-gen 3/6 success in the prior attempt (run 25030125601); this commit extends the same path to firefox/webkit. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../payment/03-failed-payment-retry.spec.ts | 39 ++++++++++++++----- 1 file changed, 29 insertions(+), 10 deletions(-) diff --git a/tests/e2e/payment/03-failed-payment-retry.spec.ts b/tests/e2e/payment/03-failed-payment-retry.spec.ts index 2d108da1..73214f5b 100644 --- a/tests/e2e/payment/03-failed-payment-retry.spec.ts +++ b/tests/e2e/payment/03-failed-payment-retry.spec.ts @@ -73,14 +73,18 @@ test.describe('Failed Payment Retry Logic', () => { ).toBeVisible(); }); - test('should display offline error banner when offline', async ({ - page, - context, - }) => { + test('should display offline error banner when offline', async ({ page }) => { // Navigate ONLINE first — the static-export build can't be loaded // without network in CI (no SW cache warmed for this URL). After the - // page is up, flip offline; useOfflineStatus listens for the browser - // 'offline' event and the banner re-renders. + // page is up, synthesize the 'offline' browser event; useOfflineStatus + // listens for it and the banner re-renders. + // + // Why dispatch instead of context.setOffline(true)? Playwright's + // setOffline blocks request traffic but does NOT fire the 'offline' + // event in Firefox/WebKit (Chromium does). Real browsers DO fire it + // on real network drops — which is exactly what the hook listens for — + // so synthesizing the event here mimics production behavior and works + // cross-browser. await page.goto('/payment-result?id=00000000-0000-0000-0000-000000000000'); await dismissCookieBanner(page); await waitForAuthenticatedState(page); @@ -94,11 +98,19 @@ test.describe('Failed Payment Retry Logic', () => { .first() .waitFor({ state: 'visible', timeout: 15_000 }); - await context.setOffline(true); + // Override navigator.onLine + dispatch the offline event. The hook + // reads navigator.onLine on its update path, so both must flip. + await page.evaluate(() => { + Object.defineProperty(navigator, 'onLine', { + configurable: true, + get: () => false, + }); + window.dispatchEvent(new Event('offline')); + }); + try { // Banner has role="status" and the offline copy is the same one - // exercised by the unit test. Polls because useOfflineStatus + the - // banner's poll interval need a moment to converge after the flip. + // exercised by the unit test. await expect(page.getByText(/you.?re offline/i)).toBeVisible({ timeout: 15_000, }); @@ -106,7 +118,14 @@ test.describe('Failed Payment Retry Logic', () => { page.getByText(/we.?ll process your payment/i) ).toBeVisible(); } finally { - await context.setOffline(false); + // Restore so subsequent tests don't see a stuck offline state. + await page.evaluate(() => { + Object.defineProperty(navigator, 'onLine', { + configurable: true, + get: () => true, + }); + window.dispatchEvent(new Event('online')); + }); } });