fix(billing): rotate week-quota snapshot on mid-week plan activation - #3989
Conversation
forceRotateForPlanChange was wired only into handleSubscriptionUpdated's plan-change block. handleCheckoutCompleted and both branches of handleSubscriptionCreated never rotated the current-week quota snapshot after a mid-week plan activation, so an upgraded org kept the previous plan's weekly quota (often 0 on free->paid) until the next weekly reset. - Wire the same non-fatal, logged forceRotateForPlanChange call into all three activation call sites (preserveUsage: true, consistent with the existing plan-change call site). - billing.usage.service.js incrementMeter now reads the live plan quota (already fetched for the snapshot write) instead of the potentially stale updatedDoc.meterQuota for overflow decisions. - Integration tests for the checkout-completed and subscription-created (existing row) activation paths, asserting the rotated snapshot and immediate new-plan quota. Closes #3988 Claude-Session: https://claude.ai/code/session_01K6NtYTXHfNcpAQ3TJXWJxL
PierreBrisorgueil
added
Fix
PierreBrisorgueil
added
Fix
|
Warning Review limit reached
Next review available in: 7 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
WalkthroughBilling webhook activation paths now rotate current-week quota snapshots without failing webhook processing. Meter overflow decisions use the live plan quota, with integration coverage for checkout and subscription-created flows. ChangesBilling quota activation
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant BillingWebhook
participant OrganizationPlan
participant BillingResetService
participant BillingUsageService
BillingWebhook->>OrganizationPlan: sync activated plan
BillingWebhook->>BillingResetService: forceRotateForPlanChange(preserveUsage: true)
BillingResetService-->>BillingWebhook: refresh current-week quota snapshot
BillingUsageService->>BillingUsageService: use live meterQuota for overflow decisions
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 #3989 +/- ##
==========================================
+ Coverage 93.29% 93.30% +0.01%
==========================================
Files 170 170
Lines 5653 5664 +11
Branches 1820 1821 +1
==========================================
+ Hits 5274 5285 +11
Misses 306 306
Partials 73 73
Flags with carried forward coverage won't be shown. Click here to find out more. Continue to review full report in Codecov by Harness.
🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@modules/billing/tests/billing.lifecycle.integration.tests.js`:
- Around line 274-282: Update the subscription-created test around
BillingWebhookService.handleSubscriptionCreated to configure the appropriate
test price-ID-to-plan mapping, then replace price.metadata.planId with a
realistic price.id payload. Keep the existing upgradePlan resolution and
rotation assertions, ensuring the test validates plan resolution through the
configured mapping.
- Around line 234-292: Add a separate integration test alongside the
existing-row test for BillingWebhookService.handleSubscriptionCreated where no
Subscription matches stripeSubscriptionId but the organization is resolved
through stripeCustomerId. Assert that a new Subscription row is created with the
upgraded plan and that the current-week BillingUsage snapshot receives the
upgraded quota and planVersion while preserving existing usage.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 435106ec-1966-418b-9931-48f3809651a3
📒 Files selected for processing (4)
ERRORS.mdmodules/billing/services/billing.usage.service.jsmodules/billing/services/billing.webhook.service.jsmodules/billing/tests/billing.lifecycle.integration.tests.js
…Created
CodeRabbit flagged missing coverage of the "new row" (Dashboard/Payment
Link) branch's forceRotateForPlanChange call added in the previous commit.
A real-DB integration test can't exercise this: `organization` is a
unique field on the Subscription schema, so `orgSub` being found via
findByStripeCustomerId always means a row already exists for that org,
and Subscription.create() for the same org always throws E11000 before
reaching the rotation call (confirmed empirically — see the existing
"race condition: E11000" test). Cover it at the unit level instead
(mocked repositories), where the success and duplicate-skip branches
can both be exercised in isolation:
- Assert forceRotateForPlanChange(orgId, { preserveUsage: true }) fires
when create() succeeds.
- Assert it does NOT fire on the E11000 duplicate-skip path (the handler
never wrote a row in that case).
Claude-Session: https://claude.ai/code/session_01K6NtYTXHfNcpAQ3TJXWJxL
The Codecov patch gate flagged 4 uncovered lines: the non-fatal error branch of the forceRotateForPlanChange call added to handleCheckoutCompleted and both handleSubscriptionCreated branches. None of the prior tests forced the rotation call to reject, so the catch/logger.warn lines never executed. - billing.webhook.checkout.unit.tests.js: promote the inline forceRotateForPlanChange stub to a named, assertable mock; add a happy-path assertion (preserveUsage: true) and a rejection test (non-fatal, logged, does not throw). - billing.webhook.priceid-map.unit.tests.js: same pair of tests for both handleSubscriptionCreated branches (existing row, new row). Claude-Session: https://claude.ai/code/session_01K6NtYTXHfNcpAQ3TJXWJxL
|
@coderabbitai full review |
✅ Action performedFull review finished. Your included review limit is currently reached under our Fair Usage Limits Policy. This review may still proceed through usage-based billing if eligible. Your next included review will be available in 23 minutes. |
/critical-review fallback finding (CodeRabbit rate-limited on this PR): the incrementMeter replay branch (idempotencyKey already consumed) still returned meterQuota: existing?.meterQuota ?? meterQuota — the stored, potentially-stale snapshot — while the non-replay path 10 lines below already reads the live quota. A replayed call after a mid-week plan upgrade (e.g. rotation hasn't run yet, or failed non-fatally) would report the pre-upgrade quota instead of the current one, inconsistent with what the same call returns when it isn't a replay. Claude-Session: https://claude.ai/code/session_01K6NtYTXHfNcpAQ3TJXWJxL
1 participant
Summary
BillingResetService.forceRotateForPlanChangeweek-quota snapshot rotation intohandleCheckoutCompletedand both branches ofhandleSubscriptionCreated(billing.webhook.service.js), mirroring the call already present inhandleSubscriptionUpdated's plan-change block. Also makesbilling.usage.service.js incrementMeterread the live plan quota (already fetched for the snapshot write) instead of the potentially-staleupdatedDoc.meterQuotafor overflow decisions.forceRotateForPlanChangewas wired only intohandleSubscriptionUpdated. Checkout activation and subscription-created (both the "new row" and "existing row" branches) never rotated the current-week quota snapshot after a mid-week plan activation — an upgraded org kept the previous plan's weekly quota (often 0 on free→paid) until the next weekly reset. This is a money bug: the org does not get its new quota immediately after upgrading.Scope
billingnonelow— additive, non-fatal (logged, try/catch) call at three existing call sites; mirrors an already-proven pattern in the same file (handleSubscriptionUpdated's plan-change block,handleSubscriptionDeleted,handleCustomerDeleted).Validation
npm run lintnpm test(full suite:npm run test:coverage— 213 suites / 2770 tests passing, coverage thresholds met, none lowered)Guardrails check
.env*,secrets/**, keys, tokens)Notes for reviewers
modules/billing/; downstream projects absorb via/update-stack.Summary by CodeRabbit
Bug Fixes
Tests