pal-e-docs

Review: Stripe webhook for subscription status sync

review-1357-2026-06-06 Review

review needs-refinement
Project

Verdict: NEEDS_REFINEMENT

Template Completeness

  • [x] Type -- Feature
  • [x] Lineage -- split from #23
  • [x] Repo -- ldraney/westside-ror
  • [x] User Story -- system actor, auto-updates roster
  • [x] Context -- explains gap (no webhook endpoint, DB columns exist)
  • [x] File Targets -- 4 modify/create, 3 do-not-touch
  • [x] Acceptance Criteria -- 8 criteria
  • [x] Test Expectations -- 7 test cases + run command
  • [x] Constraints -- 5 constraints listed
  • [x] Checklist -- standard 3-item
  • [x] Related -- references project, companion tickets, spike, arch doc

Traceability

  • [x] story:subscription-links label -- present on board item
  • [ ] story note MISSING -- project-westside-ror note does not exist in pal-e-docs (404). [SCOPE] Create project page project-westside-ror with user-stories section including subscription-links story.
  • [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 application component. Note: docs/stripe-subscription-architecture.md exists in-repo but is not a pal-e-docs note.
  • [x] Forgejo issue -- ldraney/westside-ror#27, state: open

File Targets

  • [x] config/routes.rb -- verified: exists, currently has no webhook route (line 1-39). Route addition is straightforward.
  • [x] app/controllers/webhooks_controller.rb -- correctly marked as NEW. Does not exist yet. Will need skip_forgery_protection since ApplicationController inherits from ActionController::Base (no existing CSRF skip patterns in codebase).
  • [x] app/services/stripe_client.rb -- verified: exists (127 lines). Uses raw net/http, no Stripe gem. Adding verify_webhook_signature method here is consistent with existing patterns.
  • [x] Deployment config (env var STRIPE_WEBHOOK_SECRET) -- no existing references. New secret needed.
  • [x] DB columns verified in schema.rb:

Repo Placement

OK. Issue filed on ldraney/westside-ror, all file targets are in that repo. Single-repo change.

Dependencies

  • #24 (editable fees) -- architecture doc says #24 ships first, but the issue correctly notes this webhook can ship independently. The webhook does not require fee editing to work. No blocking dependency.
  • #26 (copy links UI) -- companion ticket, explicitly independent. No dependency.
  • #22 (spike) -- closed. Produced the architecture doc. No dependency.
  • #23 (original combined ticket) -- still on board in backlog. Should be closed or updated once #26 and #27 replace it. Not a blocker but a housekeeping item.
  • No items in in_progress block this ticket. Items in in_progress (#2 Stripe payment links, #13 email) are unrelated to webhook functionality.

Acceptance Criteria

All 8 criteria are testable by an agent. The test expectations map cleanly to acceptance criteria. Run command (rails test) is valid.
Observations:
  • AC #4 references reading metadata (player_id, team) from checkout.session.completed. The ticket should clarify HOW player_id gets into session metadata -- this is set when creating the payment link (which happens in #26). The webhook needs to handle the case where metadata is missing or incomplete.
  • AC #5 references finding payment_link by stripe_payment_link_id. The checkout session object from Stripe includes a payment_link field -- this is the correct lookup key.
  • Idempotency constraint is stated but not in AC. Could be an explicit test case (processing same event twice should not error).

Blast Radius

  • subscription_status is read by admin/roster_report_controller.rb (lines 14, 24, 29, 30). Webhook updates to this field will surface automatically in the roster report. No code changes needed there -- this is the desired effect.
  • paid_at is read by StripeClient#db_payment_summary (lines 70, 84), used by the payments dashboard. Again, webhook updates will surface automatically. No blast radius concern.
  • No other controllers or services reference these columns.
  • Adding skip_forgery_protection to the new WebhooksController is scoped -- it only affects that controller, not the app globally.

Decomposition Assessment

4 file targets in 1 repo. 8 acceptance criteria. 7 test expectations. Estimated agent work: ~8-10 minutes (new controller, service method, route, tests). This is above the 5-minute threshold but all targets are in one repo and tightly coupled -- decomposing would create artificial seams. The 8 AC are all part of the same webhook handler. A single agent pass is appropriate if the agent is given clear guidance. No decomposition needed.

Recommendations

  • [SCOPE] Create project page project-westside-ror with user-stories section including the subscription-links story entry.
  • [SCOPE] Create architecture note arch-rails for the Rails application component in pal-e-docs (or re-evaluate whether arch:rails is the right label -- possibly arch:stripe would be more specific).
  • [BODY] Add note to AC #4 about handling missing metadata gracefully (payment links created outside the app may lack player_id metadata).
  • [BODY] Add explicit AC for idempotency: "Processing the same webhook event twice does not create duplicate records or errors."
  • [BODY] Clarify that #23 (the original combined ticket) should be closed now that #26 and #27 replace it.