diff --git a/test/api/v3/integration/groups/POST-groups_groupId_leave.js b/test/api/v3/integration/groups/POST-groups_groupId_leave.js new file mode 100644 index 0000000000..59031300ba --- /dev/null +++ b/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; + }); + }); + }); +}); diff --git a/test/helpers/api-integration/v3/index.js b/test/helpers/api-integration/v3/index.js index 0c440ade26..880f88f41a 100644 --- a/test/helpers/api-integration/v3/index.js +++ b/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); + }); +} diff --git a/website/src/controllers/api-v3/groups.js b/website/src/controllers/api-v3/groups.js index d7d3eedbd3..05a68831e9 100644 --- a/website/src/controllers/api-v3/groups.js +++ b/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) { diff --git a/website/src/controllers/api-v3/tasks.js b/website/src/controllers/api-v3/tasks.js index 7f2a488735..acdd456beb 100644 --- a/website/src/controllers/api-v3/tasks.js +++ b/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 diff --git a/website/src/models/group.js b/website/src/models/group.js index 5ff1eda3dd..8e21623c7c 100644 --- a/website/src/models/group.js +++ b/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; diff --git a/website/src/models/user.js b/website/src/models/user.js index 8942a8afd2..6ea4ede281 100644 --- a/website/src/models/user.js +++ b/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); } };