pal-e-docs

Review: Stripe webhook for subscription status sync (re-review)

review-1357-2026-06-06-r2 Review

review approved
Project

Verdict: APPROVED

Re-review of board item #1357 (Forgejo issue ldraney/westside-ror#27). Previous review review-1357-2026-06-06 returned NEEDS_REFINEMENT with 3 [BODY] and 2 [SCOPE] recommendations. All [BODY] items have been addressed in the updated issue. [SCOPE] items remain outstanding but are platform-level documentation tasks that do not block implementation.

Previous Findings Disposition

  • [x] [BODY] Graceful handling of missing metadata -- FIXED. New AC #6 explicitly requires graceful handling of events with missing or unrecognized metadata. Context section now documents that Marcus created some payment links outside the app. Test Expectation #8 covers this case.
  • [x] [BODY] Explicit idempotency AC -- FIXED. New AC #10: "Idempotent: processing the same event twice produces the same result without errors." Test Expectation #9: "processing the same event twice does not error or create duplicate state." Constraint section also restates this.
  • [x] [BODY] Clarify #23 closed -- FIXED. Issue #23 is confirmed closed on Forgejo. Related section in #27 now states "(closed, replaced by #26 + #27)."
  • [ ] [SCOPE] Create project-westside-ror with user-stories section -- STILL OUTSTANDING. Not a blocker for implementation.
  • [ ] [SCOPE] Create arch-rails note in pal-e-docs -- STILL OUTSTANDING. Not a blocker for implementation. In-repo architecture doc docs/stripe-subscription-architecture.md covers the webhook design in detail (Flow 3).

Template Completeness

  • [x] Type -- Feature
  • [x] Lineage -- split from #23 (Sub-B: webhook + status sync)
  • [x] Repo -- ldraney/westside-ror
  • [x] User Story -- system actor, auto-updates roster report
  • [x] Context -- explains gap, documents data format, addresses external links
  • [x] File Targets -- 4 modify/create, 3 do-not-touch
  • [x] Acceptance Criteria -- 10 criteria (up from 8, added graceful handling + idempotency)
  • [x] Test Expectations -- 9 test cases + run command (up from 7, added missing-metadata + idempotency tests)
  • [x] Constraints -- 5 constraints listed (includes idempotency)
  • [x] Checklist -- standard 3-item
  • [x] Related -- references project, companion tickets, spike, arch doc; #23 confirmed closed

Traceability

  • [x] story:subscription-links label -- present on board item
  • [ ] story note -- project-westside-ror does not exist in pal-e-docs (404). Carried forward as non-blocking [SCOPE] item.
  • [x] arch:rails label -- present on board item
  • [ ] arch note -- no arch-rails note in pal-e-docs. In-repo doc docs/stripe-subscription-architecture.md covers webhook design (Flow 3). Carried forward as non-blocking [SCOPE] item.
  • [x] Forgejo issue -- ldraney/westside-ror#27, state: open

File Targets

  • [x] config/routes.rb -- verified: exists (39 lines), no webhook route currently. Route addition straightforward.
  • [x] app/controllers/webhooks_controller.rb -- correctly marked as NEW. Does not exist. No existing skip_forgery_protection patterns in codebase; new controller will need it.
  • [x] app/services/stripe_client.rb -- verified: exists (127 lines). Uses raw net/http, no Stripe gem. Adding verify_webhook_signature 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:
  • [x] Enum values match AC: subscriptionstatus = [active, past_due, canceled, none] (schema line 37)

Repo Placement

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

Dependencies

  • #24 (editable fees) -- no blocking dependency. Webhook does not require fee editing.
  • #26 (copy links UI) -- companion ticket, explicitly independent.
  • #22 (spike) -- closed. Produced architecture doc.
  • #23 (original combined) -- confirmed closed on Forgejo. Housekeeping resolved.
  • No items in in_progress block this ticket.

Acceptance Criteria

All 10 criteria are testable by an agent. Test expectations map cleanly to AC (9 test cases cover all 10 AC). Run command (rails test) is valid. The two new additions (graceful handling AC #6, idempotency AC #10) close the gaps from the previous review.

Blast Radius

  • subscription_status read by admin/roster_report_controller.rb (lines 14, 24, 29, 30). Webhook updates surface automatically in roster report -- desired effect, no code changes needed.
  • paid_at read by StripeClient#db_payment_summary (lines 70, 84), used by payments dashboard. Same -- desired effect.
  • skip_forgery_protection scoped to new WebhooksController only. No global impact.
  • No other controllers or services reference these columns.

Decomposition Assessment

4 file targets in 1 repo. 10 AC, 9 test cases. Estimated agent work: ~8-10 minutes. Above the 5-minute threshold but all targets are tightly coupled in one repo -- decomposing would create artificial seams. Single agent pass is appropriate. No decomposition needed.

Recommendations

No blocking recommendations. Two carried-forward [SCOPE] items for platform documentation:
  • [SCOPE] Create project page project-westside-ror with user-stories section including subscription-links story. (Non-blocking)
  • [SCOPE] Create architecture note arch-rails in pal-e-docs, or reconsider label as arch:stripe. In-repo doc exists at docs/stripe-subscription-architecture.md. (Non-blocking)