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

review-455-2026-03-27 Review

review needs-refinement

Verdict: NEEDS_REFINEMENT

Summary

Board item #455 tracks Forgejo issue #111, which has been superseded by a refined split ticket. The prior review (review-455-2026-03-26) correctly identified cross-repo scope and triggered a refinement. That refinement produced:
  • westside-app#122 — frontend-only is_public toggle (board item #467, currently in_progress)
  • basketball-api#189 — PATCH visibility endpoint (now closed/merged)
Board item #455 is now stale — the active work lives on #467. Issue #111 should be closed as superseded, and board item #455 should be removed or moved to done.

Template Completeness

  • [x] Type — Feature
  • [x] Lineage — Standalone
  • [x] Repo — forgejo_admin/westside-app
  • [x] User Story — present and clear
  • [x] Context — present and informative
  • [x] File Targets — present (but cross-repo scope was the original problem)
  • [x] Acceptance Criteria — 4 criteria present
  • [x] Test Expectations — present (manual only)
  • [x] Constraints — present
  • [x] Checklist — present
  • [ ] Related — ISSUE: References basketball-api#183 which is actually "Bug: CrashLoopBackOff" not the is_public migration. Already flagged in prior review.

Traceability

  • [x] story:WS-S6 label — admin dashboard user story
  • [x] arch:admin-dashboard label — architecture component
  • [x] Forgejo issue — westside-app#111, open (but should be closed as superseded)

File Targets

  • [x] src/routes/(app)/admin/players/+page.svelte — verified: file exists (147 lines), currently a CRM player list with search/filter, no is_public toggle yet
  • [ ] basketball-api/src/basketball_api/routes/admin.pySTALE: This was the cross-repo target. Split to basketball-api#189 which is now merged. Admin.py exists but local checkout may not have the merged PATCH route yet.

Repo Placement

Resolved by split. Original issue spanned westside-app and basketball-api. Refinement correctly split into #122 (westside-app frontend) and #189 (basketball-api backend). Issue #111 body still contains cross-repo file targets.

Dependencies

  • basketball-api#189 (PATCH /admin/players/{id}/visibility) — closed/merged. Dependency satisfied.
  • Board item #467 (westside-app#122) — in_progress, same arch:admin-dashboard + story:WS-S6. This is the refined replacement.
  • scope:playground-first — westside-app-playground repo exists on Forgejo but not cloned locally.

Acceptance Criteria

The 4 criteria on #111 are reasonable but moot — #122 has its own refined 4 criteria that are more specific (visual feedback, API call details). The #122 criteria are testable by an agent.

Blast Radius

  • /public/teams endpoint — confirmed working correctly with is_public filter. No changes needed.
  • is_public model field — confirmed in models.py line 215, server_default=false. Migration 024 is the source.
  • No other admin pages reference is_public. Impact contained to admin players page.

Decomposition

Already done. #111 was split into #122 (frontend) and #189 (backend). Each fits in a single agent pass.

Recommendation

  • Close issue #111 as superseded by #122. Add a closing comment.
  • Remove or archive board item #455 — active work tracked by #467 (#122).
  • No further scope action needed — #122 is well-scoped and its API dependency (#189) is merged.