fix(organizations): replace dynamic-import cleanup with onOrganizationRemoved extension point (#3863) (#3865)

* feat(organizations): add onOrganizationRemoved subscriber registry (#3863)

* refactor(organizations): run org-removal handlers via registry, drop dynamic-import cleanup (#3863)

* test(organizations): drop dead tasks.service mock from orgRemoval suite (#3863)

* feat(tasks): register org-removal cleanup handler at boot via tasks.init (#3863)

* test(tasks): assert org-removal registration is boot-only (absent-module no-op) (#3863)

* fix(organizations): jsdoc, set-based registry, and remove() contract (#3863)

- tasks.init: add JSDoc @returns on default export (mirrors billing.init pattern)
- orgRemoval.registry: handlers Array → Set (de-dupes fn refs; clear() replaces length=0)
- organizations.crud.service remove(): update JSDoc to document cleanup handlers

Author: PierreBrisorgueil

modules/organizations/lib/orgRemoval.registry.js

@@ -0,0 +1,46 @@
+/**
+ * @module organizations/lib/orgRemoval.registry
+ * @description Subscriber registry for organization-removal cleanup.
+ *   Optional modules register a handler at load time via `onOrganizationRemoved`;
+ *   the organization crud service runs them sequentially on org delete and
+ *   propagates their errors (no silent swallow). Config-free, import-safe leaf —
+ *   it must not import organization/tasks services (avoids an import cycle).
+ */
+
+const handlers = new Set();
+
+/**
+ * @function onOrganizationRemoved
+ * @description Register a cleanup handler fired when an organization is removed.
+ *   Uses a Set so duplicate registrations (e.g. a module init running twice) are
+ *   silently de-duped — each handler function identity is unique in the Set.
+ * @param {Function} fn - async (payload) => void; payload is { organizationId, organization }.
+ * @returns {void}
+ */
+export const onOrganizationRemoved = (fn) => {
+  if (typeof fn !== 'function') throw new TypeError('onOrganizationRemoved: fn must be a function');
+  handlers.add(fn);
+};
+
+/**
+ * @function runOrganizationRemovedHandlers
+ * @description Run every registered handler sequentially. Errors propagate (not swallowed).
+ * @param {Object} payload - { organizationId, organization }.
+ * @returns {Promise<void>}
+ */
+export const runOrganizationRemovedHandlers = async (payload) => {
+  for (const fn of handlers) {
+    await fn(payload); // sequential: each handler must resolve before the next runs
+  }
+};
+
+/**
+ * @function _reset
+ * @description Test helper — clears all registered handlers.
+ * @returns {void}
+ */
+export const _reset = () => {
+  handlers.clear();
+};
+
+export default { onOrganizationRemoved, runOrganizationRemovedHandlers, _reset };

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

@@ -25,6 +25,7 @@ import MembershipRepository from '../repositories/organizations.membership.repos
 import UserService from '../../users/services/users.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';
 
 /**
  * @function list
@@ -170,7 +171,11 @@ const update = async (organization, body) => {
 
 /**
  * @function remove
- * @description Service to delete an organization and all its memberships.
+ * @description Service to delete an organization, all its memberships, and any
+ *   data owned by optional modules that registered an org-removal handler via
+ *   `onOrganizationRemoved`. Handlers run sequentially after membership cleanup;
+ *   any handler error propagates and aborts the delete before the repository
+ *   removal (no silent swallow).
  * @param {Object} organization - The organization to delete.
  * @returns {Promise<Object>} A promise resolving to a confirmation of the deletion.
  */
@@ -192,18 +197,9 @@ const remove = async (organization) => {
     await UserService.updateById(u._id, { currentOrganization: nextOrg });
   }));
 
-  // Delete all tasks belonging to this organization (Task module may not exist in all projects)
-  try {
-    const TasksService = (await import('../../tasks/services/tasks.service.js')).default;
-    await TasksService.deleteMany({ organizationId: orgId });
-  } catch (err) {
-    // Only swallow module-not-found errors; re-throw data/runtime failures
-    if (err && (err.code === 'ERR_MODULE_NOT_FOUND' || err.code === 'MODULE_NOT_FOUND')) {
-      // Tasks module not available — skip cleanup
-    } else {
-      throw err;
-    }
-  }
+  // Run org-removal cleanup handlers registered by optional modules (e.g. tasks).
+  // Errors propagate and abort the delete before the repository removal.
+  await runOrganizationRemovedHandlers({ organizationId: orgId, organization });
 
   const result = await OrganizationsRepository.remove(organization);
   return result;

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

@@ -0,0 +1,77 @@
+/**
+ * Unit tests — OrgCrudService.remove() fires the org-removal registry and propagates handler errors.
+ */
+import { jest, describe, test, expect, beforeEach } from '@jest/globals';
+
+const mockOrgRemove = jest.fn().mockResolvedValue({ acknowledged: true });
+const mockMembershipDeleteMany = jest.fn().mockResolvedValue({ deletedCount: 0 });
+const mockMembershipList = jest.fn().mockResolvedValue([]);
+const mockUpdateById = jest.fn().mockResolvedValue({});
+const mockFindWithFilter = jest.fn().mockResolvedValue([]);
+
+jest.unstable_mockModule('../../../lib/services/logger.js', () => ({
+  default: { error: jest.fn(), warn: jest.fn(), info: jest.fn() },
+}));
+
+jest.unstable_mockModule('../../../config/index.js', () => ({
+  default: { app: {}, get: jest.fn() },
+}));
+
+jest.unstable_mockModule('../repositories/organizations.repository.js', () => ({
+  default: {
+    remove: mockOrgRemove,
+    findOne: jest.fn().mockResolvedValue(null),
+  },
+}));
+
+jest.unstable_mockModule('../repositories/organizations.membership.repository.js', () => ({
+  default: {
+    deleteMany: mockMembershipDeleteMany,
+    list: mockMembershipList,
+  },
+}));
+
+jest.unstable_mockModule('../../users/services/users.service.js', () => ({
+  default: {
+    updateById: mockUpdateById,
+    findWithFilter: mockFindWithFilter,
+    getBrut: jest.fn(),
+  },
+}));
+
+const { default: OrgCrudService } = await import('../services/organizations.crud.service.js');
+const { onOrganizationRemoved, _reset } = await import('../lib/orgRemoval.registry.js');
+
+describe('OrgCrudService.remove() — org-removal registry', () => {
+  const organization = { _id: 'org-1' };
+
+  beforeEach(() => {
+    jest.clearAllMocks();
+    _reset();
+  });
+
+  test('fires every registered handler with { organizationId, organization }', async () => {
+    const handler = jest.fn().mockResolvedValue(undefined);
+    onOrganizationRemoved(handler);
+
+    await OrgCrudService.remove(organization);
+
+    expect(handler).toHaveBeenCalledTimes(1);
+    expect(handler).toHaveBeenCalledWith({ organizationId: 'org-1', organization });
+    expect(mockOrgRemove).toHaveBeenCalledWith(organization);
+  });
+
+  test('propagates a handler error and aborts before the repository remove', async () => {
+    onOrganizationRemoved(async () => {
+      throw new Error('tasks cleanup failed');
+    });
+
+    await expect(OrgCrudService.remove(organization)).rejects.toThrow('tasks cleanup failed');
+    expect(mockOrgRemove).not.toHaveBeenCalled();
+  });
+
+  test('with zero handlers registered, removes the organization without throwing', async () => {
+    await expect(OrgCrudService.remove(organization)).resolves.toEqual({ acknowledged: true });
+    expect(mockOrgRemove).toHaveBeenCalledWith(organization);
+  });
+});

modules/organizations/tests/organizations.orgRemoval.registry.unit.tests.js

@@ -0,0 +1,67 @@
+/**
+ * Unit tests for the organization-removal subscriber registry.
+ */
+import { describe, test, expect, beforeEach, jest } from '@jest/globals';
+
+import { onOrganizationRemoved, runOrganizationRemovedHandlers, _reset } from '../lib/orgRemoval.registry.js';
+
+describe('orgRemoval.registry', () => {
+  beforeEach(() => {
+    _reset();
+  });
+
+  test('runs a registered handler with the payload', async () => {
+    const handler = jest.fn().mockResolvedValue(undefined);
+    onOrganizationRemoved(handler);
+
+    const payload = { organizationId: 'org-1', organization: { _id: 'org-1' } };
+    await runOrganizationRemovedHandlers(payload);
+
+    expect(handler).toHaveBeenCalledTimes(1);
+    expect(handler).toHaveBeenCalledWith(payload);
+  });
+
+  test('runs multiple handlers sequentially in registration order', async () => {
+    const order = [];
+    onOrganizationRemoved(async () => {
+      order.push('first');
+    });
+    onOrganizationRemoved(async () => {
+      order.push('second');
+    });
+
+    await runOrganizationRemovedHandlers({ organizationId: 'org-1' });
+
+    expect(order).toEqual(['first', 'second']);
+  });
+
+  test('propagates a handler error (does not swallow) and aborts the remaining handlers', async () => {
+    const boom = new Error('cleanup failed');
+    const after = jest.fn().mockResolvedValue(undefined);
+    onOrganizationRemoved(async () => {
+      throw boom;
+    });
+    onOrganizationRemoved(after);
+
+    await expect(runOrganizationRemovedHandlers({ organizationId: 'org-1' })).rejects.toThrow('cleanup failed');
+    expect(after).not.toHaveBeenCalled();
+  });
+
+  test('runs zero handlers without throwing when none are registered', async () => {
+    await expect(runOrganizationRemovedHandlers({ organizationId: 'org-1' })).resolves.toBeUndefined();
+  });
+
+  test('rejects a non-function registration', () => {
+    expect(() => onOrganizationRemoved('not-a-fn')).toThrow(TypeError);
+  });
+
+  test('_reset clears registered handlers', async () => {
+    const handler = jest.fn().mockResolvedValue(undefined);
+    onOrganizationRemoved(handler);
+    _reset();
+
+    await runOrganizationRemovedHandlers({ organizationId: 'org-1' });
+
+    expect(handler).not.toHaveBeenCalled();
+  });
+});

modules/tasks/tasks.init.js

@@ -0,0 +1,20 @@
+/**
+ * @module tasks/tasks.init
+ * @description Boot-time wiring for the tasks module. Auto-discovered by the
+ *   modules glob and invoked by initModulesConfiguration at startup.
+ *   Registers an org-removal cleanup handler so org-scoped tasks are deleted
+ *   when an organization is removed.
+ */
+import TasksService from './services/tasks.service.js';
+import { onOrganizationRemoved } from '../organizations/lib/orgRemoval.registry.js';
+
+/**
+ * Tasks module initialisation.
+ * Registers an org-removal cleanup handler so all tasks scoped to an
+ * organization are deleted when that organization is removed.
+ *
+ * @returns {Promise<void>}
+ */
+export default async () => {
+  onOrganizationRemoved(({ organizationId }) => TasksService.deleteMany({ organizationId }));
+};

modules/tasks/tests/tasks.init.unit.tests.js

@@ -0,0 +1,45 @@
+/**
+ * Unit tests — tasks.init registers an org-removal cleanup handler that delegates to TasksService.deleteMany.
+ */
+import { jest, describe, test, expect, beforeEach } from '@jest/globals';
+
+const mockOnOrganizationRemoved = jest.fn();
+const mockDeleteMany = jest.fn().mockResolvedValue({ deletedCount: 0 });
+
+jest.unstable_mockModule('../../organizations/lib/orgRemoval.registry.js', () => ({
+  onOrganizationRemoved: mockOnOrganizationRemoved,
+}));
+
+jest.unstable_mockModule('../services/tasks.service.js', () => ({
+  default: { deleteMany: mockDeleteMany },
+}));
+
+const { default: tasksInit } = await import('../tasks.init.js');
+
+describe('tasks.init', () => {
+  beforeEach(() => {
+    jest.clearAllMocks();
+  });
+
+  test('registers a single org-removal handler at boot', async () => {
+    await tasksInit({});
+    expect(mockOnOrganizationRemoved).toHaveBeenCalledTimes(1);
+    expect(typeof mockOnOrganizationRemoved.mock.calls[0][0]).toBe('function');
+  });
+
+  test('the registered handler delegates to TasksService.deleteMany scoped by organizationId', async () => {
+    await tasksInit({});
+    const handler = mockOnOrganizationRemoved.mock.calls[0][0];
+
+    await handler({ organizationId: 'org-1', organization: { _id: 'org-1' } });
+
+    expect(mockDeleteMany).toHaveBeenCalledWith({ organizationId: 'org-1' });
+  });
+
+  test('importing tasks.init does not itself register — registration only happens on invocation (boot)', async () => {
+    // Module import alone must not register; only the default export (run at boot) does.
+    expect(mockOnOrganizationRemoved).not.toHaveBeenCalled();
+    await tasksInit({});
+    expect(mockOnOrganizationRemoved).toHaveBeenCalledTimes(1);
+  });
+});