From 5206469e909f656e9869c274eba83cccd45f9330 Mon Sep 17 00:00:00 2001 From: Matteo Pagliazzi Date: Thu, 14 Jan 2016 19:46:43 +0100 Subject: [PATCH] add unlinkTask route and refactor user.unlink (now user.unlinkChallengesTasks) --- common/locales/en/api-v3.json | 4 +- website/src/controllers/api-v3/tasks.js | 81 +++++++++++++++++++------ website/src/models/challenge.js | 2 +- website/src/models/group.js | 6 +- website/src/models/user.js | 56 ++++++++--------- 5 files changed, 95 insertions(+), 54 deletions(-) diff --git a/common/locales/en/api-v3.json b/common/locales/en/api-v3.json index 91711f15eb..dfdea49d6c 100644 --- a/common/locales/en/api-v3.json +++ b/common/locales/en/api-v3.json @@ -37,6 +37,7 @@ "memberCannotRemoveYourself": "You cannot remove yourself!", "groupMemberNotFound": "User not found among group's members", "keepOrRemoveAll": "req.query.keep must be either \"keep-all\" or \"remove-all\"", + "keepOrRemove": "req.query.keep must be either \"keep\" or \"remove\"", "canOnlyInviteEmailUuid": "Can only invite using uuids or emails.", "inviteMissingEmail": "Missing email address in invite.", "onlyGroupLeaderChal": "Only the group leader can create challenges", @@ -59,5 +60,6 @@ "userWithIDNotFound": "User with id \"<%= userId %>\" not found.", "uuidsMustBeAnArray": "UUIDs invites must be a an Array.", "emailsMustBeAnArray": "Email invites must be a an Array.", - "canOnlyInviteMaxInvites": "You can only invite \"<%= maxInvites %>\" at a time" + "canOnlyInviteMaxInvites": "You can only invite \"<%= maxInvites %>\" at a time", + "cantOnlyUnlinkChalTask": "Only challenges tasks can be unlinked." } diff --git a/website/src/controllers/api-v3/tasks.js b/website/src/controllers/api-v3/tasks.js index ffa2d20bf5..a3610f7913 100644 --- a/website/src/controllers/api-v3/tasks.js +++ b/website/src/controllers/api-v3/tasks.js @@ -540,7 +540,7 @@ api.moveTask = { }).exec(); if (!task) throw new NotFound(res.t('taskNotFound')); - if (task.type === 'todo' && task.completed) throw new NotFound(res.t('cantMoveCompletedTodo')); + if (task.type === 'todo' && task.completed) throw new BadRequest(res.t('cantMoveCompletedTodo')); let order = user.tasksOrder[`${task.type}s`]; let currentIndex = order.indexOf(task._id); @@ -837,21 +837,63 @@ api.removeTagFromTask = { }; // Remove a task from (user|challenge).tasksOrder -function _removeTaskTasksOrder (userOrChallenge, taskId) { - // Loop through all lists and when the task is found, remove it and return - for (let i = 0; i < Tasks.tasksTypes.length; i++) { - let list = userOrChallenge.tasksOrder[`${Tasks.tasksTypes[i]}s`]; - let index = list.indexOf(taskId); +function _removeTaskTasksOrder (userOrChallenge, taskId, taskType) { + let list = userOrChallenge.tasksOrder[taskType]; + let index = list.indexOf(taskId); - if (index !== -1) { - list.splice(index, 1); - break; - } - } - - return; + if (index !== -1) list.splice(index, 1); } +// TODO this method needs some limitation, like to check if the challenge is really broken? +/** + * @api {post} /tasks/unlink/:taskId Unlink a challenge task + * @apiVersion 3.0.0 + * @apiName UnlinkTask + * @apiGroup Task + * + * @apiParam {UUID} taskId The task _id + * + * @apiSuccess {object} empty An empty object + */ +api.unlinkTask = { + method: 'POST', + url: '/tasks/unlink/:taskId', + middlewares: [authWithHeaders(), cron], + async handler (req, res) { + req.checkParams('taskId', res.t('taskIdRequired')).notEmpty().isUUID(); + req.checkQuery('keep', res.t('keepOrRemove')).notEmpty().isIn(['keep', 'remove']); + + let validationErrors = req.validationErrors(); + if (validationErrors) throw validationErrors; + + let user = res.locals.user; + let keep = req.query.keep; + let taskId = req.params.taskId; + + let task = await Tasks.Task.findOne({ + _id: taskId, + userId: user._id, + }).exec(); + + if (!task) throw new NotFound(res.t('taskNotFound')); + if (!task.challenge.id) throw new BadRequest(res.t('cantOnlyUnlinkChalTask')); + + if (keep === 'keep') { + task.challenge = {}; + await task.save(); + } else { // remove + if (task.type !== 'todo' || !task.completed) { // eslint-disable-line no-lonely-if + _removeTaskTasksOrder(user, taskId, task.type); + await Q.all([user.save(), task.remove()]); + } else { + await task.remove(); + } + } + + res.respond(200, {}); // TODO what to return + }, +}; + /** * @api {delete} /task/:taskId Delete a user task given its id * @apiVersion 3.0.0 @@ -875,9 +917,8 @@ api.deleteTask = { 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.findById(taskId).exec(); if (!task) { throw new NotFound(res.t('taskNotFound')); @@ -891,8 +932,12 @@ api.deleteTask = { throw new NotAuthorized(res.t('cantDeleteChallengeTasks')); } - _removeTaskTasksOrder(challenge || user, req.params.taskId); - await Q.all([user.save(), task.remove()]); + if (task.type !== 'todo' || !task.completed) { + _removeTaskTasksOrder(challenge || user, taskId, task.type); + await Q.all([(challenge || user).save(), task.remove()]); + } else { + await task.remove(); + } res.respond(200, {}); if (challenge) challenge.removeTask(task); diff --git a/website/src/models/challenge.js b/website/src/models/challenge.js index 198ea2e476..b675a7833e 100644 --- a/website/src/models/challenge.js +++ b/website/src/models/challenge.js @@ -182,7 +182,7 @@ schema.methods.removeTask = async function challengeRemoveTask (task) { 'challenge.taskId': task._id, }, { $set: {'challenge.broken': 'TASK_DELETED'}, // TODO what about updatedAt? - }).lean().exec(); + }, {multi: true}).exec(); }; export let model = mongoose.model('Challenge', schema); diff --git a/website/src/models/group.js b/website/src/models/group.js index 91ae59e714..61c324e6c1 100644 --- a/website/src/models/group.js +++ b/website/src/models/group.js @@ -439,7 +439,7 @@ schema.statics.bossQuest = function bossQuest (user, progress) { // Remove user from this group // TODO this is highly inefficient -schema.methods.leave = function leaveGroup (user, keep = 'keep-all') { +schema.methods.leave = function leaveGroup (user, keep) { let group = this; return Q.all([ @@ -455,12 +455,12 @@ schema.methods.leave = function leaveGroup (user, keep = 'keep-all') { {_id: {$in: _.pluck(challenges, '_id')}}, {$pull: {members: user._id}}, {multi: true} - ).then(() => challenges); // pass `challenges` above to next promise TODO ok to return a non-promise? + ).then(() => challenges); // pass `challenges` above to next promise }).then(challenges => { return Q.all(challenges.map(chal => { let i = user.challenges.indexOf(chal._id); if (i !== -1) user.challenges.splice(i, 1); - return user.unlink({cid: chal._id, keep}); + return user.unlinkChallengeTasks(chal._id, keep); })); }), diff --git a/website/src/models/user.js b/website/src/models/user.js index a688453cfe..55fdd49bdb 100644 --- a/website/src/models/user.js +++ b/website/src/models/user.js @@ -643,40 +643,34 @@ schema.methods.isSubscribed = function isSubscribed () { return !!this.purchased.plan.customerId; // eslint-disable-line no-implicit-coercion }; -schema.methods.unlink = function unlink (options, cb) { - let cid = options.cid; - let keep = options.keep; - let tid = options.tid; +// Unlink challenges tasks from user +schema.methods.unlinkChallengeTasks = async function unlinkChallengeTasks (challengeId, keep) { + let user = this; + let findQuery = { + userId: user._id, + 'challenge.id': challengeId, + }; - if (!cid) { - return cb('Could not remove challenge tasks. Please delete them manually.'); - } - - let self = this; - - if (keep === 'keep') { - self.tasks[tid].challenge = {}; - } else if (keep === 'remove') { - self.ops.deleteTask({params: {id: tid}}, () => {}); - } else if (keep === 'keep-all') { - _.each(self.tasks, (t) => { - if (t.challenge && t.challenge.id === cid) { - t.challenge = {}; + if (keep === 'keep-all') { + await Tasks.Task.update(findQuery, { + $set: {challenge: {}}, // TODO what about updatedAt? + }, {multi: true}).exec(); + } else { // keep = 'remove-all' + let tasks = Tasks.Task.find(findQuery).select('_id type completed').exec(); + tasks = tasks.map(task => { + // Remove task from user.tasksOrder and delete them + if (task.type !== 'todo' || !task.completed) { + let list = user.tasksOrder[task.type]; + let index = list.indexOf(task._id); + if (index !== -1) list.splice(index, 1); } - }); - } else if (keep === 'remove-all') { - _.each(self.tasks, (t) => { - if (t.challenge && t.challenge.id === cid) { - this.ops.deleteTask({params: {id: tid}}, () => {}); - } - }); - } - self.markModified('habits'); - self.markModified('dailys'); - self.markModified('todos'); - self.markModified('rewards'); - self.save(cb); + return task.remove(); + }); + + tasks.push(user.save()); + await Q.all(tasks); + } }; export let model = mongoose.model('User', schema);