Review: Admin player list page — scrollable roster with payment status badges (re-review)

Verdict: APPROVED

Prior Review Resolution

• [x] [LABEL] story:WS-S7 → story:WS-S9 — FIXED. Board item now carries story:WS-S9.
• [x] [BODY] Route clarification (expand existing resources :players, only: []) — FIXED. Added to Constraints section.
• [x] [BODY] Outstanding balance calculation method — FIXED. Moved to companion ticket #58 (player detail page). No longer in scope for this ticket.
• [x] [DECOMPOSE] 11 AC across 2 pages → split into #55 (list) + #58 (detail) — FIXED. Issue #58 exists and is open.
• [ ] [SCOPE] Create architecture note arch-rails-views — STILL MISSING. No arch-rails-views note found in pal-e-docs. This is a documentation gap, not an issue body defect. Does not block implementation.

Template Completeness

• [x] Type — Feature
• [x] Lineage — references #54 (Keycloak auth dependency) and story WS-S9
• [x] Repo — ldraney/westside-basketball
• [x] User Story — Marcus (admin) wants scrollable player list with payment status at a glance
• [x] Context — explains current state (disjointed payments + roster pages), scope (list only, detail is #58)
• [x] File Targets — create (controller, view), modify (routes), don't-touch (payments, roster_report)
• [x] Feature Flag — none (appropriate for internal admin page)
• [x] Acceptance Criteria — 7 items
• [x] Test Expectations — 2 tests with run command
• [x] Constraints — includes route expansion note, mobile-first, no new tables, no Stripe API
• [x] Checklist — standard 3-item
• [x] Related — references #54 (auth), #58 (detail page)

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 — WS-S9 found in project-westside-basketball user-stories section under Admin (Marcus)
• [ ] arch:rails-views label — no arch-rails-views note found in pal-e-docs. [SCOPE] Create architecture note arch-rails-views. Non-blocking.
• [x] Forgejo issue — ldraney/westside-basketball#55, state: open

File Targets

• [x] app/controllers/admin/players_controller.rb — to create. Directory app/controllers/admin/ exists with payments_controller.rb and roster_report_controller.rb as reference patterns. Verified.
• [x] app/views/admin/players/index.html.erb — to create. Parent directory app/views/admin/ exists with payments/ and roster_report/ subdirectories. Verified.
• [x] config/routes.rb — to modify. Existing resources :players, only: [] at line 15 in admin namespace. Constraints section correctly notes "expand existing — do not add duplicate resource block." Verified.
• [x] app/controllers/admin/payments_controller.rb — don't touch. Exists. Verified.
• [x] app/controllers/admin/roster_report_controller.rb — don't touch. Exists. Verified.

Data Model Verification

• [x] players table — exists with subscription_status enum and jersey_order_status enum
• [x] parent_id FK — exists on players table
• [x] Player model associations: belongs_to :parent, has_many :teams through :player_teams, has_many :orders, has_many :payment_links
• [x] No migrations needed — all columns and tables exist

Repo Placement

Dependencies

• depends:54 (board item #1605) — "Add Keycloak auth with admin/parent/player roles" — currently in backlog. Correctly documented in issue body and AC ("Admin role required, gated by #54 auth"). Existing admin controllers have no auth, so this ticket can be built without it and auth layered on after #54 lands. Test expectation for auth is correctly deferred.
• Board item #1609 (issue #58, player detail page) — depends:55. This ticket is the upstream dependency. AC #5 correctly notes clicking player name navigates to detail page "built in #58." No circular dependency.

Acceptance Criteria

• AC 1-4: List display, grouping/filtering, row content, payment status badges — testable via controller test and view assertions
• AC 5: Click-through to detail page — testable via link_to path assertion (detail page built in #58, link just needs correct path)
• AC 6: Mobile-responsive — relies on CSS patterns; acceptable for feature ticket with "landscaping-assistant Today view" style reference in Constraints
• AC 7: Admin role gating — deferred to #54; noted as gated dependency. Correct.

Blast Radius

• Existing /admin/payments and /admin/roster_report pages query similar data. Issue correctly notes these are untouched ("keep existing").
• Route expansion: existing resources :players, only: [] has a member { patch :monthly_fee } nested route. Expanding to only: [:index] will not conflict — :index is a collection route, :monthly_fee is a member route. Safe.
• Top-level resources :players, only: [:index] exists at line 22. No conflict — admin namespace is separate (/admin/players vs /players). Agent should be aware but no action needed.

Decomposition Assessment

Recommendation

• [SCOPE] Create architecture note arch-rails-views for the Rails views component. This is a documentation gap that persists from review-1, but does not block implementation. Can be created during or after the sprint.