fix(billing): restore plan after dunning recovery#3627
fix(billing): restore plan after dunning recovery#3627PierreBrisorgueil merged 2 commits intomasterfrom
Conversation
…succeeded V8 audit C1: when markUnpaid downgrades a sub to plan=free and the user later pays the overdue invoice, re-fetch the live Stripe subscription and write the correct plan back alongside pastDueSince=null + status=active. Prevents paid customers being permanently stuck on free plan if customer.subscription.updated is dead-lettered. Falls back gracefully (warn log, no plan field) on Stripe re-fetch error.
…t_succeeded
/critical-review medium: add explicit `if (!stripe) throw` so the fallback warn
log is meaningful ("Stripe not configured") rather than a cryptic TypeError.
/critical-review low: update JSDoc @description to document V8 C1 plan-restore
behavior and the non-fatal Stripe re-fetch fallback contract.
|
Warning Rate limit exceeded
To continue reviewing without waiting, purchase usage credits in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (2)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
This PR addresses a billing revenue-leak edge case where dunning can downgrade a subscription’s persisted plan to 'free', and a later invoice.payment_succeeded webhook would previously clear pastDueSince/activate the sub without restoring the correct paid plan if customer.subscription.updated is dead-lettered.
Changes:
- Update
handleInvoicePaymentSucceededto re-fetch the live Stripe subscription, resolve the plan, and persistplanalongside clearingpastDueSince/ restoringstatus: 'active'. - After a successful DB update, sync the organization plan when a plan was restored.
- Add/adjust unit tests to cover plan restoration and graceful fallback when Stripe re-fetch fails.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| modules/billing/services/billing.webhook.service.js | Restores plan during invoice.payment_succeeded handling via Stripe re-fetch and syncs org plan post-update. |
| modules/billing/tests/billing.webhook.subscription.unit.tests.js | Adds test coverage for dunning recovery plan restoration and Stripe re-fetch failure fallback. |
| resolvedPlan = resolvePlan(stripeSub); | ||
| fields.plan = resolvedPlan; |
| let resolvedPlan = null; | ||
| if (isPastDue) { | ||
| try { | ||
| const stripe = getStripe(); | ||
| if (!stripe) throw new Error('Stripe not configured'); | ||
| const stripeSub = await stripe.subscriptions.retrieve(stripeSubscriptionId); | ||
| resolvedPlan = resolvePlan(stripeSub); |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #3627 +/- ##
==========================================
+ Coverage 88.82% 88.85% +0.03%
==========================================
Files 134 134
Lines 4616 4630 +14
Branches 1423 1428 +5
==========================================
+ Hits 4100 4114 +14
Misses 403 403
Partials 113 113
Flags with carried forward coverage won't be shown. Click here to find out more. Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
Summary
V8 audit C1 fix:
handleInvoicePaymentSucceedednow re-fetches the live Stripe subscription and restores the correctplanfield when clearingpastDueSinceafter a dunning sweep downgraded the sub toplan=free.markUnpaidwritesplan='free' status='unpaid'during dunning sweep. When the user later pays the overdue invoice, Stripe firesinvoice.payment_succeeded. The handler only wrote{ pastDueSince: null, status: 'active' }, relying oncustomer.subscription.updatedto restore the plan — if that event is dead-lettered, the paid customer is permanently stuck on free plan (revenue leak).stripe.subscriptions.retrieve(), resolve plan via existingresolvePlan(), addplanto the fields update, callsyncOrganizationPlan()after update succeeds.pastDueSince: null + status: 'active'update still fires.if (!stripe) throwso the fallback warn log emits "Stripe not configured" rather than a cryptic TypeError.Test plan
V8 C1 — dunning recovery: restores plan=pro after unpaid downgrade to free— assertsplan: 'pro',status: 'active',pastDueSince: null+setPlancalled on orgV8 C1 — Stripe re-fetch failure: falls back gracefully, does not restore plan— asserts noplanfield in update, no throw