diff --git a/.eslintrc b/.eslintrc index bcccde1ef6..e7df64e79e 100644 --- a/.eslintrc +++ b/.eslintrc @@ -4,6 +4,7 @@ "habitrpg/babel" ], "globals": { - "Promise": true + "Promise": true, + "Set": false } } diff --git a/common/locales/en/tasks.json b/common/locales/en/tasks.json index 7e5a87a343..ca6abea819 100644 --- a/common/locales/en/tasks.json +++ b/common/locales/en/tasks.json @@ -20,6 +20,10 @@ "extraNotes": "Extra Notes", "direction/Actions": "Direction/Actions", "advancedOptions": "Advanced Options", + "taskAlias": "Task Alias", + "taskAliasPopover": "This task alias can be used when integrating with 3rd party integrations. Only dashes, underscores, and alphanumeric characters are supported. The task alias must be unique among all your tasks.", + "taskAliasPlaceholder": "your-task-alias-here", + "taskAliasPopoverWarning": "WARNING: Changing this value will break any 3rd party integrations that rely on the task alias.", "difficulty": "Difficulty", "difficultyHelpTitle": "How difficult is this task?", "difficultyHelpContent": "The harder a task, the more Experience and Gold it awards you when you check it off... but the more it damages you if it is a Daily or Bad Habit!", @@ -113,6 +117,7 @@ "rewardHelp4": "Don't be afraid to set custom Rewards! Check out some samples here.", "clickForHelp": "Click for help", "taskIdRequired": "\"taskId\" must be a valid UUID.", + "taskAliasAlreadyUsed": "Task alias already used on another task.", "taskNotFound": "Task not found.", "invalidTaskType": "Task type must be one of \"habit\", \"daily\", \"todo\", \"reward\".", "cantDeleteChallengeTasks": "A task belonging to a challenge can't be deleted.", diff --git a/test/api/v3/integration/tasks/DELETE-tasks_id.test.js b/test/api/v3/integration/tasks/DELETE-tasks_id.test.js index bb92e4759f..450572df7b 100644 --- a/test/api/v3/integration/tasks/DELETE-tasks_id.test.js +++ b/test/api/v3/integration/tasks/DELETE-tasks_id.test.js @@ -6,7 +6,7 @@ import { describe('DELETE /tasks/:id', () => { let user; - before(async () => { + beforeEach(async () => { user = await generateUser(); }); @@ -17,6 +17,7 @@ describe('DELETE /tasks/:id', () => { task = await user.post('/tasks/user', { text: 'test habit', type: 'habit', + alias: 'task-to-be-deleted', }); }); @@ -29,6 +30,16 @@ describe('DELETE /tasks/:id', () => { message: t('taskNotFound'), }); }); + + it('can use a alias to delete a task', async () => { + await user.del(`/tasks/${task.alias}`); + + await expect(user.get(`/tasks/${task._id}`)).to.eventually.be.rejected.and.eql({ + code: 404, + error: 'NotFound', + message: t('taskNotFound'), + }); + }); }); context('task cannot be deleted', () => { 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 99f2d769f4..98c107d932 100644 --- a/test/api/v3/integration/tasks/GET-tasks_id.test.js +++ b/test/api/v3/integration/tasks/GET-tasks_id.test.js @@ -7,7 +7,7 @@ import { v4 as generateUUID } from 'uuid'; describe('GET /tasks/:id', () => { let user; - before(async () => { + beforeEach(async () => { user = await generateUser(); }); @@ -18,11 +18,19 @@ describe('GET /tasks/:id', () => { task = await user.post('/tasks/user', { text: 'test habit', type: 'habit', + alias: 'alias', }); }); it('gets specified task', async () => { let getTask = await user.get(`/tasks/${task._id}`); + + expect(getTask).to.eql(task); + }); + + it('can use alias to retrieve task', async () => { + let getTask = await user.get(`/tasks/${task.alias}`); + expect(getTask).to.eql(task); }); 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 86236fd110..e7c71c28d5 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 @@ -14,12 +14,28 @@ describe('POST /tasks/:id/score/:direction', () => { }); context('all', () => { - it('requires a task id', async () => { - await expect(user.post('/tasks/123/score/up')).to.eventually.be.rejected.and.eql({ - code: 400, - error: 'BadRequest', - message: t('invalidReqParams'), + it('can use an id to identify the task', async () => { + let todo = await user.post('/tasks/user', { + text: 'test todo', + type: 'todo', + alias: 'alias', }); + + let res = await user.post(`/tasks/${todo._id}/score/up`); + + expect(res).to.be.ok; + }); + + it('can use a alias in place of the id', async () => { + let todo = await user.post('/tasks/user', { + text: 'test todo', + type: 'todo', + alias: 'alias', + }); + + let res = await user.post(`/tasks/${todo.alias}/score/up`); + + expect(res).to.be.ok; }); it('requires a task direction', async () => { diff --git a/test/api/v3/integration/tasks/POST-tasks_move_taskId_to_position.test.js b/test/api/v3/integration/tasks/POST-tasks_move_taskId_to_position.test.js index 211c7ea975..b11143abe5 100644 --- a/test/api/v3/integration/tasks/POST-tasks_move_taskId_to_position.test.js +++ b/test/api/v3/integration/tasks/POST-tasks_move_taskId_to_position.test.js @@ -11,14 +11,6 @@ describe('POST /tasks/:taskId/move/to/:position', () => { user = await generateUser(); }); - it('requires a valid taskId', async () => { - await expect(user.post('/tasks/123/move/to/1')).to.eventually.be.rejected.and.eql({ - code: 400, - error: 'BadRequest', - message: t('invalidReqParams'), - }); - }); - it('requires a numeric position parameter', async () => { await expect(user.post(`/tasks/${generateUUID()}/move/to/notANumber`)).to.eventually.be.rejected.and.eql({ code: 400, @@ -53,6 +45,24 @@ describe('POST /tasks/:taskId/move/to/:position', () => { expect(newOrder.length).to.equal(5); }); + it('can move task to new position using alias', async () => { + let tasks = await user.post('/tasks/user', [ + {type: 'habit', text: 'habit 1'}, + {type: 'habit', text: 'habit 2', alias: 'move'}, + {type: 'daily', text: 'daily 1'}, + {type: 'habit', text: 'habit 3'}, + {type: 'habit', text: 'habit 4'}, + {type: 'todo', text: 'todo 1'}, + {type: 'habit', text: 'habit 5'}, + ]); + + let taskToMove = tasks[1]; + expect(taskToMove.text).to.equal('habit 2'); + let newOrder = await user.post(`/tasks/${taskToMove.alias}/move/to/3`); + expect(newOrder[3]).to.equal(taskToMove._id); + expect(newOrder.length).to.equal(5); + }); + it('can\'t move completed todo', async () => { let task = await user.post('/tasks/user', {type: 'todo', text: 'todo 1'}); await user.post(`/tasks/${task._id}/score/up`); diff --git a/test/api/v3/integration/tasks/POST-tasks_user.test.js b/test/api/v3/integration/tasks/POST-tasks_user.test.js index 623d9bb4a4..994bf59b04 100644 --- a/test/api/v3/integration/tasks/POST-tasks_user.test.js +++ b/test/api/v3/integration/tasks/POST-tasks_user.test.js @@ -57,7 +57,7 @@ describe('POST /tasks/user', () => { let originalHabitsOrder = (await user.get('/user')).tasksOrder.habits; await expect(user.post('/tasks/user', { type: 'habit', - })).to.eventually.be.rejected.and.eql({ // this block is necessary + })).to.eventually.be.rejected.and.eql({ code: 400, error: 'BadRequest', message: 'habit validation failed', @@ -72,7 +72,7 @@ describe('POST /tasks/user', () => { await expect(user.post('/tasks/user', [ {type: 'habit'}, // Missing text {type: 'habit', text: 'valid'}, // Valid - ])).to.eventually.be.rejected.and.eql({ // this block is necessary + ])).to.eventually.be.rejected.and.eql({ code: 400, error: 'BadRequest', message: 'habit validation failed', @@ -87,7 +87,7 @@ describe('POST /tasks/user', () => { await expect(user.post('/tasks/user', [ {type: 'habit'}, // Missing text {type: 'habit', text: 'valid'}, // Valid - ])).to.eventually.be.rejected.and.eql({ // this block is necessary + ])).to.eventually.be.rejected.and.eql({ code: 400, error: 'BadRequest', message: 'habit validation failed', @@ -142,6 +142,67 @@ describe('POST /tasks/user', () => { expect(task).not.to.have.property('notValid'); }); + + it('errors if alias already exists on another task', async () => { + await user.post('/tasks/user', { // first task that will succeed + type: 'habit', + text: 'todo text', + alias: 'alias', + }); + + await expect(user.post('/tasks/user', { + type: 'todo', + text: 'todo text', + alias: 'alias', + })).to.eventually.be.rejected.and.eql({ + code: 400, + error: 'BadRequest', + message: 'todo validation failed', + }); + }); + + it('errors if alias contains invalid values', async () => { + await expect(user.post('/tasks/user', { + type: 'todo', + text: 'todo text', + alias: 'short name!', + })).to.eventually.be.rejected.and.eql({ + code: 400, + error: 'BadRequest', + message: 'todo validation failed', + }); + }); + + it('errors if alias is a valid uuid', async () => { + await expect(user.post('/tasks/user', { + type: 'todo', + text: 'todo text', + alias: generateUUID(), + })).to.eventually.be.rejected.and.eql({ + code: 400, + error: 'BadRequest', + message: 'todo validation failed', + }); + }); + + it('errors if the same shortname is used on 2 or more tasks', async () => { + await expect(user.post('/tasks/user', [{ + type: 'habit', + text: 'habit text', + alias: 'alias', + }, { + type: 'todo', + text: 'todo text', + }, { + type: 'todo', + text: 'todo text', + alias: 'alias', + }])).to.eventually.be.rejected.and.eql({ + code: 400, + error: 'BadRequest', + message: t('taskAliasAlreadyUsed'), + }); + }); }); context('all types', () => { @@ -163,6 +224,16 @@ describe('POST /tasks/user', () => { expect(task.reminders[0].startDate).to.be.a('string'); // json doesn't have dates expect(task.reminders[0].time).to.be.a('string'); }); + + it('can create a task with a alias', async () => { + let task = await user.post('/tasks/user', { + text: 'test habit', + type: 'habit', + alias: 'a_alias012', + }); + + expect(task.alias).to.eql('a_alias012'); + }); }); context('habits', () => { diff --git a/test/api/v3/integration/tasks/PUT-tasks_id.test.js b/test/api/v3/integration/tasks/PUT-tasks_id.test.js index 566c1ebe5e..1660bb6f7f 100644 --- a/test/api/v3/integration/tasks/PUT-tasks_id.test.js +++ b/test/api/v3/integration/tasks/PUT-tasks_id.test.js @@ -9,7 +9,7 @@ import { v4 as generateUUID } from 'uuid'; describe('PUT /tasks/:id', () => { let user; - before(async () => { + beforeEach(async () => { user = await generateUser(); }); @@ -59,7 +59,7 @@ describe('PUT /tasks/:id', () => { expect(savedTask.notValid).to.be.undefined; }); - it(`only allows setting streak, reminders, checklist, notes, attribute, tags + it(`only allows setting streak, alias, reminders, checklist, notes, attribute, tags fields for challenge tasks owned by a user`, async () => { let guild = await generateGroup(user); let challenge = await generateChallenge(user, guild); @@ -87,6 +87,7 @@ describe('PUT /tasks/:id', () => { _id: 123, type: 'daily', userId: 123, + alias: 'a-short-task-name', history: [123], createdAt: 'yesterday', updatedAt: 'tomorrow', @@ -176,6 +177,44 @@ describe('PUT /tasks/:id', () => { expect(savedDaily.reminders[0].id).to.equal(id1); expect(savedDaily.reminders[1].id).to.equal(id2); }); + + it('can set a alias if no other task has that alias', async () => { + let savedDaily = await user.put(`/tasks/${daily._id}`, { + alias: 'alias', + }); + + expect(savedDaily.alias).to.eql('alias'); + }); + + it('does not set alias to a alias that is already in use', async () => { + await user.post('/tasks/user', { + type: 'todo', + text: 'a todo', + alias: 'some-alias', + }); + + await expect(user.put(`/tasks/${daily._id}`, { + alias: 'some-alias', + })).to.eventually.be.rejected.and.eql({ + code: 400, + error: 'BadRequest', + message: 'daily validation failed', + }); + }); + + it('can use alias to update a task', async () => { + daily = await user.put(`/tasks/${daily._id}`, { + alias: 'alias', + }); + + await user.put(`/tasks/${daily.alias}`, { + text: 'saved', + }); + + let fetchedDaily = await user.get(`/tasks/${daily._id}`); + + expect(fetchedDaily.text).to.eql('saved'); + }); }); context('habits', () => { diff --git a/test/api/v3/integration/tasks/challenges/POST-tasks_challenge_id.test.js b/test/api/v3/integration/tasks/challenges/POST-tasks_challenge_id.test.js index 62899ffb59..4f6fb63225 100644 --- a/test/api/v3/integration/tasks/challenges/POST-tasks_challenge_id.test.js +++ b/test/api/v3/integration/tasks/challenges/POST-tasks_challenge_id.test.js @@ -49,6 +49,18 @@ describe('POST /tasks/challenge/:challengeId', () => { }); }); + it('returns error when user tries to create task with a alias', async () => { + await expect(user.post(`/tasks/challenge/${challenge._id}`, { + text: 'test habit', + type: 'habit', + alias: 'a-alias', + })).to.eventually.be.rejected.and.eql({ + code: 400, + error: 'BadRequest', + message: 'habit validation failed', + }); + }); + it('returns error when non leader tries to edit challenge', async () => { let userThatIsNotLeaderOfChallenge = await generateUser({ challenges: [challenge._id], diff --git a/test/api/v3/integration/tasks/PUT-tasks_challenge_challengeId.test.js b/test/api/v3/integration/tasks/challenges/PUT-tasks_challenge_challengeId.test.js similarity index 95% rename from test/api/v3/integration/tasks/PUT-tasks_challenge_challengeId.test.js rename to test/api/v3/integration/tasks/challenges/PUT-tasks_challenge_challengeId.test.js index 88343cb47c..ed78d86658 100644 --- a/test/api/v3/integration/tasks/PUT-tasks_challenge_challengeId.test.js +++ b/test/api/v3/integration/tasks/challenges/PUT-tasks_challenge_challengeId.test.js @@ -3,7 +3,7 @@ import { generateGroup, generateChallenge, translate as t, -} from '../../../../helpers/api-integration/v3'; +} from '../../../../../helpers/api-integration/v3'; import { v4 as generateUUID } from 'uuid'; describe('PUT /tasks/:id', () => { @@ -54,6 +54,16 @@ describe('PUT /tasks/:id', () => { message: t('onlyChalLeaderEditTasks'), }); }); + + it('returns error when user attempts to update task with a alias', async () => { + await expect(user.put(`/tasks/${task._id}`, { + alias: 'a-alias', + })).to.eventually.be.rejected.and.eql({ + code: 400, + error: 'BadRequest', + message: 'habit validation failed', + }); + }); }); context('validates params', () => { diff --git a/test/api/v3/integration/tasks/checklists/DELETE-tasks_taskId_checklist_itemId.test.js b/test/api/v3/integration/tasks/checklists/DELETE-tasks_taskId_checklist_itemId.test.js index 2cf08bbece..940a3479b1 100644 --- a/test/api/v3/integration/tasks/checklists/DELETE-tasks_taskId_checklist_itemId.test.js +++ b/test/api/v3/integration/tasks/checklists/DELETE-tasks_taskId_checklist_itemId.test.js @@ -25,6 +25,21 @@ describe('DELETE /tasks/:taskId/checklist/:itemId', () => { expect(savedTask.checklist.length).to.equal(0); }); + it('deletes a checklist item using task alias', async () => { + let task = await user.post('/tasks/user', { + type: 'daily', + text: 'Daily with checklist', + alias: 'daily-with-alias', + }); + + let savedTask = await user.post(`/tasks/${task._id}/checklist`, {text: 'Checklist Item 1', completed: false}); + + await user.del(`/tasks/${task.alias}/checklist/${savedTask.checklist[0].id}`); + savedTask = await user.get(`/tasks/${task._id}`); + + expect(savedTask.checklist.length).to.equal(0); + }); + it('does not work with habits', async () => { let habit = await user.post('/tasks/user', { type: 'habit', diff --git a/test/api/v3/integration/tasks/checklists/POST-tasks_taskId_checklist.test.js b/test/api/v3/integration/tasks/checklists/POST-tasks_taskId_checklist.test.js index 7166db02da..488e0005d9 100644 --- a/test/api/v3/integration/tasks/checklists/POST-tasks_taskId_checklist.test.js +++ b/test/api/v3/integration/tasks/checklists/POST-tasks_taskId_checklist.test.js @@ -31,6 +31,27 @@ describe('POST /tasks/:taskId/checklist/', () => { expect(savedTask.checklist[0].ignored).to.be.an('undefined'); }); + it('can use a alias to add checklist', async () => { + let task = await user.post('/tasks/user', { + type: 'daily', + text: 'Daily with checklist', + alias: 'task-with-shortname', + }); + + let savedTask = await user.post(`/tasks/${task.alias}/checklist`, { + text: 'Checklist Item 1', + ignored: false, + _id: 123, + }); + + expect(savedTask.checklist.length).to.equal(1); + expect(savedTask.checklist[0].text).to.equal('Checklist Item 1'); + expect(savedTask.checklist[0].completed).to.equal(false); + expect(savedTask.checklist[0].id).to.be.a('string'); + expect(savedTask.checklist[0].id).to.not.equal('123'); + expect(savedTask.checklist[0].ignored).to.be.an('undefined'); + }); + it('does not add a checklist to habits', async () => { let habit = await user.post('/tasks/user', { type: 'habit', diff --git a/test/api/v3/integration/tasks/checklists/POST-tasks_taskId_checklist_itemId_score.test.js b/test/api/v3/integration/tasks/checklists/POST-tasks_taskId_checklist_itemId_score.test.js index edb65dfb65..56662bcda5 100644 --- a/test/api/v3/integration/tasks/checklists/POST-tasks_taskId_checklist_itemId_score.test.js +++ b/test/api/v3/integration/tasks/checklists/POST-tasks_taskId_checklist_itemId_score.test.js @@ -28,6 +28,24 @@ describe('POST /tasks/:taskId/checklist/:itemId/score', () => { expect(savedTask.checklist[0].completed).to.equal(true); }); + it('can use a alias to score a checklist item', async () => { + let task = await user.post('/tasks/user', { + type: 'daily', + text: 'Daily with checklist', + alias: 'daily-with-shortname', + }); + + let savedTask = await user.post(`/tasks/${task._id}/checklist`, { + text: 'Checklist Item 1', + completed: false, + }); + + savedTask = await user.post(`/tasks/${task.alias}/checklist/${savedTask.checklist[0].id}/score`); + + expect(savedTask.checklist.length).to.equal(1); + expect(savedTask.checklist[0].completed).to.equal(true); + }); + it('fails on habits', async () => { let habit = await user.post('/tasks/user', { type: 'habit', diff --git a/test/api/v3/integration/tasks/checklists/PUT-tasks_taskId_checklist_itemId.test.js b/test/api/v3/integration/tasks/checklists/PUT-tasks_taskId_checklist_itemId.test.js index 003bcb2650..2c4b259ee5 100644 --- a/test/api/v3/integration/tasks/checklists/PUT-tasks_taskId_checklist_itemId.test.js +++ b/test/api/v3/integration/tasks/checklists/PUT-tasks_taskId_checklist_itemId.test.js @@ -34,6 +34,30 @@ describe('PUT /tasks/:taskId/checklist/:itemId', () => { expect(savedTask.checklist[0].id).to.not.equal('123'); }); + it('updates a checklist item using task alias', async () => { + let task = await user.post('/tasks/user', { + type: 'daily', + text: 'Daily with checklist', + alias: 'daily-with-shortname', + }); + + let savedTask = await user.post(`/tasks/${task._id}/checklist`, { + text: 'Checklist Item 1', + completed: false, + }); + + savedTask = await user.put(`/tasks/${task.alias}/checklist/${savedTask.checklist[0].id}`, { + text: 'updated', + completed: true, + _id: 123, // ignored + }); + + expect(savedTask.checklist.length).to.equal(1); + expect(savedTask.checklist[0].text).to.equal('updated'); + expect(savedTask.checklist[0].completed).to.equal(true); + expect(savedTask.checklist[0].id).to.not.equal('123'); + }); + it('fails on habits', async () => { let habit = await user.post('/tasks/user', { type: 'habit', diff --git a/test/api/v3/integration/tasks/tags/DELETE-tasks_taskId_tags_tagId.test.js b/test/api/v3/integration/tasks/tags/DELETE-tasks_taskId_tags_tagId.test.js index ebb1a3c9e8..0a39cfbac7 100644 --- a/test/api/v3/integration/tasks/tags/DELETE-tasks_taskId_tags_tagId.test.js +++ b/test/api/v3/integration/tasks/tags/DELETE-tasks_taskId_tags_tagId.test.js @@ -27,6 +27,23 @@ describe('DELETE /tasks/:taskId/tags/:tagId', () => { expect(updatedTask.tags.length).to.equal(0); }); + it('removes a tag from a task using task short name', async () => { + let task = await user.post('/tasks/user', { + type: 'habit', + text: 'Task with tag', + alias: 'habit-with-alias', + }); + + let tag = await user.post('/tags', {name: 'Tag 1'}); + + await user.post(`/tasks/${task._id}/tags/${tag.id}`); + await user.del(`/tasks/${task.alias}/tags/${tag.id}`); + + let updatedTask = await user.get(`/tasks/${task._id}`); + + expect(updatedTask.tags.length).to.equal(0); + }); + it('only deletes existing tags', async () => { let createdTask = await user.post('/tasks/user', { type: 'habit', diff --git a/test/api/v3/integration/tasks/tags/POST-tasks_taskId_tags_tagId.test.js b/test/api/v3/integration/tasks/tags/POST-tasks_taskId_tags_tagId.test.js index d6cea02036..66fb002c69 100644 --- a/test/api/v3/integration/tasks/tags/POST-tasks_taskId_tags_tagId.test.js +++ b/test/api/v3/integration/tasks/tags/POST-tasks_taskId_tags_tagId.test.js @@ -23,6 +23,19 @@ describe('POST /tasks/:taskId/tags/:tagId', () => { expect(savedTask.tags[0]).to.equal(tag.id); }); + it('adds a tag to a task with alias', async () => { + let task = await user.post('/tasks/user', { + type: 'habit', + text: 'Task with tag', + alias: 'habit-with-alias', + }); + + let tag = await user.post('/tags', {name: 'Tag 1'}); + let savedTask = await user.post(`/tasks/${task.alias}/tags/${tag.id}`); + + expect(savedTask.tags[0]).to.equal(tag.id); + }); + it('does not add a tag to a task twice', async () => { let task = await user.post('/tasks/user', { type: 'habit', diff --git a/test/api/v3/unit/models/task.test.js b/test/api/v3/unit/models/task.test.js index 1c3082d863..7182b8e5a1 100644 --- a/test/api/v3/unit/models/task.test.js +++ b/test/api/v3/unit/models/task.test.js @@ -2,6 +2,7 @@ import { model as Challenge } from '../../../../../website/server/models/challen import { model as Group } from '../../../../../website/server/models/group'; import { model as User } from '../../../../../website/server/models/user'; import * as Tasks from '../../../../../website/server/models/task'; +import { InternalServerError } from '../../../../../website/server/libs/api-v3/errors'; import { each } from 'lodash'; import { generateHistory } from '../../../../helpers/api-unit.helper.js'; @@ -71,4 +72,105 @@ describe('Task Model', () => { }); }); }); + + describe('Static Methods', () => { + describe('findByIdOrAlias', () => { + let taskWithAlias, user; + + beforeEach(async () => { + user = new User(); + await user.save(); + + taskWithAlias = new Tasks.todo({ // eslint-disable-line babel/new-cap + text: 'some text', + alias: 'short-name', + userId: user.id, + }); + await taskWithAlias.save(); + + sandbox.spy(Tasks.Task, 'findOne'); + }); + + it('throws an error if task identifier is not passed in', async (done) => { + try { + await Tasks.Task.findByIdOrAlias(null, user._id); + } catch (err) { + expect(err).to.exist; + expect(err).to.eql(new InternalServerError('Task identifier is a required argument')); + + done(); + } + }); + + it('throws an error if user identifier is not passed in', async (done) => { + try { + await Tasks.Task.findByIdOrAlias(taskWithAlias._id); + } catch (err) { + expect(err).to.exist; + expect(err).to.eql(new InternalServerError('User identifier is a required argument')); + + done(); + } + }); + + it('returns task by id', async () => { + let foundTodo = await Tasks.Task.findByIdOrAlias(taskWithAlias._id, user._id); + + expect(foundTodo.text).to.eql(taskWithAlias.text); + }); + + it('returns task by alias', async () => { + let foundTodo = await Tasks.Task.findByIdOrAlias(taskWithAlias.alias, user._id); + + expect(foundTodo.text).to.eql(taskWithAlias.text); + }); + + it('scopes alias lookup to user', async () => { + await Tasks.Task.findByIdOrAlias(taskWithAlias.alias, user._id); + + expect(Tasks.Task.findOne).to.be.calledOnce; + expect(Tasks.Task.findOne).to.be.calledWithMatch({ + alias: taskWithAlias.alias, + userId: user._id, + }); + }); + + it('returns null if task cannot be found', async () => { + let foundTask = await Tasks.Task.findByIdOrAlias('not-found', user._id); + + expect(foundTask).to.eql(null); + }); + + it('accepts additional query parameters', async () => { + await Tasks.Task.findByIdOrAlias(taskWithAlias.alias, user._id, { foo: 'bar' }); + + expect(Tasks.Task.findOne).to.be.calledOnce; + expect(Tasks.Task.findOne).to.be.calledWithMatch({ + foo: 'bar', + alias: taskWithAlias.alias, + userId: user._id, + }); + }); + }); + + describe('sanitizeUserChallengeTask ', () => { + }); + + describe('sanitizeChecklist ', () => { + }); + + describe('sanitizeReminder ', () => { + }); + + describe('fromJSONV2 ', () => { + }); + }); + + describe('Instance Methods', () => { + describe('scoreChallengeTask', () => { + }); + + describe('toJSONV2', () => { + }); + }); }); diff --git a/test/common/ops/updateTask.js b/test/common/ops/updateTask.js index 99e8d80b22..d2205291c8 100644 --- a/test/common/ops/updateTask.js +++ b/test/common/ops/updateTask.js @@ -24,6 +24,7 @@ describe('shared.ops.updateTask', () => { text: 'updated', id: '123', _id: '123', + shortName: 'short-name', type: 'todo', tags: ['678'], checklist: [{ @@ -38,6 +39,7 @@ describe('shared.ops.updateTask', () => { expect(res._id).to.not.equal('123'); expect(res.type).to.equal('habit'); expect(res.text).to.equal('updated'); + expect(res.shortName).to.eql('short-name'); expect(res.checklist).to.eql([{ completed: false, text: 'item', diff --git a/website/server/controllers/api-v3/tasks.js b/website/server/controllers/api-v3/tasks.js index 618a6dfece..e3663118ef 100644 --- a/website/server/controllers/api-v3/tasks.js +++ b/website/server/controllers/api-v3/tasks.js @@ -16,6 +16,22 @@ import logger from '../../libs/api-v3/logger'; let api = {}; +async function _validateTaskAlias (tasks, res) { + let tasksWithAliases = tasks.filter(task => task.alias); + let aliases = tasksWithAliases.map(task => task.alias); + + // Compares the short names in tasks against + // a Set, where values cannot repeat. If the + // lengths are different, some name was duplicated + if (aliases.length !== [...new Set(aliases)].length) { + throw new BadRequest(res.t('taskAliasAlreadyUsed')); + } + + await Bluebird.map(tasksWithAliases, (task) => { + return task.validate(); + }); +} + // challenge must be passed only when a challenge task is being created async function _createTasks (req, res, user, challenge) { let toSave = Array.isArray(req.body) ? req.body : [req.body]; @@ -42,7 +58,12 @@ async function _createTasks (req, res, user, challenge) { (challenge || user).tasksOrder[`${taskType}s`].unshift(newTask._id); return newTask; - }).map(task => task.save({ // If all tasks are valid (this is why it's not in the previous .map()), save everything, withough running validation again + }); + + // tasks with aliases need to be validated asyncronously + await _validateTaskAlias(toSave, res); + + toSave = toSave.map(task => task.save({ // If all tasks are valid (this is why it's not in the previous .map()), save everything, withough running validation again validateBeforeSave: false, })); @@ -233,7 +254,7 @@ api.getChallengeTasks = { * @apiName GetTask * @apiGroup Task * - * @apiParam {UUID} taskId The task _id + * @apiParam {UUID|string} taskId The task _id or alias * * @apiSuccess {object} data The task object */ @@ -243,15 +264,8 @@ api.getTask = { middlewares: [authWithHeaders()], async handler (req, res) { let user = res.locals.user; - - req.checkParams('taskId', res.t('taskIdRequired')).notEmpty().isUUID(); - - let validationErrors = req.validationErrors(); - if (validationErrors) throw validationErrors; - - let task = await Tasks.Task.findOne({ - _id: req.params.taskId, - }).exec(); + let taskId = req.params.taskId; + let task = await Tasks.Task.findByIdOrAlias(taskId, user._id); if (!task) { throw new NotFound(res.t('taskNotFound')); @@ -274,7 +288,7 @@ api.getTask = { * @apiName UpdateTask * @apiGroup Task * - * @apiParam {UUID} taskId The task _id + * @apiParam {UUID|string} taskId The task _id or alias * * @apiSuccess {object} data The updated task */ @@ -286,14 +300,13 @@ api.updateTask = { let user = res.locals.user; let challenge; - req.checkParams('taskId', res.t('taskIdRequired')).notEmpty().isUUID(); + req.checkParams('taskId', res.t('taskIdRequired')).notEmpty(); let validationErrors = req.validationErrors(); if (validationErrors) throw validationErrors; - let task = await Tasks.Task.findOne({ - _id: req.params.taskId, - }).exec(); + let taskId = req.params.taskId; + let task = await Tasks.Task.findByIdOrAlias(taskId, user._id); if (!task) { throw new NotFound(res.t('taskNotFound')); @@ -308,7 +321,6 @@ api.updateTask = { // we have to convert task to an object because otherwise things don't get merged correctly. Bad for performances? let [updatedTaskObj] = common.ops.updateTask(task.toObject(), req); - // Sanitize differently user tasks linked to a challenge let sanitizedObj; @@ -362,7 +374,7 @@ function _generateWebhookTaskData (task, direction, delta, stats, user) { * @apiName ScoreTask * @apiGroup Task * - * @apiParam {UUID} taskId The task _id + * @apiParam {UUID|string} taskId The task _id or alias * @apiParam {string="up","down"} direction The direction for scoring the task * * @apiSuccess {object} data._tmp If an item was dropped it'll be returned in te _tmp object @@ -374,19 +386,16 @@ api.scoreTask = { url: '/tasks/:taskId/score/:direction', middlewares: [authWithHeaders()], async handler (req, res) { - req.checkParams('taskId', res.t('taskIdRequired')).notEmpty().isUUID(); req.checkParams('direction', res.t('directionUpDown')).notEmpty().isIn(['up', 'down']); let validationErrors = req.validationErrors(); if (validationErrors) throw validationErrors; let user = res.locals.user; - let direction = req.params.direction; + let {taskId} = req.params; - let task = await Tasks.Task.findOne({ - _id: req.params.taskId, - userId: user._id, - }).exec(); + let task = await Tasks.Task.findByIdOrAlias(taskId, user._id, {userId: user._id}); + let direction = req.params.direction; if (!task) throw new NotFound(res.t('taskNotFound')); @@ -448,7 +457,7 @@ api.scoreTask = { * @apiName MoveTask * @apiGroup Task * - * @apiParam {UUID} taskId The task _id + * @apiParam {UUID|string} taskId The task _id or alias * @apiParam {Number} position Query parameter - Where to move the task (-1 means push to bottom). First position is 0 * * @apiSuccess {array} data The new tasks order (user.tasksOrder.{task.type}s) @@ -458,19 +467,17 @@ api.moveTask = { url: '/tasks/:taskId/move/to/:position', middlewares: [authWithHeaders()], async handler (req, res) { - req.checkParams('taskId', res.t('taskIdRequired')).notEmpty().isUUID(); + req.checkParams('taskId', res.t('taskIdRequired')).notEmpty(); req.checkParams('position', res.t('positionRequired')).notEmpty().isNumeric(); let validationErrors = req.validationErrors(); if (validationErrors) throw validationErrors; let user = res.locals.user; + let taskId = req.params.taskId; let to = Number(req.params.position); - let task = await Tasks.Task.findOne({ - _id: req.params.taskId, - userId: user._id, - }).exec(); + let task = await Tasks.Task.findByIdOrAlias(taskId, user._id, { userId: user._id }); if (!task) throw new NotFound(res.t('taskNotFound')); if (task.type === 'todo' && task.completed) throw new BadRequest(res.t('cantMoveCompletedTodo')); @@ -503,7 +510,7 @@ api.moveTask = { * @apiName AddChecklistItem * @apiGroup Task * - * @apiParam {UUID} taskId The task _id + * @apiParam {UUID|string} taskId The task _id or alias * * @apiSuccess {object} data The updated task */ @@ -515,14 +522,13 @@ api.addChecklistItem = { let user = res.locals.user; let challenge; - req.checkParams('taskId', res.t('taskIdRequired')).notEmpty().isUUID(); + req.checkParams('taskId', res.t('taskIdRequired')).notEmpty(); let validationErrors = req.validationErrors(); if (validationErrors) throw validationErrors; - let task = await Tasks.Task.findOne({ - _id: req.params.taskId, - }).exec(); + let taskId = req.params.taskId; + let task = await Tasks.Task.findByIdOrAlias(taskId, user._id); if (!task) { throw new NotFound(res.t('taskNotFound')); @@ -552,7 +558,7 @@ api.addChecklistItem = { * @apiName ScoreChecklistItem * @apiGroup Task * - * @apiParam {UUID} taskId The task _id + * @apiParam {UUID|string} taskId The task _id or alias * @apiParam {UUID} itemId The checklist item _id * * @apiSuccess {object} data The updated task @@ -564,16 +570,14 @@ api.scoreCheckListItem = { async handler (req, res) { let user = res.locals.user; - req.checkParams('taskId', res.t('taskIdRequired')).notEmpty().isUUID(); + req.checkParams('taskId', res.t('taskIdRequired')).notEmpty(); req.checkParams('itemId', res.t('itemIdRequired')).notEmpty().isUUID(); let validationErrors = req.validationErrors(); if (validationErrors) throw validationErrors; - let task = await Tasks.Task.findOne({ - _id: req.params.taskId, - userId: user._id, - }).exec(); + let taskId = req.params.taskId; + let task = await Tasks.Task.findByIdOrAlias(taskId, user._id, { userId: user._id }); if (!task) throw new NotFound(res.t('taskNotFound')); if (task.type !== 'daily' && task.type !== 'todo') throw new BadRequest(res.t('checklistOnlyDailyTodo')); @@ -594,7 +598,7 @@ api.scoreCheckListItem = { * @apiName UpdateChecklistItem * @apiGroup Task * - * @apiParam {UUID} taskId The task _id + * @apiParam {UUID|string} taskId The task _id or alias * @apiParam {UUID} itemId The checklist item _id * * @apiSuccess {object} data The updated task @@ -607,15 +611,14 @@ api.updateChecklistItem = { let user = res.locals.user; let challenge; - req.checkParams('taskId', res.t('taskIdRequired')).notEmpty().isUUID(); + req.checkParams('taskId', res.t('taskIdRequired')).notEmpty(); req.checkParams('itemId', res.t('itemIdRequired')).notEmpty().isUUID(); let validationErrors = req.validationErrors(); if (validationErrors) throw validationErrors; - let task = await Tasks.Task.findOne({ - _id: req.params.taskId, - }).exec(); + let taskId = req.params.taskId; + let task = await Tasks.Task.findByIdOrAlias(taskId, user._id); if (!task) { throw new NotFound(res.t('taskNotFound')); @@ -647,7 +650,7 @@ api.updateChecklistItem = { * @apiName RemoveChecklistItem * @apiGroup Task * - * @apiParam {UUID} taskId The task _id + * @apiParam {UUID|string} taskId The task _id or alias * @apiParam {UUID} itemId The checklist item _id * * @apiSuccess {object} data The updated task @@ -660,15 +663,14 @@ api.removeChecklistItem = { let user = res.locals.user; let challenge; - req.checkParams('taskId', res.t('taskIdRequired')).notEmpty().isUUID(); + req.checkParams('taskId', res.t('taskIdRequired')).notEmpty(); req.checkParams('itemId', res.t('itemIdRequired')).notEmpty().isUUID(); let validationErrors = req.validationErrors(); if (validationErrors) throw validationErrors; - let task = await Tasks.Task.findOne({ - _id: req.params.taskId, - }).exec(); + let taskId = req.params.taskId; + let task = await Tasks.Task.findByIdOrAlias(taskId, user._id); if (!task) { throw new NotFound(res.t('taskNotFound')); @@ -698,7 +700,7 @@ api.removeChecklistItem = { * @apiName AddTagToTask * @apiGroup Task * - * @apiParam {UUID} taskId The task _id + * @apiParam {UUID|string} taskId The task _id or alias * @apiParam {UUID} tagId The tag id * * @apiSuccess {object} data The updated task @@ -710,17 +712,15 @@ api.addTagToTask = { async handler (req, res) { let user = res.locals.user; - req.checkParams('taskId', res.t('taskIdRequired')).notEmpty().isUUID(); + req.checkParams('taskId', res.t('taskIdRequired')).notEmpty(); let userTags = user.tags.map(tag => tag.id); req.checkParams('tagId', res.t('tagIdRequired')).notEmpty().isUUID().isIn(userTags); let validationErrors = req.validationErrors(); if (validationErrors) throw validationErrors; - let task = await Tasks.Task.findOne({ - _id: req.params.taskId, - userId: user._id, - }).exec(); + let taskId = req.params.taskId; + let task = await Tasks.Task.findByIdOrAlias(taskId, user._id, { userId: user._id }); if (!task) throw new NotFound(res.t('taskNotFound')); let tagId = req.params.tagId; @@ -741,7 +741,7 @@ api.addTagToTask = { * @apiName RemoveTagFromTask * @apiGroup Task * - * @apiParam {UUID} taskId The task _id + * @apiParam {UUID|string} taskId The task _id or alias * @apiParam {UUID} tagId The tag id * * @apiSuccess {object} data The updated task @@ -753,16 +753,14 @@ api.removeTagFromTask = { async handler (req, res) { let user = res.locals.user; - req.checkParams('taskId', res.t('taskIdRequired')).notEmpty().isUUID(); + req.checkParams('taskId', res.t('taskIdRequired')).notEmpty(); req.checkParams('tagId', res.t('tagIdRequired')).notEmpty().isUUID(); let validationErrors = req.validationErrors(); if (validationErrors) throw validationErrors; - let task = await Tasks.Task.findOne({ - _id: req.params.taskId, - userId: user._id, - }).exec(); + let taskId = req.params.taskId; + let task = await Tasks.Task.findByIdOrAlias(taskId, user._id, { userId: user._id }); if (!task) throw new NotFound(res.t('taskNotFound')); @@ -842,7 +840,7 @@ api.unlinkAllTasks = { * @apiName UnlinkOneTask * @apiGroup Task * - * @apiParam {UUID} taskId The task _id + * @apiParam {UUID|string} taskId The task _id or alias * @apiParam {string} keep Query parameter - keep or remove * * @apiSuccess {object} data An empty object @@ -862,10 +860,7 @@ api.unlinkOneTask = { let keep = req.query.keep; let taskId = req.params.taskId; - let task = await Tasks.Task.findOne({ - _id: taskId, - userId: user._id, - }).exec(); + let task = await Tasks.Task.findByIdOrAlias(taskId, user._id, { userId: user._id }); if (!task) throw new NotFound(res.t('taskNotFound')); if (!task.challenge.id) throw new BadRequest(res.t('cantOnlyUnlinkChalTask')); @@ -924,7 +919,7 @@ api.clearCompletedTodos = { * @apiName DeleteTask * @apiGroup Task * - * @apiParam {UUID} taskId The task _id + * @apiParam {UUID|string} taskId The task _id or alias * * @apiSuccess {object} data An empty object */ @@ -936,13 +931,8 @@ api.deleteTask = { let user = res.locals.user; let challenge; - req.checkParams('taskId', res.t('taskIdRequired')).notEmpty().isUUID(); - - let validationErrors = req.validationErrors(); - if (validationErrors) throw validationErrors; - let taskId = req.params.taskId; - let task = await Tasks.Task.findById(taskId).exec(); + let task = await Tasks.Task.findByIdOrAlias(taskId, user._id); if (!task) { throw new NotFound(res.t('taskNotFound')); diff --git a/website/server/models/task.js b/website/server/models/task.js index cb66aefeff..e1ba2645d7 100644 --- a/website/server/models/task.js +++ b/website/server/models/task.js @@ -3,6 +3,7 @@ import shared from '../../../common'; import validator from 'validator'; import moment from 'moment'; import baseModel from '../libs/api-v3/baseModel'; +import { InternalServerError } from '../libs/api-v3/errors'; import _ from 'lodash'; import { preenHistory } from '../libs/api-v3/preening'; @@ -25,6 +26,21 @@ export let TaskSchema = new Schema({ type: {type: String, enum: tasksTypes, required: true, default: tasksTypes[0]}, text: {type: String, required: true}, notes: {type: String, default: ''}, + alias: { + type: String, + match: [/^[a-zA-Z0-9-_]+$/, 'Task short names can only contain alphanumeric characters, underscores and dashes.'], + validate: [{ + validator () { + return Boolean(this.userId); + }, + msg: 'Task short names can only be applied to tasks in a user\'s own task list.', + }, { + validator (val) { + return !validator.isUUID(val); + }, + msg: 'Task short names cannot be uuids.', + }], + }, tags: [{ type: String, validate: [validator.isUUID, 'Invalid uuid.'], @@ -73,6 +89,25 @@ TaskSchema.plugin(baseModel, { timestamps: true, }); +TaskSchema.statics.findByIdOrAlias = async function findByIdOrAlias (identifier, userId, additionalQueries = {}) { + // not using i18n strings because these errors are meant for devs who forgot to pass some parameters + if (!identifier) throw new InternalServerError('Task identifier is a required argument'); + if (!userId) throw new InternalServerError('User identifier is a required argument'); + + let query = _.cloneDeep(additionalQueries); + + if (validator.isUUID(identifier)) { + query._id = identifier; + } else { + query.userId = userId; + query.alias = identifier; + } + + let task = await this.findOne(query).exec(); + + return task; +}; + // Sanitize user tasks linked to a challenge // See http://habitica.wikia.com/wiki/Challenges#Challenge_Participant.27s_Permissions for more info TaskSchema.statics.sanitizeUserChallengeTask = function sanitizeUserChallengeTask (taskObj) { @@ -167,6 +202,20 @@ TaskSchema.statics.fromJSONV2 = function fromJSONV2 (taskObj) { export let Task = mongoose.model('Task', TaskSchema); +Task.schema.path('alias').validate(function valiateAliasNotTaken (alias, respond) { + Task.findOne({ + _id: { $ne: this._id }, + userId: this.userId, + alias, + }).exec().then((task) => { + let aliasAvailable = !task; + + respond(aliasAvailable); + }).catch(() => { + respond(false); + }); +}, 'Task alias already used on another task.'); + // habits and dailies shared fields let habitDailySchema = () => { return {history: Array}; // [{date:Date, value:Number}], // this causes major performance problems diff --git a/website/views/shared/tasks/edit/advanced_options.jade b/website/views/shared/tasks/edit/advanced_options.jade index b75baa90e4..c78c206bee 100644 --- a/website/views/shared/tasks/edit/advanced_options.jade +++ b/website/views/shared/tasks/edit/advanced_options.jade @@ -4,6 +4,11 @@ div(ng-if='::task.type!="reward"') ng-click='task._advanced = !task._advanced', tooltip=env.t('expandCollapse')) =env.t('advancedOptions') + fieldset.option-group.advanced-option(ng-if="task.userId" ng-show="task._advanced") + legend.option-title + a.hint(href='http://habitica.wikia.com/wiki/Task_Alias', target='_blank', popover-trigger='mouseenter', popover="{{::env.t('taskAliasPopover')}} {{::task.alias ? '\n\n\' + env.t('taskAliasPopoverWarning') : ''}}")=env.t('taskAlias') + input.form-control(ng-model='task.alias' type='text' placeholder=env.t('taskAliasPlaceholder')) + div(ng-show='task._advanced') div(ng-if='::task.type == "daily"') .form-group