fix(organizations,users): address CodeRabbit review — returnDocument, membership permissions

- Replace deprecated {new: true} with {returnDocument: 'after'} in repositories
- Return empty array for non-admin users listing pending requests instead of 403
- Remove policy middleware from GET requests route (controller handles auth)
- Add integration tests for pending requests permissions
- Update e2e tests to match new behavior

Author: PierreBrisorgueil

modules/organizations/controllers/organizations.membershipRequest.controller.js

@@ -33,8 +33,8 @@ const create = async (req, res) => {
  */
 const listPending = async (req, res) => {
   try {
-    if (req.membership && req.membership.role === 'member') {
-      return responses.error(res, 403, 'Forbidden', 'Only admin or owner can list pending requests')();
+    if (!req.membership || req.membership.role === 'member') {
+      return responses.success(res, 'membership request list')([]);
     }
     const requests = await MembershipService.listPending(req.organization._id || req.organization.id);
     responses.success(res, 'membership request list')(requests);

modules/organizations/routes/organizations.membershipRequest.routes.js

@@ -24,7 +24,7 @@ export default (app) => {
     .route('/api/organizations/:organizationId/requests')
     .all(passport.authenticate('jwt', { session: false }))
     .post(limiters.api, organizations.loadMembership, policy.isAllowed, membershipRequests.create)
-    .get(organizations.loadMembership, policy.isAllowed, membershipRequests.listPending);
+    .get(organizations.loadMembership, membershipRequests.listPending);
 
   // Approve a membership request
   app

modules/organizations/tests/organizations.domainJoin.e2e.tests.js

@@ -161,17 +161,6 @@ describe('Organizations domain join E2E tests:', () => {
       }
     });
 
-    test('pending user cannot list pending requests — 403', async () => {
-      try {
-        await agentMember
-          .get(`/api/organizations/${org._id}/requests`)
-          .expect(403);
-      } catch (err) {
-        console.log(err);
-        expect(err).toBeFalsy();
-      }
-    });
-
     test('owner can list pending requests and sees the new joiner — 200', async () => {
       try {
         const result = await agentOwner

modules/organizations/tests/organizations.integration.tests.js

@@ -95,6 +95,56 @@ describe('Organizations integration tests:', () => {
     });
   });
 
+  describe('GET /api/organizations/:id/requests (pending requests permissions)', () => {
+    let owner;
+    let member;
+    let ownerAgent;
+    let memberAgent;
+    let org;
+
+    beforeAll(async () => {
+      config.organizations = { enabled: true, autoCreate: true, domainMatching: false };
+      ownerAgent = request.agent((await bootstrap()).app);
+      memberAgent = request.agent((await bootstrap()).app);
+
+      // Create owner with auto-created org
+      const ownerRes = await ownerAgent
+        .post('/api/auth/signup')
+        .send({ firstName: 'Owner', lastName: 'Test', email: 'owner-req@test.com', password: 'W@os.jsI$Aw3$0m3', provider: 'local' })
+        .expect(200);
+      owner = ownerRes.body.user;
+
+      const orgsRes = await ownerAgent.get('/api/organizations').expect(200);
+      org = orgsRes.body.data[0];
+
+      // Create member and add to same org
+      const memberRes = await memberAgent
+        .post('/api/auth/signup')
+        .send({ firstName: 'Member', lastName: 'Test', email: 'member-req@test.com', password: 'W@os.jsI$Aw3$0m3', provider: 'local' })
+        .expect(200);
+      member = memberRes.body.user;
+
+      // Add member to owner's org as 'member'
+      const MembershipService = (await import(path.resolve('./modules/organizations/services/organizations.membership.service.js'))).default;
+      await MembershipService.create({ userId: member._id || member.id, organizationId: org._id || org.id, role: 'member' });
+    });
+
+    test('owner should see pending requests — 200 with array', async () => {
+      const result = await ownerAgent.get(`/api/organizations/${org._id || org.id}/requests`).expect(200);
+      expect(result.body.data).toBeInstanceOf(Array);
+    });
+
+    test('member should get empty array for pending requests — 200', async () => {
+      const result = await memberAgent.get(`/api/organizations/${org._id || org.id}/requests`).expect(200);
+      expect(result.body.data).toEqual([]);
+    });
+
+    afterAll(async () => {
+      await cleanupUser(owner);
+      await cleanupUser(member);
+    });
+  });
+
   // Mongoose disconnect
   afterAll(async () => {
     config.organizations = { ...originalOrganizations };

modules/uploads/repositories/uploads.repository.js

@@ -44,7 +44,7 @@ const getStream = (upload) => {
  * @param {Object} update
  * @return {Object} upload updated
  */
-const update = (id, update) => Uploads.findOneAndUpdate({ _id: id }, update, { new: true }).exec();
+const update = (id, update) => Uploads.findOneAndUpdate({ _id: id }, update, { returnDocument: 'after' }).exec();
 
 /**
  * @desc Function to remove an upload from db

modules/users/repositories/users.repository.js

@@ -85,7 +85,7 @@ const search = (input) => User.find(input).exec();
  */
 const update = (user) => {
   if (user._id) {
-    return User.findByIdAndUpdate(user._id, user, { new: true, runValidators: true }).exec();
+    return User.findByIdAndUpdate(user._id, user, { returnDocument: 'after', runValidators: true }).exec();
   }
   return new User(user).save();
 };
@@ -173,7 +173,7 @@ const updateById = (id, data) => User.updateOne({ _id: id }, data, { runValidato
  * @returns {Object} updated user
  */
 const findByIdAndUpdatePopulated = (id, data, populateFields) =>
-  User.findByIdAndUpdate(id, data, { new: true, runValidators: true }).populate(populateFields).exec();
+  User.findByIdAndUpdate(id, data, { returnDocument: 'after', runValidators: true }).populate(populateFields).exec();
 
 /**
  * @desc Function to find users matching a filter with optional field selection