pal-e-docs

Review: admin UI to mark players as public for /teams page

review-455-2026-03-26 Review

review needs-refinement
Project

Verdict: NEEDS_REFINEMENT

Template Completeness

  • [x] Type -- Feature
  • [x] Lineage -- standalone, discovered after basketball-api migration fix
  • [x] Repo -- forgejo_admin/westside-app
  • [x] User Story -- well-formed (admin Marcus toggling player visibility)
  • [x] Context -- clear motivation, migration 024 context
  • [x] File Targets -- present (but see issues below)
  • [x] Acceptance Criteria -- 4 criteria, testable
  • [x] Test Expectations -- manual tests only (no automated)
  • [x] Constraints -- playground-first noted
  • [x] Checklist -- standard
  • [x] Related -- present (but wrong issue reference, see below)

Traceability

  • [x] story:WS-S6 label -- present on board item #455
  • [x] arch:admin-dashboard label -- present on board item #455
  • [x] Forgejo issue -- forgejo_admin/westside-app#111, open

File Targets

  • [x] src/routes/(app)/admin/players/+page.svelte -- verified: exists in westside-app (146 lines). Currently has no is_public support. Will need modification.
  • [x] basketball-api/src/basketball_api/routes/admin.py -- verified: exists in basketball-api (26k). Currently has zero PATCH/PUT endpoints. No is_public references. Will need a NEW endpoint pattern (PATCH for player field updates).
  • [ ] ISSUE: admin.py path uses basketball-api/ prefix -- this file lives in a DIFFERENT repo (forgejo_admin/basketball-api), not in westside-app where this issue is filed.

Repo Placement

ISSUE: Cross-repo scope. The ticket is filed on forgejo_admin/westside-app but requires changes in TWO repos:
  • forgejo_admin/westside-app -- frontend toggle UI on admin/players page
  • forgejo_admin/basketball-api -- new PATCH endpoint for is_public field (admin.py currently has no PATCH endpoints at all)
The basketball-api work needs its own Forgejo issue on forgejo_admin/basketball-api. This issue should be scoped to westside-app frontend only, with a depends: reference to the basketball-api issue.

Dependencies

  • Upstream (completed): Migration 024 added is_public column (deployed, column exists in prod).
  • Upstream (completed): /public/teams endpoint already filters by is_public=True (verified in public.py).
  • Missing dependency: basketball-api PATCH endpoint for is_public must exist BEFORE the frontend can call it. No Forgejo issue exists for this yet.
  • Board item #430 (Public coaches endpoint, column: todo) -- related WS-S26 work but not a direct dependency.
  • scope:playground-first label -- the Constraints section says playground prototype first. This means a westside-app-playground prototype is needed before touching the app.

Acceptance Criteria

Four criteria are testable but all are manual. No automated test expectations are defined. The test command is a curl to prod -- acceptable for a UI feature, but the basketball-api PATCH endpoint should have a unit test expectation (e.g., pytest tests/test_admin.py -k test_toggle_is_public).
Missing criterion: error handling -- what happens when toggle fails? What does the UI show?

Blast Radius

Low blast radius. The is_public field is isolated to the Player model. The /public/teams endpoint is already correct and tested (test_public.py exists). No sibling services consume is_public. The admin.py currently has no PATCH pattern, so this introduces a new HTTP method pattern to the admin API -- not a bug risk, but worth noting for consistency.

Recommendation

Three issues must be fixed before this ticket is READY:
  • Split cross-repo scope. Create a separate Forgejo issue on forgejo_admin/basketball-api for the PATCH /admin/players/{id}/public endpoint. Update this issue (#111) to reference it as a dependency and remove the basketball-api file target.
  • Fix Related reference. basketball-api#183 is cited as "migration that added is_public field" but #183 is actually "Bug: CrashLoopBackOff -- Alembic migration chain forked, DB in partial state" (closed). The actual migration is revision 024 (024_add_is_public_to_players.py). Find and reference the correct issue number, or reference the migration file directly.
  • Add test expectations for the API endpoint. The basketball-api issue should include automated test expectations (pytest command for the new PATCH endpoint).