From ec7ed9c90e0fcad36c59e35c4b54564fd04cafa0 Mon Sep 17 00:00:00 2001 From: Matteo Pagliazzi Date: Sun, 17 Jan 2016 18:31:03 +0100 Subject: [PATCH] add tests for getChallengeMemberProgress route and several bug fixes --- ...enges_challengeId_members_memberId.test.js | 126 ++++++++++++++++++ website/src/controllers/api-v3/challenges.js | 55 +------- website/src/controllers/api-v3/members.js | 52 ++++++++ website/src/controllers/api-v3/tasks.js | 6 +- website/src/models/challenge.js | 4 +- 5 files changed, 184 insertions(+), 59 deletions(-) create mode 100644 test/api/v3/integration/challenges/GET-challenges_challengeId_members_memberId.test.js 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 new file mode 100644 index 0000000000..e91fef8563 --- /dev/null +++ b/test/api/v3/integration/challenges/GET-challenges_challengeId_members_memberId.test.js @@ -0,0 +1,126 @@ +import { + generateUser, + generateGroup, + translate as t, +} from '../../../../helpers/api-v3-integration.helper'; +import { v4 as generateUUID } from 'uuid'; + +describe('GET /challenges/:challengeId/members/:memberId', () => { + let user; + + beforeEach(async () => { + user = await generateUser(); + }); + + it('validates req.params.memberId to be an UUID', async () => { + await expect(user.get(`/challenges/invalidUUID/members/${generateUUID()}`)).to.eventually.be.rejected.and.eql({ + code: 400, + error: 'BadRequest', + message: t('invalidReqParams'), + }); + }); + + it('validates req.params.memberId to be an UUID', async () => { + await expect(user.get(`/challenges/${generateUUID()}/members/invalidUUID`)).to.eventually.be.rejected.and.eql({ + code: 400, + error: 'BadRequest', + message: t('invalidReqParams'), + }); + }); + + it('fails if member doesn\'t exists', async () => { + let userId = generateUUID(); + await expect(user.get(`/challenges/${generateUUID()}/members/${userId}`)).to.eventually.be.rejected.and.eql({ + code: 404, + error: 'NotFound', + message: t('userWithIDNotFound', {userId}), + }); + }); + + it('fails if challenge doesn\'t exists', async () => { + let member = await generateUser(); + await expect(user.get(`/challenges/${generateUUID()}/members/${member._id}`)).to.eventually.be.rejected.and.eql({ + code: 404, + error: 'NotFound', + message: t('challengeNotFound'), + }); + }); + + it('fails if user doesn\'t have access to the challenge', async () => { + let group = await generateGroup(user, {type: 'party', name: generateUUID()}); + let challenge = await user.post('/challenges', { + name: 'test chal', + shortName: 'test-chal', + groupId: group._id, + }); + let anotherUser = await generateUser(); + let member = await generateUser(); + await expect(anotherUser.get(`/challenges/${challenge._id}/members/${member._id}`)).to.eventually.be.rejected.and.eql({ + code: 404, + error: 'NotFound', + message: t('challengeNotFound'), + }); + }); + + it('fails if member is not part of the challenge', async () => { + let group = await generateGroup(user, {type: 'party', name: generateUUID()}); + let challenge = await user.post('/challenges', { + name: 'test chal', + shortName: 'test-chal', + groupId: group._id, + }); + let member = await generateUser(); + await expect(user.get(`/challenges/${challenge._id}/members/${member._id}`)).to.eventually.be.rejected.and.eql({ + code: 404, + error: 'NotFound', + message: t('challengeMemberNotFound'), + }); + }); + + it('works with challenges belonging to a public guild', async () => { + let groupLeader = await generateUser({balance: 4}); + let group = await generateGroup(groupLeader, {type: 'guild', privacy: 'public', name: generateUUID()}); + let challenge = await groupLeader.post('/challenges', { + name: 'test chal', + shortName: 'test-chal', + groupId: group._id, + }); + let taskText = 'Test Text'; + await groupLeader.post(`/tasks/challenge/${challenge._id}`, [{type: 'habit', text: taskText}]); + + let memberProgress = await user.get(`/challenges/${challenge._id}/members/${groupLeader._id}`); + expect(memberProgress).to.have.all.keys(['_id', 'profile', 'tasks']); + expect(memberProgress.profile).to.have.all.keys(['name']); + expect(memberProgress.tasks.length).to.equal(1); + }); + + it('returns the member tasks for the challenges', async () => { + let group = await generateGroup(user, {type: 'party', name: generateUUID()}); + let challenge = await user.post('/challenges', { + name: 'test chal', + shortName: 'test-chal', + groupId: group._id, + }); + await user.post(`/tasks/challenge/${challenge._id}`, [{type: 'habit', text: 'Test Text'}]); + + let memberProgress = await user.get(`/challenges/${challenge._id}/members/${user._id}`); + let chalTasks = await user.get(`/tasks/challenge/${challenge._id}`); + expect(memberProgress.tasks.length).to.equal(chalTasks.length); + expect(memberProgress.tasks[0].challenge.id).to.equal(challenge._id); + expect(memberProgress.tasks[0].challenge.taskId).to.equal(chalTasks[0]._id); + }); + + it('returns the tasks without the tags', async () => { + let group = await generateGroup(user, {type: 'party', name: generateUUID()}); + let challenge = await user.post('/challenges', { + name: 'test chal', + shortName: 'test-chal', + groupId: group._id, + }); + let taskText = 'Test Text'; + await user.post(`/tasks/challenge/${challenge._id}`, [{type: 'habit', text: taskText}]); + + let memberProgress = await user.get(`/challenges/${challenge._id}/members/${user._id}`); + expect(memberProgress.tasks[0]).not.to.have.key('tags'); + }); +}); diff --git a/website/src/controllers/api-v3/challenges.js b/website/src/controllers/api-v3/challenges.js index d0e54667af..879b13b433 100644 --- a/website/src/controllers/api-v3/challenges.js +++ b/website/src/controllers/api-v3/challenges.js @@ -2,10 +2,7 @@ import { authWithHeaders } from '../../middlewares/api-v3/auth'; import cron from '../../middlewares/api-v3/cron'; import { model as Challenge } from '../../models/challenge'; import { model as Group } from '../../models/group'; -import { - model as User, - nameFields, -} from '../../models/user'; +import { model as User } from '../../models/user'; import { NotFound, NotAuthorized, @@ -165,56 +162,6 @@ api.getChallenge = { }, }; -/** - * @api {get} /challenges/:challengeId/members/:memberId Get a challenge member progress - * @apiVersion 3.0.0 - * @apiName GetChallenge - * @apiGroup Challenge - * - * @apiParam {UUID} challengeId The challenge _id - * @apiParam {UUID} member The member _id - * - * @apiSuccess {object} member Return an object with member _id, profile.name and a tasks object with the challenge tasks for the member - */ -api.getChallengeMemberProgress = { - method: 'GET', - url: '/challenges/:challengeId/members/:memberId', - middlewares: [authWithHeaders(), cron], - async handler (req, res) { - req.checkQuery('challengeId', res.t('challengeIdRequired')).notEmpty().isUUID(); - req.checkQuery('memberId', res.t('memberIdRequired')).notEmpty().isUUID(); - - let validationErrors = req.validationErrors(); - if (validationErrors) throw validationErrors; - - let user = res.locals.user; - let challengeId = req.params.challengeId; - let memberId = req.params.memberId; - - let member = await User.findById(memberId).select(`${nameFields} challenges`).exec(); - if (!member) throw new NotFound(res.t('userWithIDNotFound', {userId: memberId})); - - let challenge = await Challenge.findById(challengeId).exec(); - if (!challenge) throw new NotFound(res.t('challengeNotFound')); - - let group = await Group.getGroup({user, groupId: challenge.groupId, fields: '_id type privacy'}); - if (!group || !challenge.canView(user, group)) throw new NotFound(res.t('challengeNotFound')); - if (!challenge.isMember(member)) throw new NotFound(res.t('challengeMemberNotFound')); - - let chalTasks = Tasks.Task.find({ - userId: memberId, - 'challenge.id': challengeId, - }) - .select('-tags') // We don't want to return the tags publicly TODO same for other data? - .exec(); - - // manually call toJSON with minimize: true so empty paths aren't returned - let response = member.toJSON({minimize: true}); - response.tasks = chalTasks.map(chalTask => chalTask.toJSON({minimize: true})); - res.respond(200, response); - }, -}; - // TODO everything here should be moved to a worker // actually even for a worker it's probably just to big and will kill mongo function _closeChal (challenge, broken = {}) { diff --git a/website/src/controllers/api-v3/members.js b/website/src/controllers/api-v3/members.js index 5e2cb33d35..62df59b651 100644 --- a/website/src/controllers/api-v3/members.js +++ b/website/src/controllers/api-v3/members.js @@ -10,6 +10,7 @@ import { model as Challenge } from '../../models/challenge'; import { NotFound, } from '../../libs/api-v3/errors'; +import * as Tasks from '../../models/task'; let api = {}; @@ -174,4 +175,55 @@ api.getMembersForChallenge = { handler: _getMembersForItem('challenge-members'), }; +/** + * @api {get} /challenges/:challengeId/members/:memberId Get a challenge member progress + * @apiVersion 3.0.0 + * @apiName GetChallenge + * @apiGroup Challenge + * + * @apiParam {UUID} challengeId The challenge _id + * @apiParam {UUID} member The member _id + * + * @apiSuccess {object} member Return an object with member _id, profile.name and a tasks object with the challenge tasks for the member + */ +api.getChallengeMemberProgress = { + method: 'GET', + url: '/challenges/:challengeId/members/:memberId', + middlewares: [authWithHeaders(), cron], + async handler (req, res) { + req.checkParams('challengeId', res.t('challengeIdRequired')).notEmpty().isUUID(); + req.checkParams('memberId', res.t('memberIdRequired')).notEmpty().isUUID(); + + let validationErrors = req.validationErrors(); + if (validationErrors) throw validationErrors; + + let user = res.locals.user; + let challengeId = req.params.challengeId; + let memberId = req.params.memberId; + + let member = await User.findById(memberId).select(`${nameFields} challenges`).exec(); + if (!member) throw new NotFound(res.t('userWithIDNotFound', {userId: memberId})); + + let challenge = await Challenge.findById(challengeId).exec(); + if (!challenge) throw new NotFound(res.t('challengeNotFound')); + + let group = await Group.getGroup({user, groupId: challenge.groupId, fields: '_id type privacy'}); + if (!group || !challenge.canView(user, group)) throw new NotFound(res.t('challengeNotFound')); + if (!challenge.isMember(member)) throw new NotFound(res.t('challengeMemberNotFound')); + + let chalTasks = await Tasks.Task.find({ + userId: memberId, + 'challenge.id': challengeId, + }) + .select('-tags') // We don't want to return the tags publicly TODO same for other data? + .exec(); + + // manually call toJSON with minimize: true so empty paths aren't returned + let response = member.toJSON({minimize: true}); + delete response.challenges; + response.tasks = chalTasks.map(chalTask => chalTask.toJSON({minimize: true})); + res.respond(200, response); + }, +}; + export default api; diff --git a/website/src/controllers/api-v3/tasks.js b/website/src/controllers/api-v3/tasks.js index c2498b2a94..e6a420b91a 100644 --- a/website/src/controllers/api-v3/tasks.js +++ b/website/src/controllers/api-v3/tasks.js @@ -84,10 +84,10 @@ api.createUserTasks = { */ api.createChallengeTasks = { method: 'POST', - url: '/tasks/challenge/:challengeId', + url: '/tasks/challenge/:challengeId', // TODO should be /tasks/challengeS/:challengeId ? plural? middlewares: [authWithHeaders(), cron], async handler (req, res) { - req.checkQuery('challengeId', res.t('challengeIdRequired')).notEmpty().isUUID(); + req.checkParams('challengeId', res.t('challengeIdRequired')).notEmpty().isUUID(); let reqValidationErrors = req.validationErrors(); if (reqValidationErrors) throw reqValidationErrors; @@ -188,7 +188,7 @@ api.getChallengeTasks = { url: '/tasks/challenge/:challengeId', middlewares: [authWithHeaders(), cron], async handler (req, res) { - req.checkQuery('challengeId', res.t('challengeIdRequired')).notEmpty().isUUID(); + req.checkParams('challengeId', res.t('challengeIdRequired')).notEmpty().isUUID(); req.checkQuery('type', res.t('invalidTaskType')).optional().isIn(Tasks.tasksTypes); let validationErrors = req.validationErrors(); diff --git a/website/src/models/challenge.js b/website/src/models/challenge.js index 112926ef0c..8234d0b7f3 100644 --- a/website/src/models/challenge.js +++ b/website/src/models/challenge.js @@ -60,7 +60,7 @@ schema.methods.canModify = function canModifyChallenge (user) { function _syncableAttrs (task) { let t = task.toObject(); // lodash doesn't seem to like _.omit on Document // only sync/compare important attrs - let omitAttrs = ['userId', 'challenge', 'history', 'tags', 'completed', 'streak', 'notes']; // TODO what to do with updatedAt? + let omitAttrs = ['_id', 'userId', 'challenge', 'history', 'tags', 'completed', 'streak', 'notes']; // TODO what to do with updatedAt? if (t.type !== 'reward') omitAttrs.push('value'); return _.omit(t, omitAttrs); } @@ -168,7 +168,7 @@ schema.methods.addTasks = async function challengeAddTasks (tasks) { tasksOrderList.$each.unshift(userTask._id); } - toSave.push(userTask); + toSave.push(userTask.save()); }); // Update the user