From 1823f658c6573fb5ca5510ab9d33322490091b7a Mon Sep 17 00:00:00 2001
From: Aleksandr Saitgalin <32114927+Cred1Tor@users.noreply.github.com>
Date: Sat, 30 Apr 2022 01:47:17 +0400
Subject: [PATCH] update user and group in a transaction when creating a group.
 fixes #12124 (#13730)

* fix #12124

add a transaction for updating user and group
so the user doesn't lose gems when saving the group fails

* use mongoose transaction helper

use the helper instead of manually commiting/aborting
to deal with transient transaction errors

* increase timeout and add console.log outputs

add some logging to a failing test

* Revert "increase timeout and add console.log outputs"

This reverts commit 0c36aaa55fc5607211c0b96c11e865f21788b09e.

* add a test for gems when guild creation fails

test the transaction in createGroup()
make sure user keeps the gems if group.save() rejects

* fix(test): try suggested delay from PR discussion

Co-authored-by: SabreCat <sabe@habitica.com>
---
 .../v3/integration/groups/GET-groups.test.js   | 18 +++++++++++++-----
 .../v3/integration/groups/POST-groups.test.js  | 18 ++++++++++++++++++
 website/server/controllers/api-v3/groups.js    |  8 ++++++--
 3 files changed, 37 insertions(+), 7 deletions(-)

diff --git a/test/api/v3/integration/groups/GET-groups.test.js b/test/api/v3/integration/groups/GET-groups.test.js
index aaea5b397d..74c988e0b1 100644
--- a/test/api/v3/integration/groups/GET-groups.test.js
+++ b/test/api/v3/integration/groups/GET-groups.test.js
@@ -219,11 +219,19 @@ describe('GET /groups', () => {
 
     it('returns 30 guilds per page ordered by number of members', async () => {
       await user.update({ balance: 9000 });
-      const groups = await Promise.all(_.times(60, i => generateGroup(user, {
-        name: `public guild ${i} - is member`,
-        type: 'guild',
-        privacy: 'public',
-      })));
+      const delay = () => new Promise(resolve => setTimeout(resolve, 40));
+      const promises = [];
+
+      for (let i = 0; i < 60; i += 1) {
+        promises.push(generateGroup(user, {
+          name: `public guild ${i} - is member`,
+          type: 'guild',
+          privacy: 'public',
+        }));
+        await delay(); // eslint-disable-line no-await-in-loop
+      }
+
+      const groups = await Promise.all(promises);
 
       // update group number 32 and not the first to make sure sorting works
       await groups[32].update({ name: 'guild with most members', memberCount: 199 });
diff --git a/test/api/v3/integration/groups/POST-groups.test.js b/test/api/v3/integration/groups/POST-groups.test.js
index 934281209f..b507fc8165 100644
--- a/test/api/v3/integration/groups/POST-groups.test.js
+++ b/test/api/v3/integration/groups/POST-groups.test.js
@@ -2,6 +2,7 @@ import {
   generateUser,
   translate as t,
 } from '../../../../helpers/api-integration/v3';
+import { model as Group } from '../../../../../website/server/models/group';
 
 describe('POST /group', () => {
   let user;
@@ -203,6 +204,23 @@ describe('POST /group', () => {
 
         expect(updatedUser.balance).to.eql(user.balance - 1);
       });
+
+      it('does not deduct the gems from user when guild creation fails', async () => {
+        const stub = sinon.stub(Group.prototype, 'save').rejects();
+        const promise = user.post('/groups', {
+          name: groupName,
+          type: groupType,
+          privacy: groupPrivacy,
+        });
+
+        await expect(promise).to.eventually.be.rejected;
+
+        const updatedUser = await user.get('/user');
+
+        expect(updatedUser.balance).to.eql(user.balance);
+
+        stub.restore();
+      });
     });
   });
 
diff --git a/website/server/controllers/api-v3/groups.js b/website/server/controllers/api-v3/groups.js
index 5a3bc2424c..36910079e8 100644
--- a/website/server/controllers/api-v3/groups.js
+++ b/website/server/controllers/api-v3/groups.js
@@ -137,8 +137,12 @@ api.createGroup = {
       user.party._id = group._id;
     }
 
-    const results = await Promise.all([user.save(), group.save()]);
-    const savedGroup = results[1];
+    let savedGroup;
+
+    await Group.db.transaction(async session => {
+      await user.save({ session });
+      savedGroup = await group.save({ session });
+    });
 
     // Instead of populate we make a find call manually because of https://github.com/Automattic/mongoose/issues/3833
     // await Q.ninvoke(savedGroup, 'populate', ['leader', nameFields]);