Review: Fix: first-payment checkout 409 (stale pending orders)

review-1003-2026-04-13 Review

review ready

Verdict: READY

Template Completeness

  • [x] Type — Bug
  • [x] Lineage — Standalone, discovered Apr 13
  • [x] Repo — forgejo_admin/basketball-api
  • [x] What Broke — detailed symptoms, prod log evidence, order state table
  • [x] Repro Steps — 6 clear steps including bot-triggered scenario
  • [x] Expected Behavior — present
  • [x] Environment — cluster, namespace, commit SHA
  • [x] File Targets — present with explicit "do not touch" list (exceeds template minimum)
  • [x] Acceptance Criteria — 5 criteria
  • [x] Test Expectations — 3 named tests with run command (exceeds template minimum)
  • [x] Constraints — 4 constraints including audit trail and threshold rationale
  • [x] Related — project, cross-repo ticket, discovered scope noted
All required Bug template sections present. Issue also includes File Targets, Test Expectations, and Constraints sections beyond the base Bug template — excellent specificity.

Traceability

  • [x] story:WS-S11 label — "As an admin, I want to create Stripe checkout sessions for jerseys/fees so that payments are automated"
  • [x] story note verified — WS-S11 found in project-westside-basketball user-stories section under Admin (Marcus)
  • [x] arch:basketball-api label — basketball-api component
  • [ ] arch note MISSING — [SCOPE] No arch-basketball-api note exists in pal-e-docs. This is a pre-existing gap across many westside board items, not specific to this ticket.
  • [x] Forgejo issue — forgejo_admin/basketball-api#473, open

File Targets

  • [x] src/basketball_api/routes/checkout.py lines 347-361 — verified: first_payment_checkout() duplicate guard queries for OrderStatus.paid and OrderStatus.pending in an .in_() filter, then raises 409 unconditionally. Exactly matches ticket description.
  • [x] "Do not touch" files verified as existing and irrelevant to the fix

Repo Placement

OK. Issue filed on forgejo_admin/basketball-api, fix is entirely within basketball-api. Single repo.

Dependencies

No blocking dependencies found on the board. Board item #720 (legacy jersey webhook handler) touches payment status but is in backlog and unrelated. Cross-reference to pal-e-platform#290 (observability) is informational only, not a blocker.

Acceptance Criteria

All 5 AC are agent-verifiable:
  • AC1-4: testable via the 3 named pytest cases in Test Expectations
  • AC5 (no webhook regression): verifiable by running existing webhook test suite
Test command provided: pytest tests/test_first_payment.py -v. Test file confirmed to exist at tests/test_first_payment.py.

Blast Radius

Same pattern exists in 2 sibling functions — important context for the dev agent:
  • create_checkout_session() (checkout.py:181) — jersey/product checkout, same .in_([paid, pending]) guard. May need the same stale-pending fix if jersey links are also shared via email/social.
  • create_player_checkout_session() (checkout.py:485) — admin bulk checkout helper, same pattern. Less urgent since admin-triggered, not link-based.
  • admin.py:2356 — admin bulk route already handles pending differently (skips with message instead of hard 409). No fix needed here.
The ticket correctly scopes to first_payment_checkout() only. The sibling functions are discovered scope for separate tickets if the same bot-triggered 409 manifests there.

Decomposition Assessment

1 file target, 1 repo, 5 AC, 3 tests. Single function modification with clear before/after behavior. Well under the 5-minute rule. No decomposition needed.

Recommendation

No action needed — ticket is ready for dispatch.
Informational notes (not blocking):
  • [SCOPE] Create architecture note arch-basketball-api for component basketball-api (pre-existing gap, not specific to this ticket).
  • Discovered scope: sibling functions create_checkout_session() and create_player_checkout_session() have the same stale-pending vulnerability. Track as separate tickets if the pattern manifests on jersey or admin checkout links.