From 359ef47ce7ba72db36965c6a5d2d0f8358577968 Mon Sep 17 00:00:00 2001 From: Matteo Pagliazzi Date: Fri, 26 Jun 2020 16:50:49 +0200 Subject: [PATCH] fix(get tasks): improve tests and add ability to fetch group tasks (#12339) --- .../v3/integration/tasks/GET-tasks_id.test.js | 177 ++++++++++++++---- website/server/controllers/api-v3/tasks.js | 25 +-- .../server/controllers/api-v3/tasks/groups.js | 1 + 3 files changed, 156 insertions(+), 47 deletions(-) diff --git a/test/api/v3/integration/tasks/GET-tasks_id.test.js b/test/api/v3/integration/tasks/GET-tasks_id.test.js index d56b3fd151..a1b43a8378 100644 --- a/test/api/v3/integration/tasks/GET-tasks_id.test.js +++ b/test/api/v3/integration/tasks/GET-tasks_id.test.js @@ -2,6 +2,8 @@ import { v4 as generateUUID } from 'uuid'; import { generateUser, translate as t, + createAndPopulateGroup, + generateChallenge, } from '../../../../helpers/api-integration/v3'; describe('GET /tasks/:id', () => { @@ -11,55 +13,158 @@ describe('GET /tasks/:id', () => { user = await generateUser(); }); - context('task can be accessed', async () => { - let task; + context('general', () => { + context('task cannot be accessed', () => { + it('cannot get a non-existent task', async () => { + const dummyId = generateUUID(); - beforeEach(async () => { - task = await user.post('/tasks/user', { - text: 'test habit', - type: 'habit', - alias: 'alias', + await expect(user.get(`/tasks/${dummyId}`)).to.eventually.be.rejected.and.eql({ + code: 404, + error: 'NotFound', + message: t('taskNotFound'), + }); }); }); - - it('gets specified task', async () => { - const getTask = await user.get(`/tasks/${task._id}`); - - expect(getTask).to.eql(task); - }); - - it('can use alias to retrieve task', async () => { - const getTask = await user.get(`/tasks/${task.alias}`); - - expect(getTask).to.eql(task); - }); - - // TODO after challenges are implemented - it('can get active challenge task that user does not own'); // Yes? }); - context('task cannot be accessed', () => { - it('cannot get a non-existent task', async () => { - const dummyId = generateUUID(); + context('user', () => { + context('task can be accessed', async () => { + let task; - await expect(user.get(`/tasks/${dummyId}`)).to.eventually.be.rejected.and.eql({ - code: 404, - error: 'NotFound', - message: t('taskNotFound'), + beforeEach(async () => { + task = await user.post('/tasks/user', { + text: 'test habit', + type: 'habit', + alias: 'alias', + }); + }); + + it('gets specified task', async () => { + const getTask = await user.get(`/tasks/${task._id}`); + + expect(getTask).to.eql(task); + }); + + it('can use alias to retrieve task', async () => { + const getTask = await user.get(`/tasks/${task.alias}`); + + expect(getTask).to.eql(task); }); }); - it('cannot get a task owned by someone else', async () => { - const anotherUser = await generateUser(); - const task = await user.post('/tasks/user', { + context('task cannot be accessed', () => { + it('cannot get a task owned by someone else', async () => { + const anotherUser = await generateUser(); + const task = await user.post('/tasks/user', { + text: 'test habit', + type: 'habit', + }); + + await expect(anotherUser.get(`/tasks/${task._id}`)).to.eventually.be.rejected.and.eql({ + code: 404, + error: 'NotFound', + message: t('taskNotFound'), + }); + }); + }); + }); + + context('challenge', () => { + let challenge; let task; let leader; let member; + + before(async () => { + const populatedGroup = await createAndPopulateGroup({ + members: 1, + }); + + const guild = populatedGroup.group; + leader = populatedGroup.groupLeader; + member = populatedGroup.members[0]; // eslint-disable-line prefer-destructuring + + challenge = await generateChallenge(leader, guild); + await leader.post(`/challenges/${challenge._id}/join`); + + task = await leader.post(`/tasks/challenge/${challenge._id}`, { text: 'test habit', type: 'habit', }); + }); - await expect(anotherUser.get(`/tasks/${task._id}`)).to.eventually.be.rejected.and.eql({ - code: 404, - error: 'NotFound', - message: t('taskNotFound'), + context('task can be accessed', async () => { + it('can get challenge task if member of that challenge', async () => { + await member.post(`/challenges/${challenge._id}/join`); + + const getTask = await member.get(`/tasks/${task._id}`); + expect(getTask).to.eql(task); + }); + + it('can get challenge task if leader of that challenge', async () => { + const getTask = await leader.get(`/tasks/${task._id}`); + expect(getTask).to.eql(task); + }); + + it('can get challenge task if admin', async () => { + const admin = await generateUser({ + 'contributor.admin': true, + }); + + const getTask = await admin.get(`/tasks/${task._id}`); + expect(getTask).to.eql(task); + }); + }); + + context('task cannot be accessed', () => { + it('cannot get a task in a challenge i am not part of', async () => { + await expect(user.get(`/tasks/${task._id}`)).to.eventually.be.rejected.and.eql({ + code: 404, + error: 'NotFound', + message: t('taskNotFound'), + }); + }); + }); + }); + + context('group', () => { + let group; let task; let members; let leader; + + before(async () => { + const groupData = await createAndPopulateGroup({ + groupDetails: { + name: 'Test Guild', + type: 'guild', + }, + members: 1, + }); + + group = groupData.group; + members = groupData.members; + leader = groupData.groupLeader; + + task = await leader.post(`/tasks/group/${group._id}`, { + text: 'test habit', + type: 'habit', + }); + }); + + context('task can be accessed', async () => { + it('can get group task if leader of that group', async () => { + const getTask = await leader.get(`/tasks/${task._id}`); + expect(getTask).to.eql(task); + }); + + it('can get group task if member of that group', async () => { + const getTask = await members[0].get(`/tasks/${task._id}`); + expect(getTask).to.eql(task); + }); + }); + + context('task cannot be accessed', () => { + it('cannot get a task in a group i am not part of', async () => { + await expect(user.get(`/tasks/${task._id}`)).to.eventually.be.rejected.and.eql({ + code: 404, + error: 'NotFound', + message: t('taskNotFound'), + }); }); }); }); diff --git a/website/server/controllers/api-v3/tasks.js b/website/server/controllers/api-v3/tasks.js index eb2e0f855a..96f71b6f2d 100644 --- a/website/server/controllers/api-v3/tasks.js +++ b/website/server/controllers/api-v3/tasks.js @@ -26,6 +26,7 @@ import common from '../../../common'; import logger from '../../libs/logger'; import apiError from '../../libs/apiError'; +// @TODO abstract, see api-v3/tasks/groups.js function canNotEditTasks (group, user, assignedUserId, taskPayload = null) { const isNotGroupLeader = group.leader !== user._id; const isManager = Boolean(group.managers[user._id]); @@ -530,18 +531,20 @@ api.getTask = { if (!task) { throw new NotFound(res.t('taskNotFound')); + } else if (task.group.id && !task.userId) { + // @TODO: Abstract this access snippet + const fields = requiredGroupFields.concat(' managers'); + const group = await Group.getGroup({ user, groupId: task.group.id, fields }); + if (!group) throw new NotFound(res.t('taskNotFound')); - // If the task belongs to a challenge make sure the user has rights + const isNotGroupLeader = group.leader !== user._id; + if (!group.isMember(user) && isNotGroupLeader) throw new NotFound(res.t('taskNotFound')); + // If the task belongs to a challenge make sure the user has rights (leader, admin or members) } else if (task.challenge.id && !task.userId) { - const challenge = await Challenge.find({ _id: task.challenge.id }).select('leader').exec(); - if ( - !challenge - || ( - user.challenges.indexOf(task.challenge.id) === -1 - && challenge.leader !== user._id - && !user.contributor.admin - ) - ) { // eslint-disable-line no-extra-parens + const challenge = await Challenge.findOne({ _id: task.challenge.id }).select('leader').exec(); + // @TODO: Abstract this access snippet + if (!challenge) throw new NotFound(res.t('taskNotFound')); + if (!challenge.canModify(user) && !challenge.isMember(user)) { throw new NotFound(res.t('taskNotFound')); } @@ -628,7 +631,7 @@ api.updateTask = { if (!task) { throw new NotFound(res.t('taskNotFound')); } else if (task.group.id && !task.userId) { - // @TODO: Abstract this access snippet + // @TODO: Abstract this access snippet const fields = requiredGroupFields.concat(' managers'); group = await Group.getGroup({ user, groupId: task.group.id, fields }); if (!group) throw new NotFound(res.t('groupNotFound')); diff --git a/website/server/controllers/api-v3/tasks/groups.js b/website/server/controllers/api-v3/tasks/groups.js index dfbd9c0438..0adfac076c 100644 --- a/website/server/controllers/api-v3/tasks/groups.js +++ b/website/server/controllers/api-v3/tasks/groups.js @@ -21,6 +21,7 @@ const types = Tasks.tasksTypes.map(type => `${type}s`); // _allCompletedTodos is currently in BETA and is likely to be removed in future types.push('completedTodos', '_allCompletedTodos'); +// @TODO abstract this snipped (also see api-v3/tasks.js) function canNotEditTasks (group, user, assignedUserId) { const isNotGroupLeader = group.leader !== user._id; const isManager = Boolean(group.managers[user._id]);