From e68e29db8febd071ff23d96e2605bdcd9019b472 Mon Sep 17 00:00:00 2001 From: TheHollidayInn Date: Thu, 3 Sep 2015 08:55:12 -0500 Subject: [PATCH 1/7] Delete private guilds or parties when last member leaves. --- test/api/groups.coffee | 24 +++++++++- website/src/controllers/groups.js | 73 ++++++++++++++++++++++++++++++- 2 files changed, 94 insertions(+), 3 deletions(-) diff --git a/test/api/groups.coffee b/test/api/groups.coffee index 2daa783645..2c2674bbdf 100644 --- a/test/api/groups.coffee +++ b/test/api/groups.coffee @@ -11,7 +11,7 @@ describe "Guilds", -> registerNewUser -> User.findByIdAndUpdate user._id, $set: - "balance": 10 + "balance": 40 , (err, _user) -> done() , true @@ -150,6 +150,28 @@ describe "Guilds", -> expectCode res, 204 done() + it "deletes a group when the last member leaves", (done) -> + groupToDeleteAfterLeave = undefined + request.post(baseURL + "/groups").send( + name: "TestGroupToDeleteAfteLeave" + type: "guild" + privacy: "private" + ).end (res) -> + groupToDeleteAfterLeave = res.body + async.waterfall [ + (cb) -> + request.post(baseURL + "/groups/" + groupToDeleteAfterLeave._id + "/leave") + .end (res) -> + expectCode res, 204 + cb() + + (cb) -> + request.post(baseURL + "/groups/" + groupToDeleteAfterLeave._id) + .end (res) -> + expectCode res, 404 + cb() + ], done + context "removing users groups", -> it "allows guild leaders to remove a member (but not themselves)", (done) -> guildToRemoveMember = undefined diff --git a/website/src/controllers/groups.js b/website/src/controllers/groups.js index 64b45da0e8..774037b8c0 100644 --- a/website/src/controllers/groups.js +++ b/website/src/controllers/groups.js @@ -511,8 +511,77 @@ api.leave = function(req, res, next) { // When removing the user from challenges, should we keep the tasks? var keep = (/^remove-all/i).test(req.query.keep) ? 'remove-all' : 'keep-all'; - - group.leave(user, keep, function(err){ + async.parallel([ + // Remove active quest from user if they're leaving the party + function(cb){ + if (group.type != 'party') return cb(null,{},1); + user.party.quest = Group.cleanQuestProgress(); + user.save(cb); + }, + // Remove user from group challenges + function(cb){ + async.waterfall([ + // Find relevant challenges + function(cb2) { + Challenge.find({ + _id: {$in: user.challenges}, // Challenges I am in + group: group._id // that belong to the group I am leaving + }, cb2); + }, + // Update each challenge + function(challenges, cb2) { + Challenge.update( + {_id:{$in: _.pluck(challenges, '_id')}}, + {$pull:{members:user._id}}, + {multi: true}, + function(err) { + cb2(err, challenges); // pass `challenges` above to cb + } + ); + }, + // Unlink the challenge tasks from user + function(challenges, cb2) { + async.waterfall(challenges.map(function(chal) { + return function(cb3) { + var i = user.challenges.indexOf(chal._id) + if (~i) user.challenges.splice(i,1); + user.unlink({cid:chal._id, keep:keep}, cb3); + } + }), cb2); + } + ], cb); + }, + // Update the group + function(cb){ + var update = {$pull:{members:user._id}}; + if (group.type == 'party' && group.quest.key){ + update['$unset'] = {}; + update['$unset']['quest.members.' + user._id] = 1; + } + // FIXME do we want to remove the group `if group.members.length == 0` ? (well, 1 since the update hasn't gone through yet) + if (group.members.length > 1) { + var seniorMember = _.find(group.members, function (m) {return m != user._id}); + // If the leader is leaving (or if the leader previously left, and this wasn't accounted for) + var leader = group.leader; + if (leader == user._id || !~group.members.indexOf(leader)) { + update['$set'] = update['$set'] || {}; + update['$set'].leader = seniorMember; + } + leader = group.quest && group.quest.leader; + if (leader && (leader == user._id || !~group.members.indexOf(leader))) { + update['$set'] = update['$set'] || {}; + update['$set']['quest.leader'] = seniorMember; + } + update['$inc'] = {memberCount: -1}; + Group.update({_id:group._id}, update, cb); + } else if (group.members.length === 1) { + //We don't delete public groups when they are empty + if (group.privacy === 'private' || group.type === 'party') { + Group.remove({_id:group._id}, cb); + } + } + } + ],function(err){ if (err) return next(err); user = group = keep = null; return res.send(204); From 700a6ddd9e1446de5f71def86dcff6c32f993186 Mon Sep 17 00:00:00 2001 From: TheHollidayInn Date: Fri, 4 Sep 2015 14:44:47 -0500 Subject: [PATCH 2/7] Remove users inviations when guild is deleted --- test/api/groups.coffee | 45 +++++++++++++++++++++++++++++++ website/src/controllers/groups.js | 21 ++++++++++++++- 2 files changed, 65 insertions(+), 1 deletion(-) diff --git a/test/api/groups.coffee b/test/api/groups.coffee index 2c2674bbdf..5240d4c4c4 100644 --- a/test/api/groups.coffee +++ b/test/api/groups.coffee @@ -172,6 +172,51 @@ describe "Guilds", -> cb() ], done + it "deletes all invites to a group when the last member leaves", (done) -> + groupToDeleteAfterLeave = undefined + userToRemoveInvite = undefined + request.post(baseURL + "/groups").send( + name: "TestGroupToDeleteAfteLeave" + type: "guild" + privacy: "private" + ).end (res) -> + groupToDeleteAfterLeave = res.body + async.waterfall [ + (cb) -> + registerManyUsers 1, cb + + (_members, cb) -> + userToRemoveInvite = _members[0] + inviteURL = baseURL + "/groups/" + groupToDeleteAfterLeave._id + "/invite" + request.post(inviteURL).send( + uuids: [userToRemoveInvite._id] + ) + .end -> + cb() + + (cb) -> + request.post(baseURL + "/groups/" + groupToDeleteAfterLeave._id + "/leave") + .end (res) -> + expectCode res, 204 + cb() + + (cb) -> + request.get(baseURL + '/user') + .set("X-API-User", userToRemoveInvite._id) + .set("X-API-Key", userToRemoveInvite.apiToken) + .end (err, res) -> + expectCode res, 200 + groupInvitation = _.find(res.body.invitations.guilds, {id: groupToDeleteAfterLeave._id}) + expect(groupInvitation).to.not.exist + cb() + + (cb) -> + request.post(baseURL + "/groups/" + groupToDeleteAfterLeave._id) + .end (res) -> + expectCode res, 404 + cb() + ], done + context "removing users groups", -> it "allows guild leaders to remove a member (but not themselves)", (done) -> guildToRemoveMember = undefined diff --git a/website/src/controllers/groups.js b/website/src/controllers/groups.js index 774037b8c0..95f27d0c6a 100644 --- a/website/src/controllers/groups.js +++ b/website/src/controllers/groups.js @@ -577,7 +577,26 @@ api.leave = function(req, res, next) { } else if (group.members.length === 1) { //We don't delete public groups when they are empty if (group.privacy === 'private' || group.type === 'party') { - Group.remove({_id:group._id}, cb); + async.waterfall([ + function(cb2) { + User.find({ + 'invitations.guilds.id': group._id + }, cb2); + }, + function(users, cb2) { + if (users) { + users.forEach(function (user, index, array) { + var i = _.findIndex(user.invitations.guilds, {id: group._id}); + user.invitations.guilds.splice(i, 1); + user.save(); + }); + } + cb2(); + }, + function(cb2) { + Group.remove({_id:group._id}, cb2); + }, + ], cb); } } } From b839c0cd0cc2505b05b87d1e5a7d098ba85b8d74 Mon Sep 17 00:00:00 2001 From: TheHollidayInn Date: Fri, 4 Sep 2015 16:42:35 -0500 Subject: [PATCH 3/7] Change condition scope to fix updating for public guilds with a member count of 1 --- website/src/controllers/groups.js | 49 ++++++++++++++++--------------- 1 file changed, 25 insertions(+), 24 deletions(-) diff --git a/website/src/controllers/groups.js b/website/src/controllers/groups.js index 95f27d0c6a..bf9bf0397c 100644 --- a/website/src/controllers/groups.js +++ b/website/src/controllers/groups.js @@ -574,30 +574,31 @@ api.leave = function(req, res, next) { } update['$inc'] = {memberCount: -1}; Group.update({_id:group._id}, update, cb); - } else if (group.members.length === 1) { - //We don't delete public groups when they are empty - if (group.privacy === 'private' || group.type === 'party') { - async.waterfall([ - function(cb2) { - User.find({ - 'invitations.guilds.id': group._id - }, cb2); - }, - function(users, cb2) { - if (users) { - users.forEach(function (user, index, array) { - var i = _.findIndex(user.invitations.guilds, {id: group._id}); - user.invitations.guilds.splice(i, 1); - user.save(); - }); - } - cb2(); - }, - function(cb2) { - Group.remove({_id:group._id}, cb2); - }, - ], cb); - } + } + if (group.members.length === 1 && (group.privacy === 'private' || group.type === 'party') ) { + async.waterfall([ + function(cb2) { + User.find({ + 'invitations.guilds.id': group._id + }, cb2); + }, + function(users, cb2) { + if (users) { + users.forEach(function (user, index, array) { + var i = _.findIndex(user.invitations.guilds, {id: group._id}); + user.invitations.guilds.splice(i, 1); + user.save(); + }); + } + cb2(); + }, + function(cb2) { + Group.remove({_id:group._id}, cb2); + }, + ], cb); + } else { + update['$inc'] = {memberCount: -1}; + Group.update({_id:group._id}, update, cb); } } ],function(err){ From 82a3ca8a4da5b99393c0b7b083add44b7058a2b7 Mon Sep 17 00:00:00 2001 From: TheHollidayInn Date: Sun, 6 Sep 2015 16:58:44 -0500 Subject: [PATCH 4/7] Merged changes to firebase branch --- website/src/controllers/groups.js | 93 +------------------------------ website/src/models/group.js | 32 ++++++++--- 2 files changed, 27 insertions(+), 98 deletions(-) diff --git a/website/src/controllers/groups.js b/website/src/controllers/groups.js index bf9bf0397c..64b45da0e8 100644 --- a/website/src/controllers/groups.js +++ b/website/src/controllers/groups.js @@ -511,97 +511,8 @@ api.leave = function(req, res, next) { // When removing the user from challenges, should we keep the tasks? var keep = (/^remove-all/i).test(req.query.keep) ? 'remove-all' : 'keep-all'; - async.parallel([ - // Remove active quest from user if they're leaving the party - function(cb){ - if (group.type != 'party') return cb(null,{},1); - user.party.quest = Group.cleanQuestProgress(); - user.save(cb); - }, - // Remove user from group challenges - function(cb){ - async.waterfall([ - // Find relevant challenges - function(cb2) { - Challenge.find({ - _id: {$in: user.challenges}, // Challenges I am in - group: group._id // that belong to the group I am leaving - }, cb2); - }, - // Update each challenge - function(challenges, cb2) { - Challenge.update( - {_id:{$in: _.pluck(challenges, '_id')}}, - {$pull:{members:user._id}}, - {multi: true}, - function(err) { - cb2(err, challenges); // pass `challenges` above to cb - } - ); - }, - // Unlink the challenge tasks from user - function(challenges, cb2) { - async.waterfall(challenges.map(function(chal) { - return function(cb3) { - var i = user.challenges.indexOf(chal._id) - if (~i) user.challenges.splice(i,1); - user.unlink({cid:chal._id, keep:keep}, cb3); - } - }), cb2); - } - ], cb); - }, - // Update the group - function(cb){ - var update = {$pull:{members:user._id}}; - if (group.type == 'party' && group.quest.key){ - update['$unset'] = {}; - update['$unset']['quest.members.' + user._id] = 1; - } - // FIXME do we want to remove the group `if group.members.length == 0` ? (well, 1 since the update hasn't gone through yet) - if (group.members.length > 1) { - var seniorMember = _.find(group.members, function (m) {return m != user._id}); - // If the leader is leaving (or if the leader previously left, and this wasn't accounted for) - var leader = group.leader; - if (leader == user._id || !~group.members.indexOf(leader)) { - update['$set'] = update['$set'] || {}; - update['$set'].leader = seniorMember; - } - leader = group.quest && group.quest.leader; - if (leader && (leader == user._id || !~group.members.indexOf(leader))) { - update['$set'] = update['$set'] || {}; - update['$set']['quest.leader'] = seniorMember; - } - update['$inc'] = {memberCount: -1}; - Group.update({_id:group._id}, update, cb); - } - if (group.members.length === 1 && (group.privacy === 'private' || group.type === 'party') ) { - async.waterfall([ - function(cb2) { - User.find({ - 'invitations.guilds.id': group._id - }, cb2); - }, - function(users, cb2) { - if (users) { - users.forEach(function (user, index, array) { - var i = _.findIndex(user.invitations.guilds, {id: group._id}); - user.invitations.guilds.splice(i, 1); - user.save(); - }); - } - cb2(); - }, - function(cb2) { - Group.remove({_id:group._id}, cb2); - }, - ], cb); - } else { - update['$inc'] = {memberCount: -1}; - Group.update({_id:group._id}, update, cb); - } - } - ],function(err){ + + group.leave(user, keep, function(err){ if (err) return next(err); user = group = keep = null; return res.send(204); diff --git a/website/src/models/group.js b/website/src/models/group.js index 5f79f46a89..32fe6bd7a3 100644 --- a/website/src/models/group.js +++ b/website/src/models/group.js @@ -359,7 +359,7 @@ GroupSchema.methods.leave = function(user, keep, mainCb){ var group = this; var groupWasRemoved = false; - + async.parallel([ // Remove user from group challenges function(cb){ @@ -404,11 +404,29 @@ GroupSchema.methods.leave = function(user, keep, mainCb){ group.type === 'party' || (group.type === 'guild' && group.privacy === 'private') )){ - // TODO remove invitations to this group - groupWasRemoved = true; - Group.remove({ - _id: group._id - }, cb); + async.waterfall([ + function(cb2) { + User.find({ + 'invitations.guilds.id': group._id + }, cb2); + }, + function(users, cb2) { + if (users) { + users.forEach(function (user, index, array) { + var i = _.findIndex(user.invitations.guilds, {id: group._id}); + user.invitations.guilds.splice(i, 1); + user.save(); + }); + } + cb2(); + }, + function(cb2) { + groupWasRemoved = true; + Group.remove({ + _id: group._id + }, cb2); + }, + ], cb); }else{ // otherwise just remove a member var update = {$pull: {members: user._id}}; @@ -468,4 +486,4 @@ Group.count({_id: 'habitrpg'}, function(err, ct){ type: 'guild', privacy: 'public' }).save(); -}); \ No newline at end of file +}); From c7d09acd74d0658549a1d562a32aff120a99c8ce Mon Sep 17 00:00:00 2001 From: TheHollidayInn Date: Tue, 8 Sep 2015 15:58:53 -0500 Subject: [PATCH 5/7] Moved inivtation removal functions to group pre remove middleware --- website/src/models/group.js | 50 ++++++++++++++++++++----------------- 1 file changed, 27 insertions(+), 23 deletions(-) diff --git a/website/src/models/group.js b/website/src/models/group.js index 32fe6bd7a3..1ecefc7f1c 100644 --- a/website/src/models/group.js +++ b/website/src/models/group.js @@ -84,6 +84,27 @@ GroupSchema.pre('save', function(next){ next(); }) +GroupSchema.pre('remove', function(next) { + var group = this; + async.waterfall([ + function(cb2) { + User.find({ + 'invitations.guilds.id': group._id + }, cb2); + }, + function(users, cb2) { + if (users) { + users.forEach(function (user, index, array) { + var i = _.findIndex(user.invitations.guilds, {id: group._id}); + user.invitations.guilds.splice(i, 1); + user.save(); + }); + } + cb2(); + } + ], next); +}); + GroupSchema.methods.toJSON = function(){ var doc = this.toObject(); removeDuplicates(doc); @@ -404,29 +425,12 @@ GroupSchema.methods.leave = function(user, keep, mainCb){ group.type === 'party' || (group.type === 'guild' && group.privacy === 'private') )){ - async.waterfall([ - function(cb2) { - User.find({ - 'invitations.guilds.id': group._id - }, cb2); - }, - function(users, cb2) { - if (users) { - users.forEach(function (user, index, array) { - var i = _.findIndex(user.invitations.guilds, {id: group._id}); - user.invitations.guilds.splice(i, 1); - user.save(); - }); - } - cb2(); - }, - function(cb2) { - groupWasRemoved = true; - Group.remove({ - _id: group._id - }, cb2); - }, - ], cb); + groupWasRemoved = true; + Group.findOne({ + _id: group._id + }, function(err, groupDoc){ + groupDoc.remove(cb) + }); }else{ // otherwise just remove a member var update = {$pull: {members: user._id}}; From cf2548693ff3a048ed235187da1839fbe8fd455c Mon Sep 17 00:00:00 2001 From: TheHollidayInn Date: Tue, 8 Sep 2015 20:12:58 -0500 Subject: [PATCH 6/7] Changed cb2 to cb and removed extra query when removing group --- website/src/models/group.js | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/website/src/models/group.js b/website/src/models/group.js index 1ecefc7f1c..ee1fd303dc 100644 --- a/website/src/models/group.js +++ b/website/src/models/group.js @@ -87,12 +87,12 @@ GroupSchema.pre('save', function(next){ GroupSchema.pre('remove', function(next) { var group = this; async.waterfall([ - function(cb2) { + function(cb) { User.find({ 'invitations.guilds.id': group._id - }, cb2); + }, cb); }, - function(users, cb2) { + function(users, cb) { if (users) { users.forEach(function (user, index, array) { var i = _.findIndex(user.invitations.guilds, {id: group._id}); @@ -100,7 +100,7 @@ GroupSchema.pre('remove', function(next) { user.save(); }); } - cb2(); + cb(); } ], next); }); @@ -426,11 +426,7 @@ GroupSchema.methods.leave = function(user, keep, mainCb){ (group.type === 'guild' && group.privacy === 'private') )){ groupWasRemoved = true; - Group.findOne({ - _id: group._id - }, function(err, groupDoc){ - groupDoc.remove(cb) - }); + group.remove(cb) }else{ // otherwise just remove a member var update = {$pull: {members: user._id}}; From ca37f4161c2430ed907944cb7b42dd03c2805522 Mon Sep 17 00:00:00 2001 From: TheHollidayInn Date: Tue, 8 Sep 2015 21:17:28 -0500 Subject: [PATCH 7/7] Removed firebase deleteGroup to post remove --- website/src/models/group.js | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/website/src/models/group.js b/website/src/models/group.js index ee1fd303dc..25129c3c9d 100644 --- a/website/src/models/group.js +++ b/website/src/models/group.js @@ -105,6 +105,10 @@ GroupSchema.pre('remove', function(next) { ], next); }); +GroupSchema.post('remove', function(group) { + firebase.deleteGroup(group._id); +}); + GroupSchema.methods.toJSON = function(){ var doc = this.toObject(); removeDuplicates(doc); @@ -379,7 +383,6 @@ GroupSchema.methods.leave = function(user, keep, mainCb){ if(typeof keep !== 'string') keep = 'keep-all'; // can be also 'remove-all' var group = this; - var groupWasRemoved = false; async.parallel([ // Remove user from group challenges @@ -425,7 +428,6 @@ GroupSchema.methods.leave = function(user, keep, mainCb){ group.type === 'party' || (group.type === 'guild' && group.privacy === 'private') )){ - groupWasRemoved = true; group.remove(cb) }else{ // otherwise just remove a member var update = {$pull: {members: user._id}}; @@ -453,7 +455,6 @@ GroupSchema.methods.leave = function(user, keep, mainCb){ if(err) return mainCb(err); firebase.removeUserFromGroup(group._id, user._id); - if(groupWasRemoved) firebase.deleteGroup(group._id); return mainCb(); }); };