Review: C3 — app: Tournament registration section (blocked by C2)

review-1073-2026-04-22 Doc

review approved

Project

Verdict: APPROVED

Note: Both READY and APPROVED are accepted as passing verdicts by the check-board-advance hook. Item passed two prior informal review rounds; the second was APPROVED after rewriting the schema premise from registrations to orders → tournament_products → tournaments. This formal review confirms that revised scope.

Template Completeness

[x] Type — Feature
[x] Lineage — Standalone, schema premise correction noted
[x] Repo — forgejo_admin/westside-streamlit
[x] User Story — Marcus operator role, want, so-that all present
[x] Context — full join chain documented + provenance (review-994/998/1013, models.py)
[x] File Targets — single file (app.py) plus explicit do-not-touch list
[x] Acceptance Criteria — 8 criteria including dropdown query, status semantics, filters, totals, empty-state
[x] Test Expectations — ruff check + format with run commands
[x] Constraints — explicit "DO NOT use registrations", read-before-write list, tenant-scoping
[x] Checklist — present
[x] Related — links story note, parent project, blocking dependencies

Traceability

[x] story:tournament label — matches new story note
[x] story note verified — story-westside-streamlit-tournament (id 1594) exists, status active, has Data Shape section confirming orders → tournament_products → tournaments join chain
[ ] story note NOT YET in project page user-stories table — [SCOPE] Add a tournament row to the User Stories table on project-westside-streamlit (current rows: triage, blast, clusters, jerseys, audit, reachable). Sidecar hygiene; does not block this ticket.
[x] arch:streamlit-app label — present
[ ] arch note MISSING — [SCOPE] Create architecture note arch-streamlit-app. Pre-existing gap shared by every ticket on this board (jerseys/blast/triage all use the same label without a backing note); not introduced by #1073. Recommend a single hygiene ticket on board-pal-e-docs rather than gating C3.
[x] Forgejo issue — forgejo_admin/westside-streamlit#15, state open, body matches APPROVED-round-2 content

File Targets

[x] ~/westside-streamlit/app.py — verified: file exists, 211 lines total, three section headers found at lines 51, 114, 147 (Offered, Declined, Jerseys). Jerseys section spans 147-211 exactly as ticket claims, ending with st.dataframe(jview, ...). Pattern is the right reference for a new dropdown + filter + dataframe + cohort-textarea section.
[x] ~/basketball-api/src/basketball_api/models.py — verified Order (line 437), Tournament (595), TournamentProduct (613), Product (419), Player (225), Parent (204) all exist. Order has player_id, parent_id, product_id, status (OrderStatus enum), amount_cents, tenant_id — exactly what the ticket queries assume. Tournament has id, name, event_date (Date, nullable), tenant_id — the dropdown query SELECT id, name, event_date FROM tournaments WHERE tenant_id = 1 ORDER BY event_date DESC NULLS LAST is valid against this schema.
[x] ~/basketball-api/alembic/versions/044_add_westside_streamlit_ro_role.py — verified orders is in GRANT_TABLES tuple. C3's queries against orders will work as soon as the role is in the connection string.
[x] Migration 045 (045_add_tournament_tables.py) — verified present; creates tournaments and tournament_products.
[x] Migration 048 — does NOT exist yet; this is correct. C2 (basketball-api#510) is the open ticket that will create it. Latest migration in tree is 047.
[x] Files NOT to touch list — accurate: requirements.txt and Dockerfile exist but no new deps are needed (st, q() helper already imported); registrations table correctly excluded.

Repo Placement

Correct. App-layer change (Streamlit dashboard) lives in westside-streamlit; the corresponding GRANT migration is correctly placed in basketball-api as a separate ticket (C2 / #510). Per feedback_basketball_hands_off, this ticket does not touch basketball-api code — it only consumes via the existing q() helper.

Dependencies

Hard blockers:

C2 — basketball-api#510 (board item #1072, backlog): must merge AND apply migration 048 to prod before C3 PR merges. Without 048, the ro role cannot SELECT from tournaments or tournament_products; C3 will fail at runtime with permission denied. Issue body is explicit about this expected failure mode being correct.
C1 — westside-streamlit#14 (board item #1071, backlog): C3 says it appends "after the Monthly section (C1 appends Monthly first; this goes after it)". If C1 hasn't landed when C3 enters in_progress, the dev agent will need to choose: (a) wait for C1, or (b) place the Tournament section in the same relative position regardless of Monthly's presence and rebase. Recommend serializing C1 → C3, not parallel, to avoid line-number drift on the same file. This matches feedback_migration_slot_coordination applied at the file level.

Soft blockers (substrate, already noted in ticket Related):

S1 — pal-e-services#60 (Harbor + ArgoCD Application for westside-streamlit, board item #1069, backlog)
S2 — pal-e-services#61 (Keycloak client, board item #1070, backlog)

Both are marked "substrate live" prerequisites. Reachability tickets (#7, #11, #12) are in next_up/qa; once they clear, the substrate is ready.

Acceptance Criteria

All 8 ACs are agent-verifiable:

Dropdown query is given verbatim (executable check)
Query shape spelled out with named tables and exact join path
Status semantics defined unambiguously (paid = at least one paid order; unpaid = orders exist but none paid; "not signed up" explicitly out of scope for v1)
Filter set enumerated (Program: All/Kings/Queens; Payment Status: All/Paid/Unpaid)
Totals caption format given verbatim
Empty-state path defined ("No tournaments configured yet")
Read-only contract reaffirmed

Test commands (ruff check app.py, ruff format --check app.py) are real and runnable. Empty-state validation against a dev db is the correct local gate.

Blast Radius

Low. Single-file additive change in a Streamlit dashboard with no other consumers. app.py is 211 lines; new section appends after line 211 (post-Monthly per C1). No shared helpers modified — q() already exists and is cached. No basketball-api changes. No schema changes. The orders SELECT pattern is identical to the existing Jerseys section (147-211), so the same query semantics that have been validated in prod apply here. Risk note: if migration 048 GRANTs are not live when C3 deploys, the section throws permission denied on first render — caught immediately, no data corruption, recoverable by applying 048. Ticket explicitly calls this out as the correct failure mode.

Decomposition Assessment

1 file, 1 repo, 8 ACs, mirroring an existing 64-line section. Estimated agent work: 5–10 minutes (read Jerseys section → write SQL → wire filters → cohort textarea → ruff). No decomposition needed. Sits inside the 5-minute rule envelope when the agent has the join chain handed to them, which the ticket does.

Recommendation

[SCOPE] Add a tournament row to the User Stories table on project-westside-streamlit. Story note exists (id 1594) but is not linked from the project page yet. Hygiene; does not block this ticket.
[SCOPE] Create architecture note arch-streamlit-app. This is a board-wide gap (every ticket on board-westside-streamlit uses this label without a backing note). Recommend filing a single hygiene ticket on board-pal-e-docs rather than gating C3.
[BODY] (optional, non-blocking) Consider adding an explicit ordering rule to AC for the tournament dropdown when event_date is NULL across multiple rows — current ORDER BY handles it (NULLS LAST), but a tie-breaker on id DESC would make agent output deterministic. Skip if Marcus's data has only one current upcoming tournament.
Sequencing note for the dispatcher: Move C1 (#1071) → todo and dispatch first; hold C3 (#1073) until C1 PR merges to avoid line-number drift on app.py. C2 (#1072) can run in parallel with C1 — different repo, different file. C3 enters todo only after C1 merges AND C2 migration 048 is applied in prod.

Scope is solid. Ready to advance backlog → todo.