pal-e-docs

Review: Report: players behind on payment -- generate invoice list for Marcus (re-review)

review-1582-2026-06-23-r2 Review

review ready
Project

Verdict: APPROVED

Re-review of board item #1582 after refinement. Previous review (review-1582-2026-06-23) returned NEEDS_REFINEMENT with 4 [BODY] recommendations. All 4 have been addressed.

Template Completeness

  • [x] Type -- Feature
  • [x] Lineage -- Standalone, part of story:WS-S9
  • [x] Repo -- ldraney/westside-basketball
  • [x] User Story -- well-formed As/I want/So that
  • [x] Context -- excellent; now acknowledges existing /admin/payments dashboard, StripeClient#db_payment_summary, and frames this as a filtered delinquency report
  • [x] File Targets -- 4 targets, all verified (see below)
  • [x] Feature Flag -- none (appropriate for admin-only tool)
  • [x] Acceptance Criteria -- 4 criteria, all specific and testable
  • [x] Test Expectations -- present with run command
  • [x] Constraints -- present, time-sensitive noted
  • [x] Checklist -- present
  • [x] Related -- present
  • [x] Definition of "behind on payment" -- NEW SECTION, explicitly defines the filter condition with enum values

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 -- found in project-westside-basketball user-stories section under Admin (Marcus)
  • [x] arch:westside-basketball label -- references the main app repo
  • [ ] arch note MISSING -- [SCOPE] Create architecture note arch-westside-basketball for component westside-basketball. No matching note found in pal-e-docs. (Carried forward from previous review; not a blocker for this ticket.)
  • [x] Forgejo issue -- ldraney/westside-basketball#50, open

File Targets

  • [x] lib/tasks/payments.rake -- verified: lib/tasks/ directory exists (contains .keep). Valid creation target, and the issue correctly frames this as "create new".
  • [x] app/controllers/admin/payments_controller.rb -- verified: exists with index action and payment dashboard logic. Issue correctly lists as alternative endpoint location.
  • [x] app/services/stripe_client.rb -- verified: exists with db_payment_summary method at line 157. Method already queries payment_links grouped by team with total_owed/total_paid per player. Issue correctly says "reuse existing logic or extract a filtered variant".
  • [x] Database tables -- verified: payment_links table exists (schema.rb line 211) with amount_cents, paid_at, status (enum paymentlinkstatus: active/paid/canceled). players table exists (line 236) with name. parents table exists (line 185) with name and email. Issue correctly notes "there is no payments table".

Refinement Fixes Verified

All 4 recommendations from the previous review have been addressed:
  • [BODY] Fix table reference -- FIXED. Issue now says "payment_links, players, and parents tables" with explicit note that there is no payments table.
  • [BODY] Acknowledge existing infrastructure -- FIXED. Context section references /admin/payments, StripeClient#db_payment_summary, and positions this as a filtered delinquency report.
  • [BODY] Narrow AC3 output format -- FIXED. AC3 now specifies CSV format explicitly. AC4 added as exclusion criterion.
  • [BODY] Define "behind on payment" -- FIXED. New dedicated section defines: status = 'active' AND paid_at IS NULL. Enum values documented. Amount owed = sum of amount_cents on unpaid active links.

Repo Placement

OK -- issue filed on ldraney/westside-basketball, which is the correct repo. All file targets are within this repo. Single-repo scope.

Dependencies

  • No blocking dependencies. All related payment items (Phase 6 Stripe, commerce admin, webhook sync) are in the done column.
  • The existing StripeClient#db_payment_summary is a solid foundation. The new report can reuse or adapt its query pattern.
  • ActionMailer spike (#1579) is in backlog but is NOT a dependency -- the output format is now CSV (not email), so no ActionMailer needed.

Acceptance Criteria

  • [x] AC1 "Query identifies all players with outstanding payment_links (status = active with paid_at IS NULL)" -- testable, precisely defined, matches schema.
  • [x] AC2 "Output includes: player name, parent name, parent email, amount owed, last payment date" -- testable. parent.name column confirmed in schema. Existing db_payment_summary returns email but not parent name; minor addition.
  • [x] AC3 "Output is CSV format" -- testable, specific format chosen.
  • [x] AC4 "Players with no outstanding payment_links are excluded" -- testable, clear exclusion criterion.

Blast Radius

Low. Read-only report, no mutations. Reuses existing query patterns from StripeClient. No downstream consumers affected. One minor note: the existing db_payment_summary treats ALL active links as "owed" regardless of paid_at; the new definition (active AND paid_at IS NULL) is more precise, which is correct behavior for a delinquency report.

Decomposition Assessment

No decomposition needed. Single file creation (rake task) or minor controller addition in one repo, with 4 acceptance criteria. Estimated agent time: well under 5 minutes.

Recommendation

No action needed. All previous refinement issues resolved. Ticket is ready for implementation.
Note: the missing arch-westside-basketball note is a project-level gap, not a ticket blocker. It should be tracked separately.