feat(groups): update memberCount on successful user removal

- This commit adds functionality to update the `memberCount` property of a group when a
member is successfully removed from the group. 
- It also includes error handling for
scenarios where the user or group save operations fail, ensuring the `memberCount` is
not updated in those cases. 
- Additionally, a transaction-based approach is
implemented to roll back the `memberCount` update if the overall transaction fails.

test/api/v3/integration/groups/POST-groups_id_removeMember.test.js

@@ -1,10 +1,13 @@
+import mongoose from 'mongoose';
 import {
   generateUser,
   createAndPopulateGroup,
   translate as t,
   sleep,
 } from '../../../../helpers/api-integration/v3';
 import * as email from '../../../../../website/server/libs/email';
+import { schema as groupSchema } from '../../../../../website/server/models/group';
+import { schema as userSchema } from '../../../../../website/server/models/user/index';
 
 describe('POST /groups/:groupId/removeMember/:memberId', () => {
   let leader;
@@ -88,6 +91,58 @@ describe('POST /groups/:groupId/removeMember/:memberId', () => {
       await expect(leader.get(`/groups/${guild._id}`)).to.eventually.have.property('memberCount', oldMemberCount - 1);
     });
 
+    it('updates memberCount when removal is successful', async () => {
+      const oldMemberCount = guild.memberCount;
+      await leader.post(`/groups/${guild._id}/removeMember/${member._id}`);
+
+      const updatedGuild = await leader.get(`/groups/${guild._id}`);
+      expect(updatedGuild.memberCount).to.equal(oldMemberCount - 1);
+    });
+
+    it('does not update memberCount when user removal fails', async () => {
+      const oldMemberCount = guild.memberCount;
+      const user = mongoose.model('User', userSchema);
+      sandbox.stub(user.prototype, 'save').throws();
+
+      await expect(leader.post(`/groups/${guild._id}/removeMember/${member._id}`))
+        .to.eventually.be.rejected;
+
+      const updatedGuild = await leader.get(`/groups/${guild._id}`);
+      expect(updatedGuild.memberCount).to.equal(oldMemberCount);
+    });
+
+    it('does not update memberCount when group save fails', async () => {
+      const oldMemberCount = guild.memberCount;
+      const group = mongoose.model('Group', groupSchema);
+
+      sandbox.stub(group.prototype, 'save').throws(new Error('Failed to save group'));
+
+      await expect(leader.post(`/groups/${guild._id}/removeMember/${member._id}`))
+        .to.eventually.be.rejected;
+
+      const updatedGuild = await leader.get(`/groups/${guild._id}`);
+      expect(updatedGuild.memberCount).to.equal(oldMemberCount);
+    });
+
+    it('rolls back memberCount update if transaction fails', async () => {
+      const oldMemberCount = guild.memberCount;
+
+      sandbox.stub(mongoose, 'startSession').returns({
+        startTransaction: () => {},
+        commitTransaction: () => {
+          throw new Error('Transaction failed');
+        },
+        abortTransaction: () => {},
+        endSession: () => {},
+      });
+
+      await expect(leader.post(`/groups/${guild._id}/removeMember/${member._id}`))
+        .to.eventually.be.rejected;
+
+      const updatedGuild = await leader.get(`/groups/${guild._id}`);
+      expect(updatedGuild.memberCount).to.equal(oldMemberCount);
+    });
+
     it('can remove other invites', async () => {
       await leader.post(`/groups/${guild._id}/removeMember/${invitedUser._id}`);
 

website/server/controllers/api-v3/groups.js

@@ -1,6 +1,7 @@
 import _ from 'lodash';
 import nconf from 'nconf';
 import moment from 'moment';
+import mongoose from 'mongoose';
 import { authWithHeaders } from '../../middlewares/auth';
 import {
   model as Group,
@@ -14,6 +15,9 @@ import {
   NotFound,
   BadRequest,
   NotAuthorized,
+  TransactionError,
+  DatabaseError,
+  InternalServerError,
 } from '../../libs/errors';
 import { removeFromArray } from '../../libs/collectionManipulators';
 import { sendTxn as sendTxnEmail } from '../../libs/email';
@@ -957,14 +961,7 @@ api.removeGroupMember = {
     }
 
     if (isInGroup) {
-      // For parties we count the number of members from the database to get the correct value.
-      // See #12275 on why this is necessary and only done for parties.
-      if (group.type === 'party') {
-        const currentMembers = await group.getMemberCount();
-        group.memberCount = currentMembers - 1;
-      } else {
-        group.memberCount -= 1;
-      }
+      group.memberCount -= 1;
 
       if (group.quest && group.quest.leader === member._id) {
         throw new NotAuthorized(res.t('cannotRemoveQuestOwner'));
@@ -1005,10 +1002,28 @@ api.removeGroupMember = {
     const message = req.query.message || req.body.message;
     _sendMessageToRemoved(group, member, message, isInGroup);
 
-    await Promise.all([
-      member.save(),
-      group.save(),
-    ]);
+    const session = await mongoose.startSession();
+
+    try {
+      await session.withTransaction(async () => {
+        await member.save({ session });
+        await group.save({ session });
+      }, {
+        retryWrites: true,
+      });
+    } catch (err) {
+      if (err.name === 'MongoError') {
+        throw err.hasErrorLabel('TransactionTooLargeForCache')
+          ? new TransactionError(`Transaction too large for cache: ${err.message}`)
+          : new DatabaseError(`Database error: ${err.message}`);
+      } else if (err.name === 'ValidationError') {
+        throw validationErrors;
+      } else {
+        throw new InternalServerError(`Unexpected error: ${err.message}`);
+      }
+    } finally {
+      session.endSession();
+    }
 
     if (isInGroup && group.hasNotCancelled()) {
       await group.updateGroupPlan(true);

website/server/libs/errors.js

@@ -117,3 +117,39 @@ export class InternalServerError extends CustomError {
     this.message = customMessage || 'An unexpected error occurred.';
   }
 }
+
+/**
+ * @apiDefine TransactionError
+ * @apiError TransactionError An error occurred during a transaction that could not be resolved.
+ *
+ * @apiErrorExample Error-Response:
+ *     HTTP/1.1 500 Internal Server Error
+ *     {
+ *       "error": "TransactionError",
+ *       "message": "The transaction failed to commit after multiple attempts."
+ *     }
+ */
+export class TransactionError extends InternalServerError {
+  constructor (customMessage) {
+    super();
+    this.message = customMessage || 'The transaction failed to commit after multiple attempts.';
+  }
+}
+
+/**
+ * @apiDefine DatabaseError
+ * @apiError DatabaseError An error occurred while interacting with the database.
+ *
+ * @apiErrorExample Error-Response:
+ *     HTTP/1.1 500 Internal Server Error
+ *     {
+ *       "error": "DatabaseError",
+ *       "message": "A database error occurred."
+ *     }
+ */
+export class DatabaseError extends InternalServerError {
+  constructor (customMessage) {
+    super();
+    this.message = customMessage || 'A database error occurred.';
+  }
+}