Prevent progress being cleared when quest ends (#10870)

* Prevent progress being cleared when quest ends

changing group tests to make sure it keeps user's progress

fix and remove .only() from tests

* fix tests and check null case for clearing up user's quest without resetting progress

test/api/unit/models/group.test.js

@@ -32,8 +32,19 @@ describe('Group Model', () => {
       privacy: 'private',
     });
 
+    let _progress = {
+      up: 10,
+      down: 8,
+      collectedItems: 5,
+    };
+
     questLeader = new User({
-      party: { _id: party._id },
+      party: {
+        _id: party._id,
+        quest: {
+          progress: _progress,
+        },
+      },
       profile: { name: 'Quest Leader' },
       items: {
         quests: {
@@ -45,20 +56,40 @@ describe('Group Model', () => {
     party.leader = questLeader._id;
 
     participatingMember = new User({
-      party: { _id: party._id },
+      party: {
+        _id: party._id,
+        quest: {
+          progress: _progress,
+        },
+      },
       profile: { name: 'Participating Member' },
     });
     sleepingParticipatingMember = new User({
-      party: { _id: party._id },
+      party: {
+        _id: party._id,
+        quest: {
+          progress: _progress,
+        },
+      },
       profile: { name: 'Sleeping Participating Member' },
       preferences: { sleep: true },
     });
     nonParticipatingMember = new User({
-      party: { _id: party._id },
+      party: {
+        _id: party._id,
+        quest: {
+          progress: _progress,
+        },
+      },
       profile: { name: 'Non-Participating Member' },
     });
     undecidedMember = new User({
-      party: { _id: party._id },
+      party: {
+        _id: party._id,
+        quest: {
+          progress: _progress,
+        },
+      },
       profile: { name: 'Undecided Member' },
     });
 
@@ -1163,16 +1194,17 @@ describe('Group Model', () => {
           expect(party.quest.members).to.eql(expectedQuestMembers);
         });
 
-        it('applies updates to user object directly if user is participating', async () => {
+        it('applies updates to user object directly if user is participating (without resetting progress, except progress.down)', async () => {
           await party.startQuest(participatingMember);
 
           expect(participatingMember.party.quest.key).to.eql('whale');
+          expect(participatingMember.party.quest.progress.up).to.eql(10);
           expect(participatingMember.party.quest.progress.down).to.eql(0);
-          expect(participatingMember.party.quest.progress.collectedItems).to.eql(0);
+          expect(participatingMember.party.quest.progress.collectedItems).to.eql(5);
           expect(participatingMember.party.quest.completed).to.eql(null);
         });
 
-        it('applies updates to other participating members', async () => {
+        it('applies updates to other participating members (without resetting progress, except progress.down)', async () => {
           await party.startQuest(nonParticipatingMember);
 
           questLeader = await User.findById(questLeader._id);
@@ -1180,18 +1212,21 @@ describe('Group Model', () => {
           sleepingParticipatingMember = await User.findById(sleepingParticipatingMember._id);
 
           expect(participatingMember.party.quest.key).to.eql('whale');
+          expect(participatingMember.party.quest.progress.up).to.eql(10);
           expect(participatingMember.party.quest.progress.down).to.eql(0);
-          expect(participatingMember.party.quest.progress.collectedItems).to.eql(0);
+          expect(participatingMember.party.quest.progress.collectedItems).to.eql(5);
           expect(participatingMember.party.quest.completed).to.eql(null);
 
           expect(sleepingParticipatingMember.party.quest.key).to.eql('whale');
+          expect(sleepingParticipatingMember.party.quest.progress.up).to.eql(10);
           expect(sleepingParticipatingMember.party.quest.progress.down).to.eql(0);
-          expect(sleepingParticipatingMember.party.quest.progress.collectedItems).to.eql(0);
+          expect(sleepingParticipatingMember.party.quest.progress.collectedItems).to.eql(5);
           expect(sleepingParticipatingMember.party.quest.completed).to.eql(null);
 
           expect(questLeader.party.quest.key).to.eql('whale');
+          expect(questLeader.party.quest.progress.up).to.eql(10);
           expect(questLeader.party.quest.progress.down).to.eql(0);
-          expect(questLeader.party.quest.progress.collectedItems).to.eql(0);
+          expect(questLeader.party.quest.progress.collectedItems).to.eql(5);
           expect(questLeader.party.quest.completed).to.eql(null);
         });
 
@@ -1202,6 +1237,9 @@ describe('Group Model', () => {
           undecidedMember = await User.findById(undecidedMember._id);
 
           expect(nonParticipatingMember.party.quest.key).to.not.eql('whale');
+          expect(nonParticipatingMember.party.quest.progress.up).to.eql(10);
+          expect(nonParticipatingMember.party.quest.progress.down).to.eql(8);
+          expect(nonParticipatingMember.party.quest.progress.collectedItems).to.eql(5);
           expect(undecidedMember.party.quest.key).to.not.eql('whale');
         });
 
@@ -1369,8 +1407,9 @@ describe('Group Model', () => {
           let userQuest = participatingMember.party.quest;
 
           expect(userQuest.key).to.eql('whale');
+          expect(userQuest.progress.up).to.eql(10);
           expect(userQuest.progress.down).to.eql(0);
-          expect(userQuest.progress.collectedItems).to.eql(0);
+          expect(userQuest.progress.collectedItems).to.eql(5);
           expect(userQuest.completed).to.eql(null);
         });
 
@@ -1670,16 +1709,23 @@ describe('Group Model', () => {
           });
         });
 
-        it('sets user quest object to a clean state', async () => {
+        it('updates participating members quest object to a clean state (except for progress)', async () => {
           await party.finishQuest(quest);
 
-          let updatedLeader = await User.findById(questLeader._id);
+          questLeader = await User.findById(questLeader._id);
+          participatingMember = await User.findById(participatingMember._id);
 
-          expect(updatedLeader.party.quest.completed).to.eql('whale');
+          expect(questLeader.party.quest.completed).to.eql('whale');
-          expect(updatedLeader.party.quest.progress.up).to.eql(0);
+          expect(questLeader.party.quest.progress.up).to.eql(10);
-          expect(updatedLeader.party.quest.progress.down).to.eql(0);
+          expect(questLeader.party.quest.progress.down).to.eql(8);
-          expect(updatedLeader.party.quest.progress.collectedItems).to.eql(0);
+          expect(questLeader.party.quest.progress.collectedItems).to.eql(5);
-          expect(updatedLeader.party.quest.RSVPNeeded).to.eql(false);
+          expect(questLeader.party.quest.RSVPNeeded).to.eql(false);
+
+          expect(participatingMember.party.quest.completed).to.eql('whale');
+          expect(participatingMember.party.quest.progress.up).to.eql(10);
+          expect(participatingMember.party.quest.progress.down).to.eql(8);
+          expect(participatingMember.party.quest.progress.collectedItems).to.eql(5);
+          expect(participatingMember.party.quest.RSVPNeeded).to.eql(false);
         });
       });
 

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

@@ -245,7 +245,7 @@ api.rejectQuest = {
     if (group.quest.members[user._id]) throw new BadRequest(res.t('questAlreadyAccepted'));
     if (group.quest.members[user._id] === false) throw new BadRequest(res.t('questAlreadyRejected'));
 
-    user.party.quest = Group.cleanQuestProgress();
+    user.party.quest = Group.cleanQuestUser(user.party.quest.progress);
     user.markModified('party.quest');
     await user.save();
 
@@ -376,7 +376,7 @@ api.cancelQuest = {
       group.save(),
       User.update(
         {'party._id': groupId},
-        {$set: {'party.quest': Group.cleanQuestProgress()}},
+        Group.cleanQuestParty(),
         {multi: true}
       ).exec(),
     ]);
@@ -427,9 +427,8 @@ api.abortQuest = {
 
     let memberUpdates = User.update({
       'party._id': groupId,
-    }, {
+    }, Group.cleanQuestParty(),
-      $set: {'party.quest': Group.cleanQuestProgress()},
+    {multi: true}).exec();
-    }, {multi: true}).exec();
 
     let questLeaderUpdate = User.update({
       _id: group.quest.leader,
@@ -484,7 +483,7 @@ api.leaveQuest = {
     group.quest.members[user._id] = false;
     group.markModified('quest.members');
 
-    user.party.quest = Group.cleanQuestProgress();
+    user.party.quest = Group.cleanQuestUser(user.party.quest.progress);
     user.markModified('party.quest');
 
     let [savedGroup] = await Promise.all([

website/server/models/group.js

@@ -173,25 +173,41 @@ schema.pre('remove', true, async function preRemoveGroup (next, done) {
   }
 });
 
-// return a clean object for user.quest
+// return clean updates for each user in a party without resetting their progress
-function _cleanQuestProgress (merge) {
+function _cleanQuestParty (merge) {
-  let clean = {
+  let updates = {
-    key: null,
+    $set: {
-    progress: {
+      'party.quest.key': null,
+      'party.quest.completed': null,
+      'party.quest.RSVPNeeded': false,
+    },
+  };
+
+  if (merge) _.merge(updates, merge);
+
+  return updates;
+}
+
+// return a clean user.quest of a particular user while keeping his progress
+function _cleanQuestUser (userProgress) {
+  if (!userProgress) {
+    userProgress = {
       up: 0,
       down: 0,
       collect: {},
       collectedItems: 0,
-    },
+    };
+  } else {
+    userProgress = userProgress.toObject();
+  }
+
+  let clean = {
+    key: null,
+    progress: userProgress,
     completed: null,
     RSVPNeeded: false,
   };
 
-  if (merge) {
-    _.merge(clean, _.omit(merge, 'progress'));
-    if (merge.progress) _.merge(clean.progress, merge.progress);
-  }
-
   return clean;
 }
 
@@ -634,11 +650,8 @@ schema.methods.startQuest = async function startQuest (user) {
   // Do not block updates
   User.update({
     _id: { $in: nonMembers },
-  }, {
+  }, _cleanQuestParty(),
-    $set: {
+  { multi: true }).exec();
-      'party.quest': _cleanQuestProgress(),
-    },
-  }, { multi: true }).exec();
 
   const newMessage = this.sendChat(`\`Your quest, ${quest.text('en')}, has started.\``, null, {
     participatingMembers: this.getParticipatingQuestMembers().join(', '),
@@ -707,7 +720,8 @@ schema.methods.sendGroupChatReceivedWebhooks = function sendGroupChatReceivedWeb
   });
 };
 
-schema.statics.cleanQuestProgress = _cleanQuestProgress;
+schema.statics.cleanQuestParty = _cleanQuestParty;
+schema.statics.cleanQuestUser = _cleanQuestUser;
 
 // returns a clean object for group.quest
 schema.statics.cleanGroupQuest = function cleanGroupQuest () {
@@ -784,7 +798,7 @@ schema.methods.finishQuest = async function finishQuest (quest) {
   if (this._id === TAVERN_ID) {
     updates.$set['party.quest.completed'] = questK; // Just show the notif
   } else {
-    updates.$set['party.quest'] = _cleanQuestProgress({completed: questK}); // clear quest progress
+    _.merge(updates, _cleanQuestParty({$set: {'party.quest.completed': questK}})); // clear quest progress
   }
 
   _.each(_.reject(quest.drop.items, 'onlyOwner'), (item) => {