Skip to content

fix(billing): credit signupGrant on the generic org-creation path#3949

Merged
PierreBrisorgueil merged 3 commits into
masterfrom
fix/signup-grant-crud-path
Jul 7, 2026
Merged

fix(billing): credit signupGrant on the generic org-creation path#3949
PierreBrisorgueil merged 3 commits into
masterfrom
fix/signup-grant-crud-path

Conversation

@PierreBrisorgueil

@PierreBrisorgueil PierreBrisorgueil commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

What

createOrganizationForUser (invite/verify signup) credited the configured signupGrant, but the generic POST /api/organizations path (organizations.crud.service.create), used by a manual "create workspace" flow, did not. A fresh org created there started at 0 balance on a plan that defines a grant.

Changes

  • organizations.crud.service.create: call the existing idempotent BillingSignupGrantService.grantOnSignup, best-effort, outside the rollback try/catch (mirrors organizations.service.js::createOrganizationForUser).
  • New config-driven backfill migration: for every plan in planDefinitions with a positive signupGrant, credit orgs on that plan lacking a signup_grant ledger entry. Generalises the 20260511 backfill (hardcoded 500, keyed off subscriptions, missing orgs with no subscription doc) by enumerating organizations and reading the configured amount. Idempotent via signup_grant-<orgId>.
  • Unit test for the grant call on crud.create.

Notes

  • Grant is best-effort (never throws, never rolls back the org).
  • Migration down() is an intentional no-op — signup_grant entries aren't attributable to this migration; a blanket $pull would revert legitimately-earned grants.

https://claude.ai/code/session_01S8zb8xNiyALVu366vzyi8x

Summary by CodeRabbit

  • New Features

    • Organizations created through the app now automatically receive any eligible signup credit tied to their plan.
    • Missing signup credits can be backfilled for existing organizations so balances are corrected consistently.
  • Bug Fixes

    • Prevents duplicate signup credits from being applied more than once.
    • Organizations without a plan now use the default free plan when determining signup credit eligibility.

createOrganizationForUser credited the one-shot signupGrant, but the generic
POST /api/organizations path (organizations.crud.service.create) did not, so an
org created there (e.g. via a manual "create workspace" screen) started at 0
balance on a plan that defines a grant. Call the existing idempotent
BillingSignupGrantService.grantOnSignup from crud.create too, best-effort and
outside the rollback try/catch.

Add a config-driven backfill migration: for every plan in planDefinitions with a
positive signupGrant, credit orgs on that plan that lack a signup_grant ledger
entry. Generalises the 20260511 backfill (which hardcoded 500 and enumerated the
subscriptions collection, missing orgs with no subscription document) by
enumerating organizations and reading the configured amount. Idempotent via the
signup_grant-<orgId> refId.

Claude-Session: https://claude.ai/code/session_01S8zb8xNiyALVu366vzyi8x
@coderabbitai

coderabbitai Bot commented Jul 7, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

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

Next review available in: 6 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

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.

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 configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 5c4c83af-4564-40d4-8630-afdd9d2d3d1b

📥 Commits

Reviewing files that changed from the base of the PR and between 72e7033 and 04c2512.

📒 Files selected for processing (3)
  • modules/billing/migrations/20260707100000-backfill-missing-signup-grant-credits.js
  • modules/billing/tests/billing.migration.backfill-signup-grant.integration.tests.js
  • modules/organizations/tests/organizations.crud.grant.unit.tests.js

Walkthrough

This PR adds a database migration to backfill missing signup_grant ledger entries for organizations under plans with a positive signupGrant, and wires a post-creation call to BillingSignupGrantService.grantOnSignup into the organization creation flow, with accompanying unit tests.

Changes

Signup Grant Backfill and Wiring

Layer / File(s) Summary
Backfill migration for signup grant entries
modules/billing/migrations/20260707100000-backfill-missing-signup-grant-credits.js
Adds up() to build a plan-to-signupGrant map, iterate matching organizations, and idempotently upsert billingextrabalances ledger entries keyed by signup_grant-<orgId>; down() is a deliberate no-op with a warning.
Grant-on-signup wiring in org creation
modules/organizations/services/organizations.crud.service.js
Imports BillingSignupGrantService and invokes grantOnSignup with the new org id and plan (defaulting to 'free') after membership setup, outside the rollback path.
Unit tests for grant wiring
modules/organizations/tests/organizations.crud.grant.unit.tests.js
Adds Jest tests verifying grantOnSignup is called once with { orgId, planId }, including the default 'free' planId case when plan is absent.

Estimated code review effort: 2 (Simple) | ~15 minutes

Sequence Diagram(s)

sequenceDiagram
    participant Client
    participant OrgCrudService
    participant MembershipsRepo
    participant BillingSignupGrantService

    Client->>OrgCrudService: create(orgData)
    OrgCrudService->>MembershipsRepo: create owner membership / set current org
    MembershipsRepo-->>OrgCrudService: success
    OrgCrudService->>BillingSignupGrantService: grantOnSignup(orgId, planId)
    BillingSignupGrantService-->>OrgCrudService: best-effort result (no rollback)
    OrgCrudService-->>Client: created organization
Loading

Related PRs: None identified.

Suggested labels: billing, organizations, migration, tests

Suggested reviewers: None identified.

🐰 A grant hops in when orgs are born,
A migration mends the credits torn,
Ledger keys keep double-grants at bay,
Tests confirm the free plan finds its way.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Description check ⚠️ Warning The description covers the change, but it omits required template sections like Scope, Validation, Guardrails, and Related issues. Add the missing template sections with module impact, risk level, validation steps, guardrails, and any related issue references.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main change: crediting signup grants on the generic organization creation path.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/signup-grant-crud-path

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.

@codecov

codecov Bot commented Jul 7, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 92.70%. Comparing base (997c51d) to head (04c2512).
⚠️ Report is 1 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #3949   +/-   ##
=======================================
  Coverage   92.70%   92.70%           
=======================================
  Files         169      169           
  Lines        5562     5563    +1     
  Branches     1789     1791    +2     
=======================================
+ Hits         5156     5157    +1     
  Misses        326      326           
  Partials       80       80           
Flag Coverage Δ
integration 61.04% <0.00%> (+<0.01%) ⬆️
unit 74.40% <100.00%> (+<0.01%) ⬆️

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


Continue to review full report in Codecov by Harness.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 997c51d...04c2512. 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.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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/migrations/20260707100000-backfill-missing-signup-grant-credits.js`:
- Around line 64-118: The migration’s redundant `existing` pre-check in the `for
await (const org of cursor)` loop is causing an extra DB round trip per
organization. Remove the `extraBalances.findOne` lookup and rely on the atomic
`extraBalances.updateOne` with the `'ledger.refId': { $ne: idempotencyKey }`
guard plus `result.modifiedCount` to handle both existing grants and concurrent
writers in `backfill-missing-signup-grant-credits.js`.

In `@modules/organizations/tests/organizations.crud.grant.unit.tests.js`:
- Around line 67-89: Add a negative-path test in
organizations.crud.grant.unit.tests.js to verify the signup grant is not issued
when org/membership setup fails. Extend the existing OrgCrudService.create
coverage by mocking a failure in MembershipRepository.create or
UserService.updateById, then assert that grantOnSignup is never called and the
create call propagates/handles the error as expected. Use the existing
OrgCrudService.create and mockGrantOnSignup setup so the new test clearly checks
the rollback path before the grant step.
🪄 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

Run ID: d6715605-68ff-48f9-a1c3-0b7b03ecb336

📥 Commits

Reviewing files that changed from the base of the PR and between 997c51d and 72e7033.

📒 Files selected for processing (3)
  • modules/billing/migrations/20260707100000-backfill-missing-signup-grant-credits.js
  • modules/organizations/services/organizations.crud.service.js
  • modules/organizations/tests/organizations.crud.grant.unit.tests.js

Comment thread modules/organizations/tests/organizations.crud.grant.unit.tests.js
…nt path

The migration re-implemented the ledger write with the raw driver keyed on a
STRING organization, but billingextrabalances.organization is Schema.ObjectId and
every repository write casts to ObjectId — the string write would have created
orphan, app-invisible ghost documents and silently credited no one while logging
success.

Delegate to BillingSignupGrantService.grantOnSignup per org so the migration
reuses the exact runtime grant path: Mongoose ObjectId casting, the plan
co-presence guard (getActivePlan), Zod amount validation (creditGrant), and refId
idempotency. Add an integration test proving an ObjectId-keyed org is credited
(no string ghost doc), idempotent re-runs, and already-granted orgs are skipped.

Resolves the critical + two mediums raised in pre-merge review.

Claude-Session: https://claude.ai/code/session_01S8zb8xNiyALVu366vzyi8x
Cover the rollback path: when MembershipRepository.create throws, create()
re-throws before reaching the grant call, so grantOnSignup must never fire.
Locks in the placement of the best-effort grant after the rollback try/catch.

Claude-Session: https://claude.ai/code/session_01S8zb8xNiyALVu366vzyi8x
@PierreBrisorgueil PierreBrisorgueil merged commit 7c7c0bf into master Jul 7, 2026
6 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.

1 participant