From 299e88233cc137c5fd5d75c470991c96eeae1342 Mon Sep 17 00:00:00 2001 From: Asif Mallik Date: Tue, 28 Nov 2017 08:13:18 +0600 Subject: [PATCH] Fixes multiple complete and uncomplete for todos and daily (Fixes #8669) (#8971) * Fixed bug that allows users to complete todo and daily multiple times * Added tests * Fix syntax * Fix existing tests that rely on multiple complete or uncomplete * Undoes removal of website/client/README.md * Change sessionOutdated string to reflect separate client needs * Fix should update history test by changing lastCron --- .../POST-tasks_id_score_direction.test.js | 57 +++++++++++++++---- ...allengeId_tasks_id_score_direction.test.js | 7 +++ website/common/locales/en/tasks.json | 4 +- website/server/controllers/api-v3/tasks.js | 8 +++ 4 files changed, 63 insertions(+), 13 deletions(-) diff --git a/test/api/v3/integration/tasks/POST-tasks_id_score_direction.test.js b/test/api/v3/integration/tasks/POST-tasks_id_score_direction.test.js index 1baa24042e..fa435c5e31 100644 --- a/test/api/v3/integration/tasks/POST-tasks_id_score_direction.test.js +++ b/test/api/v3/integration/tasks/POST-tasks_id_score_direction.test.js @@ -130,6 +130,7 @@ describe('POST /tasks/:id/score/:direction', () => { }); it('uncompletes todo when direction is down', async () => { + await user.post(`/tasks/${todo._id}/score/up`); await user.post(`/tasks/${todo._id}/score/down`); let updatedTask = await user.get(`/tasks/${todo._id}`); @@ -137,9 +138,23 @@ describe('POST /tasks/:id/score/:direction', () => { expect(updatedTask.dateCompleted).to.be.a('undefined'); }); - it('scores up todo even if it is already completed'); // Yes? + it('doesn\'t let a todo be completed twice', async () => { + await user.post(`/tasks/${todo._id}/score/up`); + await expect(user.post(`/tasks/${todo._id}/score/up`)) + .to.eventually.be.rejected.and.eql({ + code: 401, + error: 'NotAuthorized', + message: t('sessionOutdated'), + }); + }); - it('scores down todo even if it is already uncompleted'); // Yes? + it('doesn\'t let a todo be uncompleted twice', async () => { + await expect(user.post(`/tasks/${todo._id}/score/down`)).to.eventually.be.rejected.and.eql({ + code: 401, + error: 'NotAuthorized', + message: t('sessionOutdated'), + }); + }); context('user stats when direction is up', () => { let updatedUser; @@ -163,23 +178,25 @@ describe('POST /tasks/:id/score/:direction', () => { }); context('user stats when direction is down', () => { - let updatedUser; + let updatedUser, initialUser; beforeEach(async () => { + await user.post(`/tasks/${todo._id}/score/up`); + initialUser = await user.get('/user'); await user.post(`/tasks/${todo._id}/score/down`); updatedUser = await user.get('/user'); }); it('decreases user\'s mp', () => { - expect(updatedUser.stats.mp).to.be.lessThan(user.stats.mp); + expect(updatedUser.stats.mp).to.be.lessThan(initialUser.stats.mp); }); it('decreases user\'s exp', () => { - expect(updatedUser.stats.exp).to.be.lessThan(user.stats.exp); + expect(updatedUser.stats.exp).to.be.lessThan(initialUser.stats.exp); }); it('decreases user\'s gold', () => { - expect(updatedUser.stats.gp).to.be.lessThan(user.stats.gp); + expect(updatedUser.stats.gp).to.be.lessThan(initialUser.stats.gp); }); }); }); @@ -202,6 +219,7 @@ describe('POST /tasks/:id/score/:direction', () => { }); it('uncompletes daily when direction is down', async () => { + await user.post(`/tasks/${daily._id}/score/up`); await user.post(`/tasks/${daily._id}/score/down`); let task = await user.get(`/tasks/${daily._id}`); @@ -222,9 +240,22 @@ describe('POST /tasks/:id/score/:direction', () => { expect(task.nextDue.length).to.eql(6); }); - it('scores up daily even if it is already completed'); // Yes? + it('doesn\'t let a daily be completed twice', async () => { + await user.post(`/tasks/${daily._id}/score/up`); + await expect(user.post(`/tasks/${daily._id}/score/up`)).to.eventually.be.rejected.and.eql({ + code: 401, + error: 'NotAuthorized', + message: t('sessionOutdated'), + }); + }); - it('scores down daily even if it is already uncompleted'); // Yes? + it('doesn\'t let a daily be uncompleted twice', async () => { + await expect(user.post(`/tasks/${daily._id}/score/down`)).to.eventually.be.rejected.and.eql({ + code: 401, + error: 'NotAuthorized', + message: t('sessionOutdated'), + }); + }); context('user stats when direction is up', () => { let updatedUser; @@ -248,23 +279,25 @@ describe('POST /tasks/:id/score/:direction', () => { }); context('user stats when direction is down', () => { - let updatedUser; + let updatedUser, initialUser; beforeEach(async () => { + await user.post(`/tasks/${daily._id}/score/up`); + initialUser = await user.get('/user'); await user.post(`/tasks/${daily._id}/score/down`); updatedUser = await user.get('/user'); }); it('decreases user\'s mp', () => { - expect(updatedUser.stats.mp).to.be.lessThan(user.stats.mp); + expect(updatedUser.stats.mp).to.be.lessThan(initialUser.stats.mp); }); it('decreases user\'s exp', () => { - expect(updatedUser.stats.exp).to.be.lessThan(user.stats.exp); + expect(updatedUser.stats.exp).to.be.lessThan(initialUser.stats.exp); }); it('decreases user\'s gold', () => { - expect(updatedUser.stats.gp).to.be.lessThan(user.stats.gp); + expect(updatedUser.stats.gp).to.be.lessThan(initialUser.stats.gp); }); }); }); diff --git a/test/api/v3/integration/tasks/challenges/POST-tasks_challenges_challengeId_tasks_id_score_direction.test.js b/test/api/v3/integration/tasks/challenges/POST-tasks_challenges_challengeId_tasks_id_score_direction.test.js index 0fa8bafd30..e02e6dd2fe 100644 --- a/test/api/v3/integration/tasks/challenges/POST-tasks_challenges_challengeId_tasks_id_score_direction.test.js +++ b/test/api/v3/integration/tasks/challenges/POST-tasks_challenges_challengeId_tasks_id_score_direction.test.js @@ -82,6 +82,13 @@ describe('POST /tasks/:id/score/:direction', () => { }); it('should update the history', async () => { + let newCron = new Date(2015, 11, 20); + + await user.post('/debug/set-cron', { + lastCron: newCron, + }); + + await user.post('/cron'); await user.post(`/tasks/${usersChallengeTaskId}/score/up`); let tasks = await user.get(`/tasks/challenge/${challenge._id}`); diff --git a/website/common/locales/en/tasks.json b/website/common/locales/en/tasks.json index c7eea26af1..ef1c52c135 100644 --- a/website/common/locales/en/tasks.json +++ b/website/common/locales/en/tasks.json @@ -202,5 +202,7 @@ "yesterDailiesOptionTitle": "Confirm that this Daily wasn't done before applying damage", "yesterDailiesDescription": "If this setting is applied, Habitica will ask you if you meant to leave the Daily undone before calculating and applying damage to your avatar. This can protect you against unintentional damage.", "repeatDayError": "Please ensure that you have at least one day of the week selected.", - "searchTasks": "Search titles and descriptions..." + "searchTasks": "Search titles and descriptions...", + "repeatDayError": "Please ensure that you have at least one day of the week selected.", + "sessionOutdated": "Your session is outdated. Please refresh or sync." } diff --git a/website/server/controllers/api-v3/tasks.js b/website/server/controllers/api-v3/tasks.js index 60d3b67774..f6b1215dad 100644 --- a/website/server/controllers/api-v3/tasks.js +++ b/website/server/controllers/api-v3/tasks.js @@ -536,6 +536,14 @@ api.scoreTask = { if (!task) throw new NotFound(res.t('taskNotFound')); + if (task.type === 'daily' || task.type === 'todo') { + if (task.completed && direction === 'up') { + throw new NotAuthorized(res.t('sessionOutdated')); + } else if (!task.completed && direction === 'down') { + throw new NotAuthorized(res.t('sessionOutdated')); + } + } + if (task.group.approval.required && !task.group.approval.approved) { if (task.group.approval.requested) { throw new NotAuthorized(res.t('taskRequiresApproval'));