Added intitial group leave tests

Fixed test readability, updated party test, and updated challenge update code when leaving group
Updated library, added group existance check, and reset full party
Updated syntax, added new userUnlinkChallenges, and added some initial testing for challenges
Added challenge tasks tests
Added try/catch to group remove, add more party tests, fixed broken challenge test, removed useless return value
Added public guild tests, added more tests to party, and abstracted remove invitations logic

Closes #6506

test/api/v3/integration/groups/POST-groups_groupId_leave.js

@@ -0,0 +1,195 @@
+import {
+  generateChallenge,
+  checkExistence,
+  createAndPopulateGroup,
+  sleep,
+} from '../../../../helpers/api-v3-integration.helper';
+import {
+  each,
+} from 'lodash';
+
+describe('POST /groups/:groupId/leave', () => {
+  let typesOfGroups = {
+    'public guild': { type: 'guild', privacy: 'public' },
+    'private guild': { type: 'guild', privacy: 'private' },
+    party: { type: 'party', privacy: 'private' },
+  };
+
+  each(typesOfGroups, (groupDetails, groupType) => {
+    context(`Leaving a ${groupType}`, () => {
+      let groupToLeave;
+      let leader;
+      let member;
+
+      beforeEach(async () => {
+        let { group, groupLeader, members } = await createAndPopulateGroup({
+          groupDetails,
+          members: 1,
+        });
+
+        groupToLeave = group;
+        leader = groupLeader;
+        member = members[0];
+      });
+
+      it(`lets user leave a ${groupType}`, async () => {
+        await member.post(`/groups/${groupToLeave._id}/leave`);
+
+        let userThatLeftGroup = await member.get('/user');
+
+        expect(userThatLeftGroup.guilds).to.be.empty;
+        expect(userThatLeftGroup.party._id).to.not.exist;
+      });
+
+      it(`sets a new group leader when leader leaves a ${groupType}`, async () => {
+        await leader.post(`/groups/${groupToLeave._id}/leave`);
+
+        let groupToLeaveWithNewLeader = await member.get(`/groups/${groupToLeave._id}`);
+
+        expect(groupToLeaveWithNewLeader.leader._id).to.equal(member._id);
+      });
+
+      context('With challenges', () => {
+        let challenge;
+
+        beforeEach(async () => {
+          challenge = await generateChallenge(leader, groupToLeave);
+
+          await leader.post(`/tasks/challenge/${challenge._id}`, {
+            text: 'test habit',
+            type: 'habit',
+          });
+
+          await sleep(0.5);
+        });
+
+        it('removes all challenge tasks when keep parameter is set to remove', async () => {
+          await leader.post(`/groups/${groupToLeave._id}/leave?keep=remove-all`);
+
+          let userWithoutChallengeTasks = await leader.get('/user');
+
+          expect(userWithoutChallengeTasks.challenges).to.not.include(challenge._id);
+          expect(userWithoutChallengeTasks.tasksOrder.habits).to.be.empty;
+        });
+
+        it('keeps all challenge tasks when keep parameter is not set', async () => {
+          await leader.post(`/groups/${groupToLeave._id}/leave`);
+
+          let userWithChallengeTasks = await leader.get('/user');
+
+          expect(userWithChallengeTasks.challenges).to.not.include(challenge._id);
+          // @TODO find elegant way to assert against the task existing
+          expect(userWithChallengeTasks.tasksOrder.habits).to.not.be.empty;
+        });
+      });
+
+      it('prevents quest leader from leaving a groupToLeave');
+      it('prevents a user from leaving during an active quest');
+    });
+  });
+
+  context('Leaving a group as the last member', () => {
+    context('private guild', () => {
+      let privateGuild;
+      let leader;
+      let invitedUser;
+
+      beforeEach(async () => {
+        let { group, groupLeader, invitees } = await createAndPopulateGroup({
+          groupDetails: {
+            name: 'Test Private Guild',
+            type: 'guild',
+          },
+          invites: 1,
+        });
+
+        privateGuild = group;
+        leader = groupLeader;
+        invitedUser = invitees[0];
+      });
+
+      it('removes a group when the last member leaves', async () => {
+        await leader.post(`/groups/${privateGuild._id}/leave`);
+
+        await expect(checkExistence('groups', privateGuild._id)).to.eventually.equal(false);
+      });
+
+      it('removes invitations when the last member leaves', async () => {
+        await leader.post(`/groups/${privateGuild._id}/leave`);
+
+        let userWithoutInvitation = await invitedUser.get('/user');
+
+        expect(userWithoutInvitation.invitations.guilds).to.be.empty;
+      });
+    });
+
+    context('public guild', () => {
+      let publicGuild;
+      let leader;
+      let invitedUser;
+
+      beforeEach(async () => {
+        let { group, groupLeader, invitees } = await createAndPopulateGroup({
+          groupDetails: {
+            name: 'Test Public Guild',
+            type: 'guild',
+            privacy: 'public',
+          },
+          invites: 1,
+        });
+
+        publicGuild = group;
+        leader = groupLeader;
+        invitedUser = invitees[0];
+      });
+
+      it('keeps the group when the last member leaves', async () => {
+        await leader.post(`/groups/${publicGuild._id}/leave`);
+
+        await expect(checkExistence('groups', publicGuild._id)).to.eventually.equal(true);
+      });
+
+      it('keeps the invitations when the last member leaves a public guild', async () => {
+        await leader.post(`/groups/${publicGuild._id}/leave`);
+
+        let userWithoutInvitation = await invitedUser.get('/user');
+
+        expect(userWithoutInvitation.invitations.guilds).to.not.be.empty;
+      });
+    });
+
+    context('party', () => {
+      let party;
+      let leader;
+      let invitedUser;
+
+      beforeEach(async () => {
+        let { group, groupLeader, invitees } = await createAndPopulateGroup({
+          groupDetails: {
+            name: 'Test Party',
+            type: 'party',
+          },
+          invites: 1,
+        });
+
+        party = group;
+        leader = groupLeader;
+        invitedUser = invitees[0];
+      });
+
+      it('removes a group when the last member leaves a party', async () => {
+        await leader.post(`/groups/${party._id}/leave`);
+
+        await expect(checkExistence('party', party._id)).to.eventually.equal(false);
+      });
+
+      it('removes invitations when the last member leaves a party', async () => {
+        await leader.post(`/groups/${party._id}/leave`);
+
+        let userWithoutInvitation = await invitedUser.get('/user');
+
+        expect(userWithoutInvitation.invitations.party).to.be.empty;
+      });
+    });
+  });
+});

test/helpers/api-integration/v3/index.js

@@ -8,3 +8,11 @@ export { requester };
 export { translate } from '../translate';
 export { checkExistence, resetHabiticaDB } from '../mongo';
 export * from  './object-generators';
+
+export async function sleep (seconds) {
+  let milliseconds = seconds * 1000;
+
+  return new Promise((resolve) => {
+    setTimeout(resolve, milliseconds);
+  });
+}

website/src/controllers/api-v3/groups.js

@@ -251,6 +251,8 @@ api.joinGroup = {
 
     if (group.memberCount === 0) group.leader = user._id; // If new user is only member -> set as leader
 
+    group.memberCount += 1;
+
     let promises = [group.save(), user.save()];
 
     if (group.type === 'party' && inviter) {

website/src/controllers/api-v3/tasks.js

@@ -102,6 +102,7 @@ api.createChallengeTasks = {
     if (challenge.leader !== user._id) throw new NotAuthorized(res.t('onlyChalLeaderEditTasks'));
 
     let tasks = await _createTasks(req, res, user, challenge);
+
     res.respond(201, tasks.length === 1 ? tasks[0] : tasks);
 
     // If adding tasks to a challenge -> sync users

website/src/models/group.js

@@ -98,30 +98,15 @@ schema.statics.sanitizeUpdate = function sanitizeUpdate (updateObj) {
 }*/
 
 // TODO test
-schema.pre('remove', true, function preRemoveGroup (next, done) {
+schema.pre('remove', true, async function preRemoveGroup (next, done) {
   next();
   let group = this;
-
-  // Remove invitations when group is deleted
-  // TODO verify it works fir everything
-  User.find({
-    // TODO id -> _id ?
-    [`invitations.${group.type}${group.type === 'guild' ? 's' : ''}.id`]: group._id,
-  }).exec()
-  .then(users => {
-    return Q.all(users.map(user => {
-      if (group.type === 'party') {
-        user.invitations.party = {}; // TODO mark modified
-      } else {
-        let i = _.findIndex(user.invitations.guilds, {id: group._id});
-        user.invitations.guilds.splice(i, 1);
-      }
-
-      return user.save();
-    }));
-  })
-  .then(done)
-  .catch(done);
+  try {
+    await group.removeGroupInvitations();
+    done();
+  } catch (err) {
+    done(err);
+  }
 });
 
 schema.post('remove', function postRemoveGroup (group) {
@@ -150,6 +135,27 @@ schema.statics.getGroup = function getGroup (options = {}) {
   // TODO purge chat flags info? in tojson?
 };
 
+schema.methods.removeGroupInvitations = async function removeGroupInvitations () {
+  let group = this;
+
+  let usersToRemoveInvitationsFrom = await User.find({
+    // TODO id -> _id ?
+    [`invitations.${group.type}${group.type === 'guild' ? 's' : ''}.id`]: group._id,
+  }).exec();
+
+  let userUpdates = usersToRemoveInvitationsFrom.map(user => {
+    if (group.type === 'party') {
+      user.invitations.party = {}; // TODO mark modified
+    } else {
+      let i = _.findIndex(user.invitations.guilds, {id: group._id});
+      user.invitations.guilds.splice(i, 1);
+    }
+    return user.save();
+  });
+
+  return Q.all(userUpdates);
+};
+
 // Return true if user is a member of the group
 schema.methods.isMember = function isGroupMember (user) {
   if (this._id === 'habitrpg') {
@@ -452,71 +458,46 @@ schema.statics.bossQuest = function bossQuest (user, progress) {
   });
 };
 
-// Remove user from this group
-// TODO this is highly inefficient
-schema.methods.leave = function leaveGroup (user, keep) {
+schema.methods.leave = async function leaveGroup (user, keep = 'keep-all') {
   let group = this;
 
-  return Q.all([
-    // Remove user from group challenges
-
-    // First find relevant Challenges
-    Challenge.find({
-      _id: {$in: user.challenges}, // Challenges I am in
-      group: group._id, // that belong to the group I am leaving
-    }).then(challenges => {
-      // Update each challenge
-      return Challenge.update(
-        {_id: {$in: _.pluck(challenges, '_id')}},
-        {$pull: {members: user._id}},
-        {multi: true}
-      ).then(() => challenges); // pass `challenges` above to next promise
-    }).then(challenges => {
-      return Q.all(challenges.map(chal => {
-        let i = user.challenges.indexOf(chal._id);
-        if (i !== -1) user.challenges.splice(i, 1);
-        return user.unlinkChallengeTasks(chal._id, keep);
-      }));
-    }),
-
-    // Update the group
-    (() => {
-      // If user is the last one in group and group is private, delete it
-      if (group.members.length === 1 && (
-          group.type === 'party' ||
-          group.type === 'guild' && group.privacy === 'private'
-      )) return group.remove();
-
-      let update = {};
-      // otherwise just remove a member TODO create User.methods.removeFromGroup?
-      if (group.type === 'guild') {
-        _.pull(user.guilds, group._id);
-      } else {
-        user.party._id = undefined; // TODO remove quest information too?
-      }
-
-      // If the leader is leaving (or if the leader previously left, and this wasn't accounted for)
-      let leader = group.leader;
-
-      if (leader === user._id || group.members.indexOf(leader) === -1) {
-        let seniorMember = _.find(group.members, m => m !== user._id);
-
-        // could be missing in case of public guild (that can have 0 members) with 1 member who is leaving
-        if (seniorMember) update.$set = {leader: seniorMember};
-      }
-
-      update.$inc = {memberCount: -1};
-      return Q.all([
-        model.update({_id: group._id}, update).exec(), // eslint-disable-line no-use-before-define
-        user.save(),
-      ]);
-    })(),
-  ]).then(() => {
-    firebase.removeUserFromGroup(group._id, user._id);
-    return; // TODO ok not to return promise?
-  }).catch(err => { // TODO do we have to catch err if we return the promise?
-    throw err;
+  let challenges = await Challenge.find({
+    _id: {$in: user.challenges},
+    groupId: group._id,
   });
+
+  let challengesToRemoveUserFrom = challenges.map(chal => {
+    return user.unlinkChallengeTasks(chal._id, keep);
+  });
+  await Q.all(challengesToRemoveUserFrom);
+
+  let promises = [];
+
+  // If user is the last one in group and group is private, delete it
+  if (group.memberCount <= 1 && group.privacy === 'private') {
+    return await group.remove();
+  }
+
+  // otherwise just remove a member TODO create User.methods.removeFromGroup?
+  if (group.type === 'guild') {
+    promises.push(User.update({_id: user._id}, {$pull: {guilds: group._id } }).exec());
+  } else {
+    promises.push(User.update({_id: user._id}, {$set: {party: {} } }).exec());
+  }
+
+  // If the leader is leaving (or if the leader previously left, and this wasn't accounted for)
+  let update = { memberCount: group.memberCount - 1 };
+  if (group.leader === user._id) {
+    let query = group.type === 'party' ? {'party._id': group._id} : {guilds: group._id};
+    let seniorMember = await User.findOne({query, _id: {$ne: user._id}}).exec();
+
+    // could be missing in case of public guild (that can have 0 members) with 1 member who is leaving
+    if (seniorMember) update.$set = {leader: seniorMember._id};
+  }
+  promises.push(group.update(update).exec());
+  firebase.removeUserFromGroup(group._id, user._id);
+
+  return Q.all(promises);
 };
 
 export const INVITES_LIMIT = 100;

website/src/models/user.js

@@ -661,13 +661,18 @@ schema.methods.unlinkChallengeTasks = async function unlinkChallengeTasks (chall
     'challenge.id': challengeId,
   };
 
+  let challengeIndex = user.challenges.indexOf(challengeId);
+  if (challengeIndex !== -1) user.challenges.splice(challengeIndex, 1);
+
   if (keep === 'keep-all') {
     await Tasks.Task.update(findQuery, {
       $set: {challenge: {}}, // TODO what about updatedAt?
     }, {multi: true}).exec();
+
+    await user.save();
   } else { // keep = 'remove-all'
-    let tasks = Tasks.Task.find(findQuery).select('_id type completed').exec();
-    tasks = tasks.map(task => {
+    let tasks = await Tasks.Task.find(findQuery).select('_id type completed').exec();
+    let taskPromises = tasks.map(task => {
       // Remove task from user.tasksOrder and delete them
       if (task.type !== 'todo' || !task.completed) {
         let list = user.tasksOrder[`${task.type}s`];
@@ -678,8 +683,8 @@ schema.methods.unlinkChallengeTasks = async function unlinkChallengeTasks (chall
       return task.remove();
     });
 
-    tasks.push(user.save());
-    await Q.all(tasks);
+    taskPromises.push(user.save());
+    return Q.all(taskPromises);
   }
 };