From 1daf87531aefe9a8a21bd000c68924d7c14314e6 Mon Sep 17 00:00:00 2001 From: Matteo Pagliazzi Date: Wed, 3 Feb 2016 11:29:19 +0100 Subject: [PATCH] add population to join and update routes of challenges and groups, remove chat flags info everywhere --- .../POST-challenges_challengeId_join.test.js | 16 +++++++ .../PUT-challenges_challengeId.test.js | 12 +++++- test/api/v3/integration/chat/GET-chat.test.js | 4 +- .../groups/POST-groups_groupId_join.test.js | 4 +- .../v3/integration/groups/PUT-groups.test.js | 2 + website/src/controllers/api-v3/challenges.js | 43 +++++++++++++------ website/src/controllers/api-v3/chat.js | 12 +++--- website/src/controllers/api-v3/groups.js | 43 ++++++++++++------- website/src/models/group.js | 17 +++++++- 9 files changed, 114 insertions(+), 39 deletions(-) diff --git a/test/api/v3/integration/challenges/POST-challenges_challengeId_join.test.js b/test/api/v3/integration/challenges/POST-challenges_challengeId_join.test.js index 3eda5716cd..34e018d69d 100644 --- a/test/api/v3/integration/challenges/POST-challenges_challengeId_join.test.js +++ b/test/api/v3/integration/challenges/POST-challenges_challengeId_join.test.js @@ -55,6 +55,22 @@ describe('POST /challenges/:challengeId/join', () => { }); }); + it('returns challenge data', async () => { + let res = await authorizedUser.post(`/challenges/${challenge._id}/join`); + + expect(res.group).to.eql({ + _id: group._id, + privacy: group.privacy, + name: group.name, + type: group.type, + }); + expect(res.leader).to.eql({ + _id: groupLeader._id, + profile: {name: groupLeader.profile.name}, + }); + expect(res.name).to.equal(challenge.name); + }); + it('adds challenge to user challenges', async () => { await authorizedUser.post(`/challenges/${challenge._id}/join`); diff --git a/test/api/v3/integration/challenges/PUT-challenges_challengeId.test.js b/test/api/v3/integration/challenges/PUT-challenges_challengeId.test.js index 33cc2e5166..63d2102968 100644 --- a/test/api/v3/integration/challenges/PUT-challenges_challengeId.test.js +++ b/test/api/v3/integration/challenges/PUT-challenges_challengeId.test.js @@ -63,13 +63,21 @@ describe('PUT /challenges/:challengeId', () => { }); expect(res.prize).to.equal(0); - expect(res.group).to.equal(privateGuild._id); + expect(res.group).to.eql({ + _id: privateGuild._id, + privacy: privateGuild.privacy, + name: privateGuild.name, + type: privateGuild.type, + }); expect(res.memberCount).to.equal(2); expect(res.tasksOrder).not.to.equal('new order'); expect(res.official).to.equal(false); expect(res.shortName).not.to.equal('new short name'); - expect(res.leader).to.equal(member._id); + expect(res.leader).to.eql({ + _id: member._id, + profile: {name: member.profile.name}, + }); expect(res.name).to.equal('New Challenge Name'); expect(res.description).to.equal('New challenge description.'); }); diff --git a/test/api/v3/integration/chat/GET-chat.test.js b/test/api/v3/integration/chat/GET-chat.test.js index dbd631e362..91424d655e 100644 --- a/test/api/v3/integration/chat/GET-chat.test.js +++ b/test/api/v3/integration/chat/GET-chat.test.js @@ -23,8 +23,8 @@ describe('GET /groups/:groupId/chat', () => { privacy: 'public', }, { chat: [ - 'Hello', - 'Welcome to the Guild', + {text: 'Hello', flags: {}}, + {text: 'Welcome to the Guild', flags: {}}, ], }); }); diff --git a/test/api/v3/integration/groups/POST-groups_groupId_join.test.js b/test/api/v3/integration/groups/POST-groups_groupId_join.test.js index d20c8b830d..b9b759bed9 100644 --- a/test/api/v3/integration/groups/POST-groups_groupId_join.test.js +++ b/test/api/v3/integration/groups/POST-groups_groupId_join.test.js @@ -35,9 +35,11 @@ describe('POST /group/:groupId/join', () => { }); it('allows non-invited users to join public guilds', async () => { - await joiningUser.post(`/groups/${publicGuild._id}/join`); + let res = await joiningUser.post(`/groups/${publicGuild._id}/join`); await expect(joiningUser.get('/user')).to.eventually.have.property('guilds').to.include(publicGuild._id); + expect(res.leader._id).to.eql(user._id); + expect(res.leader.profile.name).to.eql(user.profile.name); }); it('promotes joining member in a public empty guild to leader', async () => { diff --git a/test/api/v3/integration/groups/PUT-groups.test.js b/test/api/v3/integration/groups/PUT-groups.test.js index 414f495ff7..8d581d56ca 100644 --- a/test/api/v3/integration/groups/PUT-groups.test.js +++ b/test/api/v3/integration/groups/PUT-groups.test.js @@ -39,6 +39,8 @@ describe('PUT /group', () => { name: groupUpdatedName, }); + expect(updatedGroup.leader._id).to.eql(leader._id); + expect(updatedGroup.leader.profile.name).to.eql(leader.profile.name); expect(updatedGroup.name).to.equal(groupUpdatedName); }); }); diff --git a/website/src/controllers/api-v3/challenges.js b/website/src/controllers/api-v3/challenges.js index c14a12d643..bc9e6f03b4 100644 --- a/website/src/controllers/api-v3/challenges.js +++ b/website/src/controllers/api-v3/challenges.js @@ -93,20 +93,21 @@ api.createChallenge = { let results = await Q.all([challenge.save({ validateBeforeSave: false, // already validate }), group.save()]); - let savedChal = results[0]; - // TODO Instead of populate we make a find call manually because of https://github.com/Automattic/mongoose/issues/3833 - // await Q.ninvoke(savedChal, 'populate', ['leader', nameFields]); // doc.populate doesn't return a promise + + await savedChal.syncToUser(user); // (it also saves the user) + let response = savedChal.toJSON(); - response.leader = (await User.findById(response.leader).select(nameFields).exec()).toJSON({minimize: true}); - response.group = { + response.leader = { // the leader is the authenticated user + _id: user._id, + profile: {name: user.profile.name}, + }; + response.group = { // we already have the group data _id: group._id, name: group.name, type: group.type, privacy: group.privacy, }; - - await savedChal.syncToUser(user); // (it also saves the user) res.respond(201, response); }, }; @@ -136,14 +137,24 @@ api.joinChallenge = { if (!challenge) throw new NotFound(res.t('challengeNotFound')); if (challenge.isMember(user)) throw new NotAuthorized(res.t('userAlreadyInChallenge')); - let group = await Group.getGroup({user, groupId: challenge.group, fields: '_id type privacy', optionalMembership: true}); + let group = await Group.getGroup({user, groupId: challenge.group, fields: basicGroupFields, optionalMembership: true}); if (!group || !challenge.hasAccess(user, group)) throw new NotFound(res.t('challengeNotFound')); challenge.memberCount += 1; // Add all challenge's tasks to user's tasks and save the challenge - await Q.all([challenge.syncToUser(user), challenge.save()]); - res.respond(200, challenge); + let results = await Q.all([challenge.syncToUser(user), challenge.save()]); + + let response = results[1].toJSON(); + response.group = { // we already have the group data + _id: group._id, + name: group.name, + type: group.type, + privacy: group.privacy, + }; + response.leader = (await User.findById(response.leader).select(nameFields).exec()).toJSON({minimize: true}); + + res.respond(200, response); }, }; @@ -412,14 +423,22 @@ api.updateChallenge = { let challenge = await Challenge.findById(challengeId).exec(); if (!challenge) throw new NotFound(res.t('challengeNotFound')); - let group = await Group.getGroup({user, groupId: challenge.group, fields: '_id name type privacy', optionalMembership: true}); + let group = await Group.getGroup({user, groupId: challenge.group, fields: basicGroupFields, optionalMembership: true}); if (!group || !challenge.canView(user, group)) throw new NotFound(res.t('challengeNotFound')); if (!challenge.canModify(user)) throw new NotAuthorized(res.t('onlyLeaderUpdateChal')); _.merge(challenge, Challenge.sanitizeUpdate(req.body)); let savedChal = await challenge.save(); - res.respond(200, savedChal); + let response = savedChal.toJSON(); + response.group = { // we already have the group data + _id: group._id, + name: group.name, + type: group.type, + privacy: group.privacy, + }; + response.leader = (await User.findById(response.leader).select(nameFields).exec()).toJSON({minimize: true}); + res.respond(200, response); }, }; diff --git a/website/src/controllers/api-v3/chat.js b/website/src/controllers/api-v3/chat.js index 5c8d4fd3ed..079fbb66cd 100644 --- a/website/src/controllers/api-v3/chat.js +++ b/website/src/controllers/api-v3/chat.js @@ -38,7 +38,7 @@ api.getChat = { let group = await Group.getGroup({user, groupId: req.params.groupId, fields: 'chat'}); if (!group) throw new NotFound(res.t('groupNotFound')); - res.respond(200, group.chat); + res.respond(200, Group.toJSONCleanChat(group, user).chat); }, }; @@ -88,7 +88,7 @@ api.postChat = { let savedGroup = await group.save(); if (chatUpdated) { - res.respond(200, {chat: savedGroup.chat}); + res.respond(200, {chat: Group.toJSONCleanChat(savedGroup, user).chat}); } else { res.respond(200, {message: savedGroup.chat[0]}); } @@ -138,7 +138,7 @@ api.likeChat = { {_id: group._id, 'chat.id': message.id}, update ); - res.respond(200, message); + res.respond(200, message); // TODO what if the message is flagged and shouldn't be returned? }, }; @@ -385,9 +385,9 @@ api.deleteChat = { ); if (chatUpdated) { - group = group.toJSON(); - removeFromArray(group.chat, {id: chatId}); - res.respond(200, group.chat); + let chatRes = Group.toJSONCleanChat(group, user).chat; + removeFromArray(chatRes, {id: chatId}); + res.respond(200, chatRes); } else { res.respond(200, {}); } diff --git a/website/src/controllers/api-v3/groups.js b/website/src/controllers/api-v3/groups.js index ff223bcef8..d4f19ea2ec 100644 --- a/website/src/controllers/api-v3/groups.js +++ b/website/src/controllers/api-v3/groups.js @@ -59,12 +59,17 @@ api.createGroup = { let results = await Q.all([user.save(), group.save()]); let savedGroup = results[1]; + // TODO Instead of populate we make a find call manually because of https://github.com/Automattic/mongoose/issues/3833 // await Q.ninvoke(savedGroup, 'populate', ['leader', nameFields]); // doc.populate doesn't return a promise let response = savedGroup.toJSON(); - response.leader = (await User.findById(response.leader).select(nameFields).exec()).toJSON({minimize: true}); + // the leader is the authenticated user + response.leader = { + _id: user._id, + profile: {name: user.profile.name}, + }; + res.respond(201, response); // do not remove chat flags data as we've just created the group - res.respond(201, response); firebase.updateGroupData(savedGroup); firebase.addUserToGroup(savedGroup._id, user._id); }, @@ -162,16 +167,10 @@ api.getGroup = { let group = await Group.getGroup({user, groupId: req.params.groupId, populateLeader: false}); if (!group) throw new NotFound(res.t('groupNotFound')); - if (!user.contributor.admin) { - group = group.toJSON(); - _.remove(group.chat, function removeChat (chat) { - chat.flags = {}; - return chat.flagCount >= 2; - }); - } - + group = Group.toJSONCleanChat(group, user); // TODO Instead of populate we make a find call manually because of https://github.com/Automattic/mongoose/issues/3833 group.leader = (await User.findById(group.leader).select(nameFields).exec()).toJSON({minimize: true}); + res.respond(200, group); }, }; @@ -206,7 +205,18 @@ api.updateGroup = { _.assign(group, _.merge(group.toObject(), Group.sanitizeUpdate(req.body))); let savedGroup = await group.save(); - res.respond(200, savedGroup); + let response = Group.toJSONCleanChat(savedGroup, user); + // If the leader changed fetch new data, otherwise use authenticated user + if (response.leader !== user._id) { + response.leader = (await User.findById(response.leader).select(nameFields).exec()).toJSON({minimize: true}); + } else { + response.leader = { + _id: user._id, + profile: {name: user.profile.name}, + }; + } + res.respond(200, response); + firebase.updateGroupData(savedGroup); }, }; @@ -219,7 +229,7 @@ api.updateGroup = { * * @apiParam {UUID} groupId The group _id * - * @apiSuccess {Object} empty An empty object + * @apiSuccess {Object} group The group */ api.joinGroup = { method: 'POST', @@ -234,8 +244,8 @@ api.joinGroup = { let validationErrors = req.validationErrors(); if (validationErrors) throw validationErrors; - // Do not fetch chat and work even if the user is not yet a member of the group - let group = await Group.getGroup({user, groupId: req.params.groupId, fields: '-chat', optionalMembership: true}); // Do not fetch chat and work even if the user is not yet a member of the group + // Works even if the user is not yet a member of the group + let group = await Group.getGroup({user, groupId: req.params.groupId, optionalMembership: true}); // Do not fetch chat and work even if the user is not yet a member of the group if (!group) throw new NotFound(res.t('groupNotFound')); let isUserInvited = false; @@ -288,8 +298,11 @@ api.joinGroup = { await Q.all(promises); + let response = Group.toJSONCleanChat(promises[0], user); + response.leader = (await User.findById(response.leader).select(nameFields).exec()).toJSON({minimize: true}); + res.respond(200, response); + firebase.addUserToGroup(group._id, user._id); - res.respond(200, {}); // TODO what to return? }, }; diff --git a/website/src/models/group.js b/website/src/models/group.js index 890662de4c..9e0a1b7a52 100644 --- a/website/src/models/group.js +++ b/website/src/models/group.js @@ -137,7 +137,22 @@ schema.statics.getGroup = function getGroup (options = {}) { if (fields) mQuery.select(fields); if (populateLeader === true) mQuery.populate('leader', nameFields); return mQuery.exec(); - // TODO purge chat flags info? in tojson? +}; + +// When converting to json remove chat messages with more than 1 flag and remove all flags info +// unless the user is an admin +// Not putting into toJSON because there we can't access user +schema.statics.toJSONCleanChat = function groupToJSONCleanChat (group, user) { + let toJSON = group.toJSON(); + console.log(group.chat, toJSON.chat) + if (!user.contributor.admin) { + _.remove(toJSON.chat, chatMsg => { + console.log(chatMsg) + chatMsg.flags = {}; + return chatMsg.flagCount >= 2; + }); + } + return toJSON; }; schema.methods.removeGroupInvitations = async function removeGroupInvitations () {