pal-e-docs

Review: Copy-to-clipboard subscription links + webhook

review-1335-2026-06-06 Review

review needs-refinement
Project

Verdict: NEEDS_REFINEMENT

Template Completeness

  • [x] Type — Feature
  • [x] Lineage — follow-up from spike #22
  • [x] Repo — ldraney/westside-ror
  • [x] User Story — present and well-formed
  • [x] Context — thorough, includes Stripe data audit
  • [x] File Targets — 5 files listed with rationale
  • [x] Acceptance Criteria — 12 criteria
  • [x] Test Expectations — 6 test cases + run command
  • [x] Constraints — 5 constraints listed
  • [x] Open Questions — 3 questions for Lucas/Marcus
  • [x] Checklist — present
  • [x] Related — lists parent spike and upstream PR
All required sections for a Feature template are present. Issue is thorough and well-structured.

Traceability

  • [x] story:subscription-links label — present on board item
  • [ ] story note MISSING — project-westside-ror note does not exist in pal-e-docs. Cannot verify user story entry. [SCOPE] Create project page project-westside-ror with user-stories section.
  • [x] arch:rails label — present on board item
  • [ ] arch note MISSING — no arch-rails note found in pal-e-docs. [SCOPE] Create architecture note arch-rails for the Rails component.
  • [x] Forgejo issue — ldraney/westside-ror#23, open

File Targets

  • [x] config/routes.rb — verified: exists, currently has admin namespace and email routes but no webhook route. Correct target for adding post '/webhooks/stripe'.
  • [x] app/controllers/webhooks_controller.rb — verified: does NOT exist yet (new file). Correct — needs creation.
  • [x] app/services/stripe_client.rb — verified: exists at 127 lines, uses raw net/http pattern, has payment_links and checkout_sessions methods but no subscription-mode link creation. Correct target.
  • [x] app/controllers/admin/roster_report_controller.rb — verified: exists at 35 lines, loads tenant/teams/players. Currently does not load payment_links. Correct target.
  • [x] app/views/admin/roster_report/index.html.erb — verified: exists at 98 lines with desktop table and mobile card layout. No "Copy Link" button yet. Correct target.
  • [x] app/controllers/admin/payments_controller.rb — verified: exists, separate one-time payment flow. Correctly listed as "do NOT touch".
Issue: Context section references docs/reports/2026-06-01-stripe-payment-link-audit.md but this file does not exist. The actual spike output is at docs/stripe-subscription-architecture.md.

Repo Placement

OK — issue is filed on ldraney/westside-ror and all file targets are within this repo. Single-repo change.

Dependencies

  • Ordering conflict: The architecture doc (docs/stripe-subscription-architecture.md, line 172-173) specifies implementation order as #24 first, then #23. However, #23 is being reviewed for advancement while #24 (inline-editable monthly fee with Stripe sync) is also in backlog. If #24 lands first, it changes how monthly_fee works and could affect which Stripe price tier the subscription link uses. The issue should explicitly document whether it depends on #24 or can proceed independently.
  • Board item #1334 (Inline-editable monthly fee, Forgejo #24) — same story:subscription-links label, also in backlog. These two tickets share the same Stripe product and price tiers. No documented dependency between them on the board.
  • Board item #1207 (Stripe payment links + email blast, Forgejo #2) — in_progress. Uses the same StripeClient and payment_links table but for one-time payments. No conflict but blast radius overlap.
  • CSRF exemption needed: ApplicationController inherits from ActionController::Base which has CSRF protection enabled by default. The webhook controller must skip this. Issue mentions this in Constraints but not in File Targets or AC.
  • Webhook signing secret: Issue says "must go through secrets management" but does not specify the env var name or where it will be configured (docker-compose.yml currently only has STRIPE_API_KEY). File target for docker-compose.yml or deployment config is missing.

Acceptance Criteria

12 acceptance criteria — exceeds the 5-AC decomposition threshold.
  • AC 1-7 cover the copy-to-clipboard link generation flow (UI + Stripe API + storage)
  • AC 8-12 cover the webhook endpoint (signature verification, event handling, status updates)
  • These are two distinct subsystems that could be implemented and tested independently.
Schema concern: AC 9 says webhook "sets paid_at" — this column is on payment_links table (not players), which is correct but the AC text is ambiguous about which record.
Unique constraint concern: payment_links has a unique constraint on (player_id, product_id, tenant_id). If all subscription links use the same "Monthly Club Dues" product, a player can only have one link. The ticket says "reused on subsequent page loads" (AC 7) which aligns. But if a player's fee tier changes, the old link must be deactivated and a new one created — this edge case is not covered in the AC.
Missing AC: No acceptance criterion for players without a parent record (Open Question #3 acknowledges this but doesn't resolve it).

Blast Radius

  • StripeClient is used by both Admin::PaymentsController (one-time payments) and the roster report. Adding subscription link methods is additive and should not break existing one-time flows.
  • payment_links table is shared with the one-time payment system. New subscription links will coexist. The paymentlinkstatus enum already includes active, paid, canceled values.
  • No other controllers currently handle webhooks, so the new endpoint is isolated.
  • The subscriptionstatus enum already has active, past_due, canceled, none — matches webhook events perfectly.

Decomposition Assessment

NEEDS DECOMPOSITION
  • 12 acceptance criteria (threshold: 5)
  • 5 file targets (threshold: 3)
  • Two distinct subsystems: (a) link generation + copy UI, (b) webhook endpoint + status updates
  • Estimated agent work: well over 5 minutes — involves Stripe API integration, webhook signature verification, JS clipboard logic, and multiple controller/model changes
  • 8 story points further confirms this is too large for a single pass
Route to skill-decompose-ticket for sub-ticket creation.

Recommendations

  • [BODY] Fix audit doc reference: docs/reports/2026-06-01-stripe-payment-link-audit.md → docs/stripe-subscription-architecture.md
  • [BODY] Add dependency note: document whether #23 depends on #24 or can proceed independently (architecture doc says #24 first)
  • [BODY] Add file target: docker-compose.yml or deployment config for STRIPE_WEBHOOK_SECRET env var
  • [BODY] Add AC for fee-tier-change edge case: when monthly_fee changes, old link must be deactivated and new link created
  • [BODY] Add AC for players without a parent record (resolve Open Question #3)
  • [SCOPE] Create project page project-westside-ror with user-stories section including subscription-links story
  • [SCOPE] Create architecture note arch-rails for the Rails component
  • [DECOMPOSE] 12 AC across 2 subsystems (link generation + webhook), 5 file targets, 8 points — route to skill-decompose-ticket. Natural split: (a) subscription link generation + copy-to-clipboard UI (AC 1-7), (b) webhook endpoint + status sync (AC 8-12)