From 72e7033b09684f1f493ee757d8e9c39026083eda Mon Sep 17 00:00:00 2001 From: Pierre Brisorgueil Date: Tue, 7 Jul 2026 10:04:17 +0200 Subject: [PATCH 1/3] fix(billing): credit signupGrant on the generic org-creation path 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- refId. Claude-Session: https://claude.ai/code/session_01S8zb8xNiyALVu366vzyi8x --- ...0-backfill-missing-signup-grant-credits.js | 136 ++++++++++++++++++ .../services/organizations.crud.service.js | 9 ++ .../organizations.crud.grant.unit.tests.js | 89 ++++++++++++ 3 files changed, 234 insertions(+) create mode 100644 modules/billing/migrations/20260707100000-backfill-missing-signup-grant-credits.js create mode 100644 modules/organizations/tests/organizations.crud.grant.unit.tests.js diff --git a/modules/billing/migrations/20260707100000-backfill-missing-signup-grant-credits.js b/modules/billing/migrations/20260707100000-backfill-missing-signup-grant-credits.js new file mode 100644 index 000000000..a63b8bc6b --- /dev/null +++ b/modules/billing/migrations/20260707100000-backfill-missing-signup-grant-credits.js @@ -0,0 +1,136 @@ +/** + * Migration: Backfill the one-shot signupGrant to plan orgs that were created + * without it. + * + * The signupGrant was historically credited only on the invite/verify + * org-creation path (organizations.service.js::createOrganizationForUser). The + * generic path behind POST /api/organizations (organizations.crud.service.js) + * never credited it, so orgs created that way — e.g. via a manual "create + * workspace" screen — started at 0 balance. The code gap is fixed alongside + * this migration; this repairs already-affected orgs. + * + * Config-driven: for every plan in config.billing.planDefinitions that defines + * a positive signupGrant, credit that amount to orgs on that plan which have no + * signup_grant ledger entry yet. A project that configures no signupGrant is a + * no-op. This generalises the earlier 20260511 backfill, which hardcoded 500 + * and enumerated the `subscriptions` collection (missing free orgs that have no + * subscription document); this one enumerates the `organizations` collection. + * + * Idempotent: the synthetic key `signup_grant-` stored as `ledger[].refId` + * is the exact key `creditGrant` uses at signup time, so an org already credited + * (at signup, by the earlier migration, or by a prior run of this one) is skipped. + * + * The `organization` field on billingextrabalances is stored as a STRING at + * runtime (see billing.extraBalance schema / repository), so every lookup and + * write here uses `org._id.toString()` to stay consistent with app writes. + * + * Safe to run while the app is live: each updateOne is a single-document atomic + * write and the migration runner serialises execution via a DB-level claim. + */ +import mongoose from 'mongoose'; +import config from '../../../config/index.js'; + +const GRANT_SOURCE = 'signup_grant'; + +/** + * @returns {Promise} + */ +export async function up() { + const db = mongoose.connection.db; + const organizations = db.collection('organizations'); + const extraBalances = db.collection('billingextrabalances'); + + // planId -> signupGrant amount, for plans that define a positive grant. + const grantByPlan = new Map(); + for (const def of config?.billing?.planDefinitions ?? []) { + if (def?.planId && typeof def.signupGrant === 'number' && def.signupGrant > 0) { + grantByPlan.set(def.planId, def.signupGrant); + } + } + + if (grantByPlan.size === 0) { + console.info('[migration] backfill-signup-grant: no plan defines a signupGrant — nothing to do'); + return; + } + + let granted = 0; + let skipped = 0; + + const cursor = organizations.find( + { plan: { $in: [...grantByPlan.keys()] } }, + { projection: { _id: 1, plan: 1 } }, + ); + + for await (const org of cursor) { + const orgId = org._id?.toString(); + const amount = grantByPlan.get(org.plan); + if (!orgId || !amount) { skipped += 1; continue; } + + const idempotencyKey = `${GRANT_SOURCE}-${orgId}`; + + // Skip if this org already has a signup_grant entry (idempotent re-run). + const existing = await extraBalances.findOne( + { organization: orgId, 'ledger.refId': idempotencyKey }, + { projection: { _id: 1 } }, + ); + if (existing) { skipped += 1; continue; } + + // Step 1: ensure the ExtraBalance document exists (no-op if already present). + await extraBalances.updateOne( + { organization: orgId }, + { + $setOnInsert: { + organization: orgId, + ledger: [], + cachedBalance: 0, + cachedBalanceAt: new Date(), + createdAt: new Date(), + updatedAt: new Date(), + }, + }, + { upsert: true }, + ); + + // Step 2: push the grant entry (idempotency-guarded, no upsert). + const result = await extraBalances.updateOne( + { organization: orgId, 'ledger.refId': { $ne: idempotencyKey } }, + { + $push: { + ledger: { + kind: 'topup', + amount, + source: GRANT_SOURCE, + refId: idempotencyKey, + at: new Date(), + }, + }, + $inc: { cachedBalance: amount }, + $set: { cachedBalanceAt: new Date(), updatedAt: new Date() }, + }, + ); + + if (result.modifiedCount > 0) { + granted += 1; + } else { + // Another concurrent writer beat us to this org — harmless race, already credited. + skipped += 1; + } + } + + console.info(`[migration] backfill-signup-grant: complete — granted=${granted} skipped=${skipped}`); +} + +/** + * Reverse: intentional no-op. + * + * This migration shares the `signup_grant` refId scheme with the app's runtime + * grant and the earlier 20260511 backfill, so signup_grant ledger entries cannot + * be attributed to this migration specifically. A blanket $pull would revert + * legitimately-earned grants too. Reverting is left to a manual, audited + * operation if ever required. + * + * @returns {Promise} + */ +export async function down() { + console.warn('[migration] backfill-signup-grant DOWN: intentional no-op — signup_grant entries are not attributable to this migration; revert manually if required.'); +} diff --git a/modules/organizations/services/organizations.crud.service.js b/modules/organizations/services/organizations.crud.service.js index bf6ea955b..9f109b1e9 100644 --- a/modules/organizations/services/organizations.crud.service.js +++ b/modules/organizations/services/organizations.crud.service.js @@ -23,6 +23,7 @@ const normalizeDomain = (value = '') => value.trim().toLowerCase(); import OrganizationsRepository from '../repositories/organizations.repository.js'; import MembershipRepository from '../repositories/organizations.membership.repository.js'; import UserService from '../../users/services/users.service.js'; +import BillingSignupGrantService from '../../billing/services/billing.signupGrant.service.js'; import { slugify } from '../helpers/organizations.slug.js'; import { MEMBERSHIP_STATUSES, MEMBERSHIP_ROLES } from '../lib/constants.js'; import { runOrganizationRemovedHandlers } from '../lib/orgRemoval.registry.js'; @@ -120,6 +121,14 @@ const create = async (body, user) => { throw err; } + // Best-effort signup grant — mirrors organizations.service.js::createOrganizationForUser. + // Every org-creation path (including this generic one behind POST /api/organizations) + // must credit the configured one-shot signupGrant, else a fresh org on a plan that + // defines one starts at 0 balance. Called outside the rollback try/catch so a billing + // failure never rolls back the org; grantOnSignup never throws and is idempotent + // (refId signup_grant-). No-op when the plan defines no signupGrant. + await BillingSignupGrantService.grantOnSignup({ orgId: result._id.toString(), planId: result.plan || 'free' }); + return result; }; diff --git a/modules/organizations/tests/organizations.crud.grant.unit.tests.js b/modules/organizations/tests/organizations.crud.grant.unit.tests.js new file mode 100644 index 000000000..d85fa35ad --- /dev/null +++ b/modules/organizations/tests/organizations.crud.grant.unit.tests.js @@ -0,0 +1,89 @@ +/** + * Module dependencies. + */ +import { jest, describe, test, expect, beforeEach } from '@jest/globals'; + +/** + * Unit tests — verify organizations.crud.service.create() credits the one-shot + * signup grant on the generic POST /api/organizations path, mirroring the + * invite/verify path (organizations.service.js::createOrganizationForUser). A + * fresh org on a plan that defines a signupGrant must not start at 0 balance. + */ + +const mockGrantOnSignup = jest.fn().mockResolvedValue({ applied: true }); +jest.unstable_mockModule('../../billing/services/billing.signupGrant.service.js', () => ({ + default: { grantOnSignup: mockGrantOnSignup }, +})); + +const mockOrgCreate = jest.fn().mockResolvedValue({ _id: 'org1', plan: 'free' }); +const mockOrgRemove = jest.fn().mockResolvedValue({}); +jest.unstable_mockModule('../repositories/organizations.repository.js', () => ({ + default: { + create: mockOrgCreate, + findOne: jest.fn().mockResolvedValue(null), + remove: mockOrgRemove, + list: jest.fn().mockResolvedValue([]), + update: jest.fn(), + get: jest.fn(), + exists: jest.fn().mockResolvedValue(false), + }, +})); + +const mockMembershipCreate = jest.fn().mockResolvedValue({ _id: 'm1' }); +jest.unstable_mockModule('../repositories/organizations.membership.repository.js', () => ({ + default: { + create: mockMembershipCreate, + deleteMany: jest.fn(), + list: jest.fn().mockResolvedValue([]), + findOne: jest.fn().mockResolvedValue(null), + update: jest.fn(), + remove: jest.fn(), + count: jest.fn(), + }, +})); + +jest.unstable_mockModule('../../users/services/users.service.js', () => ({ + default: { + updateById: jest.fn().mockResolvedValue({}), + findWithFilter: jest.fn().mockResolvedValue([]), + getBrut: jest.fn(), + }, +})); + +jest.unstable_mockModule('../../../lib/helpers/emailVerification.js', () => ({ + assertEmailVerified: jest.fn(), +})); + +jest.unstable_mockModule('../../../lib/services/logger.js', () => ({ + default: { error: jest.fn(), warn: jest.fn(), info: jest.fn() }, +})); + +jest.unstable_mockModule('../../../config/index.js', () => ({ + default: { organizations: { enabled: false } }, +})); + +const { default: OrgCrudService } = await import('../services/organizations.crud.service.js'); + +describe('organizations.crud.service.create signup grant:', () => { + beforeEach(() => { + mockGrantOnSignup.mockClear(); + mockOrgCreate.mockResolvedValue({ _id: 'org1', plan: 'free' }); + }); + + test('credits the one-shot signup grant for the freshly created org', async () => { + const user = { id: 'u1', _id: 'u1', email: 'a@b.com', emailVerified: true }; + const result = await OrgCrudService.create({ name: 'Test Org' }, user); + + expect(result).toEqual({ _id: 'org1', plan: 'free' }); + expect(mockGrantOnSignup).toHaveBeenCalledTimes(1); + expect(mockGrantOnSignup).toHaveBeenCalledWith({ orgId: 'org1', planId: 'free' }); + }); + + test('defaults planId to free when the created org has no plan field', async () => { + mockOrgCreate.mockResolvedValueOnce({ _id: 'org2' }); + const user = { id: 'u1', _id: 'u1', email: 'a@b.com', emailVerified: true }; + await OrgCrudService.create({ name: 'No Plan Org' }, user); + + expect(mockGrantOnSignup).toHaveBeenCalledWith({ orgId: 'org2', planId: 'free' }); + }); +}); From 6dd619bc2a256e26a5dd0392081ce2dd6d82898a Mon Sep 17 00:00:00 2001 From: Pierre Brisorgueil Date: Tue, 7 Jul 2026 10:38:04 +0200 Subject: [PATCH 2/3] fix(billing): route the signup-grant backfill through the runtime grant path MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- ...0-backfill-missing-signup-grant-credits.js | 135 +++++++----------- ...backfill-signup-grant.integration.tests.js | 104 ++++++++++++++ 2 files changed, 152 insertions(+), 87 deletions(-) create mode 100644 modules/billing/tests/billing.migration.backfill-signup-grant.integration.tests.js diff --git a/modules/billing/migrations/20260707100000-backfill-missing-signup-grant-credits.js b/modules/billing/migrations/20260707100000-backfill-missing-signup-grant-credits.js index a63b8bc6b..7c261c07f 100644 --- a/modules/billing/migrations/20260707100000-backfill-missing-signup-grant-credits.js +++ b/modules/billing/migrations/20260707100000-backfill-missing-signup-grant-credits.js @@ -1,6 +1,5 @@ /** - * Migration: Backfill the one-shot signupGrant to plan orgs that were created - * without it. + * Migration: Backfill the one-shot signupGrant to plan orgs created without it. * * The signupGrant was historically credited only on the invite/verify * org-creation path (organizations.service.js::createOrganizationForUser). The @@ -9,115 +8,77 @@ * workspace" screen — started at 0 balance. The code gap is fixed alongside * this migration; this repairs already-affected orgs. * - * Config-driven: for every plan in config.billing.planDefinitions that defines - * a positive signupGrant, credit that amount to orgs on that plan which have no - * signup_grant ledger entry yet. A project that configures no signupGrant is a - * no-op. This generalises the earlier 20260511 backfill, which hardcoded 500 - * and enumerated the `subscriptions` collection (missing free orgs that have no - * subscription document); this one enumerates the `organizations` collection. + * Delegates to BillingSignupGrantService.grantOnSignup per org so it goes + * through the EXACT runtime grant path rather than re-implementing the ledger + * write. That reuse is deliberate — it inherits, for free and consistently: + * - ObjectId casting: the repository writes via the Mongoose model, whose + * `organization` field is Schema.ObjectId, so the string orgId is cast to a + * real ObjectId (a raw-driver write keyed on a string would create orphan, + * app-invisible documents that never match existing ones). + * - the plan-definition co-presence guard (BillingPlanService.getActivePlan + * refuses a signupGrant configured without oneShot). + * - Zod amount validation (creditGrant → ExtraBalanceCreditGrant.parse). + * - idempotency via the synthetic refId `signup_grant-`, so an org + * already credited (at signup, by the earlier 20260511 backfill, or by a + * prior run of this one) is a no-op. * - * Idempotent: the synthetic key `signup_grant-` stored as `ledger[].refId` - * is the exact key `creditGrant` uses at signup time, so an org already credited - * (at signup, by the earlier migration, or by a prior run of this one) is skipped. + * Config-driven: only orgs on a plan that defines a positive signupGrant are + * touched; a project that configures none is a no-op. This generalises the + * earlier 20260511 backfill, which hardcoded 500 and enumerated the + * `subscriptions` collection (missing free orgs with no subscription document); + * this one enumerates the `organizations` collection. * - * The `organization` field on billingextrabalances is stored as a STRING at - * runtime (see billing.extraBalance schema / repository), so every lookup and - * write here uses `org._id.toString()` to stay consistent with app writes. - * - * Safe to run while the app is live: each updateOne is a single-document atomic - * write and the migration runner serialises execution via a DB-level claim. + * Safe to run while the app is live: each grant is a single-document atomic + * ledger push and the migration runner serialises execution via a DB-level claim. */ import mongoose from 'mongoose'; import config from '../../../config/index.js'; - -const GRANT_SOURCE = 'signup_grant'; +import BillingSignupGrantService from '../services/billing.signupGrant.service.js'; /** * @returns {Promise} */ export async function up() { - const db = mongoose.connection.db; - const organizations = db.collection('organizations'); - const extraBalances = db.collection('billingextrabalances'); - - // planId -> signupGrant amount, for plans that define a positive grant. - const grantByPlan = new Map(); - for (const def of config?.billing?.planDefinitions ?? []) { - if (def?.planId && typeof def.signupGrant === 'number' && def.signupGrant > 0) { - grantByPlan.set(def.planId, def.signupGrant); - } - } + // Plans that define a positive signupGrant — the only orgs worth scanning. + const grantPlanIds = (config?.billing?.planDefinitions ?? []) + .filter((def) => def?.planId && typeof def.signupGrant === 'number' && def.signupGrant > 0) + .map((def) => def.planId); - if (grantByPlan.size === 0) { + if (grantPlanIds.length === 0) { console.info('[migration] backfill-signup-grant: no plan defines a signupGrant — nothing to do'); return; } - let granted = 0; - let skipped = 0; - + const organizations = mongoose.connection.db.collection('organizations'); const cursor = organizations.find( - { plan: { $in: [...grantByPlan.keys()] } }, + { plan: { $in: grantPlanIds } }, { projection: { _id: 1, plan: 1 } }, ); - for await (const org of cursor) { - const orgId = org._id?.toString(); - const amount = grantByPlan.get(org.plan); - if (!orgId || !amount) { skipped += 1; continue; } - - const idempotencyKey = `${GRANT_SOURCE}-${orgId}`; - - // Skip if this org already has a signup_grant entry (idempotent re-run). - const existing = await extraBalances.findOne( - { organization: orgId, 'ledger.refId': idempotencyKey }, - { projection: { _id: 1 } }, - ); - if (existing) { skipped += 1; continue; } - - // Step 1: ensure the ExtraBalance document exists (no-op if already present). - await extraBalances.updateOne( - { organization: orgId }, - { - $setOnInsert: { - organization: orgId, - ledger: [], - cachedBalance: 0, - cachedBalanceAt: new Date(), - createdAt: new Date(), - updatedAt: new Date(), - }, - }, - { upsert: true }, - ); + let granted = 0; + let skipped = 0; + let failed = 0; - // Step 2: push the grant entry (idempotency-guarded, no upsert). - const result = await extraBalances.updateOne( - { organization: orgId, 'ledger.refId': { $ne: idempotencyKey } }, - { - $push: { - ledger: { - kind: 'topup', - amount, - source: GRANT_SOURCE, - refId: idempotencyKey, - at: new Date(), - }, - }, - $inc: { cachedBalance: amount }, - $set: { cachedBalanceAt: new Date(), updatedAt: new Date() }, - }, - ); + for await (const org of cursor) { + // grantOnSignup is idempotent (refId) and never throws — reuses the runtime + // path so ObjectId casting + validation + co-presence guard all apply. + const result = await BillingSignupGrantService.grantOnSignup({ + orgId: org._id.toString(), + planId: org.plan, + }); - if (result.modifiedCount > 0) { - granted += 1; - } else { - // Another concurrent writer beat us to this org — harmless race, already credited. + if (result == null) { + // Plan has no exposed grant (co-presence guard) or a swallowed error — logged by the service. + failed += 1; + } else if (result.applied === false) { + // Idempotent no-op — org already had a signup_grant entry. skipped += 1; + } else { + granted += 1; } } - console.info(`[migration] backfill-signup-grant: complete — granted=${granted} skipped=${skipped}`); + console.info(`[migration] backfill-signup-grant: complete — granted=${granted} skipped=${skipped} failed=${failed}`); } /** @@ -125,7 +86,7 @@ export async function up() { * * This migration shares the `signup_grant` refId scheme with the app's runtime * grant and the earlier 20260511 backfill, so signup_grant ledger entries cannot - * be attributed to this migration specifically. A blanket $pull would revert + * be attributed to this migration specifically. A blanket removal would revert * legitimately-earned grants too. Reverting is left to a manual, audited * operation if ever required. * diff --git a/modules/billing/tests/billing.migration.backfill-signup-grant.integration.tests.js b/modules/billing/tests/billing.migration.backfill-signup-grant.integration.tests.js new file mode 100644 index 000000000..0c35a487d --- /dev/null +++ b/modules/billing/tests/billing.migration.backfill-signup-grant.integration.tests.js @@ -0,0 +1,104 @@ +/** + * Module dependencies. + */ +import mongoose from 'mongoose'; +import { describe, beforeAll, afterEach, afterAll, test, expect } from '@jest/globals'; + +import mongooseService from '../../../lib/services/mongoose.js'; +import config from '../../../config/index.js'; +import { up as backfillSignupGrants } from '../migrations/20260707100000-backfill-missing-signup-grant-credits.js'; + +/** + * Integration tests for the signup-grant backfill migration. + * + * Regression guard for the string-vs-ObjectId bug: billingextrabalances.organization + * is Schema.ObjectId, so the migration MUST credit orgs through a path that casts to + * ObjectId (it delegates to grantOnSignup → the Mongoose repository) — a raw-driver + * write keyed on a string would create orphan, app-invisible ghost documents. + */ +describe('backfill-missing-signup-grant migration (integration):', () => { + let organizations; + let extraBalances; + let grantPlanId; + let grantAmount; + let createdOrgIds = []; + + /** + * Seed an organization on the given plan and track it for cleanup. + * @param {string} plan - The plan id to set on the org. + * @returns {Promise} The new org ObjectId. + */ + const seedOrg = async (plan) => { + const _id = new mongoose.Types.ObjectId(); + await organizations.insertOne({ _id, name: `IT ${_id.toString()}`, plan }); + createdOrgIds.push(_id); + return _id; + }; + + beforeAll(async () => { + await mongooseService.loadModels(); + await mongooseService.connect(); + organizations = mongoose.connection.db.collection('organizations'); + extraBalances = mongoose.connection.db.collection('billingextrabalances'); + const def = (config?.billing?.planDefinitions ?? []).find( + (d) => typeof d.signupGrant === 'number' && d.signupGrant > 0, + ); + grantPlanId = def.planId; + grantAmount = def.signupGrant; + }); + + afterEach(async () => { + if (createdOrgIds.length) { + await organizations.deleteMany({ _id: { $in: createdOrgIds } }); + await extraBalances.deleteMany({ organization: { $in: createdOrgIds } }); + createdOrgIds = []; + } + }); + + afterAll(async () => { + await mongooseService.disconnect(); + }); + + test('credits the configured grant to an ObjectId-keyed org missing it, readable by ObjectId', async () => { + const orgId = await seedOrg(grantPlanId); + + await backfillSignupGrants(); + + // Stored + readable keyed on the ObjectId — the critical-fix assertion. + const eb = await extraBalances.findOne({ organization: orgId }); + expect(eb).not.toBeNull(); + expect(eb.cachedBalance).toBe(grantAmount); + const grant = (eb.ledger || []).find((e) => e.source === 'signup_grant'); + expect(grant).toBeDefined(); + expect(grant.amount).toBe(grantAmount); + expect(grant.refId).toBe(`signup_grant-${orgId.toString()}`); + // A string-keyed lookup must NOT find a duplicate ghost document. + expect(await extraBalances.findOne({ organization: orgId.toString() })).toBeNull(); + }); + + test('is idempotent — a second run does not double-credit', async () => { + const orgId = await seedOrg(grantPlanId); + + await backfillSignupGrants(); + await backfillSignupGrants(); + + const docs = await extraBalances.find({ organization: orgId }).toArray(); + expect(docs).toHaveLength(1); + expect(docs[0].cachedBalance).toBe(grantAmount); + const grants = (docs[0].ledger || []).filter((e) => e.source === 'signup_grant'); + expect(grants).toHaveLength(1); + }); + + test('skips an org that already holds a signup_grant entry', async () => { + const orgId = await seedOrg(grantPlanId); + const repo = (await import('../repositories/billing.extraBalance.repository.js')).default; + await repo.creditGrant(orgId.toString(), grantAmount, 'signup_grant'); + + await backfillSignupGrants(); + + const docs = await extraBalances.find({ organization: orgId }).toArray(); + expect(docs).toHaveLength(1); + const grants = (docs[0].ledger || []).filter((e) => e.source === 'signup_grant'); + expect(grants).toHaveLength(1); + }); +}); From 04c2512aa47c6f8a11f7f6d6729c4750efddcd9d Mon Sep 17 00:00:00 2001 From: Pierre Brisorgueil Date: Tue, 7 Jul 2026 11:11:38 +0200 Subject: [PATCH 3/3] test(billing): assert grant is skipped when membership creation fails 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 --- .../tests/organizations.crud.grant.unit.tests.js | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/modules/organizations/tests/organizations.crud.grant.unit.tests.js b/modules/organizations/tests/organizations.crud.grant.unit.tests.js index d85fa35ad..aa747a07d 100644 --- a/modules/organizations/tests/organizations.crud.grant.unit.tests.js +++ b/modules/organizations/tests/organizations.crud.grant.unit.tests.js @@ -86,4 +86,12 @@ describe('organizations.crud.service.create signup grant:', () => { expect(mockGrantOnSignup).toHaveBeenCalledWith({ orgId: 'org2', planId: 'free' }); }); + + test('does not credit the grant when membership creation fails (rollback exits before the grant)', async () => { + mockMembershipCreate.mockRejectedValueOnce(new Error('membership boom')); + const user = { id: 'u1', _id: 'u1', email: 'a@b.com', emailVerified: true }; + + await expect(OrgCrudService.create({ name: 'Rollback Org' }, user)).rejects.toThrow(); + expect(mockGrantOnSignup).not.toHaveBeenCalled(); + }); });