From e76bdbd62d49f47940ef13dd8dd78ce2cefe30e3 Mon Sep 17 00:00:00 2001
From: Matteo Pagliazzi <matteopagliazzi@gmail.com>
Date: Sun, 18 Nov 2018 20:48:16 +0100
Subject: [PATCH] Fix for #10814, prevent ParallelSave errors (#10852)

* fix(group leave): prevent ParallelSave errors while leaving a group with multiple group or challenge tasks

* fix typo
---
 website/server/models/challenge.js | 17 ++++++++++++++---
 website/server/models/group.js     | 23 ++++++++++++++++-------
 2 files changed, 30 insertions(+), 10 deletions(-)

diff --git a/website/server/models/challenge.js b/website/server/models/challenge.js
index 2bb773f925..492b0592dd 100644
--- a/website/server/models/challenge.js
+++ b/website/server/models/challenge.js
@@ -258,7 +258,7 @@ schema.methods.removeTask = async function challengeRemoveTask (task) {
 };
 
 // Unlink challenges tasks (and the challenge itself) from user. TODO rename to 'leave'
-schema.methods.unlinkTasks = async function challengeUnlinkTasks (user, keep) {
+schema.methods.unlinkTasks = async function challengeUnlinkTasks (user, keep, saveUser = true) {
   let challengeId = this._id;
   let findQuery = {
     userId: user._id,
@@ -273,7 +273,13 @@ schema.methods.unlinkTasks = async function challengeUnlinkTasks (user, keep) {
       $set: {challenge: {}},
     }, {multi: true}).exec();
 
-    return Promise.all([user.save(), this.save()]);
+    const promises = [this.save()];
+
+    // When multiple tasks are being unlinked at the same time,
+    // save the user once outside of this function
+    if (saveUser) promises.push(user.save());
+
+    return Promise.all(promises);
   } else { // keep = 'remove-all'
     let tasks = await Tasks.Task.find(findQuery).select('_id type completed').exec();
     let taskPromises = tasks.map(task => {
@@ -285,7 +291,12 @@ schema.methods.unlinkTasks = async function challengeUnlinkTasks (user, keep) {
       return task.remove();
     });
     user.markModified('tasksOrder');
-    taskPromises.push(user.save(), this.save());
+    taskPromises.push(this.save());
+
+    // When multiple tasks are being unlinked at the same time,
+    // save the user once outside of this function
+    if (saveUser) taskPromises.push(user.save());
+
     return Promise.all(taskPromises);
   }
 };
diff --git a/website/server/models/group.js b/website/server/models/group.js
index 82e9956acb..1bae6115f9 100644
--- a/website/server/models/group.js
+++ b/website/server/models/group.js
@@ -1148,23 +1148,25 @@ schema.methods.leave = async function leaveGroup (user, keep = 'keep-all', keepC
     }).exec();
 
     let challengesToRemoveUserFrom = challenges.map(chal => {
-      return chal.unlinkTasks(user, keep);
+      return chal.unlinkTasks(user, keep, false);
     });
     await Promise.all(challengesToRemoveUserFrom);
   }
 
-  // Unlink group tasks)
+  // Unlink group tasks
   let assignedTasks = await Tasks.Task.find({
     'group.id': group._id,
     userId: {$exists: false},
     'group.assignedUsers': user._id,
   }).exec();
   let assignedTasksToRemoveUserFrom = assignedTasks.map(task => {
-    return this.unlinkTask(task, user, keep);
+    return this.unlinkTask(task, user, keep, false);
   });
   await Promise.all(assignedTasksToRemoveUserFrom);
 
-  let promises = [];
+  // the user could be modified by calls to `unlinkTask` for challenge and group tasks
+  // it has not been saved before to avoid multiple saves in parallel
+  let promises = user.isModified() ? [user.save()] : [];
 
   // remove the group from the user's groups
   if (group.type === 'guild') {
@@ -1354,7 +1356,7 @@ schema.methods.syncTask = async function groupSyncTask (taskToSync, user) {
   return Promise.all(toSave);
 };
 
-schema.methods.unlinkTask = async function groupUnlinkTask (unlinkingTask, user, keep) {
+schema.methods.unlinkTask = async function groupUnlinkTask (unlinkingTask, user, keep, saveUser = true) {
   let findQuery = {
     'group.taskId': unlinkingTask._id,
     userId: user._id,
@@ -1368,7 +1370,9 @@ schema.methods.unlinkTask = async function groupUnlinkTask (unlinkingTask, user,
       $set: {group: {}},
     }).exec();
 
-    await user.save();
+    // When multiple tasks are being unlinked at the same time,
+    // save the user once outside of this function
+    if (saveUser) await user.save();
   } else { // keep = 'remove-all'
     let task = await Tasks.Task.findOne(findQuery).select('_id type completed').exec();
     // Remove task from user.tasksOrder and delete them
@@ -1377,7 +1381,12 @@ schema.methods.unlinkTask = async function groupUnlinkTask (unlinkingTask, user,
       user.markModified('tasksOrder');
     }
 
-    return Promise.all([task.remove(), user.save(), unlinkingTask.save()]);
+    const promises = [task.remove(), unlinkingTask.save()];
+    // When multiple tasks are being unlinked at the same time,
+    // save the user once outside of this function
+    if (saveUser) promises.push(user.save());
+
+    return Promise.all(promises);
   }
 };