Review: Checkout endpoint: GET /checkout/first-payment with Stripe redirect

review-874-2026-04-06 Doc

review ready

Verdict: READY

Template Completeness

  • [x] Type — Feature
  • [x] Lineage — Decomposed from #366, ticket B of 3
  • [x] Repo — forgejo_admin/basketball-api
  • [x] User Story — parent clicks payment link, redirects to Stripe
  • [x] Context — proration formula, existing patterns, no webhook changes
  • [x] File Targets — modify and create lists with exclusions
  • [x] Acceptance Criteria — 8 criteria
  • [x] Test Expectations — 6 unit tests + run command
  • [x] Constraints — 5 constraints documented
  • [x] Checklist — present
  • [x] Related — project, parent, dependency, plan
All required sections for template-issue-feature are present and populated.

Traceability

  • [x] story:WS-S7 label — "As an admin, I want to send branded email announcements so that parent comms are professional and consistent"
  • [x] story note verified — WS-S7 found in project-westside-basketball user-stories section (Admin list)
  • [x] arch:basketball-api label — basketball-api component
  • [ ] arch note MISSING — No arch-basketball-api note found in pal-e-docs. However, basketball-api is a well-established repo with its own project page. This is a systemic gap across most westside board items, not specific to this ticket. Acceptable for now.
  • [x] Forgejo issue — forgejo_admin/basketball-api#368, state: open

File Targets

  • [x] src/basketball_api/routes/checkout.py — verified: file exists (288 lines), contains existing Stripe Customer create/reuse pattern (lines 194-205), _SUCCESS_URL/_CANCEL_URL (lines 29-30), Order creation, duplicate prevention. All patterns the ticket references are real.
  • [x] tests/test_first_payment.py — to create, parent directory tests/ exists with conftest.py providing client fixture
  • [x] Files NOT to touch correctly identified — webhooks.py, email.py, admin.py all exist and are correctly scoped out
Model verification:
  • [x] ContractStatus — exists at models.py:104 (none, offered, signed)
  • [x] Player.contract_token — exists at models.py:232, unique=True, nullable=True
  • [x] Player.monthly_fee — exists at models.py:239, Integer, nullable=True
  • [x] Player.stripe_customer_id — confirmed in checkout.py:196 existing pattern
  • [x] ProductCategory.monthly — does NOT exist yet (current enum: jersey, contract, tournament, equipment). Ticket correctly declares dependency on migration ticket A (#366 sub-ticket) which adds this value. Dependency is accurate.
  • [x] RedirectResponse — not currently imported in checkout.py but available from fastapi.responses (used in coach.py and tryouts.py). Import is straightforward.

Repo Placement

OK. Issue filed on forgejo_admin/basketball-api, all file targets are in basketball-api. Single-repo scope.

Dependencies

  • [x] Migration ticket (A from #366) — this ticket explicitly depends on the migration that adds ProductCategory.monthly and seeds the "Monthly Fee" product. Dependency is documented in Lineage and Constraints. Must apply migration before this endpoint works. Status: pending (must run first).
  • [x] Board item #733 — "Fix 9 failing jersey/checkout tests" is in_progress. Touches checkout test infrastructure. Not a hard blocker (different test file) but agent should be aware of potential conftest changes. Status: in_progress, non-blocking.
  • [x] Board item #694 — "Add player_id param to jersey/checkout for multi-player parents" is in QA. Could affect checkout.py patterns but is additive, not conflicting. Status: QA, non-blocking.

Acceptance Criteria

All 8 criteria are testable by an agent:
  • 307 redirect — verifiable via test client response status + Location header
  • 404 on invalid/unsigned tokens — straightforward negative tests
  • 409 on duplicate orders — pattern already exists in checkout.py (lines 150-166)
  • Proration math — deterministic, three tier values + null default all specified with exact dollar/cent amounts
  • Order record fields — verifiable via DB query in test
  • Stripe Customer create/reuse — mockable, pattern exists in checkout.py
All criteria are specific, measurable, and have concrete expected values. No ambiguous language.

Blast Radius

  • New endpoint in checkout.py — additive, no existing routes affected
  • Proration formula matches westside-contracts +page.svelte:9 — formula consistency verified in issue context
  • Stripe mock pattern (@patch("basketball_api.routes.checkout.stripe")) is consistent with existing test patterns
  • No downstream consumers affected — this is a new endpoint
  • Rollback: remove one function + one route. Straightforward.

Decomposition Assessment

Apply three-thing limit and five-minute rule:
  • Discrete changes: 2 — one new endpoint in existing file + one new test file. Under the 3-change limit.
  • Estimated agent time: ~3-4 minutes. Under the 5-minute rule.
  • Parallelizable subtasks: None — endpoint and tests are tightly coupled.
  • Acceptance criteria count: 8, but 6 are simple HTTP status checks and 2 are data verification. All exercised by a single test file.
No decomposition needed.

Recommendation

No action needed. Scope is solid, file targets verified, traceability complete, fits single agent pass.
Minor systemic note: the arch:basketball-api note gap exists across most westside board items — not blocking this ticket.