pal-e-docs

Review: Public invoice page (tokenized, no-login)

review-1682-2026-07-02 Review

review needs-refinement
Project

Verdict: NEEDS_REFINEMENT

Template Completeness

  • [x] Type -- Feature
  • [x] Lineage -- Child of #144, sub-ticket 4/4, depends on #147
  • [x] Repo -- ldraney/westside-basketball
  • [x] User Story -- "As a parent I want to open a link Marcus sent me..."
  • [x] Context -- Token approach, live queries, pay button logic explained
  • [x] File Targets -- 4 create/modify, 3 do-not-touch, all verified
  • [x] Feature Flag -- "none" with justification (inert without valid token)
  • [x] Acceptance Criteria -- 10 criteria, all specific and testable
  • [x] Test Expectations -- 5 integration tests + run command
  • [x] Constraints -- 5 constraints (auth bypass, layout, CSS, mobile-first, no JS)
  • [x] Checklist -- present
  • [x] Related -- 3 references (#144 parent, #147 dependency, public views)

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 (Admin stories list)
  • [x] arch:rails-app label -- present on board item
  • [ ] arch note MISSING -- [SCOPE] Create architecture note arch-rails-app for component rails-app. Search returned no results for "arch-rails-app" or "arch rails-app westside".
  • [x] Forgejo issue -- ldraney/westside-basketball#148, open

File Targets

  • [x] app/controllers/invoices_controller.rb -- verified: does not exist yet (new file, expected)
  • [x] app/views/invoices/show.html.erb -- verified: does not exist yet (new file, expected)
  • [x] app/views/layouts/invoice.html.erb -- verified: does not exist yet (new file, expected). Existing layouts: application, admin, mailer
  • [x] config/routes.rb -- verified: exists, currently has public routes at lines 41-48 for reference pattern
  • [x] Do-not-touch: app/controllers/admin/players_controller.rb -- exists
  • [x] Do-not-touch: app/controllers/webhooks_controller.rb -- exists, uses same ActionController::Base inheritance pattern
  • [x] Do-not-touch: app/controllers/application_controller.rb -- exists, includes Authentication module
  • [x] Reference: app/views/public/ -- exists with about.html.erb, gear.html.erb, etc. CSS patterns (semantic CSS with .hero, .section, .container, .card classes) confirmed

Repo Placement

OK. Issue filed on ldraney/westside-basketball, all file targets in the same repo. Single-repo feature, no cross-repo concerns.

Dependencies

  • parent:144 -- "Player invoice builder" (board item 1678, backlog) -- documented in Lineage
  • depends:147 -- "Invoice builder UI: select items and generate tokenized link" (board item 1681, backlog) -- documented. Introduces Rails.application.message_verifier(:invoice) pattern that #148 consumes. Token payload structure (player_id, scope, item IDs) defined in #147.
  • Transitive: #147 depends on #145 -- "Mark-as-paid + paid_method migration" (board item 1679, backlog). #148 queries paid state but doesn't modify it.
  • Sibling: #146 -- "Custom charges on player detail page" (board item 1680, backlog, depends:145). Not a direct dependency of #148 but custom charges would appear as unpaid items on the invoice.
  • Note: player_tournaments table referenced in #147 token payload does not exist in DB schema yet (model file exists at app/models/player_tournament.rb, but issue #111 is still open). The invoice page handles this gracefully -- it queries whatever items exist. Not a blocker.
  • All dependency items are in backlog. Chain is clear: #145 -> #147 -> #148.

Acceptance Criteria

10 criteria, all specific, measurable, and verifiable by an agent via integration tests. Criteria cover: no-login access, correct data display, conditional pay buttons, full vs selective scope, paid-in-full state, error handling for invalid/expired tokens, mobile responsiveness, and branding consistency. Test expectations align with AC (5 integration tests covering the key states).

Blast Radius

Low. All new code (controller, view, layout) is isolated from existing auth and admin systems. The InvoicesController inherits from ActionController::Base directly (matching the existing WebhooksController pattern), so no risk of auth bypass leaking into admin routes. New layout prevents admin nav exposure. Route GET /invoice/:token does not conflict with existing routes. No existing invoice patterns in the codebase to conflict with (only one reference to "invoice" in existing views, in the delinquency report meta tag). No downstream consumers affected.

Decomposition Assessment

  • 4 file targets in 1 repo -- under the >3 across >2 repos threshold
  • 10 acceptance criteria -- exceeds the >5 threshold
  • Estimated agent work: ~3-5 minutes -- borderline on >5 min threshold
Assessment: No decomposition needed despite exceeding the AC count threshold. The 10 criteria test different states of a single page (valid/invalid token, full/selective scope, paid/unpaid, mobile, branding), not separate features. The 3 new files form one logical unit (controller + view + layout) plus one trivial route addition. The 2-point estimate is appropriate. Decomposing would create artificial sub-tickets that cannot be delivered independently.

Recommendation

  • [SCOPE] Create architecture note arch-rails-app for component rails-app. Every ticket labeled arch:rails-app (including siblings #145, #146, #147, and parent #144) has this same gap. Creating the note once resolves the traceability gap for the entire invoice builder epic.
No other issues found. The scope document is well-written with clear context, accurate file targets, proper dependency documentation, and testable acceptance criteria. Once the arch note exists, this ticket is READY.