pal-e-docs

Review: Payment recovery for abandoned Stripe registrations

review-890-2026-04-08-v2 Doc

review ready
Project

Verdict: READY

Re-review after refinements applied. All 6 issues from previous review addressed. Ticket is ready for dispatch once dependency #390 lands.

Template Completeness

  • [x] Type — Feature
  • [x] Lineage — standalone, discovered during registration flow audit
  • [x] Repo — forgejo_admin/basketball-api
  • [x] User Story — clear parent registration recovery scenario
  • [x] Context — explains PENDING flow, Stripe session expiry, lost revenue motivation
  • [x] File Targets — 4 files specified with line references and patterns
  • [x] Acceptance Criteria — 5 testable conditions
  • [x] Test Expectations — 4 unit tests with run command
  • [x] Constraints — MJML template, admin-triggered pattern, signup_method filter, dependency
  • [x] Checklist — PR, tests, no unrelated changes
  • [x] Related — westside-basketball, depends on #390

Traceability

  • [x] story:WS-S17 label — "As a parent, I want to register my child online (info + photo + payment + waiver) so that signup is one seamless flow"
  • [x] story note verified — found in project-westside-basketball user-stories section under Parent stories
  • [x] arch:registration label — component identified
  • [ ] arch note MISSING — [SCOPE] Create architecture note arch-registration for component registration (tracked separately as discovered scope, not a blocker for this ticket)
  • [x] Forgejo issue — forgejo_admin/basketball-api#389, open

File Targets

  • [x] src/basketball_api/services/email.py — verified: file exists, load_email_template pattern confirmed present
  • [x] src/basketball_api/routes/admin.py — verified: lines 899-913 confirmed as outbox pattern (@router.post, Depends(require_admin), Depends(get_db))
  • [x] src/basketball_api/models.py — verified: confirmation_email_sent at line 289 confirmed as pattern reference, signup_method at line 287 confirmed as String(50) with default "stripe"
  • [x] Alembic migration — new file for recovery_email_sent boolean column, appropriate

Repo Placement

Correct. Issue filed on forgejo_admin/basketball-api, all file targets are in that repo. Single-repo change. No multi-repo concerns.

Dependencies

  • [x] #390 (open) — "Fix: confirmation email skipped for existing Keycloak accounts + token=None in profile URL." Recovery email URLs depend on registration_token generation fix from #390. Dependency is explicitly documented in Constraints and Related sections. This ticket must not be dispatched until #390 lands.

Acceptance Criteria

5 ACs, all testable by an agent:
  • AC1: PENDING Stripe registration >2h gets recovery email — verifiable via unit test with time mock
  • AC2: Payment link completes without re-filling form — verifiable via Stripe session URL check
  • AC3: Recovery email sent only once — verifiable via recovery_email_sent column guard + unit test
  • AC4: Paid registrations unaffected — verifiable via unit test filtering on payment_status
  • AC5: Cash registrations excluded — verifiable via signup_method=="stripe" filter + unit test
All criteria are specific and programmatically verifiable. No ambiguous language. Run command: pytest tests/ -k test_payment_recovery.

Blast Radius

  • 4 files touched in 1 repo — low blast radius
  • registration_token is referenced across 8 files but ticket correctly scopes to NOT touch the Stripe webhook handler
  • The confirmation_email_sent boolean pattern is the exact model for the new recovery_email_sent column — minimal risk of unexpected side effects
  • No sibling services affected. No downstream consumers impacted.
  • Rollback is straightforward: revert migration + remove endpoint/email function

Decomposition Assessment

Apply the three-thing limit and five-minute rule:
  • 3-4 file targets in 1 repo — within the three-thing limit
  • 5 ACs — borderline but all are cohesive parts of one logical feature (email + endpoint + model + migration)
  • Estimated agent time: <5 minutes — single feature, well-defined patterns to follow
  • No independent subtasks that would benefit from parallelization — the model change, endpoint, and email function are tightly coupled
No decomposition needed.

Recommendation

No action needed. Ticket is ready for dispatch once #390 lands.

Refinements Verified (re-review delta)

  • [x] models.py + Alembic migration added as file targets
  • [x] signup_method=="stripe" filter requirement added (was incorrectly scoped before)
  • [x] Admin-triggered endpoint pattern specified with line reference (admin.py:899-913)
  • [x] AC #5 added: cash registrations must NOT receive recovery emails
  • [x] Cash exclusion unit test added to Test Expectations
  • [x] Explicit dependency on #390 documented in Constraints and Related