Skip to content

refactor(billing): extract BillingQuotaService.assertCanExecute from requireQuota middleware#3749

Merged
PierreBrisorgueil merged 3 commits into
masterfrom
feat/extract-quota-service
Jun 1, 2026
Merged

refactor(billing): extract BillingQuotaService.assertCanExecute from requireQuota middleware#3749
PierreBrisorgueil merged 3 commits into
masterfrom
feat/extract-quota-service

Conversation

@PierreBrisorgueil
Copy link
Copy Markdown
Contributor

Summary

  • Promote-up from Trawl: ports BillingQuotaService.assertCanExecute to Devkit so every downstream benefits on their next /update-stack
  • Pure refactorbilling.requireQuota.js becomes a thin wrapper; behavior is byte-identical to current, Trawl has been running this in prod for weeks
  • After merge + /update-stack on Trawl, billing.requireQuota.js on Trawl becomes byte-compatible with Devkit's (clean merge, no conflict)

Files changed

File Change
modules/billing/services/billing.quota.service.js NEWassertCanExecute({ orgId, organization, user, resource, action }) — full dual-mode (legacy quota + meter mode), admin/meterExempt bypass, fail-closed, past_due grace, 503 plan-not-configured guard
modules/billing/middlewares/billing.requireQuota.js REFACTOR — thin wrapper around the service, maps AppError → HTTP response, sets res.locals.billingDegraded
modules/billing/tests/billing.quota.service.unit.tests.js NEW — 20 service-layer unit tests
modules/billing/tests/billing.quota.unit.tests.js UPDATED — mocks at service boundary instead of inline deps; all scenario coverage preserved

De-trawlify applied

  • No trawl-specific issue refs
  • No trawl-specific PostHog analytics calls
  • Plans in tests use devkit defaults (free / starter / pro)
  • Generic JSDoc

Plan reference

Infra plan Task D.1: docs/superpowers/plans/2026-05-30-trawl-devkit-perfect-alignment.md
Issue: #3748

Test plan

  • npm test -- --testPathPatterns="billing.quota" — all 1666 tests pass (115 suites)
  • billing.quota.unit.tests.js PASS (middleware HTTP mapping)
  • billing.quota.service.unit.tests.js PASS (service-layer direct)
  • CI green on push
  • CodeRabbit 0 unresolved threads

…requireQuota middleware (#3748)

Promote-up from Trawl: extract the quota/meter enforcement logic from
billing.requireQuota.js into a dedicated BillingQuotaService so the
enforcement path can be shared with any caller (not just the HTTP
middleware), and tested independently.

- New: modules/billing/services/billing.quota.service.js
  Exports assertCanExecute({ orgId, organization, user, resource, action })
  with full dual-mode support (legacy quota + meter mode), admin bypass,
  meterExempt bypass, fail-closed statuses, past_due grace period, and
  plan-not-configured 503 guard.

- Refactor: modules/billing/middlewares/billing.requireQuota.js
  Now a thin wrapper: calls assertCanExecute, maps AppError status codes
  to HTTP responses, sets res.locals.billingDegraded on degraded mode.
  Behavior is unchanged — pure structural refactor.

- New: modules/billing/tests/billing.quota.service.unit.tests.js
  Direct service-layer unit tests (bypasses / meter mode / legacy mode).

- Updated: modules/billing/tests/billing.quota.unit.tests.js
  Adapted to mock BillingQuotaService at the service boundary instead of
  the middleware's former inline dependencies. All scenario coverage kept.

Pure refactor. Trawl has been running this extraction in production for
weeks. After merge + /update-stack, Trawl's billing.requireQuota.js
becomes byte-compatible with Devkit's. All downstreams inherit the
service-layer extraction on their next /update-stack.
Copilot AI review requested due to automatic review settings May 31, 2026 19:13
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 31, 2026

Warning

Review limit reached

@PierreBrisorgueil, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 7 minutes and 48 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 @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

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 configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: fc7c10dc-cc7b-480e-8883-8abf97b53b65

📥 Commits

Reviewing files that changed from the base of the PR and between 8393489 and f26349e.

📒 Files selected for processing (4)
  • modules/billing/middlewares/billing.requireQuota.js
  • modules/billing/services/billing.quota.service.js
  • modules/billing/tests/billing.quota.service.unit.tests.js
  • modules/billing/tests/billing.quota.unit.tests.js
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/extract-quota-service

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Promotes Trawl's BillingQuotaService.assertCanExecute extraction into Devkit so the quota/meter enforcement logic is reusable beyond the HTTP middleware. Behavior is intended to be byte-identical to the previous middleware: admin/meterExempt bypass, past_due grace period, fail-closed statuses, meter-mode + legacy-mode dual paths, and 503 PLAN_NOT_CONFIGURED guard. The middleware becomes a thin wrapper that maps AppError.status → HTTP responses and propagates degraded to res.locals.

Changes:

  • New BillingQuotaService.assertCanExecute(...) containing the full enforcement tree, throwing AppError with status 402/429/503 on denial
  • billing.requireQuota middleware reduced to organization guard + service call + AppError→HTTP mapping
  • Middleware tests retargeted to mock the service boundary; new dedicated service-layer unit tests added

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.

File Description
modules/billing/services/billing.quota.service.js New service with the extracted quota/meter enforcement logic
modules/billing/middlewares/billing.requireQuota.js Reduced to guard + service call + status→HTTP mapping (degraded flag preserved)
modules/billing/tests/billing.quota.service.unit.tests.js New direct service-layer unit tests covering both modes and bypass paths
modules/billing/tests/billing.quota.unit.tests.js Updated to mock BillingQuotaService and assert HTTP mapping; legacy nested describe retained but no longer mode-specific

Comment thread modules/billing/tests/billing.quota.unit.tests.js
Comment thread modules/billing/middlewares/billing.requireQuota.js Outdated
Comment thread modules/billing/services/billing.quota.service.js
@codecov
Copy link
Copy Markdown

codecov Bot commented May 31, 2026

Codecov Report

❌ Patch coverage is 98.68421% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 90.13%. Comparing base (8393489) to head (f26349e).

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #3749      +/-   ##
==========================================
+ Coverage   90.11%   90.13%   +0.01%     
==========================================
  Files         149      150       +1     
  Lines        4936     4956      +20     
  Branches     1564     1573       +9     
==========================================
+ Hits         4448     4467      +19     
- Misses        383      384       +1     
  Partials      105      105              
Flag Coverage Δ
integration 59.42% <0.00%> (-0.25%) ⬇️
unit 68.84% <98.68%> (+0.10%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.


Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8393489...f26349e. Read the comment docs.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

…tency

- 402 catch-all in requireQuota middleware no longer surfaces
  err.message verbatim. The service only throws known types today
  (PAYMENT_PAST_DUE / METER_EXHAUSTED) which are mapped explicitly
  above; an unknown 402 sub-type added later would leak the message.
  Send a generic "Payment required" phrase instead — new sub-types
  must be mapped explicitly above the catch-all.
- billing.quota.service.js: normalize the null-check style on the
  active plan lookup. Both branches treat "plan not configured" the
  same way; pick the !activePlan shape to match the freePlan branch
  above.

All 1666 billing unit tests pass.
@PierreBrisorgueil PierreBrisorgueil merged commit 1230870 into master Jun 1, 2026
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants