diff --git a/test/api/v3/integration/challenges/GET-challenges_challengeId_members.test.js b/test/api/v3/integration/challenges/GET-challenges_challengeId_members.test.js index da0b9162b4..01560cf037 100644 --- a/test/api/v3/integration/challenges/GET-challenges_challengeId_members.test.js +++ b/test/api/v3/integration/challenges/GET-challenges_challengeId_members.test.js @@ -117,26 +117,7 @@ describe('GET /challenges/:challengeId/members', () => { expect(res[0].profile).to.have.all.keys(['name']); }); - it('returns only first 30 members if req.query.includeAllMembers is not true and req.query.limit is undefined', async () => { - const group = await generateGroup(user, { type: 'party', name: generateUUID() }); - const challenge = await generateChallenge(user, group); - await user.post(`/challenges/${challenge._id}/join`); - - const usersToGenerate = []; - for (let i = 0; i < 31; i += 1) { - usersToGenerate.push(generateUser({ challenges: [challenge._id] })); - } - await Promise.all(usersToGenerate); - - const res = await user.get(`/challenges/${challenge._id}/members?includeAllMembers=not-true`); - expect(res.length).to.equal(30); - res.forEach(member => { - expect(member).to.have.all.keys(['_id', 'auth', 'flags', 'id', 'profile']); - expect(member.profile).to.have.all.keys(['name']); - }); - }); - - it('returns only first 30 members if req.query.includeAllMembers is not defined and req.query.limit is undefined', async () => { + it('returns only first 30 members if req.query.limit is undefined', async () => { const group = await generateGroup(user, { type: 'party', name: generateUUID() }); const challenge = await generateChallenge(user, group); await user.post(`/challenges/${challenge._id}/join`); @@ -217,25 +198,6 @@ describe('GET /challenges/:challengeId/members', () => { }); }).timeout(30000); - it('returns all members if req.query.includeAllMembers is true', async () => { - const group = await generateGroup(user, { type: 'party', name: generateUUID() }); - const challenge = await generateChallenge(user, group); - await user.post(`/challenges/${challenge._id}/join`); - - const usersToGenerate = []; - for (let i = 0; i < 31; i += 1) { - usersToGenerate.push(generateUser({ challenges: [challenge._id] })); - } - await Promise.all(usersToGenerate); - - const res = await user.get(`/challenges/${challenge._id}/members?includeAllMembers=true`); - expect(res.length).to.equal(32); - res.forEach(member => { - expect(member).to.have.all.keys(['_id', 'auth', 'flags', 'id', 'profile']); - expect(member.profile).to.have.all.keys(['name']); - }); - }); - it('supports using req.query.lastId to get more members', async function test () { this.timeout(30000); // @TODO: times out after 8 seconds const group = await generateGroup(user, { type: 'party', name: generateUUID() }); @@ -259,6 +221,34 @@ describe('GET /challenges/:challengeId/members', () => { expect(resIds).to.eql(expectedIds.sort()); }); + it('supports using req.query.includeTasks in order to add challenge-related tasks of all members', async () => { + const group = await generateGroup(user, { type: 'party', name: generateUUID() }); + const challenge = await generateChallenge(user, group); + await user.post(`/challenges/${challenge._id}/join`); + + const usersToGenerate = []; + for (let i = 0; i < 8; i += 1) { + usersToGenerate.push(generateUser({ challenges: [challenge._id] })); + } + await Promise.all(usersToGenerate); + await user.post(`/tasks/challenge/${challenge._id}`, [{ type: 'habit', text: 'Some task' }]); + await user.post(`/tasks/challenge/${challenge._id}`, [{ type: 'daily', text: 'Some different task' }]); + + const res = await user.get(`/challenges/${challenge._id}/members?includeTasks=true`); + expect(res.length).to.equal(9); + res.forEach(member => { + expect(member).to.have.property('tasks'); + expect(member.tasks).to.be.an('array'); + expect(member.tasks).to.have.lengthOf(2); + member.tasks.forEach(task => { + expect(task).to.include.all.keys(['type', 'value', 'priority', 'text', '_id', 'userId']); + expect(task).to.not.have.any.keys(['tags', 'checklist']); + expect(task.challenge.id).to.be.equal(challenge._id); + expect(task.userId).to.be.equal(member._id); + }); + }); + }); + it('supports using req.query.search to get search members', async () => { const group = await generateGroup(user, { type: 'party', name: generateUUID() }); const challenge = await generateChallenge(user, group); diff --git a/test/api/v3/integration/challenges/GET-challenges_challengeId_members_memberId.test.js b/test/api/v3/integration/challenges/GET-challenges_challengeId_members_memberId.test.js index 98455f66d3..03ffa8d8f4 100644 --- a/test/api/v3/integration/challenges/GET-challenges_challengeId_members_memberId.test.js +++ b/test/api/v3/integration/challenges/GET-challenges_challengeId_members_memberId.test.js @@ -116,7 +116,6 @@ describe('GET /challenges/:challengeId/members/:memberId', () => { }]); const memberProgress = await user.get(`/challenges/${challenge._id}/members/${user._id}`); - expect(memberProgress.tasks[0]).not.to.have.key('tags'); - expect(memberProgress.tasks[0].checklist).to.eql([]); + expect(memberProgress.tasks[0]).to.not.have.any.keys(['tags', 'checklist']); }); }); diff --git a/website/server/controllers/api-v3/members.js b/website/server/controllers/api-v3/members.js index b2f1478a03..bdd6f9020a 100644 --- a/website/server/controllers/api-v3/members.js +++ b/website/server/controllers/api-v3/members.js @@ -26,6 +26,7 @@ import { sanitizeText as sanitizeMessageText, } from '../../models/message'; import highlightMentions from '../../libs/highlightMentions'; +import { handleGetMembersForChallenge } from '../../libs/challenges/handleGetMembersForChallenge'; const { achievements } = common; @@ -278,16 +279,12 @@ api.getMemberAchievements = { // We should create factory functions. See Webhooks for a good example function _getMembersForItem (type) { // check for allowed `type` - if (['group-members', 'group-invites', 'challenge-members'].indexOf(type) === -1) { - throw new Error('Type must be one of "group-members", "group-invites", "challenge-members"'); + if (['group-members', 'group-invites'].indexOf(type) === -1) { + throw new Error('Type must be one of "group-members", "group-invites"'); } return async function handleGetMembersForItem (req, res) { - if (type === 'challenge-members') { - req.checkParams('challengeId', res.t('challengeIdRequired')).notEmpty().isUUID(); - } else { - req.checkParams('groupId', res.t('groupIdRequired')).notEmpty(); - } + req.checkParams('groupId', res.t('groupIdRequired')).notEmpty(); req.checkQuery('lastId').optional().notEmpty().isUUID(); // Allow an arbitrary number of results (up to 60) req.checkQuery('limit', res.t('groupIdRequired')).optional().notEmpty().isInt({ min: 1, max: 60 }); @@ -296,49 +293,18 @@ function _getMembersForItem (type) { if (validationErrors) throw validationErrors; const { groupId } = req.params; - const { challengeId } = req.params; const { lastId } = req.query; const { user } = res.locals; - let challenge; - let group; - if (type === 'challenge-members') { - challenge = await Challenge.findById(challengeId).select('_id type leader group').exec(); - if (!challenge) throw new NotFound(res.t('challengeNotFound')); - - // optionalMembership is set to true because even - // if you're not member of the group you may be able to access the challenge - // for example if you've been booted from it, are the leader or a site admin - group = await Group.getGroup({ - user, - groupId: challenge.group, - fields: '_id type privacy', - optionalMembership: true, - }); - - if (!group || !challenge.canView(user, group)) throw new NotFound(res.t('challengeNotFound')); - } else { - group = await Group.getGroup({ user, groupId, fields: '_id type' }); - if (!group) throw new NotFound(res.t('groupNotFound')); - } + const group = await Group.getGroup({ user, groupId, fields: '_id type' }); + if (!group) throw new NotFound(res.t('groupNotFound')); const query = {}; let fields = nameFields; // add computes stats to the member info when items and stats are available let addComputedStats = false; - if (type === 'challenge-members') { - query.challenges = challenge._id; - - if (req.query.includeAllPublicFields === 'true') { - fields = memberFields; - addComputedStats = true; - } - - if (req.query.search) { - query['auth.local.username'] = { $regex: req.query.search }; - } - } else if (type === 'group-members') { + if (type === 'group-members') { if (group.type === 'guild') { query.guilds = group._id; @@ -381,12 +347,7 @@ function _getMembersForItem (type) { if (lastId) query._id = { $gt: lastId }; - let limit = req.query.limit ? Number(req.query.limit) : 30; - - // Allow for all challenges members to be returned - if (type === 'challenge-members' && req.query.includeAllMembers === 'true') { - limit = 0; // no limit - } + const limit = req.query.limit ? Number(req.query.limit) : 30; const members = await User .find(query) @@ -420,6 +381,9 @@ function _getMembersForItem (type) { * then all public fields for members * will be returned (similar to when making * a request for a single member). + * @apiParam (Query) {Boolean} includeTasks If set to `true`, then + * response should include all tasks per user + * related to the challenge * * @apiSuccess {Array} data An array of members, sorted by _id * @@ -511,12 +475,12 @@ api.getInvitesForGroup = { * get the next batch of results. * @apiParam (Query) {Number} limit=30 BETA Query parameter to * specify the number of results to return. Max is 60. + * @apiParam (Query) {Boolean} includeTasks BETA Query parameter - If 'true' + * then include challenge tasks of each member * @apiParam (Query) {Boolean} includeAllPublicFields If set to `true` * then all public fields for members * will be returned (similar to when making * a request for a single member). - * @apiParam (Query) {String} includeAllMembers BETA Query parameter - If 'true' all - * challenge members are returned. * @apiSuccess {Array} data An array of members, sorted by _id * @@ -527,7 +491,7 @@ api.getMembersForChallenge = { method: 'GET', url: '/challenges/:challengeId/members', middlewares: [authWithHeaders()], - handler: _getMembersForItem('challenge-members'), + handler: handleGetMembersForChallenge, }; /** @@ -603,10 +567,8 @@ api.getChallengeMemberProgress = { const member = await User.findById(memberId).select(`${nameFields} challenges`).exec(); if (!member) throw new NotFound(res.t('userWithIDNotFound', { userId: memberId })); - const challenge = await Challenge.findById(challengeId).exec(); if (!challenge) throw new NotFound(res.t('challengeNotFound')); - // optionalMembership is set to true because even if you're // not member of the group you may be able to access the challenge // for example if you've been booted from it, are the leader or a site admin @@ -616,20 +578,18 @@ api.getChallengeMemberProgress = { if (!group || !challenge.canView(user, group)) throw new NotFound(res.t('challengeNotFound')); if (!challenge.isMember(member)) throw new NotFound(res.t('challengeMemberNotFound')); - const chalTasks = await Tasks.Task.find({ - userId: memberId, - 'challenge.id': challengeId, + const challengeTasks = await Tasks.Task.find({ + userId: member._id, + 'challenge.id': challenge._id, }) - .select('-tags') // We don't want to return the tags publicly TODO same for other data? + .select('-tags -checklist') // We don't want to return tags and checklists publicly + .lean() .exec(); // manually call toJSON with minimize: true so empty paths aren't returned const response = member.toJSON({ minimize: true }); delete response.challenges; - response.tasks = chalTasks.map(chalTask => { - chalTask.checklist = []; // Clear checklists as they are private - return chalTask.toJSON({ minimize: true }); - }); + response.tasks = challengeTasks; res.respond(200, response); }, }; diff --git a/website/server/libs/challenges/handleGetMembersForChallenge.js b/website/server/libs/challenges/handleGetMembersForChallenge.js new file mode 100644 index 0000000000..26dbfb9b82 --- /dev/null +++ b/website/server/libs/challenges/handleGetMembersForChallenge.js @@ -0,0 +1,90 @@ +import _ from 'lodash'; +import { + model as User, + publicFields as memberFields, + nameFields, +} from '../../models/user'; +import { model as Challenge } from '../../models/challenge'; +import { model as Group } from '../../models/group'; +import * as Tasks from '../../models/task'; +import { NotFound } from '../errors'; + +async function getMembersTasksForChallenge (members, challenge) { + const challengeTasks = await Tasks.Task.find({ + userId: { $in: members.map(m => m._id) }, + 'challenge.id': challenge._id, + }) + .select('-tags -checklist') // We don't want to return tags and checklists publicly + .lean() + .exec(); + + return _.groupBy(challengeTasks, 'userId'); +} + +export async function handleGetMembersForChallenge (req, res) { + req.checkParams('challengeId', res.t('challengeIdRequired')).notEmpty().isUUID(); + req.checkQuery('lastId').optional().notEmpty().isUUID(); + // Allow an arbitrary number of results (up to 60) + req.checkQuery('limit', res.t('groupIdRequired')).optional().notEmpty().isInt({ min: 1, max: 60 }); + + const validationErrors = req.validationErrors(); + if (validationErrors) throw validationErrors; + + const { challengeId } = req.params; + const { lastId } = req.query; + const { user } = res.locals; + + const challenge = await Challenge.findById(challengeId).select('_id type leader group').exec(); + if (!challenge) throw new NotFound(res.t('challengeNotFound')); + + // optionalMembership is set to true because even + // if you're not member of the group you may be able to access the challenge + // for example if you've been booted from it, are the leader or a site admin + const group = await Group.getGroup({ + user, + groupId: challenge.group, + fields: '_id type privacy', + optionalMembership: true, + }); + + if (!group || !challenge.canView(user, group)) throw new NotFound(res.t('challengeNotFound')); + + const query = {}; + let fields = nameFields; + // add computes stats to the member info when items and stats are available + let addComputedStats = false; + query.challenges = challenge._id; + + if (req.query.includeAllPublicFields === 'true') { + fields = memberFields; + addComputedStats = true; + } + + if (req.query.search) { + query['auth.local.username'] = { $regex: req.query.search }; + } + + if (lastId) query._id = { $gt: lastId }; + + const limit = req.query.limit ? Number(req.query.limit) : 30; + + const members = await User + .find(query) + .sort({ _id: 1 }) + .limit(limit) + .select(fields) + .lean() + .exec(); + + const includeTasks = req.query.includeTasks === 'true'; + let memberIdToTasksMap; + if (includeTasks) { + memberIdToTasksMap = await getMembersTasksForChallenge(members, challenge); + } + + members.forEach(member => { + User.transformJSONUser(member, addComputedStats); + if (includeTasks) member.tasks = memberIdToTasksMap[member._id]; + }); + res.respond(200, members); +}