From d25cb70f66f8bc6874c7e9b37afba158930619d1 Mon Sep 17 00:00:00 2001 From: Matteo Pagliazzi Date: Mon, 4 Jan 2016 19:44:39 +0100 Subject: [PATCH] wip get and create tasks plus initial user syncing --- common/locales/en/api-v3.json | 4 +- .../v3/integration/tasks/GET-tasks.test.js | 39 ++++------ website/src/controllers/api-v3/tasks.js | 76 ++++++++++++++----- website/src/models/challenge.js | 73 ++++++++++++------ 4 files changed, 124 insertions(+), 68 deletions(-) diff --git a/common/locales/en/api-v3.json b/common/locales/en/api-v3.json index 2ace1f7773..a7c62e3744 100644 --- a/common/locales/en/api-v3.json +++ b/common/locales/en/api-v3.json @@ -46,5 +46,7 @@ "winnerIdRequired": "\"winnerId\" must be a valid UUID.", "challengeNotFound": "Challenge not found.", "onlyLeaderDeleteChal": "Only the challenge leader can delete it.", - "winnerNotFound": "Winner with id \"<%= userId %>\" not found or not part of the challenge." + "winnerNotFound": "Winner with id \"<%= userId %>\" not found or not part of the challenge.", + "noCompletedTodosChallenge": "\"includeComepletedTodos\" is not supported when fetching a challenge tasks.", + "userTasksNoChallengeId": "When \"tasksOwner\" is \"user\" \"challengeId\" can't be passed." } diff --git a/test/api/v3/integration/tasks/GET-tasks.test.js b/test/api/v3/integration/tasks/GET-tasks.test.js index 0772046f0b..3f1c50448d 100644 --- a/test/api/v3/integration/tasks/GET-tasks.test.js +++ b/test/api/v3/integration/tasks/GET-tasks.test.js @@ -1,42 +1,31 @@ import { generateUser, } from '../../../../helpers/api-integration.helper'; -import Q from 'q'; describe('GET /tasks', () => { let user; - before(() => { + beforeEach(async () => { + user = await generateUser(); + }); + + before(async () => { return generateUser().then((generatedUser) => { user = generatedUser; }); }); - it('returns all user\'s tasks', () => { - let length; - return Q.all([ - user.post('/tasks', {text: 'test habit', type: 'habit'}), - ]) - .then((createdTasks) => { - length = createdTasks.length; - return user.get('/tasks'); - }) - .then((tasks) => { - expect(tasks.length).to.equal(length + 1); // + 1 because 1 is a default task - }); + it('returns all user\'s tasks', async () => { + let createdTasks = await user.post('/tasks', [{text: 'test habit', type: 'habit'}, {text: 'test todo', type: 'todo'}]); + let tasks = await user.get('/tasks/user'); + expect(tasks.length).to.equal(createdTasks.length + 1); // + 1 because 1 is a default task }); - it('returns only a type of user\'s tasks if req.query.type is specified', () => { - let habitId; - user.post('/tasks', {text: 'test habit', type: 'habit'}) - .then((task) => { - habitId = task._id; - return user.get('/tasks?type=habit'); - }) - .then((tasks) => { - expect(tasks.length).to.equal(1); - expect(tasks[0]._id).to.equal(habitId); - }); + it('returns only a type of user\'s tasks if req.query.type is specified', async () => { + let createdTasks = await user.post('/tasks', [{text: 'test habit', type: 'habit'}, {text: 'test todo', type: 'todo'}]); + let tasks = await user.get('/tasks/user?type=habit'); + expect(tasks.length).to.equal(1); + expect(tasks[0]._id).to.equal(createdTasks[0]._id); }); // TODO complete after task scoring is done diff --git a/website/src/controllers/api-v3/tasks.js b/website/src/controllers/api-v3/tasks.js index 84ccf0a221..676be05bf5 100644 --- a/website/src/controllers/api-v3/tasks.js +++ b/website/src/controllers/api-v3/tasks.js @@ -2,6 +2,7 @@ import { authWithHeaders } from '../../middlewares/api-v3/auth'; import cron from '../../middlewares/api-v3/cron'; import { sendTaskWebhook } from '../../libs/api-v3/webhook'; import * as Tasks from '../../models/task'; +import { model as Challenge } from '../../models/challenge'; import { NotFound, NotAuthorized, @@ -21,15 +22,30 @@ let api = {}; * @apiName CreateTask * @apiGroup Task * + * @apiParam {string="user","challenge"} tasksOwner Define if tasks will belong to the auhenticated user or to a challenge (specifying the "challengeId" parameter). + * @apiParam {UUID} challengeId Optional. If "tasksOwner" is "challenge" then specify the challenge id. + * * @apiSuccess {Object|Array} task The newly created task(s) */ api.createTask = { method: 'POST', - url: '/tasks', + url: '/tasks/:tasksOwner/:challengeId?', middlewares: [authWithHeaders(), cron], async handler (req, res) { let tasksData = Array.isArray(req.body) ? req.body : [req.body]; let user = res.locals.user; + let tasksOwner = req.params.tasksOwner; + let challengeId = req.params.challengeId; + let challenge; + + if (tasksOwner === 'user' && challengeId) throw new BadRequest(res.t('userTasksNoChallengeId')); + if (tasksOwner === 'challenge') { + if (!challengeId) throw new BadRequest(res.t('challengeIdRequired')); + challenge = await Challenge.findOne({_id: challengeId}).exec(); + + // If the challenge does not exist, or if it exists but user is not the leader -> throw error + if (!challenge || challenge.leader !== user._id) throw new NotFound(res.t('challengeNotFound')); + } let toSave = tasksData.map(taskData => { // Validate that task.type is valid @@ -37,15 +53,20 @@ api.createTask = { let taskType = taskData.type; let newTask = new Tasks[taskType](Tasks.Task.sanitizeCreate(taskData)); - newTask.userId = user._id; + + if (challenge) { + newTask.challenge.id = challengeId; + } else { + newTask.userId = user._id; + } // Validate that the task is valid and throw if it isn't - // otherwise since we're saving user and task in parallel it could save the user with a tasksOrder that doens't match reality + // otherwise since we're saving user/challenge and task in parallel it could save the user/challenge with a tasksOrder that doens't match reality let validationErrors = newTask.validateSync(); if (validationErrors) throw validationErrors; - // Otherwise update the user - user.tasksOrder[`${taskType}s`].unshift(newTask._id); + // Otherwise update the user/challenge + (challenge || user).tasksOrder[`${taskType}s`].unshift(newTask._id); return newTask; }); @@ -54,41 +75,60 @@ api.createTask = { toSave = toSave.map(task => task.save({ validateBeforeSave: false, })); - toSave.unshift(user.save()); - let results = await Q.all(toSave); + toSave.unshift((challenge || user).save()); - if (results.length === 2) { // Just one task created - res.respond(201, results[1]); - } else { - results.splice(0, 1); // remove the user - res.respond(201, results); - } + let tasks = await Q.all(toSave); + tasks.splice(0, 1); // remove the user/challenge + res.respond(201, tasks); + + // If adding tasks to a challenge -> sync users + if (challenge) challenge.addTasksToMembers(tasks); // TODO catch/log }, }; /** - * @api {get} /tasks Get an user's tasks + * @api {get} /tasks/:tasksOwner/:challengeId Get an user's tasks * @apiVersion 3.0.0 * @apiName GetTasks * @apiGroup Task * + * @apiParam {string="user","challenge"} tasksOwner Url parameter to return tasks belonging to a challenge (specifying the "challengeId" parameter) or to the autheticated user. + * @apiParam {UUID} challengeId Optional. If "tasksOwner" is "challenge" then specify the challenge id. * @apiParam {string="habit","daily","todo","reward"} type Optional query parameter to return just a type of tasks - * @apiParam {boolean} includeCompletedTodos Optional query parameter to include completed todos when "type" is "todo" + * @apiParam {boolean} includeCompletedTodos Optional query parameter to include completed todos when "type" is "todo". Only valid whe "tasksOwner" is "user". * * @apiSuccess {Array} tasks An array of task objects */ api.getTasks = { method: 'GET', - url: '/tasks', + url: '/tasks/:tasksOwner/:challengeId?', middlewares: [authWithHeaders(), cron], async handler (req, res) { + req.checkParams('tasksOwner', res.t('invalidTasksOwner')).isIn(['user', 'challenge']); + req.checkParams('challengeId', res.t('challengeIdRequired')).optional().isUUID(); + req.checkQuery('type', res.t('invalidTaskType')).optional().isIn(Tasks.tasksTypes); let validationErrors = req.validationErrors(); if (validationErrors) throw validationErrors; let user = res.locals.user; - let query = {userId: user._id}; + let tasksOwner = req.params.tasksOwner; + let challengeId = req.params.challengeId; + let challenge; + + if (tasksOwner === 'user' && challengeId) throw new BadRequest(res.t('userTasksNoChallengeId')); + if (tasksOwner === 'challenge') { + if (!challengeId) throw new BadRequest(res.t('challengeIdRequired')); + challenge = await Challenge.findOne({_id: challengeId}).exec(); + + // If the challenge does not exist, or if it exists but user is not a member, not the leader and not an admin -> throw error + if (!challenge || (user.challenges.indexOf(challengeId) === -1 && challenge.leader !== user._id && !user.contributor.admin)) { // eslint-disable-line no-extra-parens + throw new NotFound(res.t('challengeNotFound')); + } + } + + let query = challenge ? {'challenge.id': challengeId, userId: {$exists: false}} : {userId: user._id}; let type = req.query.type; if (type) { @@ -102,6 +142,8 @@ api.getTasks = { } if (req.query.includeCompletedTodos === 'true' && (!type || type === 'todo')) { + if (challengeId) throw new BadRequest(res.t('noCompletedTodosChallenge')); + let queryCompleted = Tasks.Task.find({ type: 'todo', completed: true, diff --git a/website/src/models/challenge.js b/website/src/models/challenge.js index d5b3bf1a1b..d3bf4550a3 100644 --- a/website/src/models/challenge.js +++ b/website/src/models/challenge.js @@ -4,6 +4,7 @@ import validator from 'validator'; import baseModel from '../libs/api-v3/baseModel'; import _ from 'lodash'; import * as Tasks from './task'; +import { model as User } from './user'; let Schema = mongoose.Schema; @@ -29,37 +30,15 @@ schema.plugin(baseModel, { noSet: ['_id', 'memberCount', 'tasksOrder'], }); - -// Syncing logic - function _syncableAttrs (task) { - let t = task.toObject(); // lodash doesn't seem to like _.omit on EmbeddedDocument + 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 use whitelist instead of blacklist? if (t.type !== 'reward') omitAttrs.push('value'); return _.omit(t, omitAttrs); } -// TODO redo -// Compare whether any changes have been made to tasks. If so, we'll want to sync those changes to subscribers -/* function comparableData(obj) { - return JSON.stringify( - _(obj.habits.concat(obj.dailys).concat(obj.todos).concat(obj.rewards)) - .sortBy('id') // we don't want to update if they're sort-order is different - .transform(function(result, task){ - result.push(syncableAttrs(task)); - }) - .value()) -} - -ChallengeSchema.methods.isOutdated = function isChallengeOutdated (newData) { - return comparableData(this) !== comparableData(newData); -}*/ - -// Syncs all new tasks, deleted tasks, etc to the user object schema.methods.syncToUser = function syncChallengeToUser (user) { - if (!user) throw new Error('User required.'); - let challenge = this; challenge.shortName = challenge.shortName || challenge.name; @@ -83,8 +62,12 @@ schema.methods.syncToUser = function syncChallengeToUser (user) { }); } + return user.save(); + + // Old logic used to sync tasks + // TODO might keep it around for when normal syncing doesn't succeed? or for first time syncing? // Sync new tasks and updated tasks - return Q.all([ + /* return Q.all([ // Find original challenge tasks Tasks.Task.find({ userId: {$exists: false}, @@ -131,7 +114,47 @@ schema.methods.syncToUser = function syncChallengeToUser (user) { toSave.push(user.save()); return Q.all(toSave); - }); + });*/ }; +schema.methods.addTasksToMembers = async function addTasksToMembers (tasks) { + let challenge = this; + + let membersIds = (await User.find({challenges: {$in: [challenge._id]}}).select('_id').exec()).map(member => member._id); + + // Add tasks to users sequentially so that we don't kill the server (hopefully); + // using a for...of loop allows each op to be run in sequence + for (let memberId of membersIds) { + let update = User.update + await db.post(doc); + } + + tasks.forEach(chalTask => { + matchingTask = new Tasks[chalTask.type](Tasks.Task.sanitizeCreate(_syncableAttrs(chalTask))); + matchingTask.challenge = {taskId: chalTask._id, id: challenge._id}; + matchingTask.userId = user._id; + + }) + +}; + +// Old Syncing logic, kept for reference and maybe will be needed to adapt v2 +/* + +// TODO redo +// Compare whether any changes have been made to tasks. If so, we'll want to sync those changes to subscribers +function comparableData(obj) { + return JSON.stringify( + _(obj.habits.concat(obj.dailys).concat(obj.todos).concat(obj.rewards)) + .sortBy('id') // we don't want to update if they're sort-order is different + .transform(function(result, task){ + result.push(syncableAttrs(task)); + }) + .value()) +} + +ChallengeSchema.methods.isOutdated = function isChallengeOutdated (newData) { + return comparableData(this) !== comparableData(newData); +}*/ + export let model = mongoose.model('Challenge', schema);