Review: Track venue court costs in DB

review-765-2026-04-03-r2 Doc

review ready

Verdict: APPROVED

Re-review after refinement. Both previous issues resolved. Scope is solid, file targets verified, fits in a single agent pass.

Template Completeness

  • [x] Type — Feature
  • [x] Lineage — Standalone, discovered during schedule review
  • [x] Repo — forgejo_admin/basketball-api
  • [x] User Story — clear admin wants/cost tracking/why
  • [x] Context — venue rates, current schema gap, Option 2 rationale
  • [x] Environment — prod / basketball-api namespace
  • [x] File Targets — 4 files to modify/create, 1 exclusion
  • [x] Acceptance Criteria — 5 testable conditions
  • [x] Test Expectations — migration up/down, CRUD, cost summary, pytest
  • [x] Constraints — backward compat, nullable FK, NULL for TBD rates
  • [x] Checklist — PR, tests, no unrelated changes
  • [x] Related — project and parent issue linked

Traceability

  • [x] story:WS-S9 — "As an admin, I want to track payment status per player so that I know who owes what" (venue costs = expense dimension of financial tracking). Verified in project-westside-basketball user-stories section under Admin (Marcus).
  • [x] arch:basketball-api — targets basketball-api repo
  • [ ] arch note MISSING — no arch-basketball-api note in pal-e-docs. [SCOPE] Acknowledged as non-blocking backlog item from prior review. Does not block dispatch.
  • [x] Forgejo issue — forgejo_admin/basketball-api#314, open

File Targets

  • [x] src/basketball_api/models.py — verified: PracticeSchedule at L505, location column at L520. Venue model will be added here.
  • [x] alembic/versions/031_add_venues_table.py — to be created. 030 confirmed as 030_add_registration_type_to_registrations.py. 031 is correct next number. (FIXED from prior review)
  • [x] src/basketball_api/routes/schedule.py — verified: exists, imports PracticeSchedule, handles schedule CRUD
  • [x] src/basketball_api/routes/admin.py — verified: exists, admin routes module. Cost summary endpoint goes here.
  • [x] src/basketball_api/routes/public.py — verified: exists, correctly listed as NOT to touch

Repo Placement

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

Dependencies

  • [x] #629 (done) — Schedule data model + migration + seed — satisfied
  • [x] #630 (done) — Schedule API endpoints CRUD — satisfied
  • [x] #739 (done) — Seed practice_schedules table — satisfied
  • [x] #722 (backlog) — West High Field House court rental — related ops ticket, not blocking
No unresolved blockers.

Acceptance Criteria

5 ACs, all verifiable programmatically by an agent:
  • [x] Venues table schema — testable via migration + model inspection
  • [x] venue_id FK on practice_schedules — testable via migration + column check
  • [x] Seed 5 venues — testable via DB query after seed
  • [x] Admin weekly cost endpoint — testable via HTTP call + JSON validation
  • [x] Existing rows migrated — testable via DB query post-migration
No ambiguous "works correctly" language. All specific and measurable.

Blast Radius

Low. "venue" only appears in subscriptions.py (Stripe context, unrelated). No downstream consumers affected. Backward compatibility enforced: venue_id nullable, location string preserved until full migration. Rollback is straightforward — migration has down step. Single namespace (basketball-api).

Decomposition Assessment

4 file targets in 1 repo. 5 ACs. All changes are tightly coupled (model + migration + routes + seed form one cohesive unit). Estimated agent time: 3-4 minutes. No independent subtasks worth parallelizing — migration must exist before routes can reference the model.
Verdict: No decomposition needed. Fits within three-thing limit (model, migration, routes) and five-minute rule.

Recommendation

No action needed. Ready for dispatch.