pal-e-docs

Review: Tournament product creation + per-player Stripe checkout links (re-review)

review-994-2026-04-12-v2 Review

review needs-refinement
Project

Verdict: APPROVED

Re-review of board item #994. Previous review (review-994-2026-04-12) flagged three issues. Only one was fixed.

Previous Findings Status

  • [x] [LABEL] arch:email → arch:checkout — FIXED. Board item #994 now has arch:checkout label.
  • [ ] [BODY] Migration number 032 — NOT FIXED. Issue still says alembic/versions/032_add_tournament_tables.py. Local repo shows 030 as latest numbered migration but previous review confirmed remote main has migrations up to 044. The next available slot is at least 045. The issue body must instruct the agent to use the next available slot from remote main HEAD, not hardcode a number.
  • [ ] [BODY] Missing error-path AC — NOT FIXED. Acceptance criteria section is unchanged — still 6 items, all happy-path. No criteria for: invalid tournament ID (404), player not on team (400), duplicate checkout link generation (idempotent or existing session).

Template Completeness

  • [x] Type
  • [x] Lineage
  • [x] Repo
  • [x] User Story
  • [x] Context
  • [x] File Targets
  • [x] Acceptance Criteria (present but incomplete — missing error paths)
  • [x] Test Expectations
  • [x] Constraints
  • [x] Checklist
  • [x] Related

Traceability

  • [x] story:WS-S33 label — "As an admin, I want to bill players for tournament entry fees at team-specific rates via Stripe so that event costs are collected before registration deadlines"
  • [x] story note verified — found in project-westside-basketball user-stories section (Admin stories list)
  • [x] arch:checkout label — FIXED from previous review (was arch:email)
  • [ ] arch note MISSING — no arch note exists for arch-checkout. [SCOPE] Create architecture note arch-checkout for the checkout/commerce component.
  • [x] Forgejo issue — #457, open

File Targets

  • [x] src/basketball_api/models.py — verified: exists. ProductCategory.tournament at L126, ProductType.one_time at L117-120. No Tournament model yet (correct).
  • [ ] alembic/versions/032_add_tournament_tables.py — ISSUE: Migration 032 is wrong. Local shows up to 030, remote main has up to 044. Next slot is at least 045. [BODY] Replace hardcoded "032" with instruction to use next available migration slot from remote main HEAD.
  • [x] src/basketball_api/routes/admin.py — verified: exists. Handles registration tokens, coach invitations, admin dashboard.
  • [x] src/basketball_api/routes/checkout.py — verified: POST /create-session at L103, Stripe session creation at L221.
  • [x] NOT-touch files correctly identified.

Repo Placement

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

Dependencies

  • Sibling: board item #993 (issue #456, generic blast system, story:WS-S32) — both in backlog. Independent — this ticket creates checkout URLs, the blast system consumes them.
  • No blocking dependencies in in_progress or next_up columns.
  • Dependencies documented in Related section.

Acceptance Criteria

6 criteria, all happy-path and testable. Still missing error-path criteria as flagged in v1 review:
  • Invalid tournament ID returns 404
  • Player not associated with team's product returns 400
  • Duplicate link generation is idempotent or returns existing session

Blast Radius

Low. New models and endpoints. Helper extraction from checkout.py is riskiest change. Test expectations cover via integration test.

Decomposition Assessment

4 file targets in 1 repo, 6 AC (would be 9 with error paths). Fits single agent pass — all changes logically cohesive. No decomposition needed.

Recommendation

  • [BODY] Fix migration number: replace 032_add_tournament_tables.py with instruction to use next available migration slot from remote main HEAD (currently at least 045).
  • [BODY] Add error-path acceptance criteria: invalid tournament ID → 404, player not on team → 400, duplicate link generation → idempotent/existing session.
  • [SCOPE] Create architecture note arch-checkout for the checkout/commerce component (non-blocking but needed for traceability).