Review: Checkout endpoint: GET /checkout/first-payment with Stripe redirect
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-apinote 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 directorytests/exists withconftest.pyprovidingclientfixture - [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.monthlyand 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.