fix(billing): resolve plan via priceId map, not price.metadata.planId (#1250)#3743
Conversation
…#3742) Port trawl's #1250 fix upstream: `customer.subscription.updated` was reading `price.metadata.planId` which is empty in real Stripe webhook payloads (planId lives on the Product, not the Price). Every paid org's plan was silently reset to `free` on every Stripe subscription update event. Fix: - Add `buildPriceIdToPlanMap()` — builds `Map<priceId, planName>` from `config.stripe.prices` at boot (monthly + annual per plan) - Rewrite `resolvePlan()` — looks up plan via priceId map first; falls back to `price.metadata.planId` for legacy test fixtures; final fallback `free` + warn - Sync `cancelAt`/`cancelAtPeriodEnd` on `customer.subscription.updated` — writes the fields added by #3741 so the UI can show pending cancellation dates - Use priceId map in `previousPlan` detection (plan change block in updated handler) - Strip trawl-specific `analytics.capture('subscription_changed')` calls (not ported) Also ports `retryWithBackoff` + `isNonTransientStripeError` patterns (already in devkit from a prior PR — kept identical to trawl). Adds 20 unit tests covering: priceId map resolution, annual/monthly variants, unknown priceId fallback to free, legacy metadata fallback, priority of map over metadata, cancelAt unix→Date conversion, null cancel clear, absent field guard, handleSubscriptionCreated race safety (E11000 idempotency), and Dashboard subscription creation path. Closes #3742
|
Warning Review limit reached
More reviews will be available in 10 minutes and 56 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, 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 include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (3)
✨ 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 |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #3743 +/- ##
==========================================
+ Coverage 89.92% 89.96% +0.04%
==========================================
Files 148 148
Lines 4865 4885 +20
Branches 1533 1542 +9
==========================================
+ Hits 4375 4395 +20
Misses 385 385
Partials 105 105
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:
|
There was a problem hiding this comment.
Pull request overview
This PR addresses a P0 billing regression where customer.subscription.updated could silently downgrade paid orgs to free because price.metadata.planId is typically absent in real Stripe webhook payloads. It introduces a boot-time priceId→plan lookup from config.stripe.prices, updates webhook handling to use it, and adds unit tests for the new behavior and edge cases.
Changes:
- Build a priceId→plan reverse lookup from
config.stripe.pricesand use it inresolvePlan()(with legacy metadata fallback). - Sync
cancelAt/cancelAtPeriodEndfrom Stripecustomer.subscription.updatedinto the subscription record. - Add a
customer.subscription.created“safety-net” handler and a new unit test suite covering the new/modified paths.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
modules/billing/services/billing.webhook.service.js |
Adds priceId-based plan resolution and extends subscription update handling to persist cancellation scheduling fields; updates plan-change detection to consider priceId. |
modules/billing/tests/billing.webhook.priceid-map.unit.tests.js |
Adds unit coverage for priceId plan resolution, cancelAt conversions, and the subscription.created safety-net logic (including E11000 race handling). |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
…+ 4 hardening) (#3758) * fix(billing): re-apply lost C.2 review nits (warn-on-unknown-priceId + 4 hardening) 5 Copilot review findings from PR #3743 (#1250 P0) were silently lost because a controller-side fix commit was never pushed to origin before the squash-merge. Re-applied fixes: 1. JSDoc for buildPriceIdToPlanMap: accurate description with @returns tag 2. resolvePlan: warn log when priceId not in map and metadata empty (operationally critical — same silent-free-downgrade shape as the P0 it was meant to prevent) 3. cancel_at truthy check → typeof === 'number' (prevents cancel_at=0 being skipped) 4. previousPlan: validatePlan() before comparison (rejects stale/invalid metadata values) 5. Test comment: corrects misleading "retryWithBackoff setTimeout suppression" claim Refs: #3743, plan 2026-05-30-trawl-devkit-perfect-alignment.md * fix(billing): gate resolvePlan warn to metadata-absent case only Copilot review finding on PR #3758: the previous warn fired whenever priceId was present and metadata didn't validate, but the log message said "metadata empty" — misleading when metadata IS present but invalid (validatePlan already warns in that case → double-warn). Gate: only emit the warn when rawMeta is also absent (!rawMeta), and update the message from "metadata empty" to "no metadata" to be precise.
Problem (P0 — silent prod downgrade affects every devkit downstream)
customer.subscription.updatedwas readingprice.metadata.planIdto resolve the plan.price.metadata.planIdis empty in real Stripe webhook payloads — Stripe only populates metadata on the Product object, not on the Price surfaced through subscription update events.Result: every paid org's plan is silently reset to
freeon everycustomer.subscription.updatedevent. Pierreb, comes, montaine, and ism are all affected in production today (trawl fixed it incomes-io/trawl_node#1250).Fix
Port trawl's
#1250fix upstream so every downstream gets it via/update-stack:buildPriceIdToPlanMap()— buildsMap<priceId, planName>fromconfig.stripe.pricesat boot (monthly + annual per plan)resolvePlan()— priceId map first →price.metadata.planIdlegacy fallback →freewith warn logcancelAt/cancelAtPeriodEndsync oncustomer.subscription.updated— writes fields added in feat(billing): subscription cancelAt + cancelAtPeriodEnd lifecycle fields #3741 (gate task C.1) so UI shows pending cancellation dates; unix-seconds → JS Date conversionpreviousPlandetection in plan change block now also uses the priceId mapanalytics.capture('subscription_changed')calls (not generic)retryWithBackoff+isNonTransientStripeErrorlib files were already in devkit — unchanged.Tests
20 new unit tests in
billing.webhook.priceid-map.unit.tests.jscovering:freeprice.metadata.planIdfallback still works (test fixtures)cancelAtunix→Date conversion exact valuecancelAt=nullclears the fieldcancel_at→ no field writtenhandleSubscriptionCreated: existing doc path, stale event, Dashboard/Payment Link path, race E11000 idempotency, no customer mapping skipAll 982 billing unit tests pass (51 suites).
References
2026-05-30-trawl-devkit-perfect-alignment.mdTask C.2comes-io/trawl_node#1250Self-review checklist
resolvePlanreads from priceId map, NOT fromprice.metadata.planIdbuildPriceIdToPlanMapiterates monthly + annual per planhandleSubscriptionCreatedrace-safe (E11000 idempotency guard)cancelAt/cancelAtPeriodEndsync writes to fields, converts unix-seconds to Date