pal-e-docs

Review: Move fee editing to player detail page, delete unused admin endpoints

review-1666-2026-06-29 Review

review needs-refinement
Project

Verdict: NEEDS_REFINEMENT

Template Completeness

  • [x] Type — Feature
  • [x] Lineage — Standalone, discovered during PR #122 review
  • [x] Repo — ldraney/westside-basketball
  • [x] User Story — As an admin (Marcus), edit fee from player detail page
  • [x] Context — Three redundant report endpoints, only unique action is update_fee
  • [x] File Targets — 3 to modify, 7 to delete
  • [x] Feature Flag — None (appropriate for relocating existing functionality)
  • [x] Acceptance Criteria — 5 criteria
  • [x] Test Expectations — Unit + integration tests, run command provided
  • [x] Constraints — Clear guidance on moving action as-is, JSON response, minimal UI
  • [x] Checklist — Standard PR/tests/no-unrelated
  • [x] Related — project-westside-basketball, PR #122, PR #21

Traceability

  • [ ] story:admin-dashboard label — NOT a recognized story code. No story:admin-dashboard entry exists in project-westside-basketball user-stories section. Closest match: story:WS-S35 ("As an admin, I want to set and manage subscription pricing on a player's detail page so that I control what each family pays for their program tier"). [LABEL] Change to story:WS-S35.
  • [ ] arch note MISSING — arch-rails-app not found in pal-e-docs. [SCOPE] Create architecture note arch-rails-app for the Rails application component.
  • [x] Forgejo issue — #123, open, body matches ticket title

File Targets

  • [x] app/controllers/admin/players_controller.rb — verified: exists, has index and show actions, no existing update_fee
  • [x] app/views/admin/players/show.html.erb — verified: exists, already displays monthly_fee in a stat card (line 21)
  • [x] config/routes.rb — verified: exists, report routes on lines 20-22, monthly_fee member route on line 26 currently dispatches to roster_report#update_fee
  • [x] app/controllers/admin/payments_controller.rb — verified: exists (to delete)
  • [x] app/controllers/admin/roster_report_controller.rb — verified: exists, 137 lines, update_fee at line 35 (~102 lines, issue says ~80)
  • [x] app/controllers/admin/delinquency_report_controller.rb — verified: exists (to delete)
  • [x] app/views/admin/payments/index.html.erb — verified: exists (to delete)
  • [x] app/views/admin/roster_report/index.html.erb — verified: exists (to delete)
  • [x] app/views/admin/roster_report/_inline_fee_editor.html.erb — verified: exists (to delete)
  • [x] app/views/admin/delinquency_report/index.html.erb — verified: exists (to delete)
  • [ ] app/javascript/controllers/inline_fee_controller.js — NOT mentioned in File Targets. This Stimulus controller handles the PATCH request for fee editing. URL path (/admin/players/:id/monthly_fee) stays the same so no code change needed, but agent should know it exists to avoid breaking data attribute contracts. [BODY] Add to "Files the agent should NOT touch" section.

Repo Placement

OK — issue filed on ldraney/westside-basketball, matches ### Repo section. All affected files are in this single repo.

Dependencies

  • PR #122 (admin dashboard shell with bottom tab navigation) — mentioned in Related section as "merge first" but NOT tracked with a depends: label on the board item. PR #122 is currently open. [LABEL] Add depends:121 label to board item.
  • No other blocking dependencies found on the board. No active items in in_progress column overlap with this work.

Acceptance Criteria

5 criteria, all agent-verifiable:
  • AC1: Tap "Edit" next to monthly fee — testable via integration test
  • AC2: Fee change updates Stripe or regenerates payment link — testable via unit test with Stripe mock
  • AC3: Inline success/error feedback — testable via integration test
  • AC4: Report routes return 404 — testable via route assertion
  • AC5: No dead files remain — testable via file existence check
Test expectations include unit and integration tests with run command bin/rails test. Complete and testable.

Blast Radius

  • inline_fee_controller.js constructs URL from data attributes (this.urlValue) — route path /admin/players/:id/monthly_fee is unchanged, only controller dispatch changes. Safe.
  • Admin layout (admin.html.erb) has NO nav links to report pages — no cleanup needed there.
  • No path helper references (payments_path, roster_report_path, delinquency_report_path) found anywhere outside the files being deleted. Clean removal.
  • No downstream consumers or sibling services affected.

Decomposition Assessment

3 files to modify + 7 files to delete = 10 files, all in 1 repo. 5 acceptance criteria (at limit, not over). Work is straightforward: move one action, add minimal UI, update one route line, delete 7 files. Estimated well under 5 minutes. No decomposition needed.

Recommendation

  • [LABEL] Change story:admin-dashboard to story:WS-S35 — the user story maps to "set and manage subscription pricing on a player's detail page"
  • [SCOPE] Create architecture note arch-rails-app for the westside-basketball Rails application component
  • [LABEL] Add depends:121 label to board item to document the dependency on issue #121 / PR #122 (admin dashboard shell)
  • [BODY] Add app/javascript/controllers/inline_fee_controller.js to "Files the agent should NOT touch" section with note: "Stimulus controller constructs URL from data attributes; route path unchanged, do not modify"