Review: Track venue court costs in DB

review-765-2026-04-03 Doc

review needs-refinement

Project

Template Completeness

[x] Type -- Feature
[x] Lineage -- Standalone, discovered during schedule review
[x] Repo -- forgejo_admin/basketball-api
[x] User Story -- clear admin story about venue cost tracking
[x] Context -- thorough venue list with known rates, design options explained, Option 2 chosen
[x] File Targets -- 4 files to modify/create, 1 exclusion documented
[x] Acceptance Criteria -- 5 testable criteria
[x] Test Expectations -- migration up/down, CRUD, cost summary, pytest command
[x] Constraints -- backward compat, nullable FK, NULL vs 0 for TBD rates
[x] Checklist -- PR, tests, no unrelated changes
[x] Related -- project and parent issue referenced

Traceability

[x] story:WS-S9 label -- "As an admin, I want to track payment status per player so that I know who owes what"
[x] story note verified -- found in project-westside-basketball user-stories section (Admin stories list)
[x] arch:basketball-api label -- basketball-api component
[ ] arch note MISSING -- [SCOPE] Create architecture note arch-basketball-api for component basketball-api
[x] Forgejo issue -- forgejo_admin/basketball-api#314, open

File Targets

[x] src/basketball_api/models.py -- verified: PracticeSchedule model at line 505, location field at line 520, no Venue model yet. Parent directory exists.
[ ] alembic/versions/030_add_venues_table.py -- ISSUE: migration 030 already exists (030_add_registration_type_to_registrations.py). Should be 031_add_venues_table.py.
[x] src/basketball_api/routes/schedule.py -- verified: exists, CRUD for PracticeSchedule with location references at lines 146, 281, 405
[x] src/basketball_api/routes/admin.py -- verified: exists, currently imports Player/Team/Coach but not schedule models. Cost summary endpoint fits here.
[x] src/basketball_api/routes/public.py -- verified: exists, correctly excluded from scope

Repo Placement

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

Dependencies

[x] basketball-api#279 (practice schedule seeding) -- referenced in Related. No blocking dependency; venues table is additive.
[x] No board items in in_progress that conflict with schedule or model changes.

Acceptance Criteria

5 AC, all testable by an agent:

Venues table schema -- verifiable via model inspection and migration run
venue_id FK on practice_schedules (nullable, with num_courts) -- verifiable via model + migration
Seed 5 venues with correct rates -- verifiable via migration data or seed script
Admin endpoint returns weekly cost breakdown -- verifiable via pytest
Existing rows migrated to venue FKs -- verifiable via migration logic

Test expectations are specific: migration up/down, CRUD endpoints, cost summary totals. Run command pytest tests/ is valid.

Blast Radius

location field on PracticeSchedule referenced in schedule.py at 4 points (lines 146, 164, 281, 405). Constraint says keep location string until all rows migrated -- no breaking change.
Event model also has a location field (line 541) -- not in scope but a future candidate for venue FK.
No downstream consumers outside basketball-api affected. westside-app reads schedule data via API but location string remains intact.
Rollback straightforward: migration down drops venues table and FK column.

Decomposition Assessment

4 file targets in 1 repo. 5 acceptance criteria. Estimated agent time ~4 minutes (model + migration + 2 route file updates). All changes are tightly coupled (venue model drives everything else). No independent subtasks worth parallelizing. No decomposition needed.

Recommendation

[BODY] Fix migration filename: alembic/versions/030_add_venues_table.py to alembic/versions/031_add_venues_table.py (030 already exists as 030_add_registration_type_to_registrations.py)
[SCOPE] Create architecture note arch-basketball-api for component basketball-api in pal-e-docs