fix(billing): requirePlan gates on subscription.status + top-level errorCode response shape#3682
Conversation
…rorCode Closes #3679. Two related drifts fixed in one change: 1. Logic: canceled / past_due / unpaid / incomplete subscriptions no longer pass requirePlan('growth') just because subscription.plan === 'growth'. Only status ∈ {active, trialing} counts; everything else falls back to 'free' plan resolution. Pre-existing gap — middleware had zero route usages in devkit until trawl_node wired it on apiKeys / webhooks / org-invites for plan-entitlement enforcement. 2. Response shape: surfaces { type: 'error', message: 'Forbidden', code: 403, status: 403, errorCode: 'PLAN_REQUIRED', description, requiredPlans, currentPlan } at the top level of the JSON body (was: nested { type: 'PLAN_REQUIRED' } under responses.error meta). Upstreams the existing trawl_node #1183 patch so /update-stack --theirs no longer silently wipes the downstream contract. ORG_CONTEXT_REQUIRED also surfaces at top level for consistency. 14 unit tests total (6 baseline updated for new shape + 8 new for status gating and response-shape assertions). Drops responses.js import (no longer needed).
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more 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 |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #3682 +/- ##
=======================================
Coverage 89.60% 89.61%
=======================================
Files 139 139
Lines 4734 4736 +2
Branches 1481 1482 +1
=======================================
+ Hits 4242 4244 +2
Misses 385 385
Partials 107 107
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 tightens billing entitlement enforcement by updating requirePlan to consider subscription status (not just plan) and by standardizing the 403 error response payload to a top-level errorCode contract used by downstream clients.
Changes:
- Gate
requirePlanonsubscription.status ∈ {'active','trialing'}; otherwise treat the effective plan asfree. - Change 403 deny responses to include top-level
errorCode(PLAN_REQUIRED,ORG_CONTEXT_REQUIRED) plusrequiredPlans/currentPlan. - Update and expand unit tests to cover status gating and the new response shape.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| modules/billing/middlewares/billing.requirePlan.js | Adds status-based plan effectiveness and returns standardized top-level errorCode response envelopes. |
| modules/billing/tests/billing.requirePlan.unit.tests.js | Updates existing assertions and adds new unit coverage for status gating and response payload shape. |
| /** | ||
| * Statuses considered "active" for plan-entitlement checks. | ||
| * canceled / past_due / unpaid / incomplete fall back to 'free' resolution. | ||
| * @type {Set<string>} | ||
| */ | ||
| const ACTIVE_STATUSES = new Set(['active', 'trialing']); |
| const subscription = await SubscriptionRepository.findByOrganization(req.organization._id); | ||
| const currentPlan = subscription?.plan || 'free'; | ||
| const isEffective = subscription && ACTIVE_STATUSES.has(subscription.status); | ||
| const currentPlan = isEffective ? subscription.plan : 'free'; | ||
|
|
| describe('requirePlan — subscription.status gating:', () => { | ||
| const fakeOrgId = new mongoose.Types.ObjectId(); | ||
|
|
||
| function mockReq(overrides = {}) { | ||
| return { | ||
| organization: { _id: fakeOrgId, name: 'Test Org' }, | ||
| ...overrides, | ||
| }; | ||
| } | ||
|
|
||
| function mockRes() { | ||
| const res = {}; | ||
| res.status = jest.fn().mockReturnValue(res); | ||
| res.json = jest.fn().mockReturnValue(res); | ||
| return res; | ||
| } |
Closes #3679.
What — two drifts fixed in one PR
1. Logic gap:
subscription.statusnot checked (pre-existing)requirePlanpreviously gated only onsubscription?.plan. Agrowthsubscription withstatus: 'canceled',past_due','unpaid', or'incomplete'still passed the check — the user kept access after their subscription lapsed.Fix: introduce
ACTIVE_STATUSES = Set(['active', 'trialing']). Plan is only effective whensubscription.status ∈ ACTIVE_STATUSES. Everything else resolvescurrentPlanto'free'.This was pre-existing but harmless in devkit (middleware had zero route usages) — became a real gate when trawl_node #1183 wired it on apiKeys / webhooks / org-invites.
2. Response-shape divergence: upstreams the trawl_node #1183 patch
trawl_node #1183 patched the response to top-level
errorCodeto make it consumable by the Vue client:{ "type": "error", "message": "Forbidden", "code": 403, "status": 403, "errorCode": "PLAN_REQUIRED", "description": "Your current plan does not allow access to this resource", "requiredPlans": ["growth", "pro"], "currentPlan": "free" }That patch lived only in trawl_node; the next
/update-stack --theirswould have silently wiped it (seefeedback_update_stack_theirs_wipes_patches.md). This PR upstreams the shape to devkit so the contract is safe on future stack updates.ORG_CONTEXT_REQUIRED follows the same top-level shape for consistency.
Decision (locked)
{active, trialing}onlytype + message + code + status + errorCode + description + requiredPlans + currentPlan)Breaking change note
This is a breaking response-shape change for any consumer reading
body.type === 'PLAN_REQUIRED'(legacy nested key). The old shape usedresponses.errorwhich wrapped meta inside the object differently. trawl_node already moved toerrorCodein #1183. devkit has zero other consumers today. Theresponses.jsimport is removed.Verification
ESLint: No issues found