From b0a889531dc4ed51eda2eff27d6b6756434083e6 Mon Sep 17 00:00:00 2001
From: Pierre Brisorgueil <pierrebrisorgueil@me.com>
Date: Sat, 13 Jun 2026 18:13:01 +0200
Subject: [PATCH 1/3] test(organizations): member-add lifecycle http e2e
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Covers POST /members, the GET /members/search before-:memberId route
ordering, the auth-only accept and decline endpoints, promote via the
:memberId param middleware, leave, pending owner_add visibility in the
members list, and the 422 duplicate-add guard — all through real routes,
passport, CASL, and param middleware. Restores the HTTP coverage lost
when the invite-lifecycle e2e was removed.

refs #3835
---
 .../organizations.memberAdd.e2e.tests.js      | 300 ++++++++++++++++++
 1 file changed, 300 insertions(+)
 create mode 100644 modules/organizations/tests/organizations.memberAdd.e2e.tests.js

diff --git a/modules/organizations/tests/organizations.memberAdd.e2e.tests.js b/modules/organizations/tests/organizations.memberAdd.e2e.tests.js
new file mode 100644
index 000000000..53a759cd9
--- /dev/null
+++ b/modules/organizations/tests/organizations.memberAdd.e2e.tests.js
@@ -0,0 +1,300 @@
+/**
+ * @desc E2E tests for the owner-add membership lifecycle through real HTTP routes
+ * (passport + CASL + param middleware): owner adds a member → pending row visible
+ * in the members list → invited user accepts → owner promotes → member leaves.
+ * Also covers the invited user's decline path and the duplicate-add guard (422).
+ */
+import request from 'supertest';
+import path from 'path';
+
+import { bootstrap } from '../../../lib/app.js';
+import mongooseService from '../../../lib/services/mongoose.js';
+import config from '../../../config/index.js';
+
+describe('Organizations member-add E2E tests:', () => {
+  let UserService;
+  let OrganizationsRepository;
+  let MembershipRepository;
+  let agent;
+
+  const password = 'W@os.jsI$Aw3$0m3';
+
+  // Store original config
+  const originalOrganizations = { ...config.organizations };
+
+  /**
+   * @description Reset organizations config to original state.
+   */
+  const resetOrgConfig = () => {
+    config.organizations = { ...originalOrganizations };
+  };
+
+  /**
+   * @description Clean up a user and their associated organizations/memberships.
+   * @param {Object} user - The user object to clean up.
+   */
+  const cleanupUser = async (user) => {
+    if (!user) return;
+    try {
+      const memberships = await MembershipRepository.list({ userId: user.id || user._id });
+      for (const m of memberships) {
+        const orgId = m.organizationId._id || m.organizationId;
+        await MembershipRepository.deleteMany({ organizationId: orgId });
+        await OrganizationsRepository.deleteMany({ _id: orgId });
+      }
+      await UserService.remove(user);
+    } catch (_) { /* cleanup — ignore errors */ }
+  };
+
+  beforeAll(async () => {
+    try {
+      const init = await bootstrap();
+      UserService = (await import(path.resolve('./modules/users/services/users.service.js'))).default;
+      OrganizationsRepository = (await import(path.resolve('./modules/organizations/repositories/organizations.repository.js'))).default;
+      MembershipRepository = (await import(path.resolve('./modules/organizations/repositories/organizations.membership.repository.js'))).default;
+      agent = request.agent(init.app);
+    } catch (err) {
+      console.log(err);
+      expect(err).toBeFalsy();
+    }
+  });
+
+  describe('Full member-add lifecycle', () => {
+    let userA;
+    let userB;
+    let orgA;
+    let agentA;
+    let agentB;
+
+    afterAll(async () => {
+      // Clean up org A and its memberships, then B before A
+      try {
+        if (orgA) {
+          await MembershipRepository.deleteMany({ organizationId: orgA._id });
+          await OrganizationsRepository.deleteMany({ _id: orgA._id });
+        }
+      } catch (_) { /* cleanup */ }
+      await cleanupUser(userB);
+      await cleanupUser(userA);
+    });
+
+    test('full lifecycle: add → pending in list → accept → promote → leave', async () => {
+      config.organizations = { enabled: true, autoCreate: true, domainMatching: false };
+      agentA = request.agent(agent.app);
+      agentB = request.agent(agent.app);
+
+      let membershipId;
+
+      // 1. Signup user A (auto-creates org A), then user B
+      try {
+        const resultA = await agentA
+          .post('/api/auth/signup')
+          .send({
+            firstName: 'MemberAddA',
+            lastName: 'User',
+            email: 'member-add-a@test.com',
+            password,
+            provider: 'local',
+          })
+          .expect(200);
+        userA = resultA.body.user;
+        orgA = resultA.body.organization;
+        expect(orgA).toBeDefined();
+        expect(orgA).not.toBeNull();
+      } catch (err) {
+        console.log(err);
+        expect(err).toBeFalsy();
+      }
+
+      try {
+        const resultB = await agentB
+          .post('/api/auth/signup')
+          .send({
+            firstName: 'MemberAddB',
+            lastName: 'User',
+            email: 'member-add-b@test.com',
+            password,
+            provider: 'local',
+          })
+          .expect(200);
+        userB = resultB.body.user;
+      } catch (err) {
+        console.log(err);
+        expect(err).toBeFalsy();
+      }
+
+      // 2. A looks up B by exact email. Asserts the /members/search route is
+      // registered BEFORE /members/:memberId — if 'search' were captured as
+      // :memberId, memberByID would 404 on the invalid ObjectId.
+      try {
+        const searchRes = await agentA
+          .get(`/api/organizations/${orgA._id}/members/search`)
+          .query({ email: 'member-add-b@test.com' })
+          .expect(200);
+        expect(searchRes.body.message).toBe('user lookup');
+        expect(searchRes.body.data).not.toBeNull();
+        expect(searchRes.body.data.id).toBe(userB.id);
+        expect(searchRes.body.data.email).toBe('member-add-b@test.com');
+      } catch (err) {
+        console.log(err);
+        expect(err).toBeFalsy();
+      }
+
+      // 3. A adds B → PENDING owner_add membership (consent not granted yet)
+      try {
+        const addRes = await agentA
+          .post(`/api/organizations/${orgA._id}/members`)
+          .send({ userId: userB.id, role: 'member' })
+          .expect(200);
+        expect(addRes.body.message).toBe('membership invitation created');
+        membershipId = addRes.body.data._id;
+        expect(membershipId).toBeDefined();
+        expect(addRes.body.data.status).toBe('pending');
+        expect(addRes.body.data.source).toBe('owner_add');
+      } catch (err) {
+        console.log(err);
+        expect(err).toBeFalsy();
+      }
+
+      // 4. The members list shows the pending owner_add row alongside the active
+      // owner (pending owner_add rows are intentionally visible to the org).
+      try {
+        const listRes = await agentA.get(`/api/organizations/${orgA._id}/members`).expect(200);
+        expect(listRes.body.message).toBe('membership list');
+        const rowB = listRes.body.data.find((m) => m._id === membershipId);
+        expect(rowB).toBeDefined();
+        expect(rowB.status).toBe('pending');
+        expect(rowB.source).toBe('owner_add');
+        const rowA = listRes.body.data.find((m) => m.userId?.email === 'member-add-a@test.com');
+        expect(rowA).toBeDefined();
+        expect(rowA.status).toBe('active');
+        expect(rowA.role).toBe('owner');
+      } catch (err) {
+        console.log(err);
+        expect(err).toBeFalsy();
+      }
+
+      // 5. B sees the invitation on the auth-only mine/pending surface
+      try {
+        const mineRes = await agentB.get('/api/membership-requests/mine/pending').expect(200);
+        expect(mineRes.body.message).toBe('membership invitation list');
+        const invite = mineRes.body.data.find((m) => m._id === membershipId);
+        expect(invite).toBeDefined();
+        expect(invite.status).toBe('pending');
+        expect(invite.source).toBe('owner_add');
+      } catch (err) {
+        console.log(err);
+        expect(err).toBeFalsy();
+      }
+
+      // 6. B accepts (auth-only route, consent gate in the service) → ACTIVE
+      try {
+        const acceptRes = await agentB
+          .put(`/api/membership-requests/${membershipId}/accept`)
+          .expect(200);
+        expect(acceptRes.body.message).toBe('membership invitation accepted');
+        expect(acceptRes.body.data.status).toBe('active');
+      } catch (err) {
+        console.log(err);
+        expect(err).toBeFalsy();
+      }
+
+      // 7. A promotes B to admin through the :memberId param middleware
+      try {
+        const promoteRes = await agentA
+          .put(`/api/organizations/${orgA._id}/members/${membershipId}`)
+          .send({ role: 'admin' })
+          .expect(200);
+        expect(promoteRes.body.message).toBe('membership updated');
+        expect(promoteRes.body.data.role).toBe('admin');
+      } catch (err) {
+        console.log(err);
+        expect(err).toBeFalsy();
+      }
+
+      // 8. B leaves → members list back to A only
+      try {
+        const leaveRes = await agentB.post(`/api/organizations/${orgA._id}/leave`).expect(200);
+        expect(leaveRes.body.message).toBe('organization left');
+        const finalList = await agentA.get(`/api/organizations/${orgA._id}/members`).expect(200);
+        expect(finalList.body.data.length).toBe(1);
+        expect(finalList.body.data[0].userId?.email).toBe('member-add-a@test.com');
+      } catch (err) {
+        console.log(err);
+        expect(err).toBeFalsy();
+      }
+    });
+
+    // Runs after the lifecycle test (Jest executes tests in declaration order with
+    // --runInBand): A owns org A, B has no membership left.
+    test('decline path: re-add → duplicate guard 422 → B declines → row gone everywhere', async () => {
+      let declineId;
+
+      // 1. A re-adds B → a fresh PENDING owner_add row
+      try {
+        const reAddRes = await agentA
+          .post(`/api/organizations/${orgA._id}/members`)
+          .send({ userId: userB.id, role: 'member' })
+          .expect(200);
+        declineId = reAddRes.body.data._id;
+        expect(declineId).toBeDefined();
+        expect(reAddRes.body.data.status).toBe('pending');
+      } catch (err) {
+        console.log(err);
+        expect(err).toBeFalsy();
+      }
+
+      // 2. Duplicate guard: adding B again while pending is rejected (422)
+      try {
+        const dupRes = await agentA
+          .post(`/api/organizations/${orgA._id}/members`)
+          .send({ userId: userB.id, role: 'member' });
+        expect(dupRes.status).toBe(422);
+        expect(dupRes.body.type).toBe('error');
+        expect(dupRes.body.description).toBe('This user already has a pending membership for this organization.');
+      } catch (err) {
+        console.log(err);
+        expect(err).toBeFalsy();
+      }
+
+      // 3. B sees the invitation, then DECLINES it (DELETE, auth-only)
+      try {
+        const mineRes = await agentB.get('/api/membership-requests/mine/pending').expect(200);
+        const invite = mineRes.body.data.find((m) => m._id === declineId);
+        expect(invite).toBeDefined();
+
+        await agentB.delete(`/api/membership-requests/${declineId}`).expect(200);
+      } catch (err) {
+        console.log(err);
+        expect(err).toBeFalsy();
+      }
+
+      // 4. The row is gone everywhere — mine/pending, the members list, the DB
+      try {
+        const mineAfter = await agentB.get('/api/membership-requests/mine/pending').expect(200);
+        expect(mineAfter.body.data.find((m) => m._id === declineId)).toBeUndefined();
+
+        const listAfter = await agentA.get(`/api/organizations/${orgA._id}/members`).expect(200);
+        expect(listAfter.body.data.find((m) => m._id === declineId)).toBeUndefined();
+        expect(listAfter.body.data.length).toBe(1);
+
+        const row = await MembershipRepository.findOne({ _id: declineId });
+        expect(row).toBeNull();
+      } catch (err) {
+        console.log(err);
+        expect(err).toBeFalsy();
+      }
+    });
+  });
+
+  // Mongoose disconnect
+  afterAll(async () => {
+    resetOrgConfig();
+    try {
+      await mongooseService.disconnect();
+    } catch (err) {
+      console.log(err);
+      expect(err).toBeFalsy();
+    }
+  });
+});

From a36228ecaf480bdbad9e551827b89d2fb54bb127 Mon Sep 17 00:00:00 2001
From: Pierre Brisorgueil <pierrebrisorgueil@me.com>
Date: Sat, 13 Jun 2026 18:30:54 +0200
Subject: [PATCH 2/3] test(organizations): harden memberAdd e2e against
 dirty-db + false-green

- unique per-run email suffixes (Date.now) eliminate dirty-db re-run flakiness
- config set in beforeAll, restored in afterAll; test-2 guards shared state with clear expect()
- duplicate-add guard chains .expect(422) so a 500 is no longer swallowed
- post-leave / post-decline assertions use identity (no B-row) instead of bare counts
- decline DELETE asserts body.message === 'membership invitation declined'

refs #3835
---
 .../organizations.memberAdd.e2e.tests.js      | 47 ++++++++++++++-----
 1 file changed, 34 insertions(+), 13 deletions(-)

diff --git a/modules/organizations/tests/organizations.memberAdd.e2e.tests.js b/modules/organizations/tests/organizations.memberAdd.e2e.tests.js
index 53a759cd9..70b9fe332 100644
--- a/modules/organizations/tests/organizations.memberAdd.e2e.tests.js
+++ b/modules/organizations/tests/organizations.memberAdd.e2e.tests.js
@@ -66,7 +66,19 @@ describe('Organizations member-add E2E tests:', () => {
     let agentA;
     let agentB;
 
+    // Finding #1: unique-per-run emails to avoid dirty-DB re-run flakiness
+    const suffix = Date.now();
+    const emailA = `member-add-a-${suffix}@test.com`;
+    const emailB = `member-add-b-${suffix}@test.com`;
+
+    // Finding #2: move config assignment into beforeAll so it is always set
+    // before any test in this describe block runs
+    beforeAll(() => {
+      config.organizations = { enabled: true, autoCreate: true, domainMatching: false };
+    });
+
     afterAll(async () => {
+      resetOrgConfig();
       // Clean up org A and its memberships, then B before A
       try {
         if (orgA) {
@@ -79,7 +91,6 @@ describe('Organizations member-add E2E tests:', () => {
     });
 
     test('full lifecycle: add → pending in list → accept → promote → leave', async () => {
-      config.organizations = { enabled: true, autoCreate: true, domainMatching: false };
       agentA = request.agent(agent.app);
       agentB = request.agent(agent.app);
 
@@ -92,7 +103,7 @@ describe('Organizations member-add E2E tests:', () => {
           .send({
             firstName: 'MemberAddA',
             lastName: 'User',
-            email: 'member-add-a@test.com',
+            email: emailA,
             password,
             provider: 'local',
           })
@@ -112,7 +123,7 @@ describe('Organizations member-add E2E tests:', () => {
           .send({
             firstName: 'MemberAddB',
             lastName: 'User',
-            email: 'member-add-b@test.com',
+            email: emailB,
             password,
             provider: 'local',
           })
@@ -129,12 +140,12 @@ describe('Organizations member-add E2E tests:', () => {
       try {
         const searchRes = await agentA
           .get(`/api/organizations/${orgA._id}/members/search`)
-          .query({ email: 'member-add-b@test.com' })
+          .query({ email: emailB })
           .expect(200);
         expect(searchRes.body.message).toBe('user lookup');
         expect(searchRes.body.data).not.toBeNull();
         expect(searchRes.body.data.id).toBe(userB.id);
-        expect(searchRes.body.data.email).toBe('member-add-b@test.com');
+        expect(searchRes.body.data.email).toBe(emailB);
       } catch (err) {
         console.log(err);
         expect(err).toBeFalsy();
@@ -165,7 +176,7 @@ describe('Organizations member-add E2E tests:', () => {
         expect(rowB).toBeDefined();
         expect(rowB.status).toBe('pending');
         expect(rowB.source).toBe('owner_add');
-        const rowA = listRes.body.data.find((m) => m.userId?.email === 'member-add-a@test.com');
+        const rowA = listRes.body.data.find((m) => m.userId?.email === emailA);
         expect(rowA).toBeDefined();
         expect(rowA.status).toBe('active');
         expect(rowA.role).toBe('owner');
@@ -213,12 +224,14 @@ describe('Organizations member-add E2E tests:', () => {
       }
 
       // 8. B leaves → members list back to A only
+      // Finding #4: assert identity (A's row) rather than a fragile bare count
       try {
         const leaveRes = await agentB.post(`/api/organizations/${orgA._id}/leave`).expect(200);
         expect(leaveRes.body.message).toBe('organization left');
         const finalList = await agentA.get(`/api/organizations/${orgA._id}/members`).expect(200);
-        expect(finalList.body.data.length).toBe(1);
-        expect(finalList.body.data[0].userId?.email).toBe('member-add-a@test.com');
+        // precondition: B must not appear in the list after leaving
+        expect(finalList.body.data.find((m) => m.userId?.email === emailB)).toBeUndefined();
+        expect(finalList.body.data[0].userId?.email).toBe(emailA);
       } catch (err) {
         console.log(err);
         expect(err).toBeFalsy();
@@ -228,6 +241,10 @@ describe('Organizations member-add E2E tests:', () => {
     // Runs after the lifecycle test (Jest executes tests in declaration order with
     // --runInBand): A owns org A, B has no membership left.
     test('decline path: re-add → duplicate guard 422 → B declines → row gone everywhere', async () => {
+      // Finding #2: guard against test-1 abort leaving shared state unpopulated
+      // precondition: the lifecycle test must have populated shared state
+      expect(orgA && userB && agentA && agentB).toBeTruthy();
+
       let declineId;
 
       // 1. A re-adds B → a fresh PENDING owner_add row
@@ -245,11 +262,12 @@ describe('Organizations member-add E2E tests:', () => {
       }
 
       // 2. Duplicate guard: adding B again while pending is rejected (422)
+      // Finding #3: chain .expect(422) so a 500 is not swallowed
       try {
         const dupRes = await agentA
           .post(`/api/organizations/${orgA._id}/members`)
-          .send({ userId: userB.id, role: 'member' });
-        expect(dupRes.status).toBe(422);
+          .send({ userId: userB.id, role: 'member' })
+          .expect(422);
         expect(dupRes.body.type).toBe('error');
         expect(dupRes.body.description).toBe('This user already has a pending membership for this organization.');
       } catch (err) {
@@ -258,25 +276,29 @@ describe('Organizations member-add E2E tests:', () => {
       }
 
       // 3. B sees the invitation, then DECLINES it (DELETE, auth-only)
+      // Finding #5: assert the decline response body message
       try {
         const mineRes = await agentB.get('/api/membership-requests/mine/pending').expect(200);
         const invite = mineRes.body.data.find((m) => m._id === declineId);
         expect(invite).toBeDefined();
 
-        await agentB.delete(`/api/membership-requests/${declineId}`).expect(200);
+        const declineRes = await agentB.delete(`/api/membership-requests/${declineId}`).expect(200);
+        expect(declineRes.body.message).toBe('membership invitation declined');
       } catch (err) {
         console.log(err);
         expect(err).toBeFalsy();
       }
 
       // 4. The row is gone everywhere — mine/pending, the members list, the DB
+      // Finding #4: assert no B-row remains (identity) rather than a bare count
       try {
         const mineAfter = await agentB.get('/api/membership-requests/mine/pending').expect(200);
         expect(mineAfter.body.data.find((m) => m._id === declineId)).toBeUndefined();
 
         const listAfter = await agentA.get(`/api/organizations/${orgA._id}/members`).expect(200);
         expect(listAfter.body.data.find((m) => m._id === declineId)).toBeUndefined();
-        expect(listAfter.body.data.length).toBe(1);
+        // assert B is not in the list rather than relying on a hard count
+        expect(listAfter.body.data.find((m) => m.userId?.email === emailB)).toBeUndefined();
 
         const row = await MembershipRepository.findOne({ _id: declineId });
         expect(row).toBeNull();
@@ -289,7 +311,6 @@ describe('Organizations member-add E2E tests:', () => {
 
   // Mongoose disconnect
   afterAll(async () => {
-    resetOrgConfig();
     try {
       await mongooseService.disconnect();
     } catch (err) {

From 6d631d2c26eda9092c54a1321c24513c3416e1bc Mon Sep 17 00:00:00 2001
From: Pierre Brisorgueil <pierrebrisorgueil@me.com>
Date: Sat, 13 Jun 2026 18:34:29 +0200
Subject: [PATCH 3/3] docs(test): add @returns + clarify test-order comment on
 memberadd e2e

Documents cleanupUser's async return and rewrites the misleading

--runInBand comment (it schedules files, not test() order). refs #3835
---
 .../tests/organizations.memberAdd.e2e.tests.js              | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/modules/organizations/tests/organizations.memberAdd.e2e.tests.js b/modules/organizations/tests/organizations.memberAdd.e2e.tests.js
index 70b9fe332..97601dd1d 100644
--- a/modules/organizations/tests/organizations.memberAdd.e2e.tests.js
+++ b/modules/organizations/tests/organizations.memberAdd.e2e.tests.js
@@ -32,6 +32,7 @@ describe('Organizations member-add E2E tests:', () => {
   /**
    * @description Clean up a user and their associated organizations/memberships.
    * @param {Object} user - The user object to clean up.
+   * @returns {Promise<void>}
    */
   const cleanupUser = async (user) => {
     if (!user) return;
@@ -238,8 +239,9 @@ describe('Organizations member-add E2E tests:', () => {
       }
     });
 
-    // Runs after the lifecycle test (Jest executes tests in declaration order with
-    // --runInBand): A owns org A, B has no membership left.
+    // Jest runs test() blocks within a file in declaration order, so this runs after
+    // the lifecycle test above (which leaves A owning org A and B with no membership);
+    // the precondition assert below guards against that test having aborted.
     test('decline path: re-add → duplicate guard 422 → B declines → row gone everywhere', async () => {
       // Finding #2: guard against test-1 abort leaving shared state unpopulated
       // precondition: the lifecycle test must have populated shared state