pal-e-docs

Review: Role-based routing: admin vs player post-login redirect

review-1646-2026-06-27 Review

review approved
Project

Verdict: APPROVED

Re-review (round 2) after round 1 refinements. All sections complete, all file targets verified, traceability intact. Scope is solid for a single agent pass.

Template Completeness

  • [x] Type — Feature
  • [x] Lineage — Gap discovered during Sprint 6 decomposition
  • [x] Repo — ldraney/westside-basketball
  • [x] User Story — WS-S38
  • [x] Context — explains current single-path redirect, need for role-based routing
  • [x] File Targets — 4 files listed
  • [x] Feature Flag — none (appropriate — internal routing, not user-visible toggle)
  • [x] Acceptance Criteria — 5 criteria
  • [x] Test Expectations — 4 integration tests + run command
  • [x] Constraints — 3 constraints listed
  • [x] Checklist — present
  • [x] Related — lists both dependencies and project page

Traceability

  • [x] story:WS-S38 label — "As a parent, I want to create an account, log in through the iOS app, and be routed to my subscription status page"
  • [x] story note verified — found in project-westside-basketball user-stories section (Parent subsection)
  • [x] arch:app-store label — App Store Billing architecture
  • [x] arch note verified — arch-app-store note exists in pal-e-docs (note ID 2224)
  • [x] Forgejo issue — westside-basketball#87, state: open

File Targets

  • [x] app/controllers/application_controller.rb — verified: exists, includes Authentication concern, has TENANT_SLUG constant
  • [x] app/controllers/sessions_controller.rb — verified: exists, contains OIDC callback (lines 11-33), already extracts realm roles (lines 17-18), currently redirects all users to root_path (line 32). This is the exact line that needs role-based branching.
  • [x] config/routes.rb — verified: exists, no /status route yet. Admin namespace exists (lines 19-28). Player route needs to be added.
  • [x] app/controllers/admin/base_controller.rb — verified: exists, already has require_role :admin (line 5). Admin guard is already in place — no changes needed here.

Repo Placement

OK. Issue filed on westside-basketball, all file targets are in westside-basketball. Single-repo change.

Dependencies

  • depends:154 — pal-e-services#154 (Keycloak player self-registration). Board item 1645, backlog. Must be done first so player accounts exist in Keycloak.
  • depends:83 — westside-basketball#83 (Player-facing subscription status page). Board item 1639, backlog. Must be done first so /status has a page to render. Note: #83 itself depends on #79 (Apple subscription model + migration) and is a child of #66.
  • Both dependencies are correctly documented in the issue body Related section and in the board item labels.

Acceptance Criteria

5 ACs, all testable via integration tests. Each maps to a specific behavior that can be verified with a test login flow. The test expectations section lists matching integration tests for each AC. Run command (bundle exec rails test) is valid. No missing criteria — the ticket covers both the redirect logic and the authorization guards.

Blast Radius

Low. The Authentication concern already has require_role and current_user_has_role? methods. Admin::BaseController already uses require_role :admin. The primary change is adding a role-based branch in SessionsController#create (line 32) and adding a /status route + controller. No existing auth behavior needs to change — admin flow stays identical.

Decomposition Assessment

4 file targets in 1 repo. 5 acceptance criteria. Estimated agent work well under 5 minutes — the auth infrastructure (role extraction, require_role) already exists. No decomposition needed.

Recommendation

No action needed. Scope is complete and verified.