From bf2e6489b7a35ace139d7b60857951a53d45efc8 Mon Sep 17 00:00:00 2001 From: Blade Barringer Date: Mon, 16 May 2016 22:40:23 -0500 Subject: [PATCH 1/6] fix: Provide default type and text for new task creation in score route --- website/server/controllers/api-v2/user.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/website/server/controllers/api-v2/user.js b/website/server/controllers/api-v2/user.js index daaa272491..fcac86e2e1 100644 --- a/website/server/controllers/api-v2/user.js +++ b/website/server/controllers/api-v2/user.js @@ -110,8 +110,8 @@ api.score = function(req, res, next) { // Defaults. Other defaults are handled in user.ops.addTask() task = new Tasks.Task({ _id: id, // TODO this might easily lead to conflicts as ids are now unique db-wide - type: body.type, - text: body.text, + type: body.type || 'habit', + text: body.text || id, userId: user._id, notes: body.notes || "This task was created by a third-party service. Feel free to edit, it won't harm the connection to that service. Additionally, multiple services may piggy-back off this task." // TODO translate }); From 4f1d738272cfff7455e570ec59a1a11b77a3191d Mon Sep 17 00:00:00 2001 From: Blade Barringer Date: Mon, 16 May 2016 22:52:32 -0500 Subject: [PATCH 2/6] fix: Provide default history [] for habit in score route --- common/script/ops/scoreTask.js | 1 + 1 file changed, 1 insertion(+) diff --git a/common/script/ops/scoreTask.js b/common/script/ops/scoreTask.js index 0508462773..f052b954e0 100644 --- a/common/script/ops/scoreTask.js +++ b/common/script/ops/scoreTask.js @@ -194,6 +194,7 @@ module.exports = function scoreTask (options = {}, req = {}) { } _gainMP(user, _.max([0.25, 0.0025 * user._statsComputed.maxMP]) * (direction === 'down' ? -1 : 1)); + task.history = task.history || []; // Add history entry, even more than 1 per day task.history.push({ date: Number(new Date()), From 5931aee26bb1bb4be75d267e493b0c3e6dca96ac Mon Sep 17 00:00:00 2001 From: Blade Barringer Date: Tue, 17 May 2016 15:12:44 -0500 Subject: [PATCH 3/6] fix: Add _legacyId prop to tasks to support non-uuid identifiers --- website/server/controllers/api-v2/user.js | 57 ++++++++++++++++------- website/server/models/task.js | 7 ++- 2 files changed, 46 insertions(+), 18 deletions(-) diff --git a/website/server/controllers/api-v2/user.js b/website/server/controllers/api-v2/user.js index fcac86e2e1..52e12d20c3 100644 --- a/website/server/controllers/api-v2/user.js +++ b/website/server/controllers/api-v2/user.js @@ -1,6 +1,7 @@ var url = require('url'); var ipn = require('paypal-ipn'); var _ = require('lodash'); +var validator = require('validator'); var nconf = require('nconf'); var asyncM = require('async'); var shared = require('../../../../common'); @@ -89,6 +90,7 @@ api.score = function(req, res, next) { direction = req.params.direction, user = res.locals.user, body = req.body || {}, + taskQuery = { userId: user._id }, task; // Send error responses for improper API call @@ -98,23 +100,33 @@ api.score = function(req, res, next) { return res.json(400, {err: ":direction must be 'up' or 'down'"}); } - Tasks.Task.findOne({ - _id: id, - userId: user._id - }, function(err, task){ + if (validator.isUUID(id)) { + taskQuery._id = id; + } else { + taskQuery._legacyId = id; + } + + Tasks.Task.findOne(taskQuery, function(err, task){ if(err) return next(err); // If exists already, score it if (!task) { // If it doesn't exist, this is likely a 3rd party up/down - create a new one, then score it // Defaults. Other defaults are handled in user.ops.addTask() - task = new Tasks.Task({ - _id: id, // TODO this might easily lead to conflicts as ids are now unique db-wide + var taskOptions = { type: body.type || 'habit', text: body.text || id, userId: user._id, notes: body.notes || "This task was created by a third-party service. Feel free to edit, it won't harm the connection to that service. Additionally, multiple services may piggy-back off this task." // TODO translate - }); + } + + if (validator.isUUID(id)) { + taskOptions._id = id; // TODO this might easily lead to conflicts as ids are now unique db-wide + } else { + taskOptions._legacyId = id; + } + + task = new Tasks.Task(taskOptions); user.tasksOrder[task.type + 's'].unshift(task._id); } @@ -206,12 +218,17 @@ api.getTasks = function(req, res, next) { * Get Task */ api.getTask = function(req, res, next) { - var user = res.locals.user; + var user = res.locals.user, + id = req.params.id, + taskQuery = { userId: user._id }; - Tasks.Task.findOne({ - userId: user._id, - _id: req.params.id, - }, function (err, task) { + if (validator.isUUID(id)) { + taskQuery._id = id; + } else { + taskQuery._legacyId = id; + } + + Tasks.Task.findOne(taskQuery, function (err, task) { if (err) return next(err); if (!task) return res.status(404).json({err: shared.i18n.t('messageTaskNotFound')}); res.status(200).json(task.toJSONV2()); @@ -830,13 +847,19 @@ api.deleteTask = function(req, res, next) { }; api.updateTask = function(req, res, next) { - var user = res.locals.user; + var user = res.locals.user, + taskQuery = { userId: user._id }, + id = req.params.id; + req.body = Tasks.Task.fromJSONV2(req.body); - Tasks.Task.findOne({ - _id: req.params.id, - userId: user._id - }, function(err, task) { + if (validator.isUUID(id)) { + taskQuery._id = id; + } else { + taskQuery._legacyId = id; + } + + Tasks.Task.findOne(taskQuery, function(err, task) { if(err) return next(err); if(!task) return res.status(404).json({err: 'Task not found.'}) diff --git a/website/server/models/task.js b/website/server/models/task.js index 27f356efa3..0116da2371 100644 --- a/website/server/models/task.js +++ b/website/server/models/task.js @@ -17,6 +17,7 @@ export let tasksTypes = ['habit', 'daily', 'todo', 'reward']; // Important // When something changes here remember to update the client side model at common/script/libs/taskDefaults export let TaskSchema = new Schema({ + _legacyId: String, // TODO Remove when v2 is deprecated type: {type: String, enum: tasksTypes, required: true, default: tasksTypes[0]}, text: {type: String, required: true}, notes: {type: String, default: ''}, @@ -119,7 +120,11 @@ TaskSchema.methods.scoreChallengeTask = async function scoreChallengeTask (delta // toJSON for API v2 TaskSchema.methods.toJSONV2 = function toJSONV2 () { let toJSON = this.toJSON(); - toJSON.id = toJSON._id; + if (toJSON._legacyId) { + toJSON.id = toJSON._legacyId; + } else { + toJSON.id = toJSON._id; + } let v3Tags = this.tags; From 990d43928b93b31874c141e743354ab49e7a79e0 Mon Sep 17 00:00:00 2001 From: Blade Barringer Date: Tue, 17 May 2016 15:25:54 -0500 Subject: [PATCH 4/6] chore: Change v3 migration to use _legacyId instead of legacyId --- migrations/api_v3/challenges.js | 6 +++--- migrations/api_v3/users.js | 10 +++++----- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/migrations/api_v3/challenges.js b/migrations/api_v3/challenges.js index eaec714858..6d066b3d63 100644 --- a/migrations/api_v3/challenges.js +++ b/migrations/api_v3/challenges.js @@ -129,16 +129,16 @@ function processChallenges (afterId) { oldTasks.forEach(function (oldTask) { oldTask._id = uuid.v4(); - oldTask.legacyId = oldTask.id; // store the old task id + oldTask._legacyId = oldTask.id; // store the old task id delete oldTask.id; oldTask.challenge = oldTask.challenge || {}; oldTask.challenge.id = newChallenge._id; - if (newTasksIds[oldTask.legacyId + '-' + newChallenge._id]) { + if (newTasksIds[oldTask._legacyId + '-' + newChallenge._id]) { throw new Error('duplicate :('); } else { - newTasksIds[oldTask.legacyId + '-' + newChallenge._id] = oldTask._id; + newTasksIds[oldTask._legacyId + '-' + newChallenge._id] = oldTask._id; } oldTask.tags = _.map(oldTask.tags || {}, function (tagPresent, tagId) { diff --git a/migrations/api_v3/users.js b/migrations/api_v3/users.js index e569699673..ef15d74f85 100644 --- a/migrations/api_v3/users.js +++ b/migrations/api_v3/users.js @@ -131,20 +131,20 @@ function processUsers (afterId) { oldTasks.forEach(function (oldTask) { oldTask._id = uuid.v4(); // create a new unique uuid oldTask.userId = newUser._id; - oldTask.legacyId = oldTask.id; // store the old task id + oldTask._legacyId = oldTask.id; // store the old task id delete oldTask.id; oldTask.challenge = oldTask.challenge || {}; if (oldTask.challenge.id) { if (oldTask.challenge.broken) { - oldTask.challenge.taskId = oldTask.legacyId; + oldTask.challenge.taskId = oldTask._legacyId; } else { - var newId = newTasksIds[oldTask.legacyId + '-' + oldTask.challenge.id]; + var newId = newTasksIds[oldTask._legacyId + '-' + oldTask.challenge.id]; // Challenges' tasks ids changed if (!newId && !oldTask.challenge.broken) { challengeTaskNoMatchingId++; - oldTask.challenge.taskId = oldTask.legacyId; + oldTask.challenge.taskId = oldTask._legacyId; oldTask.challenge.broken = 'CHALLENGE_TASK_NOT_FOUND'; } else { challengeTaskWithMatchingId++; @@ -173,7 +173,7 @@ function processUsers (afterId) { newUser.tasksOrder[`${oldTask.type}s`].push(oldTask._id); } - var allTasksFields = ['_id', 'type', 'text', 'notes', 'tags', 'value', 'priority', 'attribute', 'challenge', 'reminders', 'userId', 'legacyId', 'createdAt']; + var allTasksFields = ['_id', 'type', 'text', 'notes', 'tags', 'value', 'priority', 'attribute', 'challenge', 'reminders', 'userId', '_legacyId', 'createdAt']; // using mongoose models is too slow if (oldTask.type === 'habit') { oldTask = _.pick(oldTask, allTasksFields.concat(['history', 'up', 'down'])); From 38473f09c75d9d1a7da5131fb7af1a545bbbf69e Mon Sep 17 00:00:00 2001 From: Blade Barringer Date: Tue, 17 May 2016 16:48:42 -0500 Subject: [PATCH 5/6] fix: check for _legacyId in tasks if id does not exist --- website/server/controllers/api-v2/user.js | 70 +++++++++++++++-------- 1 file changed, 47 insertions(+), 23 deletions(-) diff --git a/website/server/controllers/api-v2/user.js b/website/server/controllers/api-v2/user.js index 52e12d20c3..0941d9d3af 100644 --- a/website/server/controllers/api-v2/user.js +++ b/website/server/controllers/api-v2/user.js @@ -90,7 +90,6 @@ api.score = function(req, res, next) { direction = req.params.direction, user = res.locals.user, body = req.body || {}, - taskQuery = { userId: user._id }, task; // Send error responses for improper API call @@ -100,14 +99,23 @@ api.score = function(req, res, next) { return res.json(400, {err: ":direction must be 'up' or 'down'"}); } - if (validator.isUUID(id)) { - taskQuery._id = id; - } else { - taskQuery._legacyId = id; - } + asyncM.waterfall([ + function (cb) { + Tasks.Task.findOne({ + _id: id, + userId: user._id, + }, cb); + }, + function (task, cb) { + if (task) return cb(null, task); - Tasks.Task.findOne(taskQuery, function(err, task){ - if(err) return next(err); + Tasks.Task.findOne({ + _legacyId: id, + userId: user._id, + }, cb); + }, + ], function (err, task) { + if (err) return next(err); // If exists already, score it if (!task) { @@ -199,7 +207,6 @@ api.score = function(req, res, next) { }); }); }); - }; /** @@ -219,16 +226,24 @@ api.getTasks = function(req, res, next) { */ api.getTask = function(req, res, next) { var user = res.locals.user, - id = req.params.id, - taskQuery = { userId: user._id }; + id = req.params.id; - if (validator.isUUID(id)) { - taskQuery._id = id; - } else { - taskQuery._legacyId = id; - } + asyncM.waterfall([ + function (cb) { + Tasks.Task.findOne({ + _id: id, + userId: user._id, + }, cb); + }, + function (task, cb) { + if (task) return cb(null, task); - Tasks.Task.findOne(taskQuery, function (err, task) { + Tasks.Task.findOne({ + _legacyId: id, + userId: user._id, + }, cb); + }, + ], function (err, task) { if (err) return next(err); if (!task) return res.status(404).json({err: shared.i18n.t('messageTaskNotFound')}); res.status(200).json(task.toJSONV2()); @@ -853,13 +868,22 @@ api.updateTask = function(req, res, next) { req.body = Tasks.Task.fromJSONV2(req.body); - if (validator.isUUID(id)) { - taskQuery._id = id; - } else { - taskQuery._legacyId = id; - } + asyncM.waterfall([ + function (cb) { + Tasks.Task.findOne({ + _id: id, + userId: user._id, + }, cb); + }, + function (task, cb) { + if (task) return cb(null, task); - Tasks.Task.findOne(taskQuery, function(err, task) { + Tasks.Task.findOne({ + _legacyId: id, + userId: user._id, + }, cb); + }, + ], function (err, task) { if(err) return next(err); if(!task) return res.status(404).json({err: 'Task not found.'}) From dba53b85a24e0510cd10c52c0ddf1f14e646fccd Mon Sep 17 00:00:00 2001 From: Blade Barringer Date: Tue, 17 May 2016 17:11:08 -0500 Subject: [PATCH 6/6] refactor: Extract out finding task by id or _legacyId into a function --- website/server/controllers/api-v2/user.js | 71 +++++++---------------- 1 file changed, 22 insertions(+), 49 deletions(-) diff --git a/website/server/controllers/api-v2/user.js b/website/server/controllers/api-v2/user.js index 0941d9d3af..323155a5bd 100644 --- a/website/server/controllers/api-v2/user.js +++ b/website/server/controllers/api-v2/user.js @@ -80,6 +80,25 @@ var findTask = function(req, res) { return res.locals.user.tasks[req.params.id]; }; +function findTaskByIdOrLegacyId (user, taskId, callback) { + asyncM.waterfall([ + function (cb) { + Tasks.Task.findOne({ + _id: taskId, + userId: user._id, + }, cb); + }, + function (task, cb) { + if (task) return cb(null, task); + + Tasks.Task.findOne({ + _legacyId: taskId, + userId: user._id, + }, cb); + }, + ], callback); +} + /* API Routes --------------- @@ -99,22 +118,7 @@ api.score = function(req, res, next) { return res.json(400, {err: ":direction must be 'up' or 'down'"}); } - asyncM.waterfall([ - function (cb) { - Tasks.Task.findOne({ - _id: id, - userId: user._id, - }, cb); - }, - function (task, cb) { - if (task) return cb(null, task); - - Tasks.Task.findOne({ - _legacyId: id, - userId: user._id, - }, cb); - }, - ], function (err, task) { + findTaskByIdOrLegacyId(user, id, function (err, task) { if (err) return next(err); // If exists already, score it @@ -228,22 +232,7 @@ api.getTask = function(req, res, next) { var user = res.locals.user, id = req.params.id; - asyncM.waterfall([ - function (cb) { - Tasks.Task.findOne({ - _id: id, - userId: user._id, - }, cb); - }, - function (task, cb) { - if (task) return cb(null, task); - - Tasks.Task.findOne({ - _legacyId: id, - userId: user._id, - }, cb); - }, - ], function (err, task) { + findTaskByIdOrLegacyId(user, id, function (err, task) { if (err) return next(err); if (!task) return res.status(404).json({err: shared.i18n.t('messageTaskNotFound')}); res.status(200).json(task.toJSONV2()); @@ -863,27 +852,11 @@ api.deleteTask = function(req, res, next) { api.updateTask = function(req, res, next) { var user = res.locals.user, - taskQuery = { userId: user._id }, id = req.params.id; req.body = Tasks.Task.fromJSONV2(req.body); - asyncM.waterfall([ - function (cb) { - Tasks.Task.findOne({ - _id: id, - userId: user._id, - }, cb); - }, - function (task, cb) { - if (task) return cb(null, task); - - Tasks.Task.findOne({ - _legacyId: id, - userId: user._id, - }, cb); - }, - ], function (err, task) { + findTaskByIdOrLegacyId(user, id, function (err, task) { if(err) return next(err); if(!task) return res.status(404).json({err: 'Task not found.'})