Fix parallel Stripe subscriptions and improve subscription sync - #1463
Open
drusepth wants to merge 2 commits into
Open
Fix parallel Stripe subscriptions and improve subscription sync#1463drusepth wants to merge 2 commits into
drusepth wants to merge 2 commits into
Conversation
Users have been accumulating multiple simultaneous Stripe subscriptions (e.g. 3 starter + 1 premium), double-charging them. Root cause: current Stripe API versions no longer include `subscriptions` on retrieved Customer objects, so every `stripe_customer.subscriptions&.data || []` read silently returned []. Every plan change therefore looked like a first-time signup and created a brand-new subscription, while the old one was never modified or canceled Stripe-side (cancellation only ever touched our local records). Repeated clicks on a slow page multiplied the effect. The same dead read also silently skipped subscription cancellation when deleting a payment method or an account, and `Stripe::Subscription.modify` no longer exists in stripe-ruby 15, so the modify path would have crashed even if a subscription had been found. Fixes: - SubscriptionService now lists subscriptions via Stripe::Subscription .list and enforces a single-subscription invariant on every plan change: keep one subscription (preferring the requested price, then healthy status, then oldest), switch its price in place, and cancel any parallel duplicates with proration so unused time is credited. Existing double-subscribed users are healed the next time they touch their plan. - Plan changes are serialized per user with a row lock, so concurrent double-clicks can't race each other into duplicate subscriptions; re-choosing the current plan is now a no-op (and un-schedules a pending cancellation instead of stacking a new subscription). - New subscriptions are created with payment_behavior: error_if_incomplete so a declined card raises Stripe::CardError (routing to the existing failed-card flow) instead of leaving an incomplete subscription behind. - delete_payment_method now cancels every paid subscription at period end (the old code no-opped on the dead read and used a removed API); delete_my_account now cancels all billable subscriptions instead of re-pricing just the first one. - Plan-change links now submit via POST with a disable-on-click guard; the GET route remains for legacy links and is safe now that changes are idempotent. - The payment page's "already paid until" notice works again and reads period ends from subscription items, where newer API versions moved them. - New rake task stripe:audit_parallel_subscriptions reports every customer with parallel subscriptions (dry run by default; APPLY=1 cancels duplicates with proration) to clean up existing damage. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GAr8oQfJnvJnUXpH2ejHRF
Auditing the rest of the billing code turned up three more places built on the same dead `stripe_customer.subscriptions&.data || []` read, plus a failure mode the first fix made more likely. - data_integrity:subscription_synced_with_stripe was a mass-downgrade landmine. It decides whether to downgrade a premium user based on that always-empty read, so on current Stripe API versions it would conclude that EVERY premium user had unsubscribed and downgrade the entire paying user base (emailing each one). It now lists subscriptions properly and checks all of them, refuses to downgrade more than 20% of premium users in a single run (a wrong-looking mass downgrade almost certainly means we are misreading Stripe rather than that everyone churned), and supports DRY_RUN=1. - Admin "unsubscribe" only ended subscriptions in our database and never cancelled them on Stripe, so unsubscribed users kept getting billed. It now cancels on Stripe first. cancel_all_existing_subscriptions is documented as local-only, with cancel_stripe_subscriptions! added for callers that are cancelling outright rather than switching plans. - New-user signup created its starter subscription with a hand-rolled Stripe::Subscription.create that did not check for existing subscriptions. It now goes through the same one-subscription invariant as every other path. - A declined card used to commit the local downgrade. process_plan_change cancels the old plan locally before adding the new one, and add_subscription swallowed Stripe::CardError and returned, so the transaction committed and the user lost the plan and bandwidth they had already paid for. Making creation raise on a declined card (error_if_ incomplete) made this reachable. CardError now propagates out of the transaction, which is opened with requires_new so it rolls back even if nested, and the controller converts it to :failed_card afterwards. Tests: 12 covering the single-subscription invariant against stubbed Stripe HTTP (cancel duplicates, modify-not-create, create-when-none, no-op on repeat, active-beats-incomplete, un-cancel, cancel-all, period-end fallback) plus an integration test asserting a declined card leaves the user's plan, bandwidth and local subscriptions untouched. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GAr8oQfJnvJnUXpH2ejHRF
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #
Changes proposed:
Refactor subscription synchronization logic: Extract complex Stripe subscription management into a dedicated
sync_stripe_subscriptions_to_planmethod that handles creating, modifying, and canceling subscriptions. This replaces the inline logic that was creating duplicate subscriptions.Prevent duplicate subscriptions: Implement proper handling of parallel subscriptions by:
customer.subscriptionsfieldpayment_behavior: 'error_if_incomplete'to prevent incomplete subscriptions from being left behind on card failuresAdd helper methods for subscription management:
billable_stripe_subscriptions(): Fetches all billable subscriptions for a customerprioritize_subscriptions_to_keep(): Sorts subscriptions by health (status) and age (creation date)subscription_price_ids(): Extracts price IDs from subscription itemssubscription_period_end(): Gets period end from subscription or items (handles API version differences)Add comprehensive test coverage: New test file with 9 test cases covering subscription filtering, creation, modification, cancellation, and edge cases like incomplete duplicates and scheduled cancellations.
Add Stripe audit rake task: New
stripe:audit_parallel_subscriptionstask to find and optionally cancel duplicate subscriptions across all customers, with dry-run mode by default.Update controllers to use new methods:
subscriptions_controller: Usebillable_stripe_subscriptions()instead of deprecatedcustomer.subscriptionsusers_controller: Cancel all billable subscriptions on account deletion instead of downgrading to starterprocess_plan_change()to prevent race conditions on concurrent plan changesUpdate views: Use new helper methods to safely access subscription data and period end times.
Fix plan change links: Change starter plan link from GET to POST with
method: :postto properly serialize plan changes and prevent accidental duplicate requests.@indentlabs/contributors
https://claude.ai/code/session_01GAr8oQfJnvJnUXpH2ejHRF