Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
**Feature ID**: 040
**Category**: payments
**Source**: ScriptHammer README (SPEC-056)
**Status**: Partial — Route shipped, UX gaps remain (2026-04-27). Built: `payment-service.ts` retry logic, `<PaymentStatusDisplay>` component, `/payment-result` route (commit `ffb33a1`, 2026-04-16) — 6-state page with retry button. Missing: idempotency-key reuse on retry, retry attempt counter + cooling period (FR-008-010), error-type categorization (FR-002), offline error banner, audit log on retry (NFR-007), update-payment-method flow (User Story 3), guided recovery wizard (User Story 4). Several E2E stubs in `tests/e2e/payment/03-failed-payment-retry.spec.ts` are still skipped pending Stripe API keys; some have been replaced with real assertions for the missing-session and malformed-ID empty states. Depends on 024 API keys.
**Status**: Mostly Shipped — recovery UX shipped; saved-method storage out of scope for static-export architecture (2026-04-28). Built: `payment-service.ts` retry logic with idempotency-key reuse + retry cap + cooling + expiry guard + audit log; `<PaymentStatusDisplay>` with categorized errors + attempt counter + cooling countdown + recovery-list disclosure; `/payment-result` route (commit `ffb33a1`, 2026-04-16); `<OfflineRetryBanner>`; `<SwitchProviderPanel>` for in-line provider switching after a decline; schema additions on `payment_intents` (`retry_count`, `parent_intent_id`) and `auth_audit_logs.event_type` (`payment_retry`). Architecture-fit reframings: US3 (Update Payment Method) became "switch payment method (provider switch)" since ScriptHammer never stores cards — every checkout is a fresh provider session; US4 (Guided Recovery Wizard) became inline progressive disclosure escalating with retry_count. Saved-card storage (Stripe Customer + saved_payment_methods table + stripe.js `<CardElement>`) remains explicitly out of scope for this template; would be a separate multi-PR feature behind a PCI scope review. Several E2E stubs in `tests/e2e/payment/03-failed-payment-retry.spec.ts` remain skipped pending Stripe API keys for the actual Checkout-flow tests.

## Description

Expand Down
31 changes: 16 additions & 15 deletions features/payments/040-payment-retry-ui/spec.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

**Feature Branch**: `040-payment-retry-ui`
**Created**: 2025-12-30
**Status**: Partial (route shipped, UX gaps remain)
**Status**: Mostly Shipped (recovery UX shipped; saved-method storage out of scope)
**Input**: User description: "Error display, retry button, and payment method update flow. Provides clear UI for handling failed payments with actionable options to resolve payment issues."

---
Expand All @@ -11,29 +11,30 @@

## Implementation Status

**Last audited**: 2026-04-27
**Real status**: Partial (route shipped, UX gaps remain)
**Last audited**: 2026-04-28
**Real status**: Mostly Shipped (recovery UX shipped; saved-method storage out of scope for static-export architecture)
**Tracking**: see gap-audit GitHub issues + STATUS.md

### Shipped

- `payment-service.ts` retry logic (`retryFailedPayment`); Stripe + PayPal webhook handlers
- `/payment-result?id=<intent-uuid>` route (commit `ffb33a1`, 2026-04-16) — 6-state page (loading, missing-id, not-configured, loaded, not-found, error), `<ProtectedRoute>` gated, `<Suspense>`-wrapped
- `<PaymentStatusDisplay>` with status badge, details panel, real-time updates via `usePaymentRealtime`, retry button for `status === 'failed'`
- `payment-service.ts` retry logic with idempotency-key reuse (FR-006), retry cap (FR-009, RETRY_LIMIT=3), cooling period (FR-010, COOLING_PERIOD_MS=30s), expiry guard, and audit logging (NFR-007)
- `/payment-result?id=<intent-uuid>` route (commit `ffb33a1`, 2026-04-16) — 6-state page, `<ProtectedRoute>` gated
- `<PaymentStatusDisplay>` — categorized error message + resolution hint (FR-001, FR-002, FR-003), transaction reference for support (FR-004), attempt counter (FR-008), cooling-state countdown, recovery-list disclosure (FR-016/017/019)
- `<OfflineRetryBanner>` — silent in steady state; surfaces queue count when offline or syncing
- `<SwitchProviderPanel>` — inline payment-method switcher reusing `<PaymentButton>`'s multi-provider machinery (Stripe / PayPal / Cash App / Chime); satisfies FR-018 "alternative payment options"
- Schema: `payment_intents.retry_count`, `payment_intents.parent_intent_id`, `auth_audit_logs` event_type extended with `payment_retry`
- E2E coverage: offline-banner test (cross-browser via dispatched `offline` event), switch-provider panel mount, recovery-list escalation

### Gaps
### Out of scope (architecture-fit reframings)

- Retry button regenerates `idempotency_key` instead of reusing the queued one (loses dedupe with offline-queue replay)
- No retry attempt counter / cooling period enforcement (FR-008, FR-009, FR-010)
- No error categorization — current UI only shows `status === 'failed'` with no reason context (FR-002 lists 8 error types, none mapped)
- No offline error banner
- No audit log on retry attempts (NFR-007)
- User Story 3 (Update Payment Method, FR-011-FR-015) — entirely unbuilt
- User Story 4 (Guided Recovery Wizard, FR-016-FR-019) — entirely unbuilt
- **US3 reframed as "switch payment method (provider switch)"**: the spec's literal "Update Payment Method" (FR-011-FR-015) assumes saved cards + stripe.js Elements + a PCI surface. ScriptHammer never stores cards — every checkout is a fresh Stripe-hosted Checkout (or PayPal redirect, or Cash App / Chime direct link). The honest interpretation in this codebase is "after a card decline, let the user pick a different provider." Implemented via `<SwitchProviderPanel>` in PR #43-followup. Saved-card storage (Stripe Customer + saved_payment_methods table + `<CardElement>`) is a separate multi-PR feature and not appropriate for this template's static-export architecture.
- **US4 reframed as inline progressive disclosure**: a separate wizard component + route is unnecessary when the failed-state block can escalate UI density based on `retry_count`. The recovery list (retry → switch method → contact support) becomes visible at retry_count=2 and emphasizes support at retry_count=3. Honors FR-016/017/018/019 without adding a stepper component.

### Notes

- Route name is `/payment-result` (kebab-case top-level, matches `/payment-demo` and 8 other flat routes); spec previously said `/payment/result` but the route was renamed at implementation time and the doc lagged. Future sibling routes (`/payment-dashboard`, `/payment-history`, `/payment-subscriptions`) are expected to follow the same convention unless a shared `/payment/` shell is justified.
- Route name is `/payment-result` (kebab-case top-level, matches `/payment-demo` and 8 other flat routes); the original spec said `/payment/result`. Renamed at implementation time; doc-correction shipped in PR #62. Future sibling routes (`/payment-dashboard`, `/payment-history`, `/payment-subscriptions`) follow the same kebab-case convention unless a shared `/payment/` shell is justified.
- The retry button reuses the parent intent's `idempotency_key` so doubled clicks become server-side ON CONFLICT no-ops via the partial unique index (PR #59 + PR #63 dedupe contract).
- Error categorization: 8 categories in `src/lib/payments/error-categorization.ts`; non-recoverable categories (`expired_card`, `limit_exceeded`) hide retry and show support-contact link.

<!-- AUDIT-IMPL-STATUS-END -->

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -421,4 +421,169 @@ describe('PaymentStatusDisplay', () => {
expect(btn).toHaveTextContent(/Try again in 24s/);
});
});

describe('US3+US4 — switch provider + recovery disclosure (#43)', () => {
// Stub SwitchProviderPanel to a sentinel so we can assert wiring without
// re-mounting the whole sub-tree (it has its own dedicated tests).
vi.mock('@/components/payment/SwitchProviderPanel', () => ({
SwitchProviderPanel: (props: Record<string, unknown>) => (
<div data-testid="switch-provider-panel">
switch panel for {String(props.parentIntentId)}
</div>
),
}));

function setupFailedRecoverable() {
const failed = createMockResult('failed');
failed.error_code = 'card_declined';
vi.mocked(usePaymentRealtime).mockReturnValue({
paymentResult: failed,
loading: false,
error: null,
});
}

it('renders the "Use a different payment method" button on recoverable failures', () => {
setupFailedRecoverable();
render(<PaymentStatusDisplay paymentResultId="test-id" />);
expect(
screen.getByRole('button', { name: /use a different payment method/i })
).toBeInTheDocument();
});

it('clicking the switch button toggles the SwitchProviderPanel', async () => {
const user = userEvent.setup();
setupFailedRecoverable();
render(<PaymentStatusDisplay paymentResultId="test-id" />);

// Closed initially
expect(
screen.queryByTestId('switch-provider-panel')
).not.toBeInTheDocument();

const switchBtn = screen.getByRole('button', {
name: /use a different payment method/i,
});
await user.click(switchBtn);

// Panel mounts with parent intent id wired through
expect(screen.getByTestId('switch-provider-panel')).toBeInTheDocument();
expect(screen.getByTestId('switch-provider-panel')).toHaveTextContent(
'456' // intent_id from createMockResult
);

// Clicking again closes
await user.click(
screen.getByRole('button', {
name: /^cancel$/i,
})
);
expect(
screen.queryByTestId('switch-provider-panel')
).not.toBeInTheDocument();
});

it('does not render the switch button for non-recoverable errors', () => {
const failed = createMockResult('failed');
failed.error_code = 'expired_card'; // not recoverable
vi.mocked(usePaymentRealtime).mockReturnValue({
paymentResult: failed,
loading: false,
error: null,
});
render(<PaymentStatusDisplay paymentResultId="test-id" />);
expect(
screen.queryByRole('button', {
name: /use a different payment method/i,
})
).not.toBeInTheDocument();
});

it('hides the recovery disclosure at retry_count=0', async () => {
setupFailedRecoverable();
const { usePaymentRetryStatus } = await import(
'@/hooks/usePaymentRetryStatus'
);
vi.mocked(usePaymentRetryStatus).mockReturnValue({
loading: false,
retryCount: 0,
maxRetries: 3,
canRetry: true,
disabledReason: null,
coolingMsRemaining: 0,
});
render(<PaymentStatusDisplay paymentResultId="test-id" />);
expect(screen.queryByText(/need more help/i)).not.toBeInTheDocument();
});

it('renders collapsed recovery disclosure at retry_count=1', async () => {
setupFailedRecoverable();
const { usePaymentRetryStatus } = await import(
'@/hooks/usePaymentRetryStatus'
);
vi.mocked(usePaymentRetryStatus).mockReturnValue({
loading: false,
retryCount: 1,
maxRetries: 3,
canRetry: true,
disabledReason: null,
coolingMsRemaining: 0,
});
render(<PaymentStatusDisplay paymentResultId="test-id" />);
const summary = screen.getByText(/need more help/i);
// <details> without `open` attribute starts collapsed
const details = summary.closest('details');
expect(details).not.toHaveAttribute('open');
});

it('renders expanded recovery list at retry_count=2 (FR-016/017)', async () => {
setupFailedRecoverable();
const { usePaymentRetryStatus } = await import(
'@/hooks/usePaymentRetryStatus'
);
vi.mocked(usePaymentRetryStatus).mockReturnValue({
loading: false,
retryCount: 2,
maxRetries: 3,
canRetry: true,
disabledReason: null,
coolingMsRemaining: 0,
});
render(<PaymentStatusDisplay paymentResultId="test-id" />);
const summary = screen.getByText(/need more help/i);
const details = summary.closest('details');
expect(details).toHaveAttribute('open');
// All three steps in priority order: retry → switch method → support.
// "use a different payment method" appears as both a button label and a
// recovery-list step, so assert that BOTH elements exist.
expect(
screen.getByText(/try again — payment failures/i)
).toBeInTheDocument();
expect(
screen.getAllByText(/use a different payment method/i).length
).toBeGreaterThanOrEqual(2);
expect(
screen.getByRole('link', { name: /contact support/i })
).toBeInTheDocument();
});

it('emphasizes support contact when retry cap is reached (FR-019)', async () => {
setupFailedRecoverable();
const { usePaymentRetryStatus } = await import(
'@/hooks/usePaymentRetryStatus'
);
vi.mocked(usePaymentRetryStatus).mockReturnValue({
loading: false,
retryCount: 3,
maxRetries: 3,
canRetry: false,
disabledReason: 'limit',
coolingMsRemaining: 0,
});
render(<PaymentStatusDisplay paymentResultId="test-id" />);
// Retry step is struck through; support link is bold
const retryStep = screen.getByText(/try again — payment failures/i);
expect(retryStep).toHaveClass('line-through');
});
});
});
Loading
Loading