fix: Cleanup quest progress for non-members when quest starts

test/api/v3/integration/quests/POST-groups_groupId_quests_accept.test.js

@@ -3,6 +3,7 @@ import {
   translate as t,
   generateUser,
 } from '../../../../helpers/api-v3-integration.helper';
+import Bluebird from 'bluebird';
 
 describe('POST /groups/:groupId/quests/accept', () => {
   const PET_QUEST = 'whale';
@@ -115,5 +116,22 @@ describe('POST /groups/:groupId/quests/accept', () => {
       await questingGroup.sync();
       expect(questingGroup.quest.active).to.equal(true);
     });
+
+    it('cleans up user quest data for non-quest members when last member accepts', async () => {
+      let rejectingMember = partyMembers[0];
+
+      await leader.post(`/groups/${questingGroup._id}/quests/invite/${PET_QUEST}`);
+      await rejectingMember.post(`/groups/${questingGroup._id}/quests/reject`);
+      // quest will start after everyone has accepted
+      await partyMembers[1].post(`/groups/${questingGroup._id}/quests/accept`);
+
+      await Bluebird.delay(500);
+
+      await rejectingMember.sync();
+
+      expect(rejectingMember.party.quest.RSVPNeeded).to.eql(false);
+      expect(rejectingMember.party.quest.key).to.not.exist;
+      expect(rejectingMember.party.quest.completed).to.not.exist;
+    });
   });
 });

test/api/v3/integration/quests/POST-groups_groupId_quests_force-start.test.js

@@ -3,6 +3,7 @@ import {
   translate as t,
   generateUser,
 } from '../../../../helpers/api-v3-integration.helper';
+import Bluebird from 'bluebird';
 
 describe('POST /groups/:groupId/quests/force-start', () => {
   const PET_QUEST = 'whale';
@@ -14,7 +15,7 @@ describe('POST /groups/:groupId/quests/force-start', () => {
   beforeEach(async () => {
     let { group, groupLeader, members } = await createAndPopulateGroup({
       groupDetails: { type: 'party', privacy: 'private' },
-      members: 2,
+      members: 3,
     });
 
     questingGroup = group;
@@ -63,8 +64,9 @@ describe('POST /groups/:groupId/quests/force-start', () => {
     it('does not force start for a quest already underway', async () => {
       await leader.post(`/groups/${questingGroup._id}/quests/invite/${PET_QUEST}`);
       await partyMembers[0].post(`/groups/${questingGroup._id}/quests/accept`);
-      // quest will start after everyone has accepted
       await partyMembers[1].post(`/groups/${questingGroup._id}/quests/accept`);
+      // quest will start after everyone has accepted
+      await partyMembers[2].post(`/groups/${questingGroup._id}/quests/accept`);
 
       await expect(leader.post(`/groups/${questingGroup._id}/quests/force-start`))
       .to.eventually.be.rejected.and.eql({
@@ -122,5 +124,30 @@ describe('POST /groups/:groupId/quests/force-start', () => {
         [`${leader._id}`]: true,
       });
     });
+
+    it('cleans up user quest data for non-quest members', async () => {
+      let partyMemberThatRejects = partyMembers[1];
+      let partyMemberThatIgnores = partyMembers[2];
+
+      await leader.post(`/groups/${questingGroup._id}/quests/invite/${PET_QUEST}`);
+      await partyMembers[0].post(`/groups/${questingGroup._id}/quests/accept`);
+      await partyMemberThatRejects.post(`/groups/${questingGroup._id}/quests/reject`);
+
+      await leader.post(`/groups/${questingGroup._id}/quests/force-start`);
+
+      await Bluebird.delay(500);
+
+      await Promise.all([
+        partyMemberThatRejects.sync(),
+        partyMemberThatIgnores.sync(),
+      ]);
+
+      expect(partyMemberThatRejects.party.quest.RSVPNeeded).to.eql(false);
+      expect(partyMemberThatRejects.party.quest.key).to.not.exist;
+      expect(partyMemberThatRejects.party.quest.completed).to.not.exist;
+      expect(partyMemberThatIgnores.party.quest.RSVPNeeded).to.eql(false);
+      expect(partyMemberThatIgnores.party.quest.key).to.not.exist;
+      expect(partyMemberThatIgnores.party.quest.completed).to.not.exist;
+    });
   });
 });

test/api/v3/integration/quests/POST-groups_groupid_quests_reject.test.js

@@ -4,6 +4,7 @@ import {
   generateUser,
 } from '../../../../helpers/api-v3-integration.helper';
 import { v4 as generateUUID } from 'uuid';
+import Bluebird from 'bluebird';
 
 describe('POST /groups/:groupId/quests/reject', () => {
   let questingGroup;
@@ -142,5 +143,26 @@ describe('POST /groups/:groupId/quests/reject', () => {
 
       expect(questingGroup.quest.active).to.be.true;
     });
+
+    it('cleans up user quest data for non-quest members when last member rejects', async () => {
+      let rejectingMember = partyMembers[1];
+
+      await leader.post(`/groups/${questingGroup._id}/quests/invite/${PET_QUEST}`);
+      await partyMembers[0].post(`/groups/${questingGroup._id}/quests/accept`);
+      // quest will start after everyone has accepted or rejected
+      await rejectingMember.post(`/groups/${questingGroup._id}/quests/reject`);
+
+      await Bluebird.delay(500);
+
+      await questingGroup.sync();
+
+      expect(questingGroup.quest.active).to.be.true;
+
+      await rejectingMember.sync();
+
+      expect(rejectingMember.party.quest.RSVPNeeded).to.eql(false);
+      expect(rejectingMember.party.quest.key).to.not.exist;
+      expect(rejectingMember.party.quest.completed).to.not.exist;
+    });
   });
 });

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

@@ -301,7 +301,7 @@ api.forceStart = {
 };
 
 /**
- * @api {post} /api/v3/groups/:groupId/quests/cancel Cancels a quest
+ * @api {post} /api/v3/groups/:groupId/quests/cancel Cancels a quest that is not active
  * @apiVersion 3.0.0
  * @apiName CancelQuest
  * @apiGroup Group

website/server/models/group.js

@@ -110,6 +110,27 @@ schema.post('remove', function postRemoveGroup (group) {
   firebase.deleteGroup(group._id);
 });
 
+// return a clean object for user.quest
+function _cleanQuestProgress (merge) {
+  let clean = {
+    key: null,
+    progress: {
+      up: 0,
+      down: 0,
+      collect: {},
+    },
+    completed: null,
+    RSVPNeeded: false,
+  };
+
+  if (merge) {
+    _.merge(clean, _.omit(merge, 'progress'));
+    if (merge.progress) _.merge(clean.progress, merge.progress);
+  }
+
+  return clean;
+}
+
 schema.statics.getGroup = async function getGroup (options = {}) {
   let {user, groupId, fields, optionalMembership = false, populateLeader = false, requireMembership = false} = options;
   let query;
@@ -332,10 +353,11 @@ schema.methods.startQuest = async function startQuest (user) {
     this.quest.progress.collect = collected;
   }
 
+  let nonMembers = Object.keys(_.pick(this.quest.members, (member) => {
+    return !member;
+  }));
+
   // Changes quest.members to only include participating members
-  // TODO: is that important? What does it matter if the non-participating members
-  // are still on the object?
-  // TODO: is it important to run clean quest progress on non-members like we did in v2?
   this.quest.members = _.pick(this.quest.members, _.identity);
   let nonUserQuestMembers = _.keys(this.quest.members);
   removeFromArray(nonUserQuestMembers, user._id);
@@ -372,6 +394,16 @@ schema.methods.startQuest = async function startQuest (user) {
     },
   }, { multi: true }).exec();
 
+  // update the users who are not participating
+  // Do not block updates
+  User.update({
+    _id: { $in: nonMembers },
+  }, {
+    $set: {
+      'party.quest': _cleanQuestProgress(),
+    },
+  }, { multi: true }).exec();
+
   // send notifications in the background without blocking
   User.find(
     { _id: { $in: nonUserQuestMembers } },
@@ -390,27 +422,6 @@ schema.methods.startQuest = async function startQuest (user) {
   });
 };
 
-// return a clean object for user.quest
-function _cleanQuestProgress (merge) {
-  let clean = {
-    key: null,
-    progress: {
-      up: 0,
-      down: 0,
-      collect: {},
-    },
-    completed: null,
-    RSVPNeeded: false,
-  };
-
-  if (merge) {
-    _.merge(clean, _.omit(merge, 'progress'));
-    if (merge.progress) _.merge(clean.progress, merge.progress);
-  }
-
-  return clean;
-}
-
 schema.statics.cleanQuestProgress = _cleanQuestProgress;
 
 // returns a clean object for group.quest