Leaving a group (#8517)

* Leaving a group or a guild no longer removes the user from the challenges of that group or guild.

* Updating api docs for leaving group to take into account the default path no longer leaving challenges when leaving a group.

* Updating api docs for leaving group to take into account the default path no longer leaving challenges when leaving a group.

* refactored according to blade's comments to not be a breaking change. The api now accepts a body parameter to specify wether the user
should remain in the groups challenges or leave them. The change also adds more tests around this behavior to confirm that it works
as expected.

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

@@ -78,7 +78,7 @@ describe('POST /groups/:groupId/leave', () => {
         expect(leader.newMessages[groupToLeave._id]).to.be.empty;
       });
 
-      context('With challenges', () => {
+      context('with challenges', () => {
         let challenge;
 
         beforeEach(async () => {
@@ -106,10 +106,25 @@ describe('POST /groups/:groupId/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('keeps the user in the challenge when the keepChallenges parameter is set to remain-in-challenges', async () => {
+          await leader.post(`/groups/${groupToLeave._id}/leave`, {keepChallenges: 'remain-in-challenges'});
+
+          let userWithChallengeTasks = await leader.get('/user');
+
+          expect(userWithChallengeTasks.challenges).to.include(challenge._id);
+        });
+
+        it('drops the user in the challenge when the keepChallenges parameter isn\'t set', async () => {
+          await leader.post(`/groups/${groupToLeave._id}/leave`);
+
+          let userWithChallengeTasks = await leader.get('/user');
+
+          expect(userWithChallengeTasks.challenges).to.not.include(challenge._id);
+        });
       });
 
       it('prevents quest leader from leaving a groupToLeave');

test/api/v3/integration/user/DELETE-user.test.js

@@ -3,6 +3,7 @@ import {
   createAndPopulateGroup,
   generateGroup,
   generateUser,
+  generateChallenge,
   translate as t,
 } from '../../../../helpers/api-integration/v3';
 import {
@@ -64,6 +65,30 @@ describe('DELETE /user', () => {
     }));
   });
 
+  it('reduces memberCount in challenges user is linked to', async () => {
+    let populatedGroup = await createAndPopulateGroup({
+      members: 2,
+    });
+
+    let group = populatedGroup.group;
+    let authorizedUser = populatedGroup.members[1];
+
+    let challenge = await generateChallenge(populatedGroup.groupLeader, group);
+    await authorizedUser.post(`/challenges/${challenge._id}/join`);
+
+    await challenge.sync();
+
+    expect(challenge.memberCount).to.eql(2);
+
+    await authorizedUser.del('/user', {
+      password,
+    });
+
+    await challenge.sync();
+
+    expect(challenge.memberCount).to.eql(1);
+  });
+
   it('deletes the user', async () => {
     await user.del('/user', {
       password,

website/client-old/js/controllers/guildsCtrl.js

@@ -71,7 +71,7 @@ habitrpg.controller("GuildsCtrl", ['$scope', 'Groups', 'User', 'Challenges', '$r
            $scope.selectedGroup = undefined;
            $scope.popoverEl.popover('destroy');
          } else {
-           Groups.Group.leave($scope.selectedGroup._id, keep)
+           Groups.Group.leave($scope.selectedGroup._id, keep, 'remain-in-challenges')
             .success(function (data) {
               var index = User.user.guilds.indexOf($scope.selectedGroup._id);
               delete User.user.guilds[index];

website/client-old/js/controllers/partyCtrl.js

@@ -134,7 +134,7 @@ habitrpg.controller("PartyCtrl", ['$rootScope','$scope','Groups','Chat','User','
           $scope.selectedGroup = undefined;
           $scope.popoverEl.popover('destroy');
         } else {
-          Groups.Group.leave($scope.selectedGroup._id, keep)
+          Groups.Group.leave($scope.selectedGroup._id, keep, 'remain-in-challenges')
             .then(function (response) {
               Analytics.updateUser({'partySize':null,'partyID':null});
               User.sync().then(function () {
@@ -197,7 +197,7 @@ habitrpg.controller("PartyCtrl", ['$rootScope','$scope','Groups','Chat','User','
 
       $scope.leaveOldPartyAndJoinNewParty = function(newPartyId, newPartyName) {
         if (confirm('Are you sure you want to delete your party and join ' + newPartyName + '?')) {
-          Groups.Group.leave(Groups.data.party._id, false)
+          Groups.Group.leave(Groups.data.party._id, false, 'remain-in-challenges')
             .then(function() {
               $rootScope.party = $scope.group = {
                 loadingParty: true

website/client-old/js/services/groupServices.js

@@ -69,12 +69,13 @@ angular.module('habitrpg')
       });
     };
 
-    Group.leave = function(gid, keep) {
+    Group.leave = function(gid, keep, keepChallenges) {
       return $http({
         method: "POST",
         url: groupApiURLPrefix + '/' + gid + '/leave',
         data: {
           keep: keep,
+          keepChallenges: keepChallenges,
         }
       });
     };

website/common/locales/en/groups.json

@@ -102,6 +102,7 @@
   "messageSentAlert": "Message sent",
   "pmHeading": "Private message to <%= name %>",
   "pmsMarkedRead": "Your private messages have been marked as read",
+  "possessiveParty": "<%= name %>'s Party",
   "clearAll": "Delete All Messages",
   "confirmDeleteAllMessages": "Are you sure you want to delete all messages in your inbox? Other users will still see messages you have sent to them.",
   "optOutPopover": "Don't like private messages? Click to completely opt out",
@@ -213,7 +214,6 @@
   "desktopNotificationsText": "We need your permission to enable desktop notifications for new messages in party chat! Follow your browser's instructions to turn them on.<br><br>You'll receive these notifications only while you have Habitica open. If you decide you don't like them, they can be disabled in your browser's settings.<br><br>This box will close automatically when a decision is made.",
   "confirmAddTag": "Do you want to assign this task to \"<%= tag %>\"?",
   "confirmRemoveTag": "Do you really want to remove \"<%= tag %>\"?",
-
   "groupHomeTitle": "Home",
   "assignTask": "Assign Task",
   "desktopNotificationsText": "We need your permission to enable desktop notifications for new messages in party chat! Follow your browser's instructions to turn them on.<br><br>You'll receive these notifications only while you have Habitica open. If you decide you don't like them, they can be disabled in your browser's settings.<br><br>This box will close automatically when a decision is made.",
@@ -253,5 +253,6 @@
   "claimedBy": "\n\nClaimed by: <%= claimingUsers %>",
   "cantDeleteAssignedGroupTasks": "Can't delete group tasks that are assigned to you.",
   "confirmGuildPlanCreation": "Create this group?",
-  "onlyGroupLeaderCanInviteToGroupPlan": "Only the group leader can invite users to a group with a subscription."
+  "onlyGroupLeaderCanInviteToGroupPlan": "Only the group leader can invite users to a group with a subscription.",
+  "remainOrLeaveChallenges": "req.query.keep must be either 'remain-in-challenges' or 'leave-challenges'"
 }

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

@@ -195,8 +195,6 @@ api.leaveChallenge = {
 
     if (!challenge.isMember(user)) throw new NotAuthorized(res.t('challengeMemberNotFound'));
 
-    challenge.memberCount -= 1;
-
     // Unlink challenge's tasks from user's tasks and save the challenge
     await Bluebird.all([challenge.unlinkTasks(user, keep), challenge.save()]);
     res.respond(200, {});

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

@@ -524,7 +524,8 @@ function _removeMessagesFromMember (member, groupId) {
  * @apiGroup Group
  *
  * @apiParam {String} groupId The group _id ('party' for the user party and 'habitrpg' for tavern are accepted)
- * @apiParam {String="remove-all","keep-all"} keep Query parameter - Whether to keep or not challenges' tasks. Defaults to keep-all
+ * @apiParam (Query) {String="remove-all","keep-all"} keep=keep-all Whether or not to keep challenge tasks belonging to the group being left.
+ * @apiParam (Body) {String="remain-in-challenges","leave-challenges"} [keepChallenges=leave-challenges] Whether or not to remain in the challenges of the group being left.
  *
  * @apiSuccess {Object} data An empty object
  *
@@ -539,6 +540,7 @@ api.leaveGroup = {
     req.checkParams('groupId', res.t('groupIdRequired')).notEmpty();
     // When removing the user from challenges, should we keep the tasks?
     req.checkQuery('keep', res.t('keepOrRemoveAll')).optional().isIn(['keep-all', 'remove-all']);
+    req.checkBody('keepChallenges', res.t('remainOrLeaveChallenges')).optional().isIn(['remain-in-challenges', 'leave-challenges']);
 
     let validationErrors = req.validationErrors();
     if (validationErrors) throw validationErrors;
@@ -560,7 +562,7 @@ api.leaveGroup = {
       }
     }
 
-    await group.leave(user, req.query.keep);
+    await group.leave(user, req.query.keep, req.body.keepChallenges);
 
     if (group.purchased.plan && group.purchased.plan.customerId) await payments.updateStripeGroupPlan(group);
 

website/server/models/challenge.js

@@ -237,13 +237,14 @@ schema.methods.unlinkTasks = async function challengeUnlinkTasks (user, keep) {
   };
 
   removeFromArray(user.challenges, challengeId);
+  this.memberCount--;
 
   if (keep === 'keep-all') {
     await Tasks.Task.update(findQuery, {
       $set: {challenge: {}},
     }, {multi: true}).exec();
 
-    await user.save();
+    return Bluebird.all([user.save(), this.save()]);
   } else { // keep = 'remove-all'
     let tasks = await Tasks.Task.find(findQuery).select('_id type completed').exec();
     let taskPromises = tasks.map(task => {
@@ -255,7 +256,7 @@ schema.methods.unlinkTasks = async function challengeUnlinkTasks (user, keep) {
       return task.remove();
     });
     user.markModified('tasksOrder');
-    taskPromises.push(user.save());
+    taskPromises.push(user.save(), this.save());
     return Bluebird.all(taskPromises);
   }
 };

website/server/models/group.js

@@ -900,7 +900,7 @@ schema.statics.tavernBoss = async function tavernBoss (user, progress) {
   }
 };
 
-schema.methods.leave = async function leaveGroup (user, keep = 'keep-all') {
+schema.methods.leave = async function leaveGroup (user, keep = 'keep-all', keepChallenges = 'leave-challenges') {
   let group = this;
   let update = {};
 
@@ -908,16 +908,18 @@ schema.methods.leave = async function leaveGroup (user, keep = 'keep-all') {
     throw new NotAuthorized(shared.i18n.t('cannotDeleteActiveGroup'));
   }
 
-  // Unlink user challenge tasks
+  // only remove user from challenges if it's set to leave-challenges
-  let challenges = await Challenge.find({
+  if (keepChallenges === 'leave-challenges') {
-    _id: {$in: user.challenges},
+    let challenges = await Challenge.find({
-    group: group._id,
+      _id: {$in: user.challenges},
-  }).exec();
+      group: group._id,
+    }).exec();
 
-  let challengesToRemoveUserFrom = challenges.map(chal => {
+    let challengesToRemoveUserFrom = challenges.map(chal => {
-    return chal.unlinkTasks(user, keep);
+      return chal.unlinkTasks(user, keep);
-  });
+    });
-  await Bluebird.all(challengesToRemoveUserFrom);
+    await Bluebird.all(challengesToRemoveUserFrom);
+  }
 
   // Unlink group tasks)
   let assignedTasks = await Tasks.Task.find({