Skip to content

Commit 2edb8e4

Browse files
refactor(organizations): emit organization.created and move signup grant to a billing subscriber (#3957)
* refactor(organizations): emit organization.created and move signup grant to a billing subscriber Organizations previously imported BillingSignupGrantService directly from two call sites (organizations.crud.service.js::create, added #3949, and the older organizations.service.js::createOrganizationForUser, since #3663), making billing<->organizations coupling bidirectional and breaking the module- removability guarantee. Adds a creation-side seam mirroring the existing organization.provisioned event: both call sites now emit `organization.created` ({ orgId, planId }) on the organizations events singleton instead of calling billing directly. billing.init.js subscribes next to the existing organization.provisioned listener and credits the one-shot signupGrant, following the same fire-and-forget / self-guarded pattern (listener errors swallowed+logged, grant failure never rolls back org creation, refId idempotence unchanged). Adapts organizations.integration.tests.js and billing.referral.integration.tests.js to the new async-listener timing (poll for the ledger entry / wait for it to settle before snapshotting balances), mirroring the existing waitForLedgerEntry pattern already used for the referral grant listener. Closes #3952 Claude-Session: https://claude.ai/code/session_01WfNC8bt1TgL4AsiYgCEGup * test(billing): assert waitForLedgerEntry result in referral integration test CodeRabbit nit on PR #3957 — the admin and referee signup-grant polls in billing.referral.integration.tests.js returned the ledger entry but never asserted it, so a poll timeout would surface as a cryptic downstream balance mismatch instead of a clear not-null failure at the source.
1 parent 5c43bb1 commit 2edb8e4

10 files changed

Lines changed: 237 additions & 73 deletions

modules/billing/billing.init.js

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import billingEvents from './lib/events.js';
99
import invitationEvents from '../invitations/lib/events.js';
1010
import organizationEvents from '../organizations/lib/events.js';
1111
import BillingUsageRepository from './repositories/billing.usage.repository.js';
12+
import BillingSignupGrantService from './services/billing.signupGrant.service.js';
1213
import { getAlertThresholdPercents } from './lib/billing.constants.js';
1314
import { setupBillingEmails } from './billing.email.js';
1415

@@ -132,6 +133,40 @@ export default async (app) => {
132133
}
133134
});
134135

136+
// Signup grant (#3952) — billing is an OPTIONAL consumer of the organizations fire-and-forget
137+
// `organization.created` event (dependency direction billing → organizations, same as
138+
// `organization.provisioned` above: billing imports the events singleton, organizations never
139+
// imports billing). BOTH organization-creation call sites (organizations.crud.service.js::create
140+
// AND organizations.service.js::createOrganizationForUser) emit this instead of calling
141+
// BillingSignupGrantService directly, so organizations stays removable without billing.
142+
// Unlike the two referral listeners above, this one is NOT config-gated — grantOnSignup is core
143+
// signup behavior (it already no-ops when the plan defines no signupGrant).
144+
/**
145+
* @desc One-shot signup grant listener for organization-creation events (#3952).
146+
* Credits the configured signupGrant to the freshly created organization via
147+
* BillingSignupGrantService.grantOnSignup (idempotent via refId `signup_grant-<orgId>`,
148+
* never throws). Self-guarded: `EventEmitter.emit` is synchronous, so the emit-site
149+
* try/catch in organizations only catches SYNC throws — an async rejection escaping
150+
* here would surface as an unhandledRejection. It never does: grantOnSignup already
151+
* swallows its own errors; this wraps it too for defense-in-depth (mirrors the instant
152+
* referee grant listener above).
153+
* @param {{orgId: string, planId: string}} payload - Created organization event payload.
154+
* @returns {Promise<void>} settles when the grant attempt completes (never rejects)
155+
*/
156+
organizationEvents.on('organization.created', async (payload) => {
157+
try {
158+
await BillingSignupGrantService.grantOnSignup({ orgId: payload?.orgId, planId: payload?.planId });
159+
} catch (err) {
160+
// ⚠️ MANDATORY self-guard (see organizations/lib/events.js): never let a rejection escape.
161+
logger.error('[billing] signup grant failed via organization.created listener', {
162+
orgId: String(payload?.orgId ?? ''),
163+
planId: String(payload?.planId ?? ''),
164+
err: err?.message,
165+
stack: err?.stack,
166+
});
167+
}
168+
});
169+
135170
// Update analytics group properties when a subscription plan changes
136171
billingEvents.on('plan.changed', ({ organizationId, newPlan }) => {
137172
try {

modules/billing/tests/billing.init.unit.tests.js

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ describe('billing.init unit tests:', () => {
1818
let mockOrganizationEvents;
1919
let mockUserService;
2020
let mockInvitationRepository;
21+
let mockSignupGrantService;
2122

2223
const mockApp = {};
2324

@@ -94,6 +95,13 @@ describe('billing.init unit tests:', () => {
9495
default: mockInvitationRepository,
9596
}));
9697

98+
// #3952: the organization.created listener delegates to BillingSignupGrantService — stub it
99+
// so the wiring tests stay isolated from the real grant/plan/repository resolution.
100+
mockSignupGrantService = { grantOnSignup: jest.fn().mockResolvedValue({ applied: true }) };
101+
jest.unstable_mockModule('../services/billing.signupGrant.service.js', () => ({
102+
default: mockSignupGrantService,
103+
}));
104+
97105
// Stub billing.email so boot validator tests don't wire real email listeners
98106
jest.unstable_mockModule('../billing.email.js', () => ({
99107
setupBillingEmails: jest.fn(),
@@ -273,6 +281,57 @@ describe('billing.init unit tests:', () => {
273281
});
274282
});
275283

284+
describe('#3952 signup grant listener (organization.created):', () => {
285+
const payload = { orgId: 'org1', planId: 'free' };
286+
287+
/**
288+
* Boot the module and return the registered organization.created handler.
289+
* @returns {Promise<Function>} The wired listener.
290+
*/
291+
const getHandler = async () => {
292+
await billingInit(mockApp);
293+
const createdCall = mockOrganizationEvents.on.mock.calls.find(([evt]) => evt === 'organization.created');
294+
expect(createdCall).toBeDefined();
295+
return createdCall[1];
296+
};
297+
298+
test('wires the listener on the organizations emitter', async () => {
299+
const handler = await getHandler();
300+
expect(typeof handler).toBe('function');
301+
});
302+
303+
test('NOT config-gated — delegates to BillingSignupGrantService.grantOnSignup with no referral config set', async () => {
304+
// mockConfig.billing has NO referral block — unlike the two referral listeners above,
305+
// this one must still fire (grantOnSignup is core signup behavior, not a referral reward).
306+
const handler = await getHandler();
307+
await handler(payload);
308+
expect(mockSignupGrantService.grantOnSignup).toHaveBeenCalledTimes(1);
309+
expect(mockSignupGrantService.grantOnSignup).toHaveBeenCalledWith({ orgId: 'org1', planId: 'free' });
310+
});
311+
312+
test('passes the payload through verbatim (orgId + planId from both org-creation call sites)', async () => {
313+
const handler = await getHandler();
314+
await handler({ orgId: 'org2', planId: 'growth' });
315+
expect(mockSignupGrantService.grantOnSignup).toHaveBeenCalledWith({ orgId: 'org2', planId: 'growth' });
316+
});
317+
318+
test('self-guard: a grant REJECTION is swallowed + logged, never escapes the listener', async () => {
319+
mockSignupGrantService.grantOnSignup.mockRejectedValue(new Error('mongo down'));
320+
const handler = await getHandler();
321+
// The emit-site catch (organizations) is sync-only — the async listener must resolve, not reject.
322+
await expect(handler(payload)).resolves.toBeUndefined();
323+
const errCall = mockLogger.error.mock.calls.find(([msg]) => msg.includes('signup grant failed via organization.created listener'));
324+
expect(errCall).toBeDefined();
325+
expect(errCall[1]).toMatchObject({ orgId: 'org1', planId: 'free', err: 'mongo down' });
326+
});
327+
328+
test('self-guard: even a malformed payload cannot make the listener throw/reject', async () => {
329+
const handler = await getHandler();
330+
await expect(handler(undefined)).resolves.toBeUndefined();
331+
expect(mockSignupGrantService.grantOnSignup).toHaveBeenCalledWith({ orgId: undefined, planId: undefined });
332+
});
333+
});
334+
276335
test('resolves without error when meterMode=true and no legacy docs', async () => {
277336
mockConfig.billing.meterMode = true;
278337
mockConfig.billing.plans = ['free', 'growth', 'pro'];

modules/billing/tests/billing.referral.integration.tests.js

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -103,6 +103,13 @@ describe('Billing referral grant (#3842):', () => {
103103
.send({ email: ADMIN_EMAIL, password: PASSWORD })
104104
.expect(200);
105105

106+
// #3952: the admin's own org creation also fires `organization.created` → billing's
107+
// fire-and-forget signupGrant listener. Wait for it to settle here so every downstream
108+
// balance-delta assertion (referral credits) starts from a stable, fully-settled baseline.
109+
const adminOrgId = String(brut.currentOrganization?._id || brut.currentOrganization);
110+
const adminGrant = await waitForLedgerEntry(adminOrgId, `signup_grant-${adminOrgId}`);
111+
expect(adminGrant).not.toBeNull();
112+
106113
return adminAgent;
107114
}
108115

@@ -165,6 +172,11 @@ describe('Billing referral grant (#3842):', () => {
165172

166173
const inviterBalanceAfter = await BillingExtraBalanceRepository.getBalance(inviterOrgId);
167174
expect(inviterBalanceAfter - inviterBalanceBefore).toBe(REFERRER_UNITS);
175+
// #3952: the referee's own org creation also fires `organization.created` → signupGrant.
176+
// Wait for it to settle so the "unchanged after replay" check below (step 5) has a stable
177+
// snapshot — otherwise a late-landing signup grant could be mistaken for a replay double-credit.
178+
const refereeGrant = await waitForLedgerEntry(refereeOrgId, `signup_grant-${refereeOrgId}`);
179+
expect(refereeGrant).not.toBeNull();
168180
const refereeBalanceAfter = await BillingExtraBalanceRepository.getBalance(refereeOrgId);
169181

170182
// 4. Replay safety (deterministic): a direct service replay is the exact grant path

modules/organizations/lib/events.js

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,20 @@ import { EventEmitter } from 'events';
77
* Singleton emitter for organization events. Config-free / import-safe.
88
*
99
* Events:
10+
* - `organization.created` — emitted (#3952) by BOTH org-creation call sites
11+
* (organizations.crud.service.js::create, the generic POST /api/organizations path, AND
12+
* organizations.service.js::createOrganizationForUser, the signup path) immediately after
13+
* the organization + owner membership are durably created. This is the sanctioned seam for
14+
* billing's one-shot signupGrant (billing.init.js), replacing a direct import of
15+
* BillingSignupGrantService from organizations — the organizations module must stay
16+
* removable without billing. Fire-and-forget; the emit-site try/catch only guards a
17+
* SYNCHRONOUS listener throw — see the `organization.provisioned` note below for the
18+
* async-rejection caveat, which applies identically here (the listener owns its own guard).
19+
* Payload: {
20+
* orgId: String — the freshly created organization's id
21+
* planId: String — the plan to evaluate the grant against (both call sites pass 'free' —
22+
* the only plan a fresh org can have at creation time)
23+
* }
1024
* - `organization.provisioned` — emitted (#3844) by OrganizationsService.handleSignupOrganization
1125
* on EVERY exit path that returns a real organization: the fresh-create paths AND the A4
1226
* idempotent-convergence path (downstream consumers must be idempotent — a converged retry

modules/organizations/services/organizations.crud.service.js

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ const normalizeDomain = (value = '') => value.trim().toLowerCase();
2323
import OrganizationsRepository from '../repositories/organizations.repository.js';
2424
import MembershipRepository from '../repositories/organizations.membership.repository.js';
2525
import UserService from '../../users/services/users.service.js';
26-
import BillingSignupGrantService from '../../billing/services/billing.signupGrant.service.js';
26+
import organizationEvents from '../lib/events.js';
2727
import { slugify } from '../helpers/organizations.slug.js';
2828
import { MEMBERSHIP_STATUSES, MEMBERSHIP_ROLES } from '../lib/constants.js';
2929
import { runOrganizationRemovedHandlers } from '../lib/orgRemoval.registry.js';
@@ -121,13 +121,17 @@ const create = async (body, user) => {
121121
throw err;
122122
}
123123

124-
// Best-effort signup grant — mirrors organizations.service.js::createOrganizationForUser.
125-
// Every org-creation path (including this generic one behind POST /api/organizations)
126-
// must credit the configured one-shot signupGrant, else a fresh org on a plan that
127-
// defines one starts at 0 balance. Called outside the rollback try/catch so a billing
128-
// failure never rolls back the org; grantOnSignup never throws and is idempotent
129-
// (refId signup_grant-<orgId>). No-op when the plan defines no signupGrant.
130-
await BillingSignupGrantService.grantOnSignup({ orgId: result._id.toString(), planId: result.plan || 'free' });
124+
// organization.created (#3952) — mirrors organizations.service.js::createOrganizationForUser.
125+
// Billing subscribes from billing.init.js and credits the configured one-shot signupGrant,
126+
// else a fresh org on a plan that defines one starts at 0 balance. Emitted outside the
127+
// rollback try/catch so a billing failure never rolls back the org; the listener owns its
128+
// idempotence (refId signup_grant-<orgId>) and never throws. Fire-and-forget, synchronous
129+
// emit — the try/catch here only guards a SYNCHRONOUS listener throw (see ../lib/events.js).
130+
try {
131+
organizationEvents.emit('organization.created', { orgId: result._id.toString(), planId: result.plan || 'free' });
132+
} catch (err) {
133+
logger.warn('organizations: organization.created listener threw', { message: err?.message });
134+
}
131135

132136
return result;
133137
};

modules/organizations/services/organizations.service.js

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@ import UserService from '../../users/services/users.service.js';
1212
import { slugify, generateOrganizationSlug } from '../helpers/organizations.slug.js';
1313
import { MEMBERSHIP_ROLES, MEMBERSHIP_STATUSES } from '../lib/constants.js';
1414
import organizationEvents from '../lib/events.js';
15-
import BillingSignupGrantService from '../../billing/services/billing.signupGrant.service.js';
1615
import { isPublicDomain, normalizeEmailDomain } from './organizations.domain.js';
1716

1817
/**
@@ -96,9 +95,17 @@ const createOrganizationForUser = async ({ name, slug, domain, user, slugGenerat
9695
}
9796

9897
if (created) {
99-
// N2: best-effort signup grant — called outside the try/catch so billing failure
100-
// never triggers org/membership rollback. grantOnSignup always resolves (never throws).
101-
await BillingSignupGrantService.grantOnSignup({ orgId: organization._id.toString(), planId: 'free' });
98+
// organization.created (#3952) — N2 signup grant moved off a direct billing import onto
99+
// this event; billing subscribes from billing.init.js and credits the one-shot signupGrant.
100+
// Emitted outside the try/catch above so billing failure never triggers org/membership
101+
// rollback; the listener owns its own guard (never throws/rejects out). Fire-and-forget,
102+
// synchronous emit — this try/catch only guards a SYNCHRONOUS listener throw (mirrors
103+
// emitProvisioned below / see ../lib/events.js).
104+
try {
105+
organizationEvents.emit('organization.created', { orgId: organization._id.toString(), planId: 'free' });
106+
} catch (err) {
107+
logger.warn('organizations: organization.created listener threw', { message: err?.message });
108+
}
102109
return { organization, membership };
103110
}
104111
}
@@ -124,8 +131,9 @@ const createOrganizationForUser = async ({ name, slug, domain, user, slugGenerat
124131
* If domainMatching is on, the user's email domain is not public, and an existing org matches,
125132
* a `suggestedJoin: { orgId, orgName }` hint (name-only) is returned alongside the new org.
126133
*
127-
* signupGrant: credited inside createOrganizationForUser (best-effort, never throws). Called
128-
* exactly once per real new org — no double-credit possible.
134+
* signupGrant: createOrganizationForUser emits `organization.created` (#3952) exactly once per
135+
* real new org — no double-credit possible; billing's subscriber (billing.init.js) credits the
136+
* grant asynchronously, best-effort, never throwing back into this flow.
129137
*
130138
* @param {Object} user - The user object returned by UserService.create (with id, email, firstName, lastName).
131139
* @returns {Promise<{organization: Object|null, membership: Object|null, abilities: Array, emailVerificationRequired?: boolean, suggestedJoin?: {orgId: string, orgName: string}}>}

modules/organizations/tests/organizations.crud.grant.unit.tests.js

Lines changed: 26 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -4,15 +4,17 @@
44
import { jest, describe, test, expect, beforeEach } from '@jest/globals';
55

66
/**
7-
* Unit tests — verify organizations.crud.service.create() credits the one-shot
8-
* signup grant on the generic POST /api/organizations path, mirroring the
9-
* invite/verify path (organizations.service.js::createOrganizationForUser). A
10-
* fresh org on a plan that defines a signupGrant must not start at 0 balance.
7+
* Unit tests — verify organizations.crud.service.create() emits `organization.created` on
8+
* the generic POST /api/organizations path, mirroring the invite/verify path
9+
* (organizations.service.js::createOrganizationForUser). Billing subscribes to this event
10+
* from billing.init.js and credits the one-shot signupGrant (#3952) — organizations no
11+
* longer imports the billing module directly (see modules/billing/tests/billing.init.unit.tests.js
12+
* for the subscriber-side coverage).
1113
*/
1214

13-
const mockGrantOnSignup = jest.fn().mockResolvedValue({ applied: true });
14-
jest.unstable_mockModule('../../billing/services/billing.signupGrant.service.js', () => ({
15-
default: { grantOnSignup: mockGrantOnSignup },
15+
const mockEmit = jest.fn();
16+
jest.unstable_mockModule('../lib/events.js', () => ({
17+
default: { emit: mockEmit, on: jest.fn() },
1618
}));
1719

1820
const mockOrgCreate = jest.fn().mockResolvedValue({ _id: 'org1', plan: 'free' });
@@ -64,34 +66,43 @@ jest.unstable_mockModule('../../../config/index.js', () => ({
6466

6567
const { default: OrgCrudService } = await import('../services/organizations.crud.service.js');
6668

67-
describe('organizations.crud.service.create signup grant:', () => {
69+
describe('organizations.crud.service.create organization.created emit:', () => {
6870
beforeEach(() => {
69-
mockGrantOnSignup.mockClear();
71+
mockEmit.mockClear();
7072
mockOrgCreate.mockResolvedValue({ _id: 'org1', plan: 'free' });
7173
});
7274

73-
test('credits the one-shot signup grant for the freshly created org', async () => {
75+
test('emits organization.created for the freshly created org', async () => {
7476
const user = { id: 'u1', _id: 'u1', email: 'a@b.com', emailVerified: true };
7577
const result = await OrgCrudService.create({ name: 'Test Org' }, user);
7678

7779
expect(result).toEqual({ _id: 'org1', plan: 'free' });
78-
expect(mockGrantOnSignup).toHaveBeenCalledTimes(1);
79-
expect(mockGrantOnSignup).toHaveBeenCalledWith({ orgId: 'org1', planId: 'free' });
80+
expect(mockEmit).toHaveBeenCalledTimes(1);
81+
expect(mockEmit).toHaveBeenCalledWith('organization.created', { orgId: 'org1', planId: 'free' });
8082
});
8183

8284
test('defaults planId to free when the created org has no plan field', async () => {
8385
mockOrgCreate.mockResolvedValueOnce({ _id: 'org2' });
8486
const user = { id: 'u1', _id: 'u1', email: 'a@b.com', emailVerified: true };
8587
await OrgCrudService.create({ name: 'No Plan Org' }, user);
8688

87-
expect(mockGrantOnSignup).toHaveBeenCalledWith({ orgId: 'org2', planId: 'free' });
89+
expect(mockEmit).toHaveBeenCalledWith('organization.created', { orgId: 'org2', planId: 'free' });
8890
});
8991

90-
test('does not credit the grant when membership creation fails (rollback exits before the grant)', async () => {
92+
test('does not emit when membership creation fails (rollback exits before the emit)', async () => {
9193
mockMembershipCreate.mockRejectedValueOnce(new Error('membership boom'));
9294
const user = { id: 'u1', _id: 'u1', email: 'a@b.com', emailVerified: true };
9395

9496
await expect(OrgCrudService.create({ name: 'Rollback Org' }, user)).rejects.toThrow();
95-
expect(mockGrantOnSignup).not.toHaveBeenCalled();
97+
expect(mockEmit).not.toHaveBeenCalled();
98+
});
99+
100+
test('a SYNCHRONOUS listener throw is swallowed — org creation still succeeds', async () => {
101+
mockEmit.mockImplementationOnce(() => { throw new Error('listener exploded'); });
102+
const user = { id: 'u1', _id: 'u1', email: 'a@b.com', emailVerified: true };
103+
104+
const result = await OrgCrudService.create({ name: 'Test Org' }, user);
105+
106+
expect(result).toEqual({ _id: 'org1', plan: 'free' });
96107
});
97108
});

modules/organizations/tests/organizations.emailVerification.policy.unit.tests.js

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -88,11 +88,6 @@ jest.unstable_mockModule('../helpers/organizations.slug.js', () => ({
8888
generateOrganizationSlug: jest.fn().mockResolvedValue('test-slug'),
8989
}));
9090

91-
const mockBillingGrantOnSignup = jest.fn().mockResolvedValue(undefined);
92-
jest.unstable_mockModule('../../billing/services/billing.signupGrant.service.js', () => ({
93-
default: { grantOnSignup: mockBillingGrantOnSignup },
94-
}));
95-
9691
// --- Dynamic imports after mocks ---
9792

9893
const { default: OrganizationsService } = await import('../services/organizations.service.js');

0 commit comments

Comments
 (0)