From c78bc981118f2ea790ff84864c78554c370faaed Mon Sep 17 00:00:00 2001 From: Pierre Brisorgueil Date: Fri, 25 Sep 2026 10:57:30 +0200 Subject: [PATCH 1/4] feat(organizations): send a welcome email once a signup provisions its workspace MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds a `welcome` mail template and fires it from both create branches of handleSignupOrganization (organizations enabled or disabled) — never on the A4 convergence path, never on the manual create-another-org flow. Fire-and- forget, gated on config.organizations.welcomeEmail.enabled (default true, fail-open) and mailer.isConfigured(); a disabled toggle, a disabled mailer, or a rejecting send can never break or delay signup. Closes #4116 Claude-Session: https://claude.ai/code/session_01TTK9g6SFCfjfuWvB3MLFr3 --- config/templates/welcome.html | 43 ++++ .../organizations.development.config.js | 4 + .../services/organizations.service.js | 52 ++++- ...ons.emailVerification.policy.unit.tests.js | 2 +- ...anizations.emailVerification.unit.tests.js | 2 +- ...organizations.service.signup.unit.tests.js | 2 +- ...zations.service.silent.catch.unit.tests.js | 2 +- ...zations.service.welcomeEmail.unit.tests.js | 221 ++++++++++++++++++ 8 files changed, 322 insertions(+), 6 deletions(-) create mode 100644 config/templates/welcome.html create mode 100644 modules/organizations/tests/organizations.service.welcomeEmail.unit.tests.js diff --git a/config/templates/welcome.html b/config/templates/welcome.html new file mode 100644 index 000000000..5192337cc --- /dev/null +++ b/config/templates/welcome.html @@ -0,0 +1,43 @@ + + + + + + + +

Hello {{displayName}},

+

+ Welcome to {{appName}}{{#if orgName}} — your workspace {{orgName}} is ready{{/if}}. +

+

Here's where to pick things up:

+ + + + +
+ Get started +
+

Button not working? {{url}}

+

The {{appName}} Team.

+
+ Please do not reply to this email, you can contact us here. + + diff --git a/modules/organizations/config/organizations.development.config.js b/modules/organizations/config/organizations.development.config.js index 636cfecd2..d89608f54 100644 --- a/modules/organizations/config/organizations.development.config.js +++ b/modules/organizations/config/organizations.development.config.js @@ -17,6 +17,10 @@ const config = { // is always auto-provisioned (same path as a mailer-not-configured env). // emailVerified stays server-only; this policy only gates the existing checks. emailVerification: { mode: 'strict' }, + // Fail-open: read with `?? true` at the call site so an absent key (a + // downstream project that predates this option) never silently disables + // the welcome email — only an explicit `false` does. + welcomeEmail: { enabled: true }, roles: ['owner', 'admin', 'member'], roleDescriptions: { owner: 'Full control — manage organization settings, members, roles, and billing.', diff --git a/modules/organizations/services/organizations.service.js b/modules/organizations/services/organizations.service.js index d4246280f..cd4c6ff41 100644 --- a/modules/organizations/services/organizations.service.js +++ b/modules/organizations/services/organizations.service.js @@ -6,6 +6,7 @@ import mailer from '../../../lib/helpers/mailer/index.js'; import logger from '../../../lib/services/logger.js'; import policy from '../../../lib/middlewares/policy.js'; import serializeAbilities from '../../../lib/helpers/abilities.js'; +import getBaseUrl from '../../../lib/helpers/getBaseUrl.js'; import OrganizationsRepository from '../repositories/organizations.repository.js'; import MembershipRepository from '../repositories/organizations.membership.repository.js'; import UserService from '../../users/services/users.service.js'; @@ -118,6 +119,48 @@ const createOrganizationForUser = async ({ name, slug, domain, user, slugGenerat throw new Error('Failed to create organization: slug conflict after maximum retries'); }; +/** + * Fire-and-forget welcome email sent once a fresh signup provisions a real + * workspace. Called from BOTH create branches of `handleSignupOrganization` + * (organizations enabled or disabled) — never from the A4 convergence path + * (existing membership, an early return above those branches) and never from + * the manual "create another org" flow (`organizations.crud.service.js`), so + * it fires exactly once per real new workspace with no dedup field needed. + * With strict email verification the send happens naturally after + * verification, since provisioning itself is deferred until then. + * + * Gated on `config.organizations.welcomeEmail.enabled` (default `true`, + * fail-open — read with `?? true` so an absent key on a downstream project + * never silently disables the email) and `mailer.isConfigured()`. Never + * awaited by the caller: a disabled toggle, a disabled mailer, a synchronous + * throw, or a rejected send must never break or delay the signup / OAuth + * redirect response. + * @param {Object} user - The newly signed-up user (id/_id, email, firstName, lastName). + * @param {string} [orgName] - Organization display name. Omitted in B2C mode + * (organizations disabled) — the template must not require it. + * @returns {void} + */ +const sendWelcomeEmail = (user, orgName) => { + if (!(config.organizations?.welcomeEmail?.enabled ?? true)) return; + if (!mailer.isConfigured()) return; + try { + mailer.sendMail({ + template: 'welcome', + to: user.email, + subject: `Welcome to ${config.app.title}`, + params: { + displayName: [user.firstName, user.lastName].filter(Boolean).join(' '), + url: getBaseUrl(), + appName: config.app.title, + appContact: config.app.contact, + ...(orgName ? { orgName } : {}), + }, + }).catch((err) => logger.warn('organizations: welcome email failed', { message: err?.message, stack: err?.stack })); + } catch (err) { + logger.warn('organizations: welcome email failed', { message: err?.message, stack: err?.stack }); + } +}; + /** * Handle organization provisioning during the signup flow. * @@ -230,7 +273,10 @@ const handleSignupOrganization = async (user) => { }); emitProvisioned(organization); - return buildResult(organization, membership); + const result = await buildResult(organization, membership); + // B2C mode — the workspace is a hidden default, never named to the user. + sendWelcomeEmail(user); + return result; } // Case 2: Organizations enabled — always provision a workspace for the user. @@ -285,8 +331,10 @@ const handleSignupOrganization = async (user) => { }); emitProvisioned(organization); + const result = await buildResult(organization, membership); + sendWelcomeEmail(user, organization.name); return { - ...(await buildResult(organization, membership)), + ...result, ...(suggestedJoin ? { suggestedJoin } : {}), }; }; diff --git a/modules/organizations/tests/organizations.emailVerification.policy.unit.tests.js b/modules/organizations/tests/organizations.emailVerification.policy.unit.tests.js index c8ba5aa9f..718c9f7ac 100644 --- a/modules/organizations/tests/organizations.emailVerification.policy.unit.tests.js +++ b/modules/organizations/tests/organizations.emailVerification.policy.unit.tests.js @@ -38,7 +38,7 @@ jest.unstable_mockModule('../../../lib/services/logger.js', () => ({ const mockIsConfigured = jest.fn(); jest.unstable_mockModule('../../../lib/helpers/mailer/index.js', () => ({ - default: { isConfigured: mockIsConfigured, sendMail: jest.fn() }, + default: { isConfigured: mockIsConfigured, sendMail: jest.fn().mockResolvedValue(null) }, })); const mockOrganizationsRepositoryCreate = jest.fn(); diff --git a/modules/organizations/tests/organizations.emailVerification.unit.tests.js b/modules/organizations/tests/organizations.emailVerification.unit.tests.js index 1eeb74665..f97101964 100644 --- a/modules/organizations/tests/organizations.emailVerification.unit.tests.js +++ b/modules/organizations/tests/organizations.emailVerification.unit.tests.js @@ -21,7 +21,7 @@ jest.unstable_mockModule('../lib/events.js', () => ({ const mockIsConfigured = jest.fn(); jest.unstable_mockModule('../../../lib/helpers/mailer/index.js', () => ({ - default: { isConfigured: mockIsConfigured, sendMail: jest.fn() }, + default: { isConfigured: mockIsConfigured, sendMail: jest.fn().mockResolvedValue(null) }, })); const mockOrganizationsRepositoryCreate = jest.fn(); diff --git a/modules/organizations/tests/organizations.service.signup.unit.tests.js b/modules/organizations/tests/organizations.service.signup.unit.tests.js index 555407c7b..55341aafe 100644 --- a/modules/organizations/tests/organizations.service.signup.unit.tests.js +++ b/modules/organizations/tests/organizations.service.signup.unit.tests.js @@ -19,7 +19,7 @@ import { jest, describe, test, expect, beforeEach } from '@jest/globals'; const mockIsConfigured = jest.fn().mockReturnValue(false); jest.unstable_mockModule('../../../lib/helpers/mailer/index.js', () => ({ - default: { isConfigured: mockIsConfigured }, + default: { isConfigured: mockIsConfigured, sendMail: jest.fn().mockResolvedValue(null) }, })); const mockOrgCreate = jest.fn(); diff --git a/modules/organizations/tests/organizations.service.silent.catch.unit.tests.js b/modules/organizations/tests/organizations.service.silent.catch.unit.tests.js index e1ac7a928..0ce9b454f 100644 --- a/modules/organizations/tests/organizations.service.silent.catch.unit.tests.js +++ b/modules/organizations/tests/organizations.service.silent.catch.unit.tests.js @@ -56,7 +56,7 @@ jest.unstable_mockModule('../../../lib/helpers/abilities.js', () => ({ })); jest.unstable_mockModule('../../../lib/helpers/mailer/index.js', () => ({ - default: { isConfigured: jest.fn().mockReturnValue(false) }, + default: { isConfigured: jest.fn().mockReturnValue(false), sendMail: jest.fn().mockResolvedValue(null) }, })); jest.unstable_mockModule('../../../config/index.js', () => ({ diff --git a/modules/organizations/tests/organizations.service.welcomeEmail.unit.tests.js b/modules/organizations/tests/organizations.service.welcomeEmail.unit.tests.js new file mode 100644 index 000000000..9b7c9af8f --- /dev/null +++ b/modules/organizations/tests/organizations.service.welcomeEmail.unit.tests.js @@ -0,0 +1,221 @@ +/** + * Unit tests — welcome email after signup (Node#4116). + * + * Contract (see `sendWelcomeEmail` in organizations.service.js): + * - Sent from BOTH create branches of `handleSignupOrganization` (organizations + * enabled or disabled) — exactly once per real new workspace. + * - NEVER sent on the A4 convergence path (existing active membership). + * - Gated on `config.organizations.welcomeEmail.enabled` (default true) AND + * `mailer.isConfigured()` — either gate off means no send. + * - B2C mode (organizations disabled) omits `orgName` from the template params. + * - Fire-and-forget: a rejecting send, or a send call that doesn't even return + * a promise, must never break the signup flow. + */ +import mongoose from 'mongoose'; +import { jest, describe, test, expect, beforeEach } from '@jest/globals'; + +// --- Mocks (must precede dynamic imports) --- + +const mockIsConfigured = jest.fn().mockReturnValue(true); +const mockSendMail = jest.fn().mockResolvedValue({ accepted: ['a@b.com'], rejected: [] }); +jest.unstable_mockModule('../../../lib/helpers/mailer/index.js', () => ({ + default: { isConfigured: mockIsConfigured, sendMail: mockSendMail }, +})); + +const mockOrgCreate = jest.fn(); +const mockOrgList = jest.fn().mockResolvedValue([]); +const mockOrgExists = jest.fn().mockResolvedValue(false); +jest.unstable_mockModule('../repositories/organizations.repository.js', () => ({ + default: { + create: mockOrgCreate, + list: mockOrgList, + exists: mockOrgExists, + remove: jest.fn().mockResolvedValue({}), + }, +})); + +const mockMembershipCreate = jest.fn(); +const mockMembershipFindOne = jest.fn().mockResolvedValue(null); +jest.unstable_mockModule('../repositories/organizations.membership.repository.js', () => ({ + default: { + create: mockMembershipCreate, + deleteMany: jest.fn().mockResolvedValue({}), + list: jest.fn().mockResolvedValue([]), + findOne: mockMembershipFindOne, + }, +})); + +const mockUpdateById = jest.fn().mockResolvedValue({}); +jest.unstable_mockModule('../../users/services/users.service.js', () => ({ + default: { updateById: mockUpdateById }, +})); + +const mockDefineAbilityFor = jest.fn().mockResolvedValue({ rules: [] }); +jest.unstable_mockModule('../../../lib/middlewares/policy.js', () => ({ + default: { defineAbilityFor: mockDefineAbilityFor }, +})); + +jest.unstable_mockModule('../../../lib/helpers/abilities.js', () => ({ + default: jest.fn().mockReturnValue(['ability-stub']), +})); + +jest.unstable_mockModule('../helpers/organizations.slug.js', () => ({ + slugify: (str) => str.toLowerCase().replace(/\s+/g, '-'), + generateOrganizationSlug: jest.fn().mockResolvedValue('alice-org'), +})); + +const mockLoggerWarn = jest.fn(); +jest.unstable_mockModule('../../../lib/services/logger.js', () => ({ + default: { error: jest.fn(), warn: mockLoggerWarn, info: jest.fn() }, +})); + +jest.unstable_mockModule('../lib/events.js', () => ({ + default: { emit: jest.fn(), on: jest.fn() }, +})); + +// Config store — MUST be mutated in-place (jest.unstable_mockModule captures the +// default export value at import time; reassigning the variable breaks the binding). +const configStore = { organizations: {}, app: { title: 'Acme App', contact: 'hi@acme.test' }, cors: { origin: 'https://app.acme.test' } }; +jest.unstable_mockModule('../../../config/index.js', () => ({ + default: configStore, +})); + +// --- Dynamic import after all mocks --- +const { default: OrganizationsService } = await import('../services/organizations.service.js'); + +/** + * Configure config mock and repository happy-path defaults for a fresh signup. + * Must mutate configStore's `organizations` key in-place. + * @param {Object} orgConfig - `config.organizations` values. + */ +function setupConfig(orgConfig) { + configStore.organizations = { publicDomains: [], ...orgConfig }; + const fakeOrg = { _id: new mongoose.Types.ObjectId(), name: 'Acme Corp', slug: 'acme', domain: '', plan: 'free', toJSON() { return { _id: this._id, name: this.name }; } }; + mockOrgCreate.mockResolvedValue(fakeOrg); + mockMembershipCreate.mockResolvedValue({ _id: new mongoose.Types.ObjectId(), role: 'owner' }); + return fakeOrg; +} + +/** + * Build a minimal user object for testing. + * @param {string} email + * @returns {Object} + */ +function makeUser(email = 'alice@example.com') { + return { + id: new mongoose.Types.ObjectId().toString(), + _id: new mongoose.Types.ObjectId().toString(), + email, + firstName: 'Alice', + lastName: 'Smith', + emailVerified: true, + }; +} + +describe('handleSignupOrganization — welcome email (Node#4116):', () => { + beforeEach(() => { + jest.clearAllMocks(); + mockIsConfigured.mockReturnValue(true); + mockSendMail.mockResolvedValue({ accepted: ['a@b.com'], rejected: [] }); + mockOrgExists.mockResolvedValue(false); + mockOrgList.mockResolvedValue([]); + mockMembershipFindOne.mockResolvedValue(null); + mockUpdateById.mockResolvedValue({}); + mockDefineAbilityFor.mockResolvedValue({ rules: [] }); + }); + + test('sent once on a fresh create, orgs enabled — includes orgName', async () => { + const fakeOrg = setupConfig({ enabled: true, autoCreate: false, domainMatching: false }); + const user = makeUser('alice@corp.example.com'); + + const result = await OrganizationsService.handleSignupOrganization(user); + + expect(result.organization).not.toBeNull(); + expect(mockSendMail).toHaveBeenCalledTimes(1); + expect(mockSendMail).toHaveBeenCalledWith({ + template: 'welcome', + to: user.email, + subject: 'Welcome to Acme App', + params: { + displayName: 'Alice Smith', + url: 'https://app.acme.test', + appName: 'Acme App', + appContact: 'hi@acme.test', + orgName: fakeOrg.name, + }, + }); + }); + + test('sent once on a fresh create, orgs disabled (B2C) — no orgName in params', async () => { + setupConfig({ enabled: false }); + const user = makeUser('bob@example.com'); + + const result = await OrganizationsService.handleSignupOrganization(user); + + expect(result.organization).not.toBeNull(); + expect(mockSendMail).toHaveBeenCalledTimes(1); + const { params } = mockSendMail.mock.calls[0][0]; + expect(params).not.toHaveProperty('orgName'); + }); + + test('NOT sent on the A4 convergence path (existing active membership)', async () => { + setupConfig({ enabled: true }); + const existingOrg = { _id: new mongoose.Types.ObjectId(), name: 'Existing Org' }; + mockMembershipFindOne.mockResolvedValue({ _id: new mongoose.Types.ObjectId(), role: 'owner', status: 'active', organizationId: existingOrg }); + const user = makeUser('carol@example.com'); + + const result = await OrganizationsService.handleSignupOrganization(user); + + expect(result.organization).toBe(existingOrg); + expect(mockOrgCreate).not.toHaveBeenCalled(); + expect(mockSendMail).not.toHaveBeenCalled(); + }); + + test('NOT sent when config.organizations.welcomeEmail.enabled is false', async () => { + setupConfig({ enabled: true, welcomeEmail: { enabled: false } }); + const user = makeUser('dave@example.com'); + + const result = await OrganizationsService.handleSignupOrganization(user); + + expect(result.organization).not.toBeNull(); + expect(mockOrgCreate).toHaveBeenCalled(); + expect(mockSendMail).not.toHaveBeenCalled(); + }); + + test('NOT sent when the mailer is not configured — signup still succeeds', async () => { + setupConfig({ enabled: true }); + mockIsConfigured.mockReturnValue(false); + const user = makeUser('erin@example.com'); + + const result = await OrganizationsService.handleSignupOrganization(user); + + expect(result.organization).not.toBeNull(); + expect(mockSendMail).not.toHaveBeenCalled(); + }); + + test('a rejecting sendMail does not break signup', async () => { + setupConfig({ enabled: true }); + mockSendMail.mockRejectedValueOnce(new Error('smtp down')); + const user = makeUser('frank@example.com'); + + const result = await OrganizationsService.handleSignupOrganization(user); + // Flush the fire-and-forget promise chain so its .catch() runs before we assert. + await new Promise((resolve) => setImmediate(resolve)); + + expect(result.organization).not.toBeNull(); + expect(mockSendMail).toHaveBeenCalledTimes(1); + expect(mockLoggerWarn).toHaveBeenCalledWith('organizations: welcome email failed', expect.objectContaining({ message: 'smtp down' })); + }); + + test('a sendMail call that does not return a promise does not break signup', async () => { + setupConfig({ enabled: true }); + mockSendMail.mockReturnValueOnce(undefined); + const user = makeUser('grace@example.com'); + + const result = await OrganizationsService.handleSignupOrganization(user); + + expect(result.organization).not.toBeNull(); + expect(mockSendMail).toHaveBeenCalledTimes(1); + expect(mockLoggerWarn).toHaveBeenCalledWith('organizations: welcome email failed', expect.anything()); + }); +}); From 2530962aaec92d287f12e94359f6d7955334fe80 Mon Sep 17 00:00:00 2001 From: Pierre Brisorgueil Date: Fri, 25 Sep 2026 10:59:59 +0200 Subject: [PATCH 2/4] refactor(simplify): dedupe the welcome-email error handler sendWelcomeEmail logged the identical warn line from both the outer try/catch (guards a synchronous throw building sendMail's args) and the async .catch() (guards a rejected send). Extract one onError closure and reuse it in both spots instead of repeating the log call. Claude-Session: https://claude.ai/code/session_01TTK9g6SFCfjfuWvB3MLFr3 --- modules/organizations/services/organizations.service.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/modules/organizations/services/organizations.service.js b/modules/organizations/services/organizations.service.js index cd4c6ff41..f0273bd9a 100644 --- a/modules/organizations/services/organizations.service.js +++ b/modules/organizations/services/organizations.service.js @@ -143,6 +143,7 @@ const createOrganizationForUser = async ({ name, slug, domain, user, slugGenerat const sendWelcomeEmail = (user, orgName) => { if (!(config.organizations?.welcomeEmail?.enabled ?? true)) return; if (!mailer.isConfigured()) return; + const onError = (err) => logger.warn('organizations: welcome email failed', { message: err?.message, stack: err?.stack }); try { mailer.sendMail({ template: 'welcome', @@ -155,9 +156,9 @@ const sendWelcomeEmail = (user, orgName) => { appContact: config.app.contact, ...(orgName ? { orgName } : {}), }, - }).catch((err) => logger.warn('organizations: welcome email failed', { message: err?.message, stack: err?.stack })); + }).catch(onError); } catch (err) { - logger.warn('organizations: welcome email failed', { message: err?.message, stack: err?.stack }); + onError(err); } }; From dac9cab6a3993f82f4631569cf17cc986b35ad48 Mon Sep 17 00:00:00 2001 From: Pierre Brisorgueil Date: Fri, 25 Sep 2026 11:14:53 +0200 Subject: [PATCH 3/4] docs(organizations): migration note + traceable welcome-email log Applies the fallback /critical-review findings on #4125: - MIGRATIONS.md entry for the new default-on welcome email (opt-out via organizations.welcomeEmail.enabled: false, customize via config/templates/welcome.html) - sendWelcomeEmail's warn log now includes userId and orgId (org is in scope on both call sites, including B2C) so a send failure is traceable - welcome.html now renders {{appName}} instead of empty Claude-Session: https://claude.ai/code/session_01TTK9g6SFCfjfuWvB3MLFr3 --- MIGRATIONS.md | 18 +++++++++++++ config/templates/welcome.html | 2 +- .../services/organizations.service.js | 16 +++++++++--- ...zations.service.welcomeEmail.unit.tests.js | 25 ++++++++++++++++--- 4 files changed, 53 insertions(+), 8 deletions(-) diff --git a/MIGRATIONS.md b/MIGRATIONS.md index 7a9ddf3dd..d4ff9e9a6 100644 --- a/MIGRATIONS.md +++ b/MIGRATIONS.md @@ -4,6 +4,24 @@ Breaking changes and upgrade notes for downstream projects. --- +## Organizations: welcome email now sent after signup provisioning (2026-09-25) + +Every successful signup that provisions a workspace — organizations enabled or +disabled — now fires a fire-and-forget `welcome` email to the new user. Never +sent on the A4 convergence path (existing membership) and never on the manual +"create another org" flow, so it fires exactly once per real new workspace. +Gated on `mailer.isConfigured()`: a disabled mailer, a synchronous throw, or a +rejected send can never break or delay the signup / OAuth redirect response. + +**What you will see:** every new signup receives a welcome email once the +mailer is configured. To opt out, set `organizations.welcomeEmail.enabled: +false` in the project config (default `true`, fail-open — read with `?? true` +so an absent key never silently disables it). To customize the copy, edit the +same-named template at `config/templates/welcome.html` (`{{displayName}}`, +`{{appName}}`, `{{url}}`, `{{appContact}}`, and an optional +`{{#if orgName}}` block, omitted in B2C mode). No schema change, no migration +to run. + ## Billing: public plans listing now requires a plan tag (2026-09-25) `GET /api/billing/plans` no longer falls back to a Stripe product's raw id when diff --git a/config/templates/welcome.html b/config/templates/welcome.html index 5192337cc..856bd3f21 100644 --- a/config/templates/welcome.html +++ b/config/templates/welcome.html @@ -1,7 +1,7 @@ <!doctype html> <html lang="en" xmlns="http://www.w3.org/1999/xhtml"> <head> - <title> + {{appName}} diff --git a/modules/organizations/services/organizations.service.js b/modules/organizations/services/organizations.service.js index f0273bd9a..35ab9af3a 100644 --- a/modules/organizations/services/organizations.service.js +++ b/modules/organizations/services/organizations.service.js @@ -138,12 +138,20 @@ const createOrganizationForUser = async ({ name, slug, domain, user, slugGenerat * @param {Object} user - The newly signed-up user (id/_id, email, firstName, lastName). * @param {string} [orgName] - Organization display name. Omitted in B2C mode * (organizations disabled) — the template must not require it. + * @param {string} [orgId] - Organization id, for failure logging only (never + * passed to the template) — always in scope, including B2C mode. * @returns {void} */ -const sendWelcomeEmail = (user, orgName) => { +const sendWelcomeEmail = (user, orgName, orgId) => { if (!(config.organizations?.welcomeEmail?.enabled ?? true)) return; if (!mailer.isConfigured()) return; - const onError = (err) => logger.warn('organizations: welcome email failed', { message: err?.message, stack: err?.stack }); + const userId = user.id || user._id; + const onError = (err) => logger.warn('organizations: welcome email failed', { + userId: userId ? String(userId) : undefined, + ...(orgId ? { orgId: String(orgId) } : {}), + message: err?.message, + stack: err?.stack, + }); try { mailer.sendMail({ template: 'welcome', @@ -276,7 +284,7 @@ const handleSignupOrganization = async (user) => { emitProvisioned(organization); const result = await buildResult(organization, membership); // B2C mode — the workspace is a hidden default, never named to the user. - sendWelcomeEmail(user); + sendWelcomeEmail(user, undefined, organization._id); return result; } @@ -333,7 +341,7 @@ const handleSignupOrganization = async (user) => { emitProvisioned(organization); const result = await buildResult(organization, membership); - sendWelcomeEmail(user, organization.name); + sendWelcomeEmail(user, organization.name, organization._id); return { ...result, ...(suggestedJoin ? { suggestedJoin } : {}), diff --git a/modules/organizations/tests/organizations.service.welcomeEmail.unit.tests.js b/modules/organizations/tests/organizations.service.welcomeEmail.unit.tests.js index 9b7c9af8f..91cc80c46 100644 --- a/modules/organizations/tests/organizations.service.welcomeEmail.unit.tests.js +++ b/modules/organizations/tests/organizations.service.welcomeEmail.unit.tests.js @@ -193,8 +193,8 @@ describe('handleSignupOrganization — welcome email (Node#4116):', () => { expect(mockSendMail).not.toHaveBeenCalled(); }); - test('a rejecting sendMail does not break signup', async () => { - setupConfig({ enabled: true }); + test('a rejecting sendMail does not break signup — failure is traceable (userId + orgId)', async () => { + const fakeOrg = setupConfig({ enabled: true }); mockSendMail.mockRejectedValueOnce(new Error('smtp down')); const user = makeUser('frank@example.com'); @@ -204,7 +204,26 @@ describe('handleSignupOrganization — welcome email (Node#4116):', () => { expect(result.organization).not.toBeNull(); expect(mockSendMail).toHaveBeenCalledTimes(1); - expect(mockLoggerWarn).toHaveBeenCalledWith('organizations: welcome email failed', expect.objectContaining({ message: 'smtp down' })); + expect(mockLoggerWarn).toHaveBeenCalledWith('organizations: welcome email failed', expect.objectContaining({ + message: 'smtp down', + userId: user.id, + orgId: String(fakeOrg._id), + })); + }); + + test('a rejecting sendMail in B2C mode still logs orgId (hidden default org is in scope)', async () => { + const fakeOrg = setupConfig({ enabled: false }); + mockSendMail.mockRejectedValueOnce(new Error('smtp down')); + const user = makeUser('heidi@example.com'); + + const result = await OrganizationsService.handleSignupOrganization(user); + await new Promise((resolve) => setImmediate(resolve)); + + expect(result.organization).not.toBeNull(); + expect(mockLoggerWarn).toHaveBeenCalledWith('organizations: welcome email failed', expect.objectContaining({ + userId: user.id, + orgId: String(fakeOrg._id), + })); }); test('a sendMail call that does not return a promise does not break signup', async () => { From 65ad745d7a191b3a3e22447885b1599d54b3f638 Mon Sep 17 00:00:00 2001 From: Pierre Brisorgueil Date: Fri, 25 Sep 2026 11:40:15 +0200 Subject: [PATCH 4/4] test(organizations): complete JSDoc on welcome-email test helpers CodeRabbit nit on #4125: setupConfig, slugify (mock) and the fakeOrg toJSON stub were missing @returns / a header per the coding guideline requiring JSDoc on every new or modified function. Claude-Session: https://claude.ai/code/session_01TTK9g6SFCfjfuWvB3MLFr3 --- ...zations.service.welcomeEmail.unit.tests.js | 21 ++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) diff --git a/modules/organizations/tests/organizations.service.welcomeEmail.unit.tests.js b/modules/organizations/tests/organizations.service.welcomeEmail.unit.tests.js index 91cc80c46..92f3e7d13 100644 --- a/modules/organizations/tests/organizations.service.welcomeEmail.unit.tests.js +++ b/modules/organizations/tests/organizations.service.welcomeEmail.unit.tests.js @@ -60,6 +60,11 @@ jest.unstable_mockModule('../../../lib/helpers/abilities.js', () => ({ })); jest.unstable_mockModule('../helpers/organizations.slug.js', () => ({ + /** + * Lowercase and hyphenate a string for use as a slug (test stub). + * @param {string} str - The string to slugify. + * @returns {string} The slugified string. + */ slugify: (str) => str.toLowerCase().replace(/\s+/g, '-'), generateOrganizationSlug: jest.fn().mockResolvedValue('alice-org'), })); @@ -87,10 +92,24 @@ const { default: OrganizationsService } = await import('../services/organization * Configure config mock and repository happy-path defaults for a fresh signup. * Must mutate configStore's `organizations` key in-place. * @param {Object} orgConfig - `config.organizations` values. + * @returns {Object} The fake organization document `OrganizationsRepository.create` resolves to. */ function setupConfig(orgConfig) { configStore.organizations = { publicDomains: [], ...orgConfig }; - const fakeOrg = { _id: new mongoose.Types.ObjectId(), name: 'Acme Corp', slug: 'acme', domain: '', plan: 'free', toJSON() { return { _id: this._id, name: this.name }; } }; + const fakeOrg = { + _id: new mongoose.Types.ObjectId(), + name: 'Acme Corp', + slug: 'acme', + domain: '', + plan: 'free', + /** + * Serialize the fake organization to its public JSON shape (test stub). + * @returns {{_id: import('mongoose').Types.ObjectId, name: string}} The serialized organization. + */ + toJSON() { + return { _id: this._id, name: this.name }; + }, + }; mockOrgCreate.mockResolvedValue(fakeOrg); mockMembershipCreate.mockResolvedValue({ _id: new mongoose.Types.ObjectId(), role: 'owner' }); return fakeOrg;