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..7c261c07f --- /dev/null +++ b/modules/billing/migrations/20260707100000-backfill-missing-signup-grant-credits.js @@ -0,0 +1,97 @@ +/** + * 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 + * 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. + * + * 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. + * + * 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. + * + * 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'; +import BillingSignupGrantService from '../services/billing.signupGrant.service.js'; + +/** + * @returns {Promise} + */ +export async function up() { + // 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 (grantPlanIds.length === 0) { + console.info('[migration] backfill-signup-grant: no plan defines a signupGrant — nothing to do'); + return; + } + + const organizations = mongoose.connection.db.collection('organizations'); + const cursor = organizations.find( + { plan: { $in: grantPlanIds } }, + { projection: { _id: 1, plan: 1 } }, + ); + + let granted = 0; + let skipped = 0; + let failed = 0; + + 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 == 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} failed=${failed}`); +} + +/** + * 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 removal 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/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); + }); +}); 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..aa747a07d --- /dev/null +++ b/modules/organizations/tests/organizations.crud.grant.unit.tests.js @@ -0,0 +1,97 @@ +/** + * 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' }); + }); + + 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(); + }); +});