From 434cac944c936422c2b330253c402e6f5f0405eb Mon Sep 17 00:00:00 2001 From: Blade Barringer Date: Wed, 1 Jun 2016 12:43:44 -0500 Subject: [PATCH] fix: Cleanup quest progress for non-members when quest starts --- .../POST-groups_groupId_quests_accept.test.js | 18 ++++++ ...-groups_groupId_quests_force-start.test.js | 31 +++++++++- .../POST-groups_groupid_quests_reject.test.js | 22 +++++++ website/server/controllers/api-v3/quests.js | 2 +- website/server/models/group.js | 59 +++++++++++-------- 5 files changed, 105 insertions(+), 27 deletions(-) diff --git a/test/api/v3/integration/quests/POST-groups_groupId_quests_accept.test.js b/test/api/v3/integration/quests/POST-groups_groupId_quests_accept.test.js index 665a185279..fec6c5b88d 100644 --- a/test/api/v3/integration/quests/POST-groups_groupId_quests_accept.test.js +++ b/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; + }); }); }); diff --git a/test/api/v3/integration/quests/POST-groups_groupId_quests_force-start.test.js b/test/api/v3/integration/quests/POST-groups_groupId_quests_force-start.test.js index b6d43f826b..e569b14c33 100644 --- a/test/api/v3/integration/quests/POST-groups_groupId_quests_force-start.test.js +++ b/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; + }); }); }); diff --git a/test/api/v3/integration/quests/POST-groups_groupid_quests_reject.test.js b/test/api/v3/integration/quests/POST-groups_groupid_quests_reject.test.js index 2dcddfe727..596ad2f360 100644 --- a/test/api/v3/integration/quests/POST-groups_groupid_quests_reject.test.js +++ b/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; + }); }); }); diff --git a/website/server/controllers/api-v3/quests.js b/website/server/controllers/api-v3/quests.js index 9107fda8d6..56fabbbd34 100644 --- a/website/server/controllers/api-v3/quests.js +++ b/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 diff --git a/website/server/models/group.js b/website/server/models/group.js index cff731c034..fed02f4619 100644 --- a/website/server/models/group.js +++ b/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