From 7c0b3612f09a8637471ecb34aee9e8ae97cc98b0 Mon Sep 17 00:00:00 2001 From: Shadow Date: Thu, 24 Dec 2020 11:37:09 -0600 Subject: [PATCH] Change $type of date for todos (#12779) * change $type to date for task and add new test * adjust apidocs to reflect type change * migration test for api date $type change * minor fixes to migration * unset instead of set empty string * add type filter * fix(todo date migration): make sure the update command works and limit update ops Co-authored-by: Matteo Pagliazzi --- migrations/archive/2020/20201111_api_date.js | 61 +++++++++++++++++++ .../integration/tasks/POST-tasks_user.test.js | 12 ++++ website/server/controllers/api-v3/tasks.js | 6 +- website/server/models/task.js | 3 +- 4 files changed, 77 insertions(+), 5 deletions(-) create mode 100644 migrations/archive/2020/20201111_api_date.js diff --git a/migrations/archive/2020/20201111_api_date.js b/migrations/archive/2020/20201111_api_date.js new file mode 100644 index 0000000000..c3b65657af --- /dev/null +++ b/migrations/archive/2020/20201111_api_date.js @@ -0,0 +1,61 @@ +/* + * Fix dates in the database that were stored as $type string instead of Date + */ +/* eslint-disable no-console */ + +const MIGRATION_NAME = '20201111_api_date'; + +import * as Tasks from '../../../website/server/models/task'; + +const progressCount = 1000; +let count = 0; + +async function updateUser (todo) { + count++; + + if (count % progressCount === 0) console.warn(`${count} ${todo._id}`); + + const newDate = new Date(todo.date); + if (isValidDate(newDate)) return; + + return await Tasks.Task.update({_id: todo._id, type: 'todo'}, {$unset: {date: ''}}).exec(); +} + +module.exports = async function processUsers () { + let query = { + type: 'todo', + date: {$exists: true} + }; + + const fields = { + _id: 1, + type: 1, + date: 1, + }; + + while (true) { // eslint-disable-line no-constant-condition + const users = await Tasks.Task // eslint-disable-line no-await-in-loop + .find(query) + .select(fields) + .limit(250) + .sort({_id: 1}) + .lean() + .exec(); + + if (users.length === 0) { + console.warn('All appropriate tasks found and modified.'); + console.warn(`\n${count} tasks processed\n`); + break; + } else { + query._id = { + $gt: users[users.length - 1], + }; + } + + await Promise.all(users.map(updateUser)); // eslint-disable-line no-await-in-loop + } +}; + +function isValidDate(d) { + return !isNaN(d.getTime()); +} 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 4de093022c..8ffbce920a 100644 --- a/test/api/v3/integration/tasks/POST-tasks_user.test.js +++ b/test/api/v3/integration/tasks/POST-tasks_user.test.js @@ -220,6 +220,18 @@ describe('POST /tasks/user', () => { }); }); + it('errors if todo due date supplied is an invalid date', async () => { + await expect(user.post('/tasks/user', { + type: 'todo', + text: 'todo text', + date: 'invalid date', + })).to.eventually.be.rejected.and.eql({ + code: 400, + error: 'BadRequest', + message: 'todo validation failed', + }); + }); + context('sending task activity webhooks', () => { before(async () => { await server.start(); diff --git a/website/server/controllers/api-v3/tasks.js b/website/server/controllers/api-v3/tasks.js index 3815346dc2..8e7d1fe663 100644 --- a/website/server/controllers/api-v3/tasks.js +++ b/website/server/controllers/api-v3/tasks.js @@ -80,7 +80,7 @@ const requiredGroupFields = '_id leader tasksOrder name'; * "per", "con" * @apiParam (Body) {Boolean} [collapseChecklist=false] Determines if a checklist will be displayed * @apiParam (Body) {String} [notes] Extra notes - * @apiParam (Body) {String} [date] Due date to be shown in task list. Only valid for type "todo." + * @apiParam (Body) {Date} [date] Due date to be shown in task list. Only valid for type "todo." * @apiParam (Body) {Number="0.1","1","1.5","2"} [priority=1] Difficulty, options are 0.1, 1, * 1.5, 2; eqivalent of Trivial, * Easy, Medium, Hard. @@ -253,7 +253,7 @@ api.createUserTasks = { * "int", "per", "con". * @apiParam (Body) {Boolean} [collapseChecklist=false] Determines if a checklist will be displayed * @apiParam (Body) {String} [notes] Extra notes - * @apiParam (Body) {String} [date] Due date to be shown in task list. Only valid for type "todo." + * @apiParam (Body) {Date} [date] Due date to be shown in task list. Only valid for type "todo." * @apiParam (Body) {Number="0.1","1","1.5","2"} [priority=1] Difficulty, options are 0.1, 1, * 1.5, 2; eqivalent of Trivial, * Easy, Medium, Hard. @@ -567,7 +567,7 @@ api.getTask = { * "per", "con". * @apiParam (Body) {Boolean} [collapseChecklist=false] Determines if a checklist will be displayed * @apiParam (Body) {String} [notes] Extra notes - * @apiParam (Body) {String} [date] Due date to be shown in task list. Only valid for type "todo." + * @apiParam (Body) {Date} [date] Due date to be shown in task list. Only valid for type "todo." * @apiParam (Body) {Number="0.1","1","1.5","2"} [priority=1] Difficulty, options are 0.1, 1, * 1.5, 2; eqivalent of Trivial, * Easy, Medium, Hard. diff --git a/website/server/models/task.js b/website/server/models/task.js index 96cbeec00b..b948eaa441 100644 --- a/website/server/models/task.js +++ b/website/server/models/task.js @@ -404,8 +404,7 @@ export const daily = Task.discriminator('daily', DailySchema); export const TodoSchema = new Schema(_.defaults({ dateCompleted: Date, - // TODO we're getting parse errors, people have stored as "today" and "3/13". Need to run a migration & put this back to $type: Date see http://stackoverflow.com/questions/1353684/detecting-an-invalid-date-date-instance-in-javascript - date: String, // due date for todos + date: Date, // due date for todos }, dailyTodoSchema()), subDiscriminatorOptions); export const todo = Task.discriminator('todo', TodoSchema);