From 382e391fd0f95bc88faef69353cd2dac3ffe7060 Mon Sep 17 00:00:00 2001 From: Matteo Pagliazzi Date: Sun, 3 Apr 2016 21:50:32 +0200 Subject: [PATCH] port updateTask, addTask, clearCompleted, taskDefaults, uuid --- common/script/index.js | 4 + common/script/libs/taskDefaults.js | 83 ++++++++------- common/script/libs/uuid.js | 13 +-- common/script/ops/addTask.js | 23 ++-- common/script/ops/clearCompleted.js | 14 ++- common/script/ops/updateTask.js | 35 +++--- package.json | 4 +- tasks/gulp-eslint.js | 5 - test/common/ops/addTask.js | 135 ++++++++++++++++++++++++ test/common/ops/clearCompleted.js | 37 +++++++ test/common/ops/updateTask.js | 53 ++++++++++ website/src/controllers/api-v3/tasks.js | 9 +- website/src/libs/api-v3/baseModel.js | 2 +- website/src/models/task.js | 4 +- 14 files changed, 322 insertions(+), 99 deletions(-) create mode 100644 test/common/ops/addTask.js create mode 100644 test/common/ops/clearCompleted.js create mode 100644 test/common/ops/updateTask.js diff --git a/common/script/index.js b/common/script/index.js index 7482a09fd7..2efe3371cf 100644 --- a/common/script/index.js +++ b/common/script/index.js @@ -121,6 +121,8 @@ import openMysteryItem from './ops/openMysteryItem'; import releasePets from './ops/releasePets'; import releaseBoth from './ops/releaseBoth'; import releaseMounts from './ops/releaseMounts'; +import updateTask from './ops/updateTask'; +import clearCompleted from './ops/clearCompleted'; api.ops = { scoreTask, @@ -143,6 +145,8 @@ api.ops = { releasePets, releaseBoth, releaseMounts, + updateTask, + clearCompleted, }; import handleTwoHanded from './fns/handleTwoHanded'; diff --git a/common/script/libs/taskDefaults.js b/common/script/libs/taskDefaults.js index 69c815e4fd..cb14115bca 100644 --- a/common/script/libs/taskDefaults.js +++ b/common/script/libs/taskDefaults.js @@ -1,71 +1,74 @@ -import uuid from './uuid'; +import { v4 as uuid } from 'uuid'; import _ from 'lodash'; +import moment from 'moment'; -/* -Even though Mongoose handles task defaults, we want to make sure defaults are set on the client-side before -sending up to the server for performance - */ +// Even though Mongoose handles task defaults, we want to make sure defaults are set on the client-side before +// sending up to the server for performance -// TODO revisit +// TODO move to client code? -module.exports = function(task) { - var defaults, ref, ref1, ref2; - if (task == null) { - task = {}; - } - if (!(task.type && ((ref = task.type) === 'habit' || ref === 'daily' || ref === 'todo' || ref === 'reward'))) { +const tasksTypes = ['habit', 'daily', 'todo', 'reward']; + +module.exports = function taskDefaults (task = {}) { + if (!task.type || tasksTypes.indexOf(task.type) === -1) { task.type = 'habit'; } - defaults = { - id: uuid(), - text: task.id != null ? task.id : '', + + let defaultId = uuid(); + let defaults = { + _id: defaultId, // TODO convert all occurencies of id to _id + text: task._id || defaultId, notes: '', + tags: [], + value: task.type === 'reward' ? 10 : 0, priority: 1, challenge: {}, + reminders: {}, attribute: 'str', - dateCreated: new Date() + createdAt: new Date(), // TODO these are going to be overwritten by the server... + updatedAt: new Date(), }; + _.defaults(task, defaults); + + if (task.type === 'habit' || task.type === 'daily') { + _.defaults(task, { + history: [], + }); + } + + if (task.type === 'todo' || task.type === 'daily') { + _.defaults(task, { + completed: false, + collapseChecklist: false, + checklist: [], + }); + } + if (task.type === 'habit') { _.defaults(task, { up: true, - down: true - }); - } - if ((ref1 = task.type) === 'habit' || ref1 === 'daily') { - _.defaults(task, { - history: [] - }); - } - if ((ref2 = task.type) === 'daily' || ref2 === 'todo') { - _.defaults(task, { - completed: false + down: true, }); } + if (task.type === 'daily') { _.defaults(task, { streak: 0, repeat: { - su: true, m: true, t: true, w: true, th: true, f: true, - s: true - } - }, { - startDate: new Date(), + s: true, + su: true, + }, + startDate: moment().startOf('day').toDate(), everyX: 1, - frequency: 'weekly' + frequency: 'weekly', }); } - task._id = task.id; - if (task.value == null) { - task.value = task.type === 'reward' ? 10 : 0; - } - if (!_.isNumber(task.priority)) { - task.priority = 1; - } + return task; }; diff --git a/common/script/libs/uuid.js b/common/script/libs/uuid.js index 4a26440d7d..63f75cf398 100644 --- a/common/script/libs/uuid.js +++ b/common/script/libs/uuid.js @@ -1,9 +1,4 @@ -// TODO use node-uuid module -module.exports = function() { - return "xxxxxxxx-xxxx-4xxx-yxxx-xxxxxxxxxxxx".replace(/[xy]/g, function(c) { - var r, v; - r = Math.random() * 16 | 0; - v = (c === "x" ? r : r & 0x3 | 0x8); - return v.toString(16); - }); -}; +import uuid from 'uuid'; + +// TODO remove this file completely +module.exports = uuid.v4; diff --git a/common/script/ops/addTask.js b/common/script/ops/addTask.js index a2d1895e1e..81f642ab02 100644 --- a/common/script/ops/addTask.js +++ b/common/script/ops/addTask.js @@ -1,27 +1,22 @@ import taskDefaults from '../libs/taskDefaults'; -import i18n from '../i18n'; -module.exports = function(user, req, cb) { - var task; - task = taskDefaults(req.body); - if (user.tasks[task.id] != null) { - return typeof cb === "function" ? cb({ - code: 409, - message: i18n.t('messageDuplicateTaskID', req.language) - }) : void 0; - } - user[task.type + "s"].unshift(task); +// TODO move to client since it's only used there? + +module.exports = function addTask (user, req = {body: {}}) { + let task = taskDefaults(req.body); + user.tasksOrder[`${task.type}s`].unshift(task._id); + if (user.preferences.newTaskEdit) { task._editing = true; } + if (user.preferences.tagsCollapsed) { task._tags = true; } + if (!user.preferences.advancedCollapsed) { task._advanced = true; } - if (typeof cb === "function") { - cb(null, task); - } + return task; }; diff --git a/common/script/ops/clearCompleted.js b/common/script/ops/clearCompleted.js index d60f12704f..26fb1727d9 100644 --- a/common/script/ops/clearCompleted.js +++ b/common/script/ops/clearCompleted.js @@ -1,12 +1,10 @@ import _ from 'lodash'; -module.exports = function(user, req, cb) { - _.remove(user.todos, function(t) { - var ref; - return t.completed && !((ref = t.challenge) != null ? ref.id : void 0); +// TODO move to client since it's only used there? +// TODO rename file to clearCompletedTodos + +module.exports = function clearCompletedTodos (todos) { + _.remove(todos, todo => { + return todo.completed && (!todo.challenge || !todo.challenge.id || todo.challenge.broken); }); - if (typeof user.markModified === "function") { - user.markModified('todos'); - } - return typeof cb === "function" ? cb(null, user.todos) : void 0; }; diff --git a/common/script/ops/updateTask.js b/common/script/ops/updateTask.js index 427104f15e..a128b40fc1 100644 --- a/common/script/ops/updateTask.js +++ b/common/script/ops/updateTask.js @@ -1,23 +1,26 @@ -import i18n from '../i18n'; import _ from 'lodash'; -module.exports = function(user, req, cb) { - var ref, task; - if (!(task = user.tasks[(ref = req.params) != null ? ref.id : void 0])) { - return typeof cb === "function" ? cb({ - code: 404, - message: i18n.t('messageTaskNotFound', req.language) - }) : void 0; - } - _.merge(task, _.omit(req.body, ['checklist', 'reminders', 'id', 'type'])); - if (req.body.checklist) { - task.checklist = req.body.checklist; - } +// From server pass task.toObject() not the task document directly +module.exports = function updateTask (task, req = {}) { + // If reminders are updated -> replace the original ones if (req.body.reminders) { task.reminders = req.body.reminders; + delete req.body.reminders; } - if (typeof task.markModified === "function") { - task.markModified('tags'); + + // If checklist is updated -> replace the original one + if (req.body.checklist) { + task.checklist = req.body.checklist; + delete req.body.checklist; } - return typeof cb === "function" ? cb(null, task) : void 0; + + // If tags are updated -> replace the original ones + if (req.body.tags) { + task.tags = req.body.tags; + delete req.body.tags; + } + + _.merge(task, _.omit(req.body, ['_id', 'id', 'type'])); + + return task; }; diff --git a/package.json b/package.json index 85f9d207f3..2279d48fe0 100644 --- a/package.json +++ b/package.json @@ -90,7 +90,8 @@ "validator": "~4.2.1", "vinyl-buffer": "^1.0.0", "vinyl-source-stream": "^1.1.0", - "winston": "^2.1.0" + "winston": "^2.1.0", + "uuid": "^2.0.1" }, "private": true, "engines": { @@ -153,7 +154,6 @@ "sinon": "^1.17.2", "sinon-chai": "^2.8.0", "superagent-defaults": "^0.1.13", - "uuid": "^2.0.1", "vinyl-source-stream": "^1.0.0", "vinyl-transform": "^1.0.0", "xml2js": "^0.4.16" diff --git a/tasks/gulp-eslint.js b/tasks/gulp-eslint.js index e71cde8258..fc542d4faf 100644 --- a/tasks/gulp-eslint.js +++ b/tasks/gulp-eslint.js @@ -12,10 +12,8 @@ const COMMON_FILES = [ '!./common/script/content/index.js', '!./common/script/ops/addPushDevice.js', '!./common/script/ops/addTag.js', - '!./common/script/ops/addTask.js', '!./common/script/ops/addWebhook.js', '!./common/script/ops/blockUser.js', - '!./common/script/ops/clearCompleted.js', '!./common/script/ops/clearPMs.js', '!./common/script/ops/deletePM.js', '!./common/script/ops/deleteTag.js', @@ -36,7 +34,6 @@ const COMMON_FILES = [ '!./common/script/ops/unlock.js', '!./common/script/ops/update.js', '!./common/script/ops/updateTag.js', - '!./common/script/ops/updateTask.js', '!./common/script/ops/updateWebhook.js', '!./common/script/fns/crit.js', '!./common/script/fns/cron.js', @@ -63,8 +60,6 @@ const COMMON_FILES = [ '!./common/script/libs/silver.js', '!./common/script/libs/splitWhitespace.js', '!./common/script/libs/taskClasses.js', - '!./common/script/libs/taskDefaults.js', - '!./common/script/libs/uuid.js', '!./common/script/public/**/*.js', ]; const TEST_FILES = [ diff --git a/test/common/ops/addTask.js b/test/common/ops/addTask.js new file mode 100644 index 0000000000..2d5febe8d2 --- /dev/null +++ b/test/common/ops/addTask.js @@ -0,0 +1,135 @@ +import addTask from '../../../common/script/ops/addTask'; +import { + generateUser, +} from '../../helpers/common.helper'; + +describe('shared.ops.addTask', () => { + let user; + + beforeEach(() => { + user = generateUser(); + }); + + it('adds an habit', () => { + let habit = addTask(user, { + body: { + type: 'habit', + text: 'habit', + down: false, + }, + }); + + expect(user.tasksOrder.habits).to.eql([ + habit._id, + ]); + expect(habit._id).to.be.a('string'); + expect(habit.text).to.equal('habit'); + expect(habit.type).to.equal('habit'); + expect(habit.up).to.equal(true); + expect(habit.down).to.equal(false); + expect(habit.history).to.eql([]); + expect(habit.checklist).to.not.exists; + }); + + it('adds an habtit when type is invalid', () => { + let habit = addTask(user, { + body: { + type: 'invalid', + text: 'habit', + down: false, + }, + }); + + expect(user.tasksOrder.habits).to.eql([ + habit._id, + ]); + expect(habit._id).to.be.a('string'); + expect(habit.text).to.equal('habit'); + expect(habit.type).to.equal('habit'); + expect(habit.up).to.equal(true); + expect(habit.down).to.equal(false); + expect(habit.history).to.eql([]); + expect(habit.checklist).to.not.exists; + }); + + it('adds a daily', () => { + let daily = addTask(user, { + body: { + type: 'daily', + text: 'daily', + }, + }); + + expect(user.tasksOrder.dailys).to.eql([ + daily._id, + ]); + expect(daily._id).to.be.a('string'); + expect(daily.type).to.equal('daily'); + expect(daily.text).to.equal('daily'); + expect(daily.history).to.eql([]); + expect(daily.checklist).to.eql([]); + expect(daily.completed).to.be.false; + expect(daily.up).to.not.exists; + }); + + it('adds a todo', () => { + let todo = addTask(user, { + body: { + type: 'todo', + text: 'todo', + }, + }); + + expect(user.tasksOrder.todos).to.eql([ + todo._id, + ]); + expect(todo._id).to.be.a('string'); + expect(todo.type).to.equal('todo'); + expect(todo.text).to.equal('todo'); + expect(todo.checklist).to.eql([]); + expect(todo.completed).to.be.false; + expect(todo.up).to.not.exists; + }); + + it('adds a reward', () => { + let reward = addTask(user, { + body: { + type: 'reward', + text: 'reward', + }, + }); + + expect(user.tasksOrder.rewards).to.eql([ + reward._id, + ]); + expect(reward._id).to.be.a('string'); + expect(reward.type).to.equal('reward'); + expect(reward.text).to.equal('reward'); + expect(reward.value).to.equal(10); + expect(reward.up).to.not.exists; + }); + + context('respects preferences', () => { + it('true', () => { + user.preferences.newTaskEdit = true; + user.preferences.tagsCollapsed = true; + user.preferences.advancedCollapsed = false; + let task = addTask(user); + + expect(task._editing).to.be.true; + expect(task._tags).to.be.true; + expect(task._advanced).to.be.true; + }); + + it('false', () => { + user.preferences.newTaskEdit = false; + user.preferences.tagsCollapsed = false; + user.preferences.advancedCollapsed = true; + let task = addTask(user); + + expect(task._editing).to.not.exists; + expect(task._tags).to.not.exists; + expect(task._advanced).to.not.exists; + }); + }); +}); diff --git a/test/common/ops/clearCompleted.js b/test/common/ops/clearCompleted.js new file mode 100644 index 0000000000..4dceb3c1f0 --- /dev/null +++ b/test/common/ops/clearCompleted.js @@ -0,0 +1,37 @@ +import clearCompleted from '../../../common/script/ops/clearCompleted'; +import { + generateTodo, +} from '../../helpers/common.helper'; + +describe('shared.ops.clearCompleted', () => { + it('clear completed todos', () => { + let todos = [ + generateTodo({text: 'todo'}), + generateTodo({ + text: 'done', + completed: true, + }), + generateTodo({ + text: 'done chellenge broken', + completed: true, + challenge: { + id: 123, + broken: 'TASK_DELETED', + }, + }), + generateTodo({ + text: 'done chellenge not broken', + completed: true, + challenge: { + id: 123, + }, + }), + ]; + + clearCompleted(todos); + + expect(todos.length).to.equal(2); + expect(todos[0].text).to.equal('todo'); + expect(todos[1].text).to.equal('done chellenge not broken'); + }); +}); diff --git a/test/common/ops/updateTask.js b/test/common/ops/updateTask.js new file mode 100644 index 0000000000..834aa0c419 --- /dev/null +++ b/test/common/ops/updateTask.js @@ -0,0 +1,53 @@ +import updateTask from '../../../common/script/ops/updateTask'; +import { + generateHabit, +} from '../../helpers/common.helper'; + +describe('shared.ops.updateTask', () => { + it('updates a task', () => { + let now = new Date(); + let habit = generateHabit({ + tags: [ + '123', + '456', + ], + + reminders: [{ + _id: '123', + startDate: now, + time: now, + }], + }); + + let res = updateTask(habit, { + body: { + text: 'updated', + id: '123', + _id: '123', + type: 'todo', + tags: ['678'], + checklist: [{ + completed: false, + text: 'item', + _id: '123', + }], + }, + }); + + expect(res.id).to.not.equal('123'); + expect(res._id).to.not.equal('123'); + expect(res.type).to.equal('habit'); + expect(res.text).to.equal('updated'); + expect(res.checklist).to.eql([{ + completed: false, + text: 'item', + _id: '123', + }]); + expect(res.reminders).to.eql([{ + _id: '123', + startDate: now, + time: now, + }]); + expect(res.tags).to.eql(['678']); + }); +}); diff --git a/website/src/controllers/api-v3/tasks.js b/website/src/controllers/api-v3/tasks.js index f931283102..527bb90265 100644 --- a/website/src/controllers/api-v3/tasks.js +++ b/website/src/controllers/api-v3/tasks.js @@ -324,7 +324,7 @@ api.updateTask = { // TODO we have to convert task to an object because otherwise things don't get merged correctly. Bad for performances? // TODO regarding comment above, make sure other models with nested fields are using this trick too - _.assign(task, _.merge(task.toObject(), Tasks.Task.sanitizeUpdate(req.body))); + _.assign(task, common.ops.updateTask(task.toObject(), req)); // TODO console.log(task.modifiedPaths(), task.toObject().repeat === tep) // repeat is always among modifiedPaths because mongoose changes the other of the keys when using .toObject() // see https://github.com/Automattic/mongoose/issues/2749 @@ -836,12 +836,15 @@ api.clearCompletedTodos = { let user = res.locals.user; // Clear completed todos - // Do not delete challenges completed todos TODO unless the task is broken? + // Do not delete challenges completed todos unless the task is broken await Tasks.Task.remove({ userId: user._id, type: 'todo', completed: true, - 'challenge.id': {$exists: false}, + $or: [ + {'challenge.id': {$exists: false}}, + {'challenge.broken': {$exists: true}}, + ], }).exec(); res.respond(200, {}); diff --git a/website/src/libs/api-v3/baseModel.js b/website/src/libs/api-v3/baseModel.js index e1b69216b6..878bebfa14 100644 --- a/website/src/libs/api-v3/baseModel.js +++ b/website/src/libs/api-v3/baseModel.js @@ -1,4 +1,4 @@ -import { uuid } from '../../../../common'; +import { v4 as uuid } from 'uuid'; import validator from 'validator'; import objectPath from 'object-path'; // TODO use lodash's unset once v4 is out import _ from 'lodash'; diff --git a/website/src/models/task.js b/website/src/models/task.js index 30c68969c4..66cd0ef524 100644 --- a/website/src/models/task.js +++ b/website/src/models/task.js @@ -14,6 +14,8 @@ let subDiscriminatorOptions = _.defaults(_.cloneDeep(discriminatorOptions), {_id 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({ type: {type: String, enum: tasksTypes, required: true, default: tasksTypes[0]}, text: {type: String, required: true}, @@ -35,7 +37,7 @@ export let TaskSchema = new Schema({ }, reminders: [{ - id: {type: String, validate: [validator.isUUID, 'Invalid uuid.'], default: shared.uuid, required: true}, + _id: {type: String, validate: [validator.isUUID, 'Invalid uuid.'], default: shared.uuid, required: true}, startDate: {type: Date, required: true}, time: {type: Date, required: true}, }],