Review: Copy-to-clipboard subscription links (UI)

review-1356-2026-06-06 Review

review ready

Verdict: READY

Template Completeness

  • [x] Type — Feature
  • [x] Lineage — Split from #23, depends on #24
  • [x] Repo — ldraney/westside-ror
  • [x] User Story — Marcus (admin) copy-to-clipboard workflow
  • [x] Context — Thorough: spike #22 findings, existing state, dependency chain
  • [x] File Targets — 3 files to modify, 2 exclusions documented
  • [x] Acceptance Criteria — 7 criteria, all testable
  • [x] Test Expectations — 2 unit tests, 1 integration test, run command
  • [x] Constraints — Product reuse, raw net/http pattern, cents conversion
  • [x] Checklist — Present
  • [x] Related — Links to project, prerequisite, spike, and arch doc

Traceability

  • [x] story:subscription-links label — present on board item
  • [ ] story note MISSING — no project-westside-ror project page exists in pal-e-docs; no user-stories section to verify. Acceptable: the user story is fully stated inline in the issue body. [SCOPE] Create project page project-westside-ror with user-stories section (low priority, not blocking).
  • [x] arch:rails label — present on board item
  • [ ] arch note MISSING — search for arch-rails returned no results. Acceptable: "rails" is the application framework, not a discrete architecture component. The actual architecture doc exists at docs/stripe-subscription-architecture.md in the repo and is referenced in the issue. [SCOPE] Consider creating arch-stripe note in pal-e-docs to back the scope:stripe label (low priority, not blocking).
  • [x] Forgejo issue — ldraney/westside-ror#26, open

File Targets

  • [x] app/services/stripe_client.rb — verified: exists (3.6k). Currently has only GET methods (paginate, get). Adding create_subscription_link requires a new private post method. Correct target.
  • [x] app/controllers/admin/roster_report_controller.rb — verified: exists (1.3k). Currently loads teams/players with includes(players: :parent). Will need to add :payment_links to the includes and instantiate StripeClient. Correct target.
  • [x] app/views/admin/roster_report/index.html.erb — verified: exists (4.0k). Currently renders player rows with name/parent/phone/email/monthly/status columns. "Copy Link" button + JS clipboard logic will be added here. Correct target.
  • [x] Exclusions verified — payments_controller.rb and webhooks_controller.rb correctly excluded (separate concerns).

Repo Placement

Correct. Issue filed on ldraney/westside-ror, all file targets are in the same repo. Single-repo change.

Dependencies

  • #24 (inline-editable monthly fee) — documented as prerequisite, confirmed still open. Payment link amount derives from player.monthly_fee. Correctly identified. Board item #1334 is in backlog.
  • #27 (Stripe webhook) — correctly split out as separate ticket. Board item #1357 is in backlog.
  • #23 (original combined ticket) — parent ticket, board item #1335 still in backlog. Should be closed/superseded once #26 and #27 replace it.
  • Spike #22 — closed, findings documented in issue context.

Acceptance Criteria

7 criteria, all verifiable by an agent:
  • [x] AC1: Copy Link button per row — testable via view rendering
  • [x] AC2: Clipboard copy without navigation — testable via JS behavior
  • [x] AC3: "Copied!" feedback for 2s — testable via JS
  • [x] AC4: Subscription mode with correct price — testable via unit test on StripeClient
  • [x] AC5: Metadata includes player_id, player_name, team — testable via unit test
  • [x] AC6: Pre-fill parent email — testable via unit test
  • [x] AC7: Links stored and reused — testable via integration test
Minor note: AC1 mentions "players with no parent email show 'No email' indicator" but per schema, players.parent_id is NOT NULL and parents.email is NOT NULL. Every player has a parent with an email. The "No email" edge case may never trigger in practice. Not blocking — the code can handle it defensively, but the agent should be aware.

Blast Radius

  • PaymentLink model is used by payments_controller (separate concern) — no conflict.
  • Unique constraint uq_payment_links_player_product_tenant on [player_id, product_id, tenant_id] — since product_id is nullable, subscription links can coexist with one-time payment links for the same player. No conflict.
  • Adding a POST method to StripeClient is additive — no existing code affected.
  • payment_links.parent_id is NOT NULL — creating a payment link requires a parent record. This is fine since every player has a parent.
  • No other views or controllers reference clipboard or copy-link functionality.

Decomposition Assessment

3 file targets in 1 repo. 7 acceptance criteria (over 5 threshold but tightly coupled). All changes are in a single feature flow: service method + controller wiring + view rendering. Estimated agent work: ~4 minutes. No decomposition needed — this is a cohesive single-pass feature.

Recommendations

No blocking recommendations. Minor items for awareness:
  • [SCOPE] Create project page project-westside-ror with user-stories section in pal-e-docs (low priority, does not block this ticket).
  • [SCOPE] Consider creating arch-stripe note in pal-e-docs to back the scope:stripe label (low priority, does not block this ticket).
  • [BODY] Minor: "No email" edge case in AC1 is impossible per current schema (players.parent_id NOT NULL, parents.email NOT NULL). Agent should implement defensively but be aware this path may never trigger. Not worth changing the AC — defensive coding is fine.