pal-e-docs

Review: Player invoice builder: mark paid, custom charges, generate tokenized invoice link

review-1678-2026-07-02 Review

review needs-refinement
Project

Verdict: NEEDS_REFINEMENT

Template Completeness

  • [x] Type — Feature
  • [x] Lineage — Standalone, discovered during data audit discussion
  • [x] Repo — ldraney/westside-basketball
  • [x] User Story — As an admin (Marcus), mark payments, add charges, generate invoice link
  • [x] Context — Detailed table of current state, Stripe flow, critical gap, approach
  • [x] File Targets — 7 files to modify/create, 4 exclusions
  • [x] Feature Flag — none (admin-only + signed token)
  • [x] Acceptance Criteria — 9 criteria
  • [x] Test Expectations — 5 test items + run command
  • [x] Constraints — 6 constraints
  • [x] Checklist — 3 items
  • [x] Related — 4 references

Traceability

  • [x] story:WS-S9 — "As an admin, I want to track payment status per player so that I know who owes what" — verified in project-westside-basketball user-stories section
  • [x] 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" — verified in project-westside-basketball user-stories section
  • [ ] arch:rails-app — arch note MISSING — [SCOPE] Create architecture note arch-rails-app for the Rails application component
  • [x] Forgejo issue — ldraney/westside-basketball#144, state: open

File Targets

  • [x] app/controllers/admin/players_controller.rb — verified: exists, has show action with payment_links/orders/player_tournaments queries
  • [x] app/views/admin/players/show.html.erb — verified: exists, 287 lines, shows outstanding balances and payment history
  • [x] config/routes.rb — verified: exists, admin namespace with players resources
  • [x] app/controllers/invoices_controller.rb — does not exist yet (to be created), no conflict
  • [x] app/views/invoices/show.html.erb — does not exist yet (to be created), no conflict
  • [x] app/javascript/controllers/clipboard_controller.js — does not exist yet (to be created), no conflict. Existing Stimulus controllers (inline_fee, program) confirm pattern.
  • [x] db/migrate/YYYYMMDD_add_paid_method_columns.rb — migration, will be generated
  • [ ] ISSUE: Migration scope underestimated — see Recommendation below

Repo Placement

OK — issue filed on ldraney/westside-basketball, all file targets are in this repo. No multi-repo concerns.

Dependencies

No blocking dependencies found on the board. Issues #131/#132/#133/#134 (Stripe DB reconciliation) are closed predecessors. Issue #123 (fee editing to player detail page) is open but addresses a different concern. Phase 14 (Billing Tiers) is in_progress but doesn't block this ticket.

Acceptance Criteria

9 criteria, all in "When X, then Y" format, each testable by an agent. Well-written and specific. However, 9 AC exceeds the decomposition threshold of 5.

Blast Radius

  • webhooks_controller.rb — Currently marks payment_links as paid at line 75: payment_link.update!(paid_at: Time.current, status: "paid"). After adding paid_method, webhook-originated payments will have NULL paid_method unless the webhook is updated to set paid_method: 'stripe'. The ticket says NOT to touch the webhook, but doesn't address this gap (no DB default, no backfill strategy).
  • MessageVerifier — No existing usage in the codebase. New pattern introduction, low blast radius.
  • Schema constraint conflict — payment_links.stripe_payment_link_id (NOT NULL, limit 200) and payment_links.stripe_payment_link_url (NOT NULL, limit 500) prevent storing DB-only custom charges. The ticket's Constraints section claims "no schema change needed for this" but the NOT NULL constraints must be relaxed.

Decomposition Assessment

NEEDS DECOMPOSITION — route to skill-decompose-ticket
  • 7 file targets in 1 repo (threshold: >3 across >2 repos — not met, but close)
  • 9 acceptance criteria (threshold: >5 — exceeded)
  • Estimated agent work: well over 5 minutes — migration with schema changes, 3+ new controller actions, 1 new controller, significant view modifications, 1 new public view, 1 new Stimulus controller, multiple test files
  • 4 independent concerns: (1) mark-as-paid with method tracking, (2) custom charges, (3) invoice builder UI + tokenized URL generation, (4) public invoice page

Recommendation

  • [BODY] Fix schema claim: payment_links.stripe_payment_link_id and stripe_payment_link_url are NOT NULL — migration must ALTER these columns to be nullable for DB-only custom charges. Update the Constraints section that says "no schema change needed for this" and expand the migration file target description.
  • [BODY] Address webhook blast radius: webhooks_controller.rb line 75 sets status: "paid" without paid_method. Either (a) add paid_method: 'stripe' to the webhook update (contradicts "don't touch" directive — update that list), or (b) add a DB default of 'stripe' for paid_method so webhook-originated payments are correctly tagged, or (c) explicitly document that webhook payments will have NULL paid_method and that's acceptable.
  • [SCOPE] Create architecture note arch-rails-app for the Rails application component in pal-e-docs.
  • [DECOMPOSE] 9 AC across 7 file targets with 4 independent concerns (mark-paid, custom charges, invoice builder, public invoice page). Exceeds 5-minute rule. Route to skill-decompose-ticket with suggested sub-tickets: