From d25cb70f66f8bc6874c7e9b37afba158930619d1 Mon Sep 17 00:00:00 2001
From: Matteo Pagliazzi <matteopagliazzi@gmail.com>
Date: Mon, 4 Jan 2016 19:44:39 +0100
Subject: [PATCH 01/10] 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);

From b73f5a8f402d8dd70e5afe18b7d2b6b893b10336 Mon Sep 17 00:00:00 2001
From: Matteo Pagliazzi <matteopagliazzi@gmail.com>
Date: Mon, 4 Jan 2016 19:57:20 +0100
Subject: [PATCH 02/10] add support for getting single challenges tasks

---
 website/src/controllers/api-v3/tasks.js | 14 ++++++++++++--
 website/src/models/challenge.js         |  5 ++++-
 2 files changed, 16 insertions(+), 3 deletions(-)

diff --git a/website/src/controllers/api-v3/tasks.js b/website/src/controllers/api-v3/tasks.js
index 676be05bf5..45339dc49a 100644
--- a/website/src/controllers/api-v3/tasks.js
+++ b/website/src/controllers/api-v3/tasks.js
@@ -120,7 +120,7 @@ api.getTasks = {
     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();
+      challenge = await Challenge.findOne({_id: challengeId}).select('leader').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
@@ -188,10 +188,20 @@ api.getTask = {
 
     let task = await Tasks.Task.findOne({
       _id: req.params.taskId,
-      userId: user._id,
     }).exec();
 
     if (!task) throw new NotFound(res.t('taskNotFound'));
+
+    // If the task belongs to a challenge make sure the user has rights
+    if (!task.userId) {
+      let challenge = await Challenge.find().selec({_id: task.challenge.id}).select('leader').exec();
+      if (user.challenges.indexOf(task.challenge.id) === -1 && challenge.leader !== user._id && !user.contributor.admin) {
+        throw new NotFound(res.t('taskNotFound'));
+      }
+    } else if (task.userId !== user._id) { // If the task is owned by an user make it's the current one
+      throw new NotFound(res.t('taskNotFound'));
+    }
+
     res.respond(200, task);
   },
 };
diff --git a/website/src/models/challenge.js b/website/src/models/challenge.js
index d3bf4550a3..6e7e179047 100644
--- a/website/src/models/challenge.js
+++ b/website/src/models/challenge.js
@@ -125,7 +125,10 @@ schema.methods.addTasksToMembers = async function addTasksToMembers (tasks) {
   // 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
+    let updateQ = {$push: {}};
+    tasks.forEach(chalTask => {
+
+    })
     await db.post(doc);
   }
 

From 6680853078aa8a4811d7a430513c894e6e66f0d8 Mon Sep 17 00:00:00 2001
From: Matteo Pagliazzi <matteopagliazzi@gmail.com>
Date: Mon, 4 Jan 2016 20:48:48 +0100
Subject: [PATCH 03/10] finish adding challenges support for tasks (except
 syncing)

---
 common/locales/en/api-v3.json           |  3 +-
 website/src/controllers/api-v3/tasks.js | 94 +++++++++++++++++++------
 2 files changed, 73 insertions(+), 24 deletions(-)

diff --git a/common/locales/en/api-v3.json b/common/locales/en/api-v3.json
index a7c62e3744..77930e4583 100644
--- a/common/locales/en/api-v3.json
+++ b/common/locales/en/api-v3.json
@@ -48,5 +48,6 @@
   "onlyLeaderDeleteChal": "Only the challenge leader can delete it.",
   "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."
+  "userTasksNoChallengeId": "When \"tasksOwner\" is \"user\" \"challengeId\" can't be passed.",
+  "onlyChalLeaderEditTasks": "Tasks belonging to a challenge can only be edited by the leader."
 }
diff --git a/website/src/controllers/api-v3/tasks.js b/website/src/controllers/api-v3/tasks.js
index 45339dc49a..40425c27a3 100644
--- a/website/src/controllers/api-v3/tasks.js
+++ b/website/src/controllers/api-v3/tasks.js
@@ -44,7 +44,8 @@ api.createTask = {
       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'));
+      if (!challenge) throw new NotFound(res.t('challengeNotFound'));
+      if (challenge.leader !== user._id) throw new NotAuthorized(res.t('onlyChalLeaderEditTasks'));
     }
 
     let toSave = tasksData.map(taskData => {
@@ -82,7 +83,7 @@ api.createTask = {
     res.respond(201, tasks);
 
     // If adding tasks to a challenge -> sync users
-    if (challenge) challenge.addTasksToMembers(tasks); // TODO catch/log
+    if (challenge) challenge.addTasks(tasks); // TODO catch/log
   },
 };
 
@@ -190,12 +191,11 @@ api.getTask = {
       _id: req.params.taskId,
     }).exec();
 
-    if (!task) throw new NotFound(res.t('taskNotFound'));
-
-    // If the task belongs to a challenge make sure the user has rights
-    if (!task.userId) {
+    if (!task) {
+      throw new NotFound(res.t('taskNotFound'));
+    } else if (!task.userId) { // If the task belongs to a challenge make sure the user has rights
       let challenge = await Challenge.find().selec({_id: task.challenge.id}).select('leader').exec();
-      if (user.challenges.indexOf(task.challenge.id) === -1 && challenge.leader !== user._id && !user.contributor.admin) {
+      if (!challenge || (user.challenges.indexOf(task.challenge.id) === -1 && challenge.leader !== user._id && !user.contributor.admin)) { // eslint-disable-line no-extra-parens
         throw new NotFound(res.t('taskNotFound'));
       }
     } else if (task.userId !== user._id) { // If the task is owned by an user make it's the current one
@@ -222,6 +222,7 @@ api.updateTask = {
   middlewares: [authWithHeaders(), cron],
   async handler (req, res) {
     let user = res.locals.user;
+    let challenge;
 
     req.checkParams('taskId', res.t('taskIdRequired')).notEmpty().isUUID();
     // TODO check that req.body isn't empty
@@ -232,10 +233,17 @@ api.updateTask = {
 
     let task = await Tasks.Task.findOne({
       _id: req.params.taskId,
-      userId: user._id,
     }).exec();
 
-    if (!task) throw new NotFound(res.t('taskNotFound'));
+    if (!task) {
+      throw new NotFound(res.t('taskNotFound'));
+    } else if (!task.userId) { // If the task belongs to a challenge make sure the user has rights
+      challenge = await Challenge.find().selec({_id: task.challenge.id}).select('leader').exec();
+      if (!challenge) throw new NotFound(res.t('challengeNotFound'));
+      if (challenge.leader !== user._id) throw new NotAuthorized(res.t('onlyChalLeaderEditTasks'));
+    } else if (task.userId !== user._id) { // If the task is owned by an user make it's the current one
+      throw new NotFound(res.t('taskNotFound'));
+    }
 
     // If checklist is updated -> replace the original one
     if (req.body.checklist) {
@@ -258,6 +266,7 @@ api.updateTask = {
 
     let savedTask = await task.save();
     res.respond(200, savedTask);
+    if (challenge) challenge.updateTask(savedTask); // TODO catch/log
   },
 };
 
@@ -382,6 +391,7 @@ api.scoreTask = {
 
 // completed todos cannot be moved, they'll be returned ordered by date of completion
 // TODO check that it works when a tag is selected or todos are split between dated and due
+// TODO support challenges?
 /**
  * @api {post} /tasks/move/:taskId/to/:position Move a task to a new position
  * @apiVersion 3.0.0
@@ -449,6 +459,7 @@ api.addChecklistItem = {
   middlewares: [authWithHeaders(), cron],
   async handler (req, res) {
     let user = res.locals.user;
+    let challenge;
 
     req.checkParams('taskId', res.t('taskIdRequired')).notEmpty().isUUID();
     // TODO check that req.body isn't empty and is an array
@@ -458,16 +469,25 @@ api.addChecklistItem = {
 
     let task = await Tasks.Task.findOne({
       _id: req.params.taskId,
-      userId: user._id,
     }).exec();
 
-    if (!task) throw new NotFound(res.t('taskNotFound'));
+    if (!task) {
+      throw new NotFound(res.t('taskNotFound'));
+    } else if (!task.userId) { // If the task belongs to a challenge make sure the user has rights
+      challenge = await Challenge.find().selec({_id: task.challenge.id}).select('leader').exec();
+      if (!challenge) throw new NotFound(res.t('challengeNotFound'));
+      if (challenge.leader !== user._id) throw new NotAuthorized(res.t('onlyChalLeaderEditTasks'));
+    } else if (task.userId !== user._id) { // If the task is owned by an user make it's the current one
+      throw new NotFound(res.t('taskNotFound'));
+    }
+
     if (task.type !== 'daily' && task.type !== 'todo') throw new BadRequest(res.t('checklistOnlyDailyTodo'));
 
     task.checklist.push(Tasks.Task.sanitizeChecklist(req.body));
     let savedTask = await task.save();
 
     res.respond(200, savedTask); // TODO what to return
+    if (challenge) challenge.updateTask(savedTask);
   },
 };
 
@@ -530,6 +550,7 @@ api.updateChecklistItem = {
   middlewares: [authWithHeaders(), cron],
   async handler (req, res) {
     let user = res.locals.user;
+    let challenge;
 
     req.checkParams('taskId', res.t('taskIdRequired')).notEmpty().isUUID();
     req.checkParams('itemId', res.t('itemIdRequired')).notEmpty().isUUID();
@@ -539,10 +560,17 @@ api.updateChecklistItem = {
 
     let task = await Tasks.Task.findOne({
       _id: req.params.taskId,
-      userId: user._id,
     }).exec();
 
-    if (!task) throw new NotFound(res.t('taskNotFound'));
+    if (!task) {
+      throw new NotFound(res.t('taskNotFound'));
+    } else if (!task.userId) { // If the task belongs to a challenge make sure the user has rights
+      challenge = await Challenge.find().selec({_id: task.challenge.id}).select('leader').exec();
+      if (!challenge) throw new NotFound(res.t('challengeNotFound'));
+      if (challenge.leader !== user._id) throw new NotAuthorized(res.t('onlyChalLeaderEditTasks'));
+    } else if (task.userId !== user._id) { // If the task is owned by an user make it's the current one
+      throw new NotFound(res.t('taskNotFound'));
+    }
     if (task.type !== 'daily' && task.type !== 'todo') throw new BadRequest(res.t('checklistOnlyDailyTodo'));
 
     let item = _.find(task.checklist, {_id: req.params.itemId});
@@ -552,6 +580,7 @@ api.updateChecklistItem = {
     let savedTask = await task.save();
 
     res.respond(200, savedTask); // TODO what to return
+    if (challenge) challenge.updateTask(savedTask);
   },
 };
 
@@ -572,6 +601,7 @@ api.removeChecklistItem = {
   middlewares: [authWithHeaders(), cron],
   async handler (req, res) {
     let user = res.locals.user;
+    let challenge;
 
     req.checkParams('taskId', res.t('taskIdRequired')).notEmpty().isUUID();
     req.checkParams('itemId', res.t('itemIdRequired')).notEmpty().isUUID();
@@ -581,10 +611,17 @@ api.removeChecklistItem = {
 
     let task = await Tasks.Task.findOne({
       _id: req.params.taskId,
-      userId: user._id,
     }).exec();
 
-    if (!task) throw new NotFound(res.t('taskNotFound'));
+    if (!task) {
+      throw new NotFound(res.t('taskNotFound'));
+    } else if (!task.userId) { // If the task belongs to a challenge make sure the user has rights
+      challenge = await Challenge.find().selec({_id: task.challenge.id}).select('leader').exec();
+      if (!challenge) throw new NotFound(res.t('challengeNotFound'));
+      if (challenge.leader !== user._id) throw new NotAuthorized(res.t('onlyChalLeaderEditTasks'));
+    } else if (task.userId !== user._id) { // If the task is owned by an user make it's the current one
+      throw new NotFound(res.t('taskNotFound'));
+    }
     if (task.type !== 'daily' && task.type !== 'todo') throw new BadRequest(res.t('checklistOnlyDailyTodo'));
 
     let itemI = _.findIndex(task.checklist, {_id: req.params.itemId});
@@ -592,8 +629,9 @@ api.removeChecklistItem = {
 
     task.checklist.splice(itemI, 1);
 
-    await task.save();
+    let savedTask = await task.save();
     res.respond(200, {}); // TODO what to return
+    if (challenge) challenge.updateTask(savedTask);
   },
 };
 
@@ -681,11 +719,11 @@ api.removeTagFromTask = {
   },
 };
 
-// Remove a task from user.tasksOrder
-function _removeTaskTasksOrder (user, taskId) {
+// 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 = user.tasksOrder[`${Tasks.tasksTypes[i]}s`];
+    let list = userOrChallenge.tasksOrder[`${Tasks.tasksTypes[i]}s`];
     let index = list.indexOf(taskId);
 
     if (index !== -1) {
@@ -713,6 +751,7 @@ api.deleteTask = {
   middlewares: [authWithHeaders(), cron],
   async handler (req, res) {
     let user = res.locals.user;
+    let challenge;
 
     req.checkParams('taskId', res.t('taskIdRequired')).notEmpty().isUUID();
 
@@ -721,16 +760,25 @@ api.deleteTask = {
 
     let task = await Tasks.Task.findOne({
       _id: req.params.taskId,
-      userId: user._id,
     }).exec();
 
-    if (!task) throw new NotFound(res.t('taskNotFound'));
-    if (task.challenge.id) throw new NotAuthorized(res.t('cantDeleteChallengeTasks'));
+    if (!task) {
+      throw new NotFound(res.t('taskNotFound'));
+    } else if (!task.userId) { // If the task belongs to a challenge make sure the user has rights
+      challenge = await Challenge.find().selec({_id: task.challenge.id}).select('leader').exec();
+      if (!challenge) throw new NotFound(res.t('challengeNotFound'));
+      if (challenge.leader !== user._id) throw new NotAuthorized(res.t('onlyChalLeaderEditTasks'));
+    } else if (task.userId !== user._id) { // If the task is owned by an user make it's the current one
+      throw new NotFound(res.t('taskNotFound'));
+    } else if (task.userId && task.challenge.id) {
+      throw new NotAuthorized(res.t('cantDeleteChallengeTasks'));
+    }
 
-    _removeTaskTasksOrder(user, req.params.taskId);
+    _removeTaskTasksOrder(challenge || user, req.params.taskId);
     await Q.all([user.save(), task.remove()]);
 
     res.respond(200, {});
+    if (challenge) challenge.removeTask(task);
   },
 };
 

From 3bc8945bcc652a83bc0308dc40c0435ee85f63f9 Mon Sep 17 00:00:00 2001
From: Matteo Pagliazzi <matteopagliazzi@gmail.com>
Date: Mon, 4 Jan 2016 22:03:21 +0100
Subject: [PATCH 04/10] adapt current tests and change urls to avoid conflicts

---
 common/locales/en/api-v3.json                 |  3 +-
 .../integration/tasks/DELETE-tasks_id.test.js |  4 +-
 .../v3/integration/tasks/GET-tasks.test.js    |  8 +-
 .../v3/integration/tasks/GET-tasks_id.test.js |  4 +-
 .../v3/integration/tasks/POST-tasks.test.js   | 76 +++++++++----------
 .../POST-tasks_id_score_direction.test.js     | 14 ++--
 .../v3/integration/tasks/PUT-tasks_id.test.js | 10 +--
 ...LETE-tasks_taskId_checklist_itemId.test.js |  8 +-
 .../POST-tasks_taskId_checklist.test.js       |  6 +-
 ...asks_taskId_checklist_itemId_score.test.js |  8 +-
 .../PUT-tasks_taskId_checklist_itemId.test.js |  8 +-
 .../DELETE-tasks_taskId_tags_tagId.test.js    |  4 +-
 .../tags/POST-tasks_taskId_tags_tagId.test.js |  6 +-
 website/src/controllers/api-v3/tasks.js       | 40 ++++++----
 14 files changed, 105 insertions(+), 94 deletions(-)

diff --git a/common/locales/en/api-v3.json b/common/locales/en/api-v3.json
index 77930e4583..a97308e064 100644
--- a/common/locales/en/api-v3.json
+++ b/common/locales/en/api-v3.json
@@ -49,5 +49,6 @@
   "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.",
-  "onlyChalLeaderEditTasks": "Tasks belonging to a challenge can only be edited by the leader."
+  "onlyChalLeaderEditTasks": "Tasks belonging to a challenge can only be edited by the leader.",
+  "invalidTasksOwner": "\"tasksOwner\" must be \"user\" or \"challenge\"."
 }
diff --git a/test/api/v3/integration/tasks/DELETE-tasks_id.test.js b/test/api/v3/integration/tasks/DELETE-tasks_id.test.js
index 1d3e59ef15..503e484e66 100644
--- a/test/api/v3/integration/tasks/DELETE-tasks_id.test.js
+++ b/test/api/v3/integration/tasks/DELETE-tasks_id.test.js
@@ -16,7 +16,7 @@ describe('DELETE /tasks/:id', () => {
     let task;
 
     beforeEach(() => {
-      return user.post('/tasks', {
+      return user.post('/tasks?tasksOwner=user', {
         text: 'test habit',
         type: 'habit',
       }).then((createdTask) => {
@@ -48,7 +48,7 @@ describe('DELETE /tasks/:id', () => {
     it('cannot delete a task owned by someone else', () => {
       return generateUser()
         .then((anotherUser) => {
-          return anotherUser.post('/tasks', {
+          return anotherUser.post('/tasks?tasksOwner=user', {
             text: 'test habit',
             type: 'habit',
           });
diff --git a/test/api/v3/integration/tasks/GET-tasks.test.js b/test/api/v3/integration/tasks/GET-tasks.test.js
index 3f1c50448d..bf77442ea4 100644
--- a/test/api/v3/integration/tasks/GET-tasks.test.js
+++ b/test/api/v3/integration/tasks/GET-tasks.test.js
@@ -16,14 +16,14 @@ describe('GET /tasks', () => {
   });
 
   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');
+    let createdTasks = await user.post('/tasks?tasksOwner=user', [{text: 'test habit', type: 'habit'}, {text: 'test todo', type: 'todo'}]);
+    let tasks = await user.get('/tasks?tasksOwner=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', 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');
+    let createdTasks = await user.post('/tasks?tasksOwner=user', [{text: 'test habit', type: 'habit'}, {text: 'test todo', type: 'todo'}]);
+    let tasks = await user.get('/tasks?tasksOwner=user&type=habit');
     expect(tasks.length).to.equal(1);
     expect(tasks[0]._id).to.equal(createdTasks[0]._id);
   });
diff --git a/test/api/v3/integration/tasks/GET-tasks_id.test.js b/test/api/v3/integration/tasks/GET-tasks_id.test.js
index a4ae455d6a..b88d8a95a6 100644
--- a/test/api/v3/integration/tasks/GET-tasks_id.test.js
+++ b/test/api/v3/integration/tasks/GET-tasks_id.test.js
@@ -17,7 +17,7 @@ describe('GET /tasks/:id', () => {
     let task;
 
     beforeEach(() => {
-      return user.post('/tasks', {
+      return user.post('/tasks?tasksOwner=user', {
         text: 'test habit',
         type: 'habit',
       }).then((createdTask) => {
@@ -54,7 +54,7 @@ describe('GET /tasks/:id', () => {
         .then((user2) => {
           anotherUser = user2;
 
-          return user.post('/tasks', {
+          return user.post('/tasks?tasksOwner=user', {
             text: 'test habit',
             type: 'habit',
           });
diff --git a/test/api/v3/integration/tasks/POST-tasks.test.js b/test/api/v3/integration/tasks/POST-tasks.test.js
index 245cea7d28..a002119634 100644
--- a/test/api/v3/integration/tasks/POST-tasks.test.js
+++ b/test/api/v3/integration/tasks/POST-tasks.test.js
@@ -14,7 +14,7 @@ describe('POST /tasks', () => {
 
   context('validates params', () => {
     it('returns an error if req.body.type is absent', async () => {
-      return expect(user.post('/tasks', {
+      return expect(user.post('/tasks?tasksOwner=user', {
         notType: 'habit',
       })).to.eventually.be.rejected.and.eql({
         code: 400,
@@ -24,7 +24,7 @@ describe('POST /tasks', () => {
     });
 
     it('returns an error if req.body.type is not valid', async () => {
-      return expect(user.post('/tasks', {
+      return expect(user.post('/tasks?tasksOwner=user', {
         type: 'habitF',
       })).to.eventually.be.rejected.and.eql({
         code: 400,
@@ -34,7 +34,7 @@ describe('POST /tasks', () => {
     });
 
     it('returns an error if one object inside an array is invalid', async () => {
-      return expect(user.post('/tasks', [
+      return expect(user.post('/tasks?tasksOwner=user', [
         {type: 'habitF'},
         {type: 'habit'},
       ])).to.eventually.be.rejected.and.eql({
@@ -45,7 +45,7 @@ describe('POST /tasks', () => {
     });
 
     it('returns an error if req.body.text is absent', async () => {
-      return expect(user.post('/tasks', {
+      return expect(user.post('/tasks?tasksOwner=user', {
         type: 'habit',
       })).to.eventually.be.rejected.and.eql({
         code: 400,
@@ -56,7 +56,7 @@ describe('POST /tasks', () => {
 
     it('does not update user.tasksOrder.{taskType} when the task is not saved because invalid', async () => {
       let originalHabitsOrder = (await user.get('/user')).tasksOrder.habits;
-      return expect(user.post('/tasks', {
+      return expect(user.post('/tasks?tasksOwner=user', {
         type: 'habit',
       })).to.eventually.be.rejected.and.eql({ // this block is necessary
         code: 400,
@@ -71,7 +71,7 @@ describe('POST /tasks', () => {
 
     it('does not update user.tasksOrder.{taskType} when a task inside an array is not saved because invalid', async () => {
       let originalHabitsOrder = (await user.get('/user')).tasksOrder.habits;
-      return expect(user.post('/tasks', [
+      return expect(user.post('/tasks?tasksOwner=user', [
         {type: 'habit'}, // Missing text
         {type: 'habit', text: 'valid'}, // Valid
       ])).to.eventually.be.rejected.and.eql({ // this block is necessary
@@ -86,8 +86,8 @@ describe('POST /tasks', () => {
     });
 
     it('does not save any task sent in an array when 1 is invalid', async () => {
-      let originalTasks = await user.get('/tasks');
-      return expect(user.post('/tasks', [
+      let originalTasks = await user.get('/tasks?tasksOwner=user');
+      return expect(user.post('/tasks?tasksOwner=user', [
         {type: 'habit'}, // Missing text
         {type: 'habit', text: 'valid'}, // Valid
       ])).to.eventually.be.rejected.and.eql({ // this block is necessary
@@ -95,14 +95,14 @@ describe('POST /tasks', () => {
         error: 'BadRequest',
         message: 'habit validation failed',
       }).then(async () => {
-        let updatedTasks = await user.get('/tasks');
+        let updatedTasks = await user.get('/tasks?tasksOwner=user');
 
         expect(updatedTasks).to.eql(originalTasks);
       });
     });
 
     it('automatically sets "task.userId" to user\'s uuid', async () => {
-      let task = await user.post('/tasks', {
+      let task = await user.post('/tasks?tasksOwner=user', {
         text: 'test habit',
         type: 'habit',
       });
@@ -113,7 +113,7 @@ describe('POST /tasks', () => {
     it(`ignores setting userId, history, createdAt,
                         updatedAt, challenge, completed, streak,
                         dateCompleted fields`, async () => {
-      let task = await user.post('/tasks', {
+      let task = await user.post('/tasks?tasksOwner=user', {
         text: 'test daily',
         type: 'daily',
         userId: 123,
@@ -137,7 +137,7 @@ describe('POST /tasks', () => {
     });
 
     it('ignores invalid fields', async () => {
-      let task = await user.post('/tasks', {
+      let task = await user.post('/tasks?tasksOwner=user', {
         text: 'test daily',
         type: 'daily',
         notValid: true,
@@ -149,7 +149,7 @@ describe('POST /tasks', () => {
 
   context('habits', () => {
     it('creates a habit', async () => {
-      let task = await user.post('/tasks', {
+      let task = await user.post('/tasks?tasksOwner=user', {
         text: 'test habit',
         type: 'habit',
         up: false,
@@ -167,7 +167,7 @@ describe('POST /tasks', () => {
 
     it('updates user.tasksOrder.habits when a new habit is created', async () => {
       let originalHabitsOrderLen = (await user.get('/user')).tasksOrder.habits.length;
-      let task = await user.post('/tasks', {
+      let task = await user.post('/tasks?tasksOwner=user', {
         type: 'habit',
         text: 'an habit',
       });
@@ -179,7 +179,7 @@ describe('POST /tasks', () => {
 
     it('updates user.tasksOrder.habits when multiple habits are created', async () => {
       let originalHabitsOrderLen = (await user.get('/user')).tasksOrder.habits.length;
-      let [task, task2] = await user.post('/tasks', [{
+      let [task, task2] = await user.post('/tasks?tasksOwner=user', [{
         type: 'habit',
         text: 'an habit',
       }, {
@@ -194,7 +194,7 @@ describe('POST /tasks', () => {
     });
 
     it('creates multiple habits', async () => {
-      let [task, task2] = await user.post('/tasks', [{
+      let [task, task2] = await user.post('/tasks?tasksOwner=user', [{
         text: 'test habit',
         type: 'habit',
         up: false,
@@ -224,7 +224,7 @@ describe('POST /tasks', () => {
     });
 
     it('defaults to setting up and down to true', async () => {
-      let task = await user.post('/tasks', {
+      let task = await user.post('/tasks?tasksOwner=user', {
         text: 'test habit',
         type: 'habit',
         notes: 1976,
@@ -235,7 +235,7 @@ describe('POST /tasks', () => {
     });
 
     it('cannot create checklists', async () => {
-      let task = await user.post('/tasks', {
+      let task = await user.post('/tasks?tasksOwner=user', {
         text: 'test habit',
         type: 'habit',
         checklist: [
@@ -249,7 +249,7 @@ describe('POST /tasks', () => {
 
   context('todos', () => {
     it('creates a todo', async () => {
-      let task = await user.post('/tasks', {
+      let task = await user.post('/tasks?tasksOwner=user', {
         text: 'test todo',
         type: 'todo',
         notes: 1976,
@@ -262,7 +262,7 @@ describe('POST /tasks', () => {
     });
 
     it('creates multiple todos', async () => {
-      let [task, task2] = await user.post('/tasks', [{
+      let [task, task2] = await user.post('/tasks?tasksOwner=user', [{
         text: 'test todo',
         type: 'todo',
         notes: 1976,
@@ -285,7 +285,7 @@ describe('POST /tasks', () => {
 
     it('updates user.tasksOrder.todos when a new todo is created', async () => {
       let originalTodosOrderLen = (await user.get('/user')).tasksOrder.todos.length;
-      let task = await user.post('/tasks', {
+      let task = await user.post('/tasks?tasksOwner=user', {
         type: 'todo',
         text: 'a todo',
       });
@@ -297,7 +297,7 @@ describe('POST /tasks', () => {
 
     it('updates user.tasksOrder.todos when multiple todos are created', async () => {
       let originalTodosOrderLen = (await user.get('/user')).tasksOrder.todos.length;
-      let [task, task2] = await user.post('/tasks', [{
+      let [task, task2] = await user.post('/tasks?tasksOwner=user', [{
         type: 'todo',
         text: 'a todo',
       }, {
@@ -312,7 +312,7 @@ describe('POST /tasks', () => {
     });
 
     it('can create checklists', async () => {
-      let task = await user.post('/tasks', {
+      let task = await user.post('/tasks?tasksOwner=user', {
         text: 'test todo',
         type: 'todo',
         checklist: [
@@ -333,7 +333,7 @@ describe('POST /tasks', () => {
     it('creates a daily', async () => {
       let now = new Date();
 
-      let task = await user.post('/tasks', {
+      let task = await user.post('/tasks?tasksOwner=user', {
         text: 'test daily',
         type: 'daily',
         notes: 1976,
@@ -352,7 +352,7 @@ describe('POST /tasks', () => {
     });
 
     it('creates multiple dailys', async () => {
-      let [task, task2] = await user.post('/tasks', [{
+      let [task, task2] = await user.post('/tasks?tasksOwner=user', [{
         text: 'test daily',
         type: 'daily',
         notes: 1976,
@@ -375,7 +375,7 @@ describe('POST /tasks', () => {
 
     it('updates user.tasksOrder.dailys when a new daily is created', async () => {
       let originalDailysOrderLen = (await user.get('/user')).tasksOrder.dailys.length;
-      let task = await user.post('/tasks', {
+      let task = await user.post('/tasks?tasksOwner=user', {
         type: 'daily',
         text: 'a daily',
       });
@@ -387,7 +387,7 @@ describe('POST /tasks', () => {
 
     it('updates user.tasksOrder.dailys when multiple dailys are created', async () => {
       let originalDailysOrderLen = (await user.get('/user')).tasksOrder.dailys.length;
-      let [task, task2] = await user.post('/tasks', [{
+      let [task, task2] = await user.post('/tasks?tasksOwner=user', [{
         type: 'daily',
         text: 'a daily',
       }, {
@@ -402,7 +402,7 @@ describe('POST /tasks', () => {
     });
 
     it('defaults to a weekly frequency, with every day set', async () => {
-      let task = await user.post('/tasks', {
+      let task = await user.post('/tasks?tasksOwner=user', {
         text: 'test daily',
         type: 'daily',
       });
@@ -421,7 +421,7 @@ describe('POST /tasks', () => {
     });
 
     it('allows repeat field to be configured', async () => {
-      let task = await user.post('/tasks', {
+      let task = await user.post('/tasks?tasksOwner=user', {
         text: 'test daily',
         type: 'daily',
         repeat: {
@@ -445,7 +445,7 @@ describe('POST /tasks', () => {
     it('defaults startDate to today', async () => {
       let today = (new Date()).getDay();
 
-      let task = await user.post('/tasks', {
+      let task = await user.post('/tasks?tasksOwner=user', {
         text: 'test daily',
         type: 'daily',
       });
@@ -454,7 +454,7 @@ describe('POST /tasks', () => {
     });
 
     it('can create checklists', async () => {
-      let task = await user.post('/tasks', {
+      let task = await user.post('/tasks?tasksOwner=user', {
         text: 'test daily',
         type: 'daily',
         checklist: [
@@ -473,7 +473,7 @@ describe('POST /tasks', () => {
 
   context('rewards', () => {
     it('creates a reward', async () => {
-      let task = await user.post('/tasks', {
+      let task = await user.post('/tasks?tasksOwner=user', {
         text: 'test reward',
         type: 'reward',
         notes: 1976,
@@ -488,7 +488,7 @@ describe('POST /tasks', () => {
     });
 
     it('creates multiple rewards', async () => {
-      let [task, task2] = await user.post('/tasks', [{
+      let [task, task2] = await user.post('/tasks?tasksOwner=user', [{
         text: 'test reward',
         type: 'reward',
         notes: 1976,
@@ -515,7 +515,7 @@ describe('POST /tasks', () => {
 
     it('updates user.tasksOrder.rewards when a new reward is created', async () => {
       let originalRewardsOrderLen = (await user.get('/user')).tasksOrder.rewards.length;
-      let task = await user.post('/tasks', {
+      let task = await user.post('/tasks?tasksOwner=user', {
         type: 'reward',
         text: 'a reward',
       });
@@ -527,7 +527,7 @@ describe('POST /tasks', () => {
 
     it('updates user.tasksOrder.dreward when multiple rewards are created', async () => {
       let originalRewardsOrderLen = (await user.get('/user')).tasksOrder.rewards.length;
-      let [task, task2] = await user.post('/tasks', [{
+      let [task, task2] = await user.post('/tasks?tasksOwner=user', [{
         type: 'reward',
         text: 'a reward',
       }, {
@@ -542,7 +542,7 @@ describe('POST /tasks', () => {
     });
 
     it('defaults to a 0 value', async () => {
-      let task = await user.post('/tasks', {
+      let task = await user.post('/tasks?tasksOwner=user', {
         text: 'test reward',
         type: 'reward',
       });
@@ -551,7 +551,7 @@ describe('POST /tasks', () => {
     });
 
     it('requires value to be coerced into a number', async () => {
-      let task = await user.post('/tasks', {
+      let task = await user.post('/tasks?tasksOwner=user', {
         text: 'test reward',
         type: 'reward',
         value: '10',
@@ -561,7 +561,7 @@ describe('POST /tasks', () => {
     });
 
     it('cannot create checklists', async () => {
-      let task = await user.post('/tasks', {
+      let task = await user.post('/tasks?tasksOwner=user', {
         text: 'test reward',
         type: 'reward',
         checklist: [
diff --git a/test/api/v3/integration/tasks/POST-tasks_id_score_direction.test.js b/test/api/v3/integration/tasks/POST-tasks_id_score_direction.test.js
index 2978d93657..1c552613b4 100644
--- a/test/api/v3/integration/tasks/POST-tasks_id_score_direction.test.js
+++ b/test/api/v3/integration/tasks/POST-tasks_id_score_direction.test.js
@@ -37,7 +37,7 @@ describe('POST /tasks/:id/score/:direction', () => {
     let todo;
 
     beforeEach(() => {
-      return user.post('/tasks', {
+      return user.post('/tasks?tasksOwner=user', {
         text: 'test todo',
         type: 'todo',
       }).then((task) => {
@@ -149,7 +149,7 @@ describe('POST /tasks/:id/score/:direction', () => {
     let daily;
 
     beforeEach(() => {
-      return user.post('/tasks', {
+      return user.post('/tasks?tasksOwner=user', {
         text: 'test daily',
         type: 'daily',
       }).then((task) => {
@@ -226,26 +226,26 @@ describe('POST /tasks/:id/score/:direction', () => {
     let habit, minusHabit, plusHabit, neitherHabit; // eslint-disable-line no-unused-vars
 
     beforeEach(() => {
-      return user.post('/tasks', {
+      return user.post('/tasks?tasksOwner=user', {
         text: 'test habit',
         type: 'habit',
       }).then((task) => {
         habit = task;
-        return user.post('/tasks', {
+        return user.post('/tasks?tasksOwner=user', {
           text: 'test min habit',
           type: 'habit',
           up: false,
         });
       }).then((task) => {
         minusHabit = task;
-        return user.post('/tasks', {
+        return user.post('/tasks?tasksOwner=user', {
           text: 'test plus habit',
           type: 'habit',
           down: false,
         });
       }).then((task) => {
         plusHabit = task;
-        user.post('/tasks', {
+        user.post('/tasks?tasksOwner=user', {
           text: 'test neither habit',
           type: 'habit',
           up: false,
@@ -297,7 +297,7 @@ describe('POST /tasks/:id/score/:direction', () => {
     let reward;
 
     beforeEach(() => {
-      return user.post('/tasks', {
+      return user.post('/tasks?tasksOwner=user', {
         text: 'test reward',
         type: 'reward',
         value: 5,
diff --git a/test/api/v3/integration/tasks/PUT-tasks_id.test.js b/test/api/v3/integration/tasks/PUT-tasks_id.test.js
index 3092d3bd84..408ef92e99 100644
--- a/test/api/v3/integration/tasks/PUT-tasks_id.test.js
+++ b/test/api/v3/integration/tasks/PUT-tasks_id.test.js
@@ -16,7 +16,7 @@ describe('PUT /tasks/:id', () => {
     let task;
 
     beforeEach(() => {
-      return user.post('/tasks', {
+      return user.post('/tasks?tasksOwner=user', {
         text: 'test habit',
         type: 'habit',
       }).then((createdTask) => {
@@ -65,7 +65,7 @@ describe('PUT /tasks/:id', () => {
     let habit;
 
     beforeEach(() => {
-      return user.post('/tasks', {
+      return user.post('/tasks?tasksOwner=user', {
         text: 'test habit',
         type: 'habit',
         notes: 1976,
@@ -93,7 +93,7 @@ describe('PUT /tasks/:id', () => {
     let todo;
 
     beforeEach(() => {
-      return user.post('/tasks', {
+      return user.post('/tasks?tasksOwner=user', {
         text: 'test todo',
         type: 'todo',
         notes: 1976,
@@ -150,7 +150,7 @@ describe('PUT /tasks/:id', () => {
     let daily;
 
     beforeEach(() => {
-      return user.post('/tasks', {
+      return user.post('/tasks?tasksOwner=user', {
         text: 'test daily',
         type: 'daily',
         notes: 1976,
@@ -254,7 +254,7 @@ describe('PUT /tasks/:id', () => {
     let reward;
 
     beforeEach(() => {
-      return user.post('/tasks', {
+      return user.post('/tasks?tasksOwner=user', {
         text: 'test reward',
         type: 'reward',
         notes: 1976,
diff --git a/test/api/v3/integration/tasks/checklists/DELETE-tasks_taskId_checklist_itemId.test.js b/test/api/v3/integration/tasks/checklists/DELETE-tasks_taskId_checklist_itemId.test.js
index d013878a74..738255996a 100644
--- a/test/api/v3/integration/tasks/checklists/DELETE-tasks_taskId_checklist_itemId.test.js
+++ b/test/api/v3/integration/tasks/checklists/DELETE-tasks_taskId_checklist_itemId.test.js
@@ -16,7 +16,7 @@ describe('DELETE /tasks/:taskId/checklist/:itemId', () => {
   it('deletes a checklist item', () => {
     let task;
 
-    return user.post('/tasks', {
+    return user.post('/tasks?tasksOwner=user', {
       type: 'daily',
       text: 'Daily with checklist',
     }).then(createdTask => {
@@ -33,7 +33,7 @@ describe('DELETE /tasks/:taskId/checklist/:itemId', () => {
 
   it('does not work with habits', () => {
     let habit;
-    return expect(user.post('/tasks', {
+    return expect(user.post('/tasks?tasksOwner=user', {
       type: 'habit',
       text: 'habit with checklist',
     }).then(createdTask => {
@@ -47,7 +47,7 @@ describe('DELETE /tasks/:taskId/checklist/:itemId', () => {
   });
 
   it('does not work with rewards', async () => {
-    let reward = await user.post('/tasks', {
+    let reward = await user.post('/tasks?tasksOwner=user', {
       type: 'reward',
       text: 'reward with checklist',
     });
@@ -68,7 +68,7 @@ describe('DELETE /tasks/:taskId/checklist/:itemId', () => {
   });
 
   it('fails on checklist item not found', () => {
-    return expect(user.post('/tasks', {
+    return expect(user.post('/tasks?tasksOwner=user', {
       type: 'daily',
       text: 'daily with checklist',
     }).then(createdTask => {
diff --git a/test/api/v3/integration/tasks/checklists/POST-tasks_taskId_checklist.test.js b/test/api/v3/integration/tasks/checklists/POST-tasks_taskId_checklist.test.js
index e9b695effd..1ad4f7f58c 100644
--- a/test/api/v3/integration/tasks/checklists/POST-tasks_taskId_checklist.test.js
+++ b/test/api/v3/integration/tasks/checklists/POST-tasks_taskId_checklist.test.js
@@ -16,7 +16,7 @@ describe('POST /tasks/:taskId/checklist/', () => {
   it('adds a checklist item to a task', () => {
     let task;
 
-    return user.post('/tasks', {
+    return user.post('/tasks?tasksOwner=user', {
       type: 'daily',
       text: 'Daily with checklist',
     }).then(createdTask => {
@@ -36,7 +36,7 @@ describe('POST /tasks/:taskId/checklist/', () => {
   it('does not add a checklist to habits', () => {
     let habit;
 
-    return expect(user.post('/tasks', {
+    return expect(user.post('/tasks?tasksOwner=user', {
       type: 'habit',
       text: 'habit with checklist',
     }).then(createdTask => {
@@ -51,7 +51,7 @@ describe('POST /tasks/:taskId/checklist/', () => {
 
   it('does not add a checklist to rewards', () => {
     let reward;
-    return expect(user.post('/tasks', {
+    return expect(user.post('/tasks?tasksOwner=user', {
       type: 'reward',
       text: 'reward with checklist',
     }).then(createdTask => {
diff --git a/test/api/v3/integration/tasks/checklists/POST-tasks_taskId_checklist_itemId_score.test.js b/test/api/v3/integration/tasks/checklists/POST-tasks_taskId_checklist_itemId_score.test.js
index 667ef41446..862b67b638 100644
--- a/test/api/v3/integration/tasks/checklists/POST-tasks_taskId_checklist_itemId_score.test.js
+++ b/test/api/v3/integration/tasks/checklists/POST-tasks_taskId_checklist_itemId_score.test.js
@@ -16,7 +16,7 @@ describe('POST /tasks/:taskId/checklist/:itemId/score', () => {
   it('scores a checklist item', () => {
     let task;
 
-    return user.post('/tasks', {
+    return user.post('/tasks?tasksOwner=user', {
       type: 'daily',
       text: 'Daily with checklist',
     }).then(createdTask => {
@@ -32,7 +32,7 @@ describe('POST /tasks/:taskId/checklist/:itemId/score', () => {
 
   it('fails on habits', () => {
     let habit;
-    return expect(user.post('/tasks', {
+    return expect(user.post('/tasks?tasksOwner=user', {
       type: 'habit',
       text: 'habit with checklist',
     }).then(createdTask => {
@@ -46,7 +46,7 @@ describe('POST /tasks/:taskId/checklist/:itemId/score', () => {
   });
 
   it('fails on rewards', async () => {
-    let reward = await user.post('/tasks', {
+    let reward = await user.post('/tasks?tasksOwner=user', {
       type: 'reward',
       text: 'reward with checklist',
     });
@@ -67,7 +67,7 @@ describe('POST /tasks/:taskId/checklist/:itemId/score', () => {
   });
 
   it('fails on checklist item not found', () => {
-    return expect(user.post('/tasks', {
+    return expect(user.post('/tasks?tasksOwner=user', {
       type: 'daily',
       text: 'daily with checklist',
     }).then(createdTask => {
diff --git a/test/api/v3/integration/tasks/checklists/PUT-tasks_taskId_checklist_itemId.test.js b/test/api/v3/integration/tasks/checklists/PUT-tasks_taskId_checklist_itemId.test.js
index 988574bbf8..ae6babf778 100644
--- a/test/api/v3/integration/tasks/checklists/PUT-tasks_taskId_checklist_itemId.test.js
+++ b/test/api/v3/integration/tasks/checklists/PUT-tasks_taskId_checklist_itemId.test.js
@@ -16,7 +16,7 @@ describe('PUT /tasks/:taskId/checklist/:itemId', () => {
   it('updates a checklist item', () => {
     let task;
 
-    return user.post('/tasks', {
+    return user.post('/tasks?tasksOwner=user', {
       type: 'daily',
       text: 'Daily with checklist',
     }).then(createdTask => {
@@ -33,7 +33,7 @@ describe('PUT /tasks/:taskId/checklist/:itemId', () => {
   });
 
   it('fails on habits', async () => {
-    let habit = await user.post('/tasks', {
+    let habit = await user.post('/tasks?tasksOwner=user', {
       type: 'habit',
       text: 'habit with checklist',
     });
@@ -46,7 +46,7 @@ describe('PUT /tasks/:taskId/checklist/:itemId', () => {
   });
 
   it('fails on rewards', async () => {
-    let reward = await user.post('/tasks', {
+    let reward = await user.post('/tasks?tasksOwner=user', {
       type: 'reward',
       text: 'reward with checklist',
     });
@@ -67,7 +67,7 @@ describe('PUT /tasks/:taskId/checklist/:itemId', () => {
   });
 
   it('fails on checklist item not found', () => {
-    return expect(user.post('/tasks', {
+    return expect(user.post('/tasks?tasksOwner=user', {
       type: 'daily',
       text: 'daily with checklist',
     }).then(createdTask => {
diff --git a/test/api/v3/integration/tasks/tags/DELETE-tasks_taskId_tags_tagId.test.js b/test/api/v3/integration/tasks/tags/DELETE-tasks_taskId_tags_tagId.test.js
index c01b71fa2c..27d57bb2f0 100644
--- a/test/api/v3/integration/tasks/tags/DELETE-tasks_taskId_tags_tagId.test.js
+++ b/test/api/v3/integration/tasks/tags/DELETE-tasks_taskId_tags_tagId.test.js
@@ -17,7 +17,7 @@ describe('DELETE /tasks/:taskId/tags/:tagId', () => {
     let tag;
     let task;
 
-    return user.post('/tasks', {
+    return user.post('/tasks?tasksOwner=user', {
       type: 'habit',
       text: 'Task with tag',
     }).then(createdTask => {
@@ -35,7 +35,7 @@ describe('DELETE /tasks/:taskId/tags/:tagId', () => {
   });
 
   it('only deletes existing tags', () => {
-    return expect(user.post('/tasks', {
+    return expect(user.post('/tasks?tasksOwner=user', {
       type: 'habit',
       text: 'Task with tag',
     }).then(createdTask => {
diff --git a/test/api/v3/integration/tasks/tags/POST-tasks_taskId_tags_tagId.test.js b/test/api/v3/integration/tasks/tags/POST-tasks_taskId_tags_tagId.test.js
index 6e4c2ba510..9887a5bedb 100644
--- a/test/api/v3/integration/tasks/tags/POST-tasks_taskId_tags_tagId.test.js
+++ b/test/api/v3/integration/tasks/tags/POST-tasks_taskId_tags_tagId.test.js
@@ -17,7 +17,7 @@ describe('POST /tasks/:taskId/tags/:tagId', () => {
     let tag;
     let task;
 
-    return user.post('/tasks', {
+    return user.post('/tasks?tasksOwner=user', {
       type: 'habit',
       text: 'Task with tag',
     }).then(createdTask => {
@@ -35,7 +35,7 @@ describe('POST /tasks/:taskId/tags/:tagId', () => {
     let tag;
     let task;
 
-    return expect(user.post('/tasks', {
+    return expect(user.post('/tasks?tasksOwner=user', {
       type: 'habit',
       text: 'Task with tag',
     }).then(createdTask => {
@@ -54,7 +54,7 @@ describe('POST /tasks/:taskId/tags/:tagId', () => {
   });
 
   it('does not add a non existing tag to a task', () => {
-    return expect(user.post('/tasks', {
+    return expect(user.post('/tasks?tasksOwner=user', {
       type: 'habit',
       text: 'Task with tag',
     }).then((task) => {
diff --git a/website/src/controllers/api-v3/tasks.js b/website/src/controllers/api-v3/tasks.js
index 40425c27a3..6ea130e843 100644
--- a/website/src/controllers/api-v3/tasks.js
+++ b/website/src/controllers/api-v3/tasks.js
@@ -22,20 +22,26 @@ 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.
+ * @apiParam {string="user","challenge"} tasksOwner Query parameter to define if tasks will belong to the auhenticated user or to a challenge (specifying the "challengeId" parameter).
+ * @apiParam {UUID} challengeId Optional. Query parameter. If "tasksOwner" is "challenge" then specify the challenge id.
  *
  * @apiSuccess {Object|Array} task The newly created task(s)
  */
 api.createTask = {
   method: 'POST',
-  url: '/tasks/:tasksOwner/:challengeId?',
+  url: '/tasks',
   middlewares: [authWithHeaders(), cron],
   async handler (req, res) {
+    req.checkQuery('tasksOwner', res.t('invalidTasksOwner')).isIn(['user', 'challenge']);
+    req.checkQuery('challengeId', res.t('challengeIdRequired')).optional().isUUID();
+
+    let validationErrors = req.validationErrors();
+    if (validationErrors) throw validationErrors;
+
     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 tasksOwner = req.query.tasksOwner;
+    let challengeId = req.query.challengeId;
     let challenge;
 
     if (tasksOwner === 'user' && challengeId) throw new BadRequest(res.t('userTasksNoChallengeId'));
@@ -79,8 +85,13 @@ api.createTask = {
     toSave.unshift((challenge || user).save());
 
     let tasks = await Q.all(toSave);
-    tasks.splice(0, 1); // remove the user/challenge
-    res.respond(201, tasks);
+
+    if (tasks.length === 2) {
+      res.respond(201, tasks[1]);
+    } else {
+      tasks.splice(0, 1); // remove the user/challenge
+      res.respond(201, tasks);
+    }
 
     // If adding tasks to a challenge -> sync users
     if (challenge) challenge.addTasks(tasks); // TODO catch/log
@@ -93,8 +104,8 @@ api.createTask = {
  * @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="user","challenge"} tasksOwner Query parameter to return tasks belonging to a challenge (specifying the "challengeId" parameter) or to the autheticated user.
+ * @apiParam {UUID} challengeId Optional query parameter. If "tasksOwner" is "challenge" then required to 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". Only valid whe "tasksOwner" is "user".
  *
@@ -102,20 +113,19 @@ api.createTask = {
  */
 api.getTasks = {
   method: 'GET',
-  url: '/tasks/:tasksOwner/:challengeId?',
+  url: '/tasks',
   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('tasksOwner', res.t('invalidTasksOwner')).isIn(['user', 'challenge']);
+    req.checkQuery('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 tasksOwner = req.params.tasksOwner;
-    let challengeId = req.params.challengeId;
+    let tasksOwner = req.query.tasksOwner;
+    let challengeId = req.query.challengeId;
     let challenge;
 
     if (tasksOwner === 'user' && challengeId) throw new BadRequest(res.t('userTasksNoChallengeId'));

From a5b1dfd32d7b14371082541bc9544bb91b048f7f Mon Sep 17 00:00:00 2001
From: Matteo Pagliazzi <matteopagliazzi@gmail.com>
Date: Tue, 5 Jan 2016 20:19:55 +0100
Subject: [PATCH 05/10] finish implementing tasks syncing for challenges

---
 website/src/controllers/api-v3/groups.js |  11 +-
 website/src/controllers/api-v3/tasks.js  |   4 +-
 website/src/models/challenge.js          | 194 +++++++++++++----------
 website/src/models/task.js               |   2 +-
 4 files changed, 123 insertions(+), 88 deletions(-)

diff --git a/website/src/controllers/api-v3/groups.js b/website/src/controllers/api-v3/groups.js
index 80b2972217..169512b0da 100644
--- a/website/src/controllers/api-v3/groups.js
+++ b/website/src/controllers/api-v3/groups.js
@@ -112,12 +112,13 @@ api.getGroups = {
     // If no valid value for type was supplied, return an error
     if (queries.length === 0) throw new BadRequest(res.t('groupTypesRequired'));
 
-    let results = await Q.all(queries); // TODO we would like not to return a single big array but Q doesn't support the funtionality https://github.com/kriskowal/q/issues/328
+    // TODO we would like not to return a single big array but Q doesn't support the funtionality https://github.com/kriskowal/q/issues/328
+    let results = _.reduce(await Q.all(queries), (previousValue, currentValue) => {
+      if (_.isEmpty(currentValue)) return previousValue; // don't add anything to the results if the query returned null or an empty array
+      return previousValue.concat(Array.isArray(currentValue) ? currentValue : [currentValue]); // otherwise concat the new results to the previousValue
+    }, []);
 
-    res.respond(200, _.reduce(results, (m, v) => {
-      if (_.isEmpty(v)) return m;
-      return m.concat(Array.isArray(v) ? v : [v]);
-    }, []));
+    res.respond(200, results);
   },
 };
 
diff --git a/website/src/controllers/api-v3/tasks.js b/website/src/controllers/api-v3/tasks.js
index 6ea130e843..0f5ecc314c 100644
--- a/website/src/controllers/api-v3/tasks.js
+++ b/website/src/controllers/api-v3/tasks.js
@@ -35,8 +35,8 @@ api.createTask = {
     req.checkQuery('tasksOwner', res.t('invalidTasksOwner')).isIn(['user', 'challenge']);
     req.checkQuery('challengeId', res.t('challengeIdRequired')).optional().isUUID();
 
-    let validationErrors = req.validationErrors();
-    if (validationErrors) throw validationErrors;
+    let reqValidationErrors = req.validationErrors();
+    if (reqValidationErrors) throw reqValidationErrors;
 
     let tasksData = Array.isArray(req.body) ? req.body : [req.body];
     let user = res.locals.user;
diff --git a/website/src/models/challenge.js b/website/src/models/challenge.js
index 6e7e179047..bf009314a9 100644
--- a/website/src/models/challenge.js
+++ b/website/src/models/challenge.js
@@ -30,15 +30,18 @@ schema.plugin(baseModel, {
   noSet: ['_id', 'memberCount', 'tasksOrder'],
 });
 
+// Takes a Task document and return a plain object of attributes that can be synced to the user
 function _syncableAttrs (task) {
   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?
+  let omitAttrs = ['userId', 'challenge', 'history', 'tags', 'completed', 'streak', 'notes']; // TODO what to do with updatedAt?
   if (t.type !== 'reward') omitAttrs.push('value');
   return _.omit(t, omitAttrs);
 }
 
-schema.methods.syncToUser = function syncChallengeToUser (user) {
+// Sync challenge to user, including tasks and tags.
+// Used when user joins the challenge or to force sync.
+schema.methods.syncToUser = async function syncChallengeToUser (user) {
   let challenge = this;
   challenge.shortName = challenge.shortName || challenge.name;
 
@@ -62,12 +65,7 @@ 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([
+  let [challengeTasks, userTasks] = await Q.all([
     // Find original challenge tasks
     Tasks.Task.find({
       userId: {$exists: false},
@@ -78,86 +76,122 @@ schema.methods.syncToUser = function syncChallengeToUser (user) {
       userId: user._id,
       'challenge.id': challenge._id,
     }).exec(),
-  ])
-  .then(results => {
-    let challengeTasks = results[0];
-    let userTasks = results[1];
-    let toSave = []; // An array of things to save
+  ]);
 
-    challengeTasks.forEach(chalTask => {
-      let matchingTask = _.find(userTasks, userTask => userTask.challenge.taskId === chalTask._id);
+  let toSave = []; // An array of things to save
 
-      if (!matchingTask) { // If the task is new, create it
-        matchingTask = new Tasks[chalTask.type](Tasks.Task.sanitizeCreate(_syncableAttrs(chalTask)));
-        matchingTask.challenge = {taskId: chalTask._id, id: challenge._id};
-        matchingTask.userId = user._id;
-        user.tasksOrder[`${chalTask.type}s`].push(matchingTask._id);
-      } else {
-        _.merge(matchingTask, _syncableAttrs(chalTask));
-        // Make sure the task is in user.tasksOrder TODO necessary?
-        let orderList = user.tasksOrder[`${chalTask.type}s`];
-        if (orderList.indexOf(matchingTask._id) === -1 && (matchingTask.type !== 'todo' || !matchingTask.completed)) orderList.push(matchingTask._id);
-      }
+  challengeTasks.forEach(chalTask => {
+    let matchingTask = _.find(userTasks, userTask => userTask.challenge.taskId === chalTask._id);
 
-      if (!matchingTask.notes) matchingTask.notes = chalTask.notes; // don't override the notes, but provide it if not provided
-      if (matchingTask.tags.indexOf(challenge._id) === -1) matchingTask.tags.push(challenge._id); // add tag if missing
-      toSave.push(matchingTask.save());
-    });
+    if (!matchingTask) { // If the task is new, create it
+      matchingTask = new Tasks[chalTask.type](Tasks.Task.sanitizeCreate(_syncableAttrs(chalTask)));
+      matchingTask.challenge = {taskId: chalTask._id, id: challenge._id};
+      matchingTask.userId = user._id;
+      user.tasksOrder[`${chalTask.type}s`].push(matchingTask._id);
+    } else {
+      _.merge(matchingTask, _syncableAttrs(chalTask));
+      // Make sure the task is in user.tasksOrder TODO necessary?
+      let orderList = user.tasksOrder[`${chalTask.type}s`];
+      if (orderList.indexOf(matchingTask._id) === -1 && (matchingTask.type !== 'todo' || !matchingTask.completed)) orderList.push(matchingTask._id);
+    }
 
-    // Flag deleted tasks as "broken"
-    userTasks.forEach(userTask => {
-      if (!_.find(challengeTasks, chalTask => chalTask._id === userTask.challenge.taskId)) {
-        userTask.challenge.broken = 'TASK_DELETED';
-        toSave.push(userTask.save());
-      }
-    });
+    if (!matchingTask.notes) matchingTask.notes = chalTask.notes; // don't override the notes, but provide it if not provided
+    if (matchingTask.tags.indexOf(challenge._id) === -1) matchingTask.tags.push(challenge._id); // add tag if missing
+    toSave.push(matchingTask.save());
+  });
 
-    toSave.push(user.save());
-    return Q.all(toSave);
-  });*/
+  // Flag deleted tasks as "broken"
+  userTasks.forEach(userTask => {
+    if (!_.find(challengeTasks, chalTask => chalTask._id === userTask.challenge.taskId)) {
+      userTask.challenge.broken = 'TASK_DELETED';
+      toSave.push(userTask.save());
+    }
+  });
+
+  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 updateQ = {$push: {}};
-    tasks.forEach(chalTask => {
-
-    })
-    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())
+async function _fetchMembersIds (challengeId) {
+  return (await User.find({challenges: {$in: [challengeId]}}).select('_id').lean().exec()).map(member => member._id);
 }
 
-ChallengeSchema.methods.isOutdated = function isChallengeOutdated (newData) {
-  return comparableData(this) !== comparableData(newData);
-}*/
+// Add a new task to challenge members
+schema.methods.addTasks = async function challengeAddTasks (tasks) {
+  let challenge = this;
+  let membersIds = await _fetchMembersIds(challenge._id);
+
+  // Sync each user sequentially
+  for (let memberId of membersIds) {
+    let updateTasksOrderQ = {$push: {}};
+    let toSave = [];
+
+    // TODO eslint complaints about ahving a function inside a loop -> make sure it works
+    tasks.forEach(chalTask => { // eslint-disable-line no-loop-func
+      let userTask = new Tasks[chalTask.type](Tasks.Task.sanitizeCreate(_syncableAttrs(chalTask)));
+      userTask.challenge = {taskId: chalTask._id, id: challenge._id};
+      userTask.userId = memberId;
+
+      let tasksOrderList = updateTasksOrderQ.$push[`tasksOrder.${chalTask.type}s`];
+      if (!tasksOrderList) {
+        updateTasksOrderQ.$push[`tasksOrder.${chalTask.type}s`] = {
+          $position: 0, // unshift
+          $each: [userTask._id],
+        };
+      } else {
+        tasksOrderList.$each.unshift(userTask._id);
+      }
+
+      toSave.push(userTask);
+    });
+
+    // Update the user
+    toSave.unshift(User.update({_id: memberId}, updateTasksOrderQ).exec());
+    await Q.all(toSave); // eslint-disable-line babel/no-await-in-loop
+  }
+};
+
+// Sync updated task to challenge members
+schema.methods.updateTask = async function challengeUpdateTask (task) {
+  let challenge = this;
+
+  let updateCmd = {$set: {}};
+
+  _syncableAttrs(task).forEach((value, key) => {
+    updateCmd.$set[key] = value;
+  });
+
+  // TODO reveiw
+  // Updating instead of loading and saving for performances, risks becoming a problem if we introduce more complexity in tasks
+  await Tasks.Task.update({
+    userId: {$exists: true},
+    'challenge.id': challenge.id,
+    'challenge.taskId': task._id,
+  }, updateCmd, {multi: true}).exec();
+};
+
+// Remove a task from challenge members
+schema.methods.removeTask = async function challengeRemoveTask (task) {
+  let challenge = this;
+
+  // Remove the tasks from users' and map each of them to an update query to remove the task from tasksOrder
+  let updateQueries = (await Tasks.Task.findOneAndRemove({
+    userId: {$exists: true},
+    'challenge.id': challenge.id,
+    'challenge.taskId': task._id,
+  }, {
+    fields: {userId: 1, type: 1}, // fetch only what's necessary
+  }).lean().exec())
+    .map(removedTask => {
+      return User.update({_id: removedTask.userId}, {
+        $pull: {[`tasksOrder${removedTask.type}s`]: removedTask._id},
+      });
+    });
+
+  // Execute each update sequentially
+  for (let query of updateQueries) {
+    await query.exec(); // eslint-disable-line babel/no-await-in-loop
+  }
+};
 
 export let model = mongoose.model('Challenge', schema);
diff --git a/website/src/models/task.js b/website/src/models/task.js
index b31676da10..3fd574af93 100644
--- a/website/src/models/task.js
+++ b/website/src/models/task.js
@@ -28,7 +28,7 @@ export let TaskSchema = new Schema({
 
   challenge: {
     id: {type: String, ref: 'Challenge', validate: [validator.isUUID, 'Invalid uuid.']}, // When set (and userId not set) it's the original task
-    taskId: {type: String, ref: 'Task', validate: [validator.isUUID, 'Invalid uuid.']}, // When not set but challenge.id defined it's the original task
+    taskId: {type: String, ref: 'Task', validate: [validator.isUUID, 'Invalid uuid.']}, // When not set but challenge.id defined it's the original task TODO unique index?
     broken: {type: String, enum: ['CHALLENGE_DELETED', 'TASK_DELETED', 'UNSUBSCRIBED', 'CHALLENGE_CLOSED']},
     winner: String, // user.profile.name TODO necessary?
   },

From 3a6d7bd466f2f5d8b6cfeae4881d5007aef27fc7 Mon Sep 17 00:00:00 2001
From: Matteo Pagliazzi <matteopagliazzi@gmail.com>
Date: Wed, 6 Jan 2016 12:16:41 +0100
Subject: [PATCH 06/10] fix sync of remove challenge task

---
 website/src/models/challenge.js | 18 ++++--------------
 1 file changed, 4 insertions(+), 14 deletions(-)

diff --git a/website/src/models/challenge.js b/website/src/models/challenge.js
index bf009314a9..430a7e0443 100644
--- a/website/src/models/challenge.js
+++ b/website/src/models/challenge.js
@@ -174,24 +174,14 @@ schema.methods.updateTask = async function challengeUpdateTask (task) {
 schema.methods.removeTask = async function challengeRemoveTask (task) {
   let challenge = this;
 
-  // Remove the tasks from users' and map each of them to an update query to remove the task from tasksOrder
-  let updateQueries = (await Tasks.Task.findOneAndRemove({
+  // Set the task as broken
+  await Tasks.Task.update({
     userId: {$exists: true},
     'challenge.id': challenge.id,
     'challenge.taskId': task._id,
   }, {
-    fields: {userId: 1, type: 1}, // fetch only what's necessary
-  }).lean().exec())
-    .map(removedTask => {
-      return User.update({_id: removedTask.userId}, {
-        $pull: {[`tasksOrder${removedTask.type}s`]: removedTask._id},
-      });
-    });
-
-  // Execute each update sequentially
-  for (let query of updateQueries) {
-    await query.exec(); // eslint-disable-line babel/no-await-in-loop
-  }
+    $set: {'challenge.broken': 'TASK_DELETED'}, // TODO what about updatedAt?
+  }).lean().exec();
 };
 
 export let model = mongoose.model('Challenge', schema);

From 2b2dcfe7ce66fafb53410e45ba13a6b343223ff6 Mon Sep 17 00:00:00 2001
From: Matteo Pagliazzi <matteopagliazzi@gmail.com>
Date: Wed, 6 Jan 2016 12:25:53 +0100
Subject: [PATCH 07/10] do not delete completed todos that belongs to a
 challenge

---
 website/src/middlewares/api-v3/cron.js | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/website/src/middlewares/api-v3/cron.js b/website/src/middlewares/api-v3/cron.js
index 0d6d46b900..f606a72aea 100644
--- a/website/src/middlewares/api-v3/cron.js
+++ b/website/src/middlewares/api-v3/cron.js
@@ -34,6 +34,7 @@ export default function cronMiddleware (req, res, next) {
     cron({user, tasksByType, now, daysMissed, analytics});
 
     // Clean completed todos - 30 days for free users, 90 for subscribers
+    // Do not delete challenges completed todos TODO unless the task is broken?
     Task.remove({
       userId: user._id,
       type: 'todo',
@@ -41,6 +42,7 @@ export default function cronMiddleware (req, res, next) {
       dateCompleted: {
         $lt: moment(now).subtract(user.isSubscribed() ? 90 : 30, 'days'),
       },
+      'challenge.id': {$exists: false},
     }).exec(); // TODO catch error or at least log it
 
     let ranCron = user.isModified();

From 8ba486ec12ee83350c759fdf2c048760189e613a Mon Sep 17 00:00:00 2001
From: Matteo Pagliazzi <matteopagliazzi@gmail.com>
Date: Wed, 6 Jan 2016 18:40:11 +0100
Subject: [PATCH 08/10] change tasks routes to /tasks/(user|challenge)

---
 .../integration/tasks/DELETE-tasks_id.test.js |   4 +-
 .../v3/integration/tasks/GET-tasks.test.js    |  10 +-
 .../v3/integration/tasks/GET-tasks_id.test.js |   4 +-
 .../v3/integration/tasks/POST-tasks.test.js   |  76 +++---
 .../POST-tasks_id_score_direction.test.js     |  14 +-
 .../v3/integration/tasks/PUT-tasks_id.test.js |  10 +-
 ...LETE-tasks_taskId_checklist_itemId.test.js |   8 +-
 .../POST-tasks_taskId_checklist.test.js       |   6 +-
 ...asks_taskId_checklist_itemId_score.test.js |   8 +-
 .../PUT-tasks_taskId_checklist_itemId.test.js |   8 +-
 .../DELETE-tasks_taskId_tags_tagId.test.js    |   4 +-
 .../tags/POST-tasks_taskId_tags_tagId.test.js |   6 +-
 website/src/controllers/api-v3/tasks.js       | 248 ++++++++++++------
 13 files changed, 248 insertions(+), 158 deletions(-)

diff --git a/test/api/v3/integration/tasks/DELETE-tasks_id.test.js b/test/api/v3/integration/tasks/DELETE-tasks_id.test.js
index 503e484e66..053176a9ee 100644
--- a/test/api/v3/integration/tasks/DELETE-tasks_id.test.js
+++ b/test/api/v3/integration/tasks/DELETE-tasks_id.test.js
@@ -16,7 +16,7 @@ describe('DELETE /tasks/:id', () => {
     let task;
 
     beforeEach(() => {
-      return user.post('/tasks?tasksOwner=user', {
+      return user.post('/tasks/user', {
         text: 'test habit',
         type: 'habit',
       }).then((createdTask) => {
@@ -48,7 +48,7 @@ describe('DELETE /tasks/:id', () => {
     it('cannot delete a task owned by someone else', () => {
       return generateUser()
         .then((anotherUser) => {
-          return anotherUser.post('/tasks?tasksOwner=user', {
+          return anotherUser.post('/tasks/user', {
             text: 'test habit',
             type: 'habit',
           });
diff --git a/test/api/v3/integration/tasks/GET-tasks.test.js b/test/api/v3/integration/tasks/GET-tasks.test.js
index bf77442ea4..bf652777e4 100644
--- a/test/api/v3/integration/tasks/GET-tasks.test.js
+++ b/test/api/v3/integration/tasks/GET-tasks.test.js
@@ -2,7 +2,7 @@ import {
   generateUser,
 } from '../../../../helpers/api-integration.helper';
 
-describe('GET /tasks', () => {
+describe('GET /tasks/user', () => {
   let user;
 
   beforeEach(async () => {
@@ -16,14 +16,14 @@ describe('GET /tasks', () => {
   });
 
   it('returns all user\'s tasks', async () => {
-    let createdTasks = await user.post('/tasks?tasksOwner=user', [{text: 'test habit', type: 'habit'}, {text: 'test todo', type: 'todo'}]);
-    let tasks = await user.get('/tasks?tasksOwner=user');
+    let createdTasks = await user.post('/tasks/user', [{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', async () => {
-    let createdTasks = await user.post('/tasks?tasksOwner=user', [{text: 'test habit', type: 'habit'}, {text: 'test todo', type: 'todo'}]);
-    let tasks = await user.get('/tasks?tasksOwner=user&type=habit');
+    let createdTasks = await user.post('/tasks/user', [{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);
   });
diff --git a/test/api/v3/integration/tasks/GET-tasks_id.test.js b/test/api/v3/integration/tasks/GET-tasks_id.test.js
index b88d8a95a6..d3fe449b8d 100644
--- a/test/api/v3/integration/tasks/GET-tasks_id.test.js
+++ b/test/api/v3/integration/tasks/GET-tasks_id.test.js
@@ -17,7 +17,7 @@ describe('GET /tasks/:id', () => {
     let task;
 
     beforeEach(() => {
-      return user.post('/tasks?tasksOwner=user', {
+      return user.post('/tasks/user', {
         text: 'test habit',
         type: 'habit',
       }).then((createdTask) => {
@@ -54,7 +54,7 @@ describe('GET /tasks/:id', () => {
         .then((user2) => {
           anotherUser = user2;
 
-          return user.post('/tasks?tasksOwner=user', {
+          return user.post('/tasks/user', {
             text: 'test habit',
             type: 'habit',
           });
diff --git a/test/api/v3/integration/tasks/POST-tasks.test.js b/test/api/v3/integration/tasks/POST-tasks.test.js
index a002119634..3fd81aa7cd 100644
--- a/test/api/v3/integration/tasks/POST-tasks.test.js
+++ b/test/api/v3/integration/tasks/POST-tasks.test.js
@@ -14,7 +14,7 @@ describe('POST /tasks', () => {
 
   context('validates params', () => {
     it('returns an error if req.body.type is absent', async () => {
-      return expect(user.post('/tasks?tasksOwner=user', {
+      return expect(user.post('/tasks/user', {
         notType: 'habit',
       })).to.eventually.be.rejected.and.eql({
         code: 400,
@@ -24,7 +24,7 @@ describe('POST /tasks', () => {
     });
 
     it('returns an error if req.body.type is not valid', async () => {
-      return expect(user.post('/tasks?tasksOwner=user', {
+      return expect(user.post('/tasks/user', {
         type: 'habitF',
       })).to.eventually.be.rejected.and.eql({
         code: 400,
@@ -34,7 +34,7 @@ describe('POST /tasks', () => {
     });
 
     it('returns an error if one object inside an array is invalid', async () => {
-      return expect(user.post('/tasks?tasksOwner=user', [
+      return expect(user.post('/tasks/user', [
         {type: 'habitF'},
         {type: 'habit'},
       ])).to.eventually.be.rejected.and.eql({
@@ -45,7 +45,7 @@ describe('POST /tasks', () => {
     });
 
     it('returns an error if req.body.text is absent', async () => {
-      return expect(user.post('/tasks?tasksOwner=user', {
+      return expect(user.post('/tasks/user', {
         type: 'habit',
       })).to.eventually.be.rejected.and.eql({
         code: 400,
@@ -56,7 +56,7 @@ describe('POST /tasks', () => {
 
     it('does not update user.tasksOrder.{taskType} when the task is not saved because invalid', async () => {
       let originalHabitsOrder = (await user.get('/user')).tasksOrder.habits;
-      return expect(user.post('/tasks?tasksOwner=user', {
+      return expect(user.post('/tasks/user', {
         type: 'habit',
       })).to.eventually.be.rejected.and.eql({ // this block is necessary
         code: 400,
@@ -71,7 +71,7 @@ describe('POST /tasks', () => {
 
     it('does not update user.tasksOrder.{taskType} when a task inside an array is not saved because invalid', async () => {
       let originalHabitsOrder = (await user.get('/user')).tasksOrder.habits;
-      return expect(user.post('/tasks?tasksOwner=user', [
+      return expect(user.post('/tasks/user', [
         {type: 'habit'}, // Missing text
         {type: 'habit', text: 'valid'}, // Valid
       ])).to.eventually.be.rejected.and.eql({ // this block is necessary
@@ -86,8 +86,8 @@ describe('POST /tasks', () => {
     });
 
     it('does not save any task sent in an array when 1 is invalid', async () => {
-      let originalTasks = await user.get('/tasks?tasksOwner=user');
-      return expect(user.post('/tasks?tasksOwner=user', [
+      let originalTasks = await user.get('/tasks/user');
+      return expect(user.post('/tasks/user', [
         {type: 'habit'}, // Missing text
         {type: 'habit', text: 'valid'}, // Valid
       ])).to.eventually.be.rejected.and.eql({ // this block is necessary
@@ -95,14 +95,14 @@ describe('POST /tasks', () => {
         error: 'BadRequest',
         message: 'habit validation failed',
       }).then(async () => {
-        let updatedTasks = await user.get('/tasks?tasksOwner=user');
+        let updatedTasks = await user.get('/tasks/user');
 
         expect(updatedTasks).to.eql(originalTasks);
       });
     });
 
     it('automatically sets "task.userId" to user\'s uuid', async () => {
-      let task = await user.post('/tasks?tasksOwner=user', {
+      let task = await user.post('/tasks/user', {
         text: 'test habit',
         type: 'habit',
       });
@@ -113,7 +113,7 @@ describe('POST /tasks', () => {
     it(`ignores setting userId, history, createdAt,
                         updatedAt, challenge, completed, streak,
                         dateCompleted fields`, async () => {
-      let task = await user.post('/tasks?tasksOwner=user', {
+      let task = await user.post('/tasks/user', {
         text: 'test daily',
         type: 'daily',
         userId: 123,
@@ -137,7 +137,7 @@ describe('POST /tasks', () => {
     });
 
     it('ignores invalid fields', async () => {
-      let task = await user.post('/tasks?tasksOwner=user', {
+      let task = await user.post('/tasks/user', {
         text: 'test daily',
         type: 'daily',
         notValid: true,
@@ -149,7 +149,7 @@ describe('POST /tasks', () => {
 
   context('habits', () => {
     it('creates a habit', async () => {
-      let task = await user.post('/tasks?tasksOwner=user', {
+      let task = await user.post('/tasks/user', {
         text: 'test habit',
         type: 'habit',
         up: false,
@@ -167,7 +167,7 @@ describe('POST /tasks', () => {
 
     it('updates user.tasksOrder.habits when a new habit is created', async () => {
       let originalHabitsOrderLen = (await user.get('/user')).tasksOrder.habits.length;
-      let task = await user.post('/tasks?tasksOwner=user', {
+      let task = await user.post('/tasks/user', {
         type: 'habit',
         text: 'an habit',
       });
@@ -179,7 +179,7 @@ describe('POST /tasks', () => {
 
     it('updates user.tasksOrder.habits when multiple habits are created', async () => {
       let originalHabitsOrderLen = (await user.get('/user')).tasksOrder.habits.length;
-      let [task, task2] = await user.post('/tasks?tasksOwner=user', [{
+      let [task, task2] = await user.post('/tasks/user', [{
         type: 'habit',
         text: 'an habit',
       }, {
@@ -194,7 +194,7 @@ describe('POST /tasks', () => {
     });
 
     it('creates multiple habits', async () => {
-      let [task, task2] = await user.post('/tasks?tasksOwner=user', [{
+      let [task, task2] = await user.post('/tasks/user', [{
         text: 'test habit',
         type: 'habit',
         up: false,
@@ -224,7 +224,7 @@ describe('POST /tasks', () => {
     });
 
     it('defaults to setting up and down to true', async () => {
-      let task = await user.post('/tasks?tasksOwner=user', {
+      let task = await user.post('/tasks/user', {
         text: 'test habit',
         type: 'habit',
         notes: 1976,
@@ -235,7 +235,7 @@ describe('POST /tasks', () => {
     });
 
     it('cannot create checklists', async () => {
-      let task = await user.post('/tasks?tasksOwner=user', {
+      let task = await user.post('/tasks/user', {
         text: 'test habit',
         type: 'habit',
         checklist: [
@@ -249,7 +249,7 @@ describe('POST /tasks', () => {
 
   context('todos', () => {
     it('creates a todo', async () => {
-      let task = await user.post('/tasks?tasksOwner=user', {
+      let task = await user.post('/tasks/user', {
         text: 'test todo',
         type: 'todo',
         notes: 1976,
@@ -262,7 +262,7 @@ describe('POST /tasks', () => {
     });
 
     it('creates multiple todos', async () => {
-      let [task, task2] = await user.post('/tasks?tasksOwner=user', [{
+      let [task, task2] = await user.post('/tasks/user', [{
         text: 'test todo',
         type: 'todo',
         notes: 1976,
@@ -285,7 +285,7 @@ describe('POST /tasks', () => {
 
     it('updates user.tasksOrder.todos when a new todo is created', async () => {
       let originalTodosOrderLen = (await user.get('/user')).tasksOrder.todos.length;
-      let task = await user.post('/tasks?tasksOwner=user', {
+      let task = await user.post('/tasks/user', {
         type: 'todo',
         text: 'a todo',
       });
@@ -297,7 +297,7 @@ describe('POST /tasks', () => {
 
     it('updates user.tasksOrder.todos when multiple todos are created', async () => {
       let originalTodosOrderLen = (await user.get('/user')).tasksOrder.todos.length;
-      let [task, task2] = await user.post('/tasks?tasksOwner=user', [{
+      let [task, task2] = await user.post('/tasks/user', [{
         type: 'todo',
         text: 'a todo',
       }, {
@@ -312,7 +312,7 @@ describe('POST /tasks', () => {
     });
 
     it('can create checklists', async () => {
-      let task = await user.post('/tasks?tasksOwner=user', {
+      let task = await user.post('/tasks/user', {
         text: 'test todo',
         type: 'todo',
         checklist: [
@@ -333,7 +333,7 @@ describe('POST /tasks', () => {
     it('creates a daily', async () => {
       let now = new Date();
 
-      let task = await user.post('/tasks?tasksOwner=user', {
+      let task = await user.post('/tasks/user', {
         text: 'test daily',
         type: 'daily',
         notes: 1976,
@@ -352,7 +352,7 @@ describe('POST /tasks', () => {
     });
 
     it('creates multiple dailys', async () => {
-      let [task, task2] = await user.post('/tasks?tasksOwner=user', [{
+      let [task, task2] = await user.post('/tasks/user', [{
         text: 'test daily',
         type: 'daily',
         notes: 1976,
@@ -375,7 +375,7 @@ describe('POST /tasks', () => {
 
     it('updates user.tasksOrder.dailys when a new daily is created', async () => {
       let originalDailysOrderLen = (await user.get('/user')).tasksOrder.dailys.length;
-      let task = await user.post('/tasks?tasksOwner=user', {
+      let task = await user.post('/tasks/user', {
         type: 'daily',
         text: 'a daily',
       });
@@ -387,7 +387,7 @@ describe('POST /tasks', () => {
 
     it('updates user.tasksOrder.dailys when multiple dailys are created', async () => {
       let originalDailysOrderLen = (await user.get('/user')).tasksOrder.dailys.length;
-      let [task, task2] = await user.post('/tasks?tasksOwner=user', [{
+      let [task, task2] = await user.post('/tasks/user', [{
         type: 'daily',
         text: 'a daily',
       }, {
@@ -402,7 +402,7 @@ describe('POST /tasks', () => {
     });
 
     it('defaults to a weekly frequency, with every day set', async () => {
-      let task = await user.post('/tasks?tasksOwner=user', {
+      let task = await user.post('/tasks/user', {
         text: 'test daily',
         type: 'daily',
       });
@@ -421,7 +421,7 @@ describe('POST /tasks', () => {
     });
 
     it('allows repeat field to be configured', async () => {
-      let task = await user.post('/tasks?tasksOwner=user', {
+      let task = await user.post('/tasks/user', {
         text: 'test daily',
         type: 'daily',
         repeat: {
@@ -445,7 +445,7 @@ describe('POST /tasks', () => {
     it('defaults startDate to today', async () => {
       let today = (new Date()).getDay();
 
-      let task = await user.post('/tasks?tasksOwner=user', {
+      let task = await user.post('/tasks/user', {
         text: 'test daily',
         type: 'daily',
       });
@@ -454,7 +454,7 @@ describe('POST /tasks', () => {
     });
 
     it('can create checklists', async () => {
-      let task = await user.post('/tasks?tasksOwner=user', {
+      let task = await user.post('/tasks/user', {
         text: 'test daily',
         type: 'daily',
         checklist: [
@@ -473,7 +473,7 @@ describe('POST /tasks', () => {
 
   context('rewards', () => {
     it('creates a reward', async () => {
-      let task = await user.post('/tasks?tasksOwner=user', {
+      let task = await user.post('/tasks/user', {
         text: 'test reward',
         type: 'reward',
         notes: 1976,
@@ -488,7 +488,7 @@ describe('POST /tasks', () => {
     });
 
     it('creates multiple rewards', async () => {
-      let [task, task2] = await user.post('/tasks?tasksOwner=user', [{
+      let [task, task2] = await user.post('/tasks/user', [{
         text: 'test reward',
         type: 'reward',
         notes: 1976,
@@ -515,7 +515,7 @@ describe('POST /tasks', () => {
 
     it('updates user.tasksOrder.rewards when a new reward is created', async () => {
       let originalRewardsOrderLen = (await user.get('/user')).tasksOrder.rewards.length;
-      let task = await user.post('/tasks?tasksOwner=user', {
+      let task = await user.post('/tasks/user', {
         type: 'reward',
         text: 'a reward',
       });
@@ -527,7 +527,7 @@ describe('POST /tasks', () => {
 
     it('updates user.tasksOrder.dreward when multiple rewards are created', async () => {
       let originalRewardsOrderLen = (await user.get('/user')).tasksOrder.rewards.length;
-      let [task, task2] = await user.post('/tasks?tasksOwner=user', [{
+      let [task, task2] = await user.post('/tasks/user', [{
         type: 'reward',
         text: 'a reward',
       }, {
@@ -542,7 +542,7 @@ describe('POST /tasks', () => {
     });
 
     it('defaults to a 0 value', async () => {
-      let task = await user.post('/tasks?tasksOwner=user', {
+      let task = await user.post('/tasks/user', {
         text: 'test reward',
         type: 'reward',
       });
@@ -551,7 +551,7 @@ describe('POST /tasks', () => {
     });
 
     it('requires value to be coerced into a number', async () => {
-      let task = await user.post('/tasks?tasksOwner=user', {
+      let task = await user.post('/tasks/user', {
         text: 'test reward',
         type: 'reward',
         value: '10',
@@ -561,7 +561,7 @@ describe('POST /tasks', () => {
     });
 
     it('cannot create checklists', async () => {
-      let task = await user.post('/tasks?tasksOwner=user', {
+      let task = await user.post('/tasks/user', {
         text: 'test reward',
         type: 'reward',
         checklist: [
diff --git a/test/api/v3/integration/tasks/POST-tasks_id_score_direction.test.js b/test/api/v3/integration/tasks/POST-tasks_id_score_direction.test.js
index 1c552613b4..139db4310f 100644
--- a/test/api/v3/integration/tasks/POST-tasks_id_score_direction.test.js
+++ b/test/api/v3/integration/tasks/POST-tasks_id_score_direction.test.js
@@ -37,7 +37,7 @@ describe('POST /tasks/:id/score/:direction', () => {
     let todo;
 
     beforeEach(() => {
-      return user.post('/tasks?tasksOwner=user', {
+      return user.post('/tasks/user', {
         text: 'test todo',
         type: 'todo',
       }).then((task) => {
@@ -149,7 +149,7 @@ describe('POST /tasks/:id/score/:direction', () => {
     let daily;
 
     beforeEach(() => {
-      return user.post('/tasks?tasksOwner=user', {
+      return user.post('/tasks/user', {
         text: 'test daily',
         type: 'daily',
       }).then((task) => {
@@ -226,26 +226,26 @@ describe('POST /tasks/:id/score/:direction', () => {
     let habit, minusHabit, plusHabit, neitherHabit; // eslint-disable-line no-unused-vars
 
     beforeEach(() => {
-      return user.post('/tasks?tasksOwner=user', {
+      return user.post('/tasks/user', {
         text: 'test habit',
         type: 'habit',
       }).then((task) => {
         habit = task;
-        return user.post('/tasks?tasksOwner=user', {
+        return user.post('/tasks/user', {
           text: 'test min habit',
           type: 'habit',
           up: false,
         });
       }).then((task) => {
         minusHabit = task;
-        return user.post('/tasks?tasksOwner=user', {
+        return user.post('/tasks/user', {
           text: 'test plus habit',
           type: 'habit',
           down: false,
         });
       }).then((task) => {
         plusHabit = task;
-        user.post('/tasks?tasksOwner=user', {
+        user.post('/tasks/user', {
           text: 'test neither habit',
           type: 'habit',
           up: false,
@@ -297,7 +297,7 @@ describe('POST /tasks/:id/score/:direction', () => {
     let reward;
 
     beforeEach(() => {
-      return user.post('/tasks?tasksOwner=user', {
+      return user.post('/tasks/user', {
         text: 'test reward',
         type: 'reward',
         value: 5,
diff --git a/test/api/v3/integration/tasks/PUT-tasks_id.test.js b/test/api/v3/integration/tasks/PUT-tasks_id.test.js
index 408ef92e99..453fb03cea 100644
--- a/test/api/v3/integration/tasks/PUT-tasks_id.test.js
+++ b/test/api/v3/integration/tasks/PUT-tasks_id.test.js
@@ -16,7 +16,7 @@ describe('PUT /tasks/:id', () => {
     let task;
 
     beforeEach(() => {
-      return user.post('/tasks?tasksOwner=user', {
+      return user.post('/tasks/user', {
         text: 'test habit',
         type: 'habit',
       }).then((createdTask) => {
@@ -65,7 +65,7 @@ describe('PUT /tasks/:id', () => {
     let habit;
 
     beforeEach(() => {
-      return user.post('/tasks?tasksOwner=user', {
+      return user.post('/tasks/user', {
         text: 'test habit',
         type: 'habit',
         notes: 1976,
@@ -93,7 +93,7 @@ describe('PUT /tasks/:id', () => {
     let todo;
 
     beforeEach(() => {
-      return user.post('/tasks?tasksOwner=user', {
+      return user.post('/tasks/user', {
         text: 'test todo',
         type: 'todo',
         notes: 1976,
@@ -150,7 +150,7 @@ describe('PUT /tasks/:id', () => {
     let daily;
 
     beforeEach(() => {
-      return user.post('/tasks?tasksOwner=user', {
+      return user.post('/tasks/user', {
         text: 'test daily',
         type: 'daily',
         notes: 1976,
@@ -254,7 +254,7 @@ describe('PUT /tasks/:id', () => {
     let reward;
 
     beforeEach(() => {
-      return user.post('/tasks?tasksOwner=user', {
+      return user.post('/tasks/user', {
         text: 'test reward',
         type: 'reward',
         notes: 1976,
diff --git a/test/api/v3/integration/tasks/checklists/DELETE-tasks_taskId_checklist_itemId.test.js b/test/api/v3/integration/tasks/checklists/DELETE-tasks_taskId_checklist_itemId.test.js
index 738255996a..2c6de8e570 100644
--- a/test/api/v3/integration/tasks/checklists/DELETE-tasks_taskId_checklist_itemId.test.js
+++ b/test/api/v3/integration/tasks/checklists/DELETE-tasks_taskId_checklist_itemId.test.js
@@ -16,7 +16,7 @@ describe('DELETE /tasks/:taskId/checklist/:itemId', () => {
   it('deletes a checklist item', () => {
     let task;
 
-    return user.post('/tasks?tasksOwner=user', {
+    return user.post('/tasks/user', {
       type: 'daily',
       text: 'Daily with checklist',
     }).then(createdTask => {
@@ -33,7 +33,7 @@ describe('DELETE /tasks/:taskId/checklist/:itemId', () => {
 
   it('does not work with habits', () => {
     let habit;
-    return expect(user.post('/tasks?tasksOwner=user', {
+    return expect(user.post('/tasks/user', {
       type: 'habit',
       text: 'habit with checklist',
     }).then(createdTask => {
@@ -47,7 +47,7 @@ describe('DELETE /tasks/:taskId/checklist/:itemId', () => {
   });
 
   it('does not work with rewards', async () => {
-    let reward = await user.post('/tasks?tasksOwner=user', {
+    let reward = await user.post('/tasks/user', {
       type: 'reward',
       text: 'reward with checklist',
     });
@@ -68,7 +68,7 @@ describe('DELETE /tasks/:taskId/checklist/:itemId', () => {
   });
 
   it('fails on checklist item not found', () => {
-    return expect(user.post('/tasks?tasksOwner=user', {
+    return expect(user.post('/tasks/user', {
       type: 'daily',
       text: 'daily with checklist',
     }).then(createdTask => {
diff --git a/test/api/v3/integration/tasks/checklists/POST-tasks_taskId_checklist.test.js b/test/api/v3/integration/tasks/checklists/POST-tasks_taskId_checklist.test.js
index 1ad4f7f58c..fd79b89ae9 100644
--- a/test/api/v3/integration/tasks/checklists/POST-tasks_taskId_checklist.test.js
+++ b/test/api/v3/integration/tasks/checklists/POST-tasks_taskId_checklist.test.js
@@ -16,7 +16,7 @@ describe('POST /tasks/:taskId/checklist/', () => {
   it('adds a checklist item to a task', () => {
     let task;
 
-    return user.post('/tasks?tasksOwner=user', {
+    return user.post('/tasks/user', {
       type: 'daily',
       text: 'Daily with checklist',
     }).then(createdTask => {
@@ -36,7 +36,7 @@ describe('POST /tasks/:taskId/checklist/', () => {
   it('does not add a checklist to habits', () => {
     let habit;
 
-    return expect(user.post('/tasks?tasksOwner=user', {
+    return expect(user.post('/tasks/user', {
       type: 'habit',
       text: 'habit with checklist',
     }).then(createdTask => {
@@ -51,7 +51,7 @@ describe('POST /tasks/:taskId/checklist/', () => {
 
   it('does not add a checklist to rewards', () => {
     let reward;
-    return expect(user.post('/tasks?tasksOwner=user', {
+    return expect(user.post('/tasks/user', {
       type: 'reward',
       text: 'reward with checklist',
     }).then(createdTask => {
diff --git a/test/api/v3/integration/tasks/checklists/POST-tasks_taskId_checklist_itemId_score.test.js b/test/api/v3/integration/tasks/checklists/POST-tasks_taskId_checklist_itemId_score.test.js
index 862b67b638..cc386341b7 100644
--- a/test/api/v3/integration/tasks/checklists/POST-tasks_taskId_checklist_itemId_score.test.js
+++ b/test/api/v3/integration/tasks/checklists/POST-tasks_taskId_checklist_itemId_score.test.js
@@ -16,7 +16,7 @@ describe('POST /tasks/:taskId/checklist/:itemId/score', () => {
   it('scores a checklist item', () => {
     let task;
 
-    return user.post('/tasks?tasksOwner=user', {
+    return user.post('/tasks/user', {
       type: 'daily',
       text: 'Daily with checklist',
     }).then(createdTask => {
@@ -32,7 +32,7 @@ describe('POST /tasks/:taskId/checklist/:itemId/score', () => {
 
   it('fails on habits', () => {
     let habit;
-    return expect(user.post('/tasks?tasksOwner=user', {
+    return expect(user.post('/tasks/user', {
       type: 'habit',
       text: 'habit with checklist',
     }).then(createdTask => {
@@ -46,7 +46,7 @@ describe('POST /tasks/:taskId/checklist/:itemId/score', () => {
   });
 
   it('fails on rewards', async () => {
-    let reward = await user.post('/tasks?tasksOwner=user', {
+    let reward = await user.post('/tasks/user', {
       type: 'reward',
       text: 'reward with checklist',
     });
@@ -67,7 +67,7 @@ describe('POST /tasks/:taskId/checklist/:itemId/score', () => {
   });
 
   it('fails on checklist item not found', () => {
-    return expect(user.post('/tasks?tasksOwner=user', {
+    return expect(user.post('/tasks/user', {
       type: 'daily',
       text: 'daily with checklist',
     }).then(createdTask => {
diff --git a/test/api/v3/integration/tasks/checklists/PUT-tasks_taskId_checklist_itemId.test.js b/test/api/v3/integration/tasks/checklists/PUT-tasks_taskId_checklist_itemId.test.js
index ae6babf778..2df3213f2b 100644
--- a/test/api/v3/integration/tasks/checklists/PUT-tasks_taskId_checklist_itemId.test.js
+++ b/test/api/v3/integration/tasks/checklists/PUT-tasks_taskId_checklist_itemId.test.js
@@ -16,7 +16,7 @@ describe('PUT /tasks/:taskId/checklist/:itemId', () => {
   it('updates a checklist item', () => {
     let task;
 
-    return user.post('/tasks?tasksOwner=user', {
+    return user.post('/tasks/user', {
       type: 'daily',
       text: 'Daily with checklist',
     }).then(createdTask => {
@@ -33,7 +33,7 @@ describe('PUT /tasks/:taskId/checklist/:itemId', () => {
   });
 
   it('fails on habits', async () => {
-    let habit = await user.post('/tasks?tasksOwner=user', {
+    let habit = await user.post('/tasks/user', {
       type: 'habit',
       text: 'habit with checklist',
     });
@@ -46,7 +46,7 @@ describe('PUT /tasks/:taskId/checklist/:itemId', () => {
   });
 
   it('fails on rewards', async () => {
-    let reward = await user.post('/tasks?tasksOwner=user', {
+    let reward = await user.post('/tasks/user', {
       type: 'reward',
       text: 'reward with checklist',
     });
@@ -67,7 +67,7 @@ describe('PUT /tasks/:taskId/checklist/:itemId', () => {
   });
 
   it('fails on checklist item not found', () => {
-    return expect(user.post('/tasks?tasksOwner=user', {
+    return expect(user.post('/tasks/user', {
       type: 'daily',
       text: 'daily with checklist',
     }).then(createdTask => {
diff --git a/test/api/v3/integration/tasks/tags/DELETE-tasks_taskId_tags_tagId.test.js b/test/api/v3/integration/tasks/tags/DELETE-tasks_taskId_tags_tagId.test.js
index 27d57bb2f0..8a9617c803 100644
--- a/test/api/v3/integration/tasks/tags/DELETE-tasks_taskId_tags_tagId.test.js
+++ b/test/api/v3/integration/tasks/tags/DELETE-tasks_taskId_tags_tagId.test.js
@@ -17,7 +17,7 @@ describe('DELETE /tasks/:taskId/tags/:tagId', () => {
     let tag;
     let task;
 
-    return user.post('/tasks?tasksOwner=user', {
+    return user.post('/tasks/user', {
       type: 'habit',
       text: 'Task with tag',
     }).then(createdTask => {
@@ -35,7 +35,7 @@ describe('DELETE /tasks/:taskId/tags/:tagId', () => {
   });
 
   it('only deletes existing tags', () => {
-    return expect(user.post('/tasks?tasksOwner=user', {
+    return expect(user.post('/tasks/user', {
       type: 'habit',
       text: 'Task with tag',
     }).then(createdTask => {
diff --git a/test/api/v3/integration/tasks/tags/POST-tasks_taskId_tags_tagId.test.js b/test/api/v3/integration/tasks/tags/POST-tasks_taskId_tags_tagId.test.js
index 9887a5bedb..ddc0c0fe6b 100644
--- a/test/api/v3/integration/tasks/tags/POST-tasks_taskId_tags_tagId.test.js
+++ b/test/api/v3/integration/tasks/tags/POST-tasks_taskId_tags_tagId.test.js
@@ -17,7 +17,7 @@ describe('POST /tasks/:taskId/tags/:tagId', () => {
     let tag;
     let task;
 
-    return user.post('/tasks?tasksOwner=user', {
+    return user.post('/tasks/user', {
       type: 'habit',
       text: 'Task with tag',
     }).then(createdTask => {
@@ -35,7 +35,7 @@ describe('POST /tasks/:taskId/tags/:tagId', () => {
     let tag;
     let task;
 
-    return expect(user.post('/tasks?tasksOwner=user', {
+    return expect(user.post('/tasks/user', {
       type: 'habit',
       text: 'Task with tag',
     }).then(createdTask => {
@@ -54,7 +54,7 @@ describe('POST /tasks/:taskId/tags/:tagId', () => {
   });
 
   it('does not add a non existing tag to a task', () => {
-    return expect(user.post('/tasks?tasksOwner=user', {
+    return expect(user.post('/tasks/user', {
       type: 'habit',
       text: 'Task with tag',
     }).then((task) => {
diff --git a/website/src/controllers/api-v3/tasks.js b/website/src/controllers/api-v3/tasks.js
index 0f5ecc314c..a061ac28e5 100644
--- a/website/src/controllers/api-v3/tasks.js
+++ b/website/src/controllers/api-v3/tasks.js
@@ -16,88 +16,197 @@ import { preenHistory } from '../../../../common/script/api-v3/preenHistory';
 
 let api = {};
 
+// challenge must be passed only when a challenge task is being created
+async function _createTasks (req, res, user, challenge) {
+  let toSave = Array.isArray(req.body) ? req.body : [req.body];
+
+  toSave = toSave.map(taskData => {
+    // Validate that task.type is valid
+    if (!taskData || Tasks.tasksTypes.indexOf(taskData.type) === -1) throw new BadRequest(res.t('invalidTaskType'));
+
+    let taskType = taskData.type;
+    let newTask = new Tasks[taskType](Tasks.Task.sanitizeCreate(taskData));
+
+    if (challenge) {
+      newTask.challenge.id = challenge.id;
+    } else {
+      newTask.userId = user._id;
+    }
+
+    // Validate that the task is valid and throw if it isn't
+    // 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/challenge
+    (challenge || user).tasksOrder[`${taskType}s`].unshift(newTask._id);
+
+    return newTask;
+  }).map(task => task.save({ // If all tasks are valid (this is why it's not in the previous .map()), save everything, withough running validation again
+    validateBeforeSave: false,
+  }));
+
+  toSave.unshift((challenge || user).save());
+
+  let tasks = await Q.all(toSave);
+  tasks.splice(0, 1); // Remove user or challenge
+  return tasks;
+}
+
 /**
- * @api {post} /tasks Create a new task. Can be passed an object to create a single task or an array of objects to create multiple tasks.
+ * @api {post} /tasks/user Create a new task belonging to the autheticated user. Can be passed an object to create a single task or an array of objects to create multiple tasks.
  * @apiVersion 3.0.0
- * @apiName CreateTask
+ * @apiName CreateUserTasks
  * @apiGroup Task
  *
- * @apiParam {string="user","challenge"} tasksOwner Query parameter to define if tasks will belong to the auhenticated user or to a challenge (specifying the "challengeId" parameter).
- * @apiParam {UUID} challengeId Optional. Query parameter. If "tasksOwner" is "challenge" then specify the challenge id.
- *
  * @apiSuccess {Object|Array} task The newly created task(s)
  */
-api.createTask = {
+api.createUserTasks = {
   method: 'POST',
-  url: '/tasks',
+  url: '/tasks/user',
   middlewares: [authWithHeaders(), cron],
   async handler (req, res) {
-    req.checkQuery('tasksOwner', res.t('invalidTasksOwner')).isIn(['user', 'challenge']);
-    req.checkQuery('challengeId', res.t('challengeIdRequired')).optional().isUUID();
+    let tasks = await _createTasks(req, res, res.locals.user);
+    res.respond(201, tasks.length === 1 ? tasks[0] : tasks);
+  },
+};
+
+/**
+ * @api {post} /tasks/challenge/:challengeId Create a new task belonging to the challenge. Can be passed an object to create a single task or an array of objects to create multiple tasks.
+ * @apiVersion 3.0.0
+ * @apiName CreateChallengeTasks
+ * @apiGroup Task
+ *
+ * @apiParam {UUID} challengeId The id of the challenge the new task(s) will belong to.
+ *
+ * @apiSuccess {Object|Array} task The newly created task(s)
+ */
+api.createChallengeTasks = {
+  method: 'POST',
+  url: '/tasks/challenge/:challengeId',
+  middlewares: [authWithHeaders(), cron],
+  async handler (req, res) {
+    req.checkQuery('challengeId', res.t('challengeIdRequired')).notEmpty().isUUID();
 
     let reqValidationErrors = req.validationErrors();
     if (reqValidationErrors) throw reqValidationErrors;
 
-    let tasksData = Array.isArray(req.body) ? req.body : [req.body];
-    let user = res.locals.user;
-    let tasksOwner = req.query.tasksOwner;
-    let challengeId = req.query.challengeId;
-    let challenge;
+    let user = res.local.user;
+    let challengeId = req.params.challengeId;
 
-    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();
+    let 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) throw new NotFound(res.t('challengeNotFound'));
-      if (challenge.leader !== user._id) throw new NotAuthorized(res.t('onlyChalLeaderEditTasks'));
-    }
+    // If the challenge does not exist, or if it exists but user is not the leader -> throw error
+    if (!challenge || user.challenges.indexOf(challengeId) === -1) throw new NotFound(res.t('challengeNotFound'));
+    if (challenge.leader !== user._id) throw new NotAuthorized(res.t('onlyChalLeaderEditTasks'));
 
-    let toSave = tasksData.map(taskData => {
-      // Validate that task.type is valid
-      if (!taskData || Tasks.tasksTypes.indexOf(taskData.type) === -1) throw new BadRequest(res.t('invalidTaskType'));
-
-      let taskType = taskData.type;
-      let newTask = new Tasks[taskType](Tasks.Task.sanitizeCreate(taskData));
-
-      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/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/challenge
-      (challenge || user).tasksOrder[`${taskType}s`].unshift(newTask._id);
-
-      return newTask;
-    });
-
-    // If all tasks are valid, save everything, withough running validation again
-    toSave = toSave.map(task => task.save({
-      validateBeforeSave: false,
-    }));
-    toSave.unshift((challenge || user).save());
-
-    let tasks = await Q.all(toSave);
-
-    if (tasks.length === 2) {
-      res.respond(201, tasks[1]);
-    } else {
-      tasks.splice(0, 1); // remove the user/challenge
-      res.respond(201, tasks);
-    }
+    let tasks = await _createTasks(req, res, user, challenge);
+    res.respond(201, tasks.length === 1 ? tasks[0] : tasks);
 
     // If adding tasks to a challenge -> sync users
     if (challenge) challenge.addTasks(tasks); // TODO catch/log
   },
 };
 
+// challenge must be passed only when a challenge task is being created
+async function _getTasks (req, res, user, challenge) {
+  let query = challenge ? {'challenge.id': challenge.id, userId: {$exists: false}} : {userId: user._id};
+  let type = req.query.type;
+
+  if (type) {
+    query.type = type;
+    if (type === 'todo') query.completed = false; // Exclude completed todos
+  } else {
+    query.$or = [ // Exclude completed todos
+      {type: 'todo', completed: false},
+      {type: {$in: ['habit', 'daily', 'reward']}},
+    ];
+  }
+
+  if (req.query.includeCompletedTodos === 'true' && (!type || type === 'todo')) {
+    if (challenge) throw new BadRequest(res.t('noCompletedTodosChallenge')); // no completed todos for challenges
+
+    let queryCompleted = Tasks.Task.find({
+      type: 'todo',
+      completed: true,
+    }).limit(30).sort({ // TODO add ability to pick more than 30 completed todos
+      dateCompleted: 1,
+    });
+
+    let results = await Q.all([
+      queryCompleted.exec(),
+      Tasks.Task.find(query).exec(),
+    ]);
+
+    res.respond(200, results[1].concat(results[0]));
+  } else {
+    let tasks = await Tasks.Task.find(query).exec();
+    res.respond(200, tasks);
+  }
+}
+
+/**
+ * @api {get} /tasks/user Get an user's tasks
+ * @apiVersion 3.0.0
+ * @apiName GetUserTasks
+ * @apiGroup Task
+ *
+ * @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". Only valid whe "tasksOwner" is "user".
+ *
+ * @apiSuccess {Array} tasks An array of task objects
+ */
+api.getUserTasks = {
+  method: 'GET',
+  url: '/tasks/user',
+  middlewares: [authWithHeaders(), cron],
+  async handler (req, res) {
+    req.checkQuery('type', res.t('invalidTaskType')).optional().isIn(Tasks.tasksTypes);
+
+    let validationErrors = req.validationErrors();
+    if (validationErrors) throw validationErrors;
+
+    await _getTasks(req, res, res.locals.user);
+  },
+};
+
+/**
+ * @api {get} /tasks/challenge/:challengeId Get a challenge's tasks
+ * @apiVersion 3.0.0
+ * @apiName GetChallengeTasks
+ * @apiGroup Task
+ *
+ * @apiParam {UUID} challengeId The id of the challenge from which to retrieve the tasks.
+ *
+ * @apiParam {string="habit","daily","todo","reward"} type Optional query parameter to return just a type of tasks
+ *
+ * @apiSuccess {Array} tasks An array of task objects
+ */
+api.getChallengeTasks = {
+  method: 'GET',
+  url: '/tasks/challenge/:challengeId',
+  middlewares: [authWithHeaders(), cron],
+  async handler (req, res) {
+    req.checkQuery('challengeId', res.t('challengeIdRequired')).notEmpty().isUUID();
+    req.checkQuery('type', res.t('invalidTaskType')).optional().isIn(Tasks.tasksTypes);
+
+    let validationErrors = req.validationErrors();
+    if (validationErrors) throw validationErrors;
+
+    let user = res.local.user;
+    let challengeId = req.params.challengeId;
+
+    let challenge = await Challenge.findOne({_id: challengeId}).select('leader').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'));
+    }
+
+    await _getTasks(req, res, res.locals.user, challenge);
+  },
+};
+
 /**
  * @api {get} /tasks/:tasksOwner/:challengeId Get an user's tasks
  * @apiVersion 3.0.0
@@ -116,29 +225,10 @@ api.getTasks = {
   url: '/tasks',
   middlewares: [authWithHeaders(), cron],
   async handler (req, res) {
-    req.checkQuery('tasksOwner', res.t('invalidTasksOwner')).isIn(['user', 'challenge']);
-    req.checkQuery('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 tasksOwner = req.query.tasksOwner;
     let challengeId = req.query.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}).select('leader').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;
 

From 9dfcad238cf7ad0b4d4afa9448bf001a02d39ccc Mon Sep 17 00:00:00 2001
From: Matteo Pagliazzi <matteopagliazzi@gmail.com>
Date: Wed, 6 Jan 2016 18:41:25 +0100
Subject: [PATCH 09/10] rename tasks tests to match new routes

---
 .../tasks/{GET-tasks.test.js => GET-tasks_user.test.js}         | 0
 .../tasks/{POST-tasks.test.js => POST-tasks_user.test.js}       | 2 +-
 2 files changed, 1 insertion(+), 1 deletion(-)
 rename test/api/v3/integration/tasks/{GET-tasks.test.js => GET-tasks_user.test.js} (100%)
 rename test/api/v3/integration/tasks/{POST-tasks.test.js => POST-tasks_user.test.js} (99%)

diff --git a/test/api/v3/integration/tasks/GET-tasks.test.js b/test/api/v3/integration/tasks/GET-tasks_user.test.js
similarity index 100%
rename from test/api/v3/integration/tasks/GET-tasks.test.js
rename to test/api/v3/integration/tasks/GET-tasks_user.test.js
diff --git a/test/api/v3/integration/tasks/POST-tasks.test.js b/test/api/v3/integration/tasks/POST-tasks_user.test.js
similarity index 99%
rename from test/api/v3/integration/tasks/POST-tasks.test.js
rename to test/api/v3/integration/tasks/POST-tasks_user.test.js
index 3fd81aa7cd..27da94bb50 100644
--- a/test/api/v3/integration/tasks/POST-tasks.test.js
+++ b/test/api/v3/integration/tasks/POST-tasks_user.test.js
@@ -3,7 +3,7 @@ import {
   translate as t,
 } from '../../../../helpers/api-integration.helper';
 
-describe('POST /tasks', () => {
+describe('POST /tasks/user', () => {
   let user;
 
   before(async () => {

From 9a908785c130c1c32a963b9055176955691f3ca5 Mon Sep 17 00:00:00 2001
From: Matteo Pagliazzi <matteopagliazzi@gmail.com>
Date: Wed, 13 Jan 2016 23:07:08 +0100
Subject: [PATCH 10/10] new history preening, save tasks after cron

---
 common/script/api-v3/cron.js            |   6 +-
 common/script/api-v3/preenHistory.js    |  81 ----------------
 common/script/api-v3/preening.js        |  82 ++++++++++++++++
 test/common/algos.mocha.js              | 118 ------------------------
 test/common/preening.test.js            |  70 ++++++++++++++
 website/src/controllers/api-v3/tasks.js |  27 +++++-
 website/src/middlewares/api-v3/cron.js  |  15 ++-
 7 files changed, 189 insertions(+), 210 deletions(-)
 delete mode 100644 common/script/api-v3/preenHistory.js
 create mode 100644 common/script/api-v3/preening.js
 create mode 100644 test/common/preening.test.js

diff --git a/common/script/api-v3/cron.js b/common/script/api-v3/cron.js
index 604b6254f4..6f8626c7d8 100644
--- a/common/script/api-v3/cron.js
+++ b/common/script/api-v3/cron.js
@@ -1,7 +1,7 @@
 import moment from 'moment';
 import _ from 'lodash';
 import scoreTask from './scoreTask';
-import preenUserHistory from './preenHistory';
+import { preenUserHistory } from './preening';
 import common from '../../';
 import {
   shouldDo,
@@ -65,7 +65,7 @@ export default function cron (options = {}) {
         gemCapExtra: 0,
       });
 
-      user.markModified('purchased.plan'); // TODO necessary?
+      user.markModified('purchased.plan');
     }
   }
 
@@ -197,7 +197,7 @@ export default function cron (options = {}) {
   // preen user history so that it doesn't become a performance problem
   // also for subscribed users but differentyly
   // premium subscribers can keep their full history.
-  preenUserHistory(user, tasksByType);
+  preenUserHistory(user, tasksByType, user.preferences.timezoneOffset);
 
   if (perfect) {
     user.achievements.perfect++;
diff --git a/common/script/api-v3/preenHistory.js b/common/script/api-v3/preenHistory.js
deleted file mode 100644
index 64ae4e4715..0000000000
--- a/common/script/api-v3/preenHistory.js
+++ /dev/null
@@ -1,81 +0,0 @@
-import moment from 'moment';
-import _ from 'lodash';
-
-function _preen (newHistory, history, amount, groupBy) {
-  _.chain(history)
-    .groupBy(h => moment(h.date).format(groupBy))
-    .sortBy((h, k) => k)
-    .slice(-amount)
-    .pop()
-    .each((group) => {
-      newHistory.push({
-        date: moment(group[0].date).toDate(),
-        value: _.reduce(group, (m, obj) => m + obj.value, 0) / group.length,
-      });
-    })
-    .value();
-}
-
-// Free users:
-//  Preen history for users with > 7 history entries
-//  This takes an infinite array of single day entries [day day day day day...], and turns it into a condensed array
-//  of averages, condensing more the further back in time we go. Eg, 7 entries each for last 7 days; 1 entry each week
-//  of this month; 1 entry for each month of this year; 1 entry per previous year: [day*7 week*4 month*12 year*infinite]
-//
-// Subscribers:
-//  TODO implement
-
-// TODO Probably the description ^ is not too correct, this method actually takes 1 value each for the last 50 years,
-// then the X last months, where X is the month we're in (september = 8 starting from 0)
-// and all the days in this month
-// Allowing for multiple values in a single day for habits we probably want something different:
-// For free users:
-// - At max 30 values for today (max 30)
-// - 1 value each for the previous 61 days (2 months)
-// - 1 value each for the previous 10 months (max 10)
-// - 1 value each for the previous 50 years
-// - Total: 30+61+10+ a few years ~= 105
-//
-// For subscribed users
-// - At max 30 values for today (max 30)
-// - 1 value each for the previous 364 days (max 364)
-// - 1 value each for the previous 12 months (max 12)
-// - 1 value each for the previous 50 years
-// - Total: 30+364+12+ a few years ~= 410
-//
-export function preenHistory (history) {
-  // TODO remember to add this to migration
-  /* history = _.filter(history, function(h) {
-    return !!h;
-  }); */
-  let newHistory = [];
-
-  _preen(newHistory, history, 50, 'YYYY');
-  _preen(newHistory, history, moment().format('MM'), 'YYYYMM');
-
-  let thisMonth = moment().format('YYYYMM');
-  newHistory = newHistory.concat(history.filter(h => {
-    return moment(h.date).format('YYYYMM') === thisMonth;
-  }));
-
-  return newHistory;
-}
-
-export function preenUserHistory (user, tasksByType, minHistLen = 7) {
-  tasksByType.habits.concat(tasksByType.dailys).forEach((task) => {
-    if (task.history.length > minHistLen) {
-      task.history = preenHistory(user, task.history);
-      task.markModified('history');
-    }
-  });
-
-  if (user.history.exp.length > minHistLen) {
-    user.history.exp = preenHistory(user, user.history.exp);
-    user.markModified('history.exp');
-  }
-
-  if (user.history.todos.length > minHistLen) {
-    user.history.todos = preenHistory(user, user.history.todos);
-    user.markModified('history.todos');
-  }
-}
diff --git a/common/script/api-v3/preening.js b/common/script/api-v3/preening.js
new file mode 100644
index 0000000000..ee6a201b3a
--- /dev/null
+++ b/common/script/api-v3/preening.js
@@ -0,0 +1,82 @@
+import _ from 'lodash';
+import moment from 'moment';
+
+// Aggregate entries
+function _aggregate (history, aggregateBy) {
+  return _.chain(history)
+    .groupBy(entry => { // group entries by aggregateBy
+      return moment(entry.date).format(aggregateBy);
+    })
+    .sortBy((entry, key) => key) // sort by date
+    .map(entries => {
+      return {
+        date: Number(entries[0].date),
+        value: _.reduce(entries, (previousValue, entry) => {
+          return previousValue + entry.value;
+        }, 0) / entries.length,
+      };
+    })
+    .value();
+}
+
+/* Preen an array of history entries
+Free users:
+- 1 value for each day of the past 60 days (no compression)
+- 1 value each month for the previous 10 months
+- 1 value each year for the previous years
+Subscribers and challenges:
+- 1 value for each day of the past 365 days (no compression)
+- 1 value each month for the previous 12 months
+- 1 value each year for the previous years
+ */
+export function preenHistory (history, isSubscribed, timezoneOffset) {
+  // history = _.filter(history, historyEntry => Boolean(historyEntry)); // Filter missing entries TODO add to migration
+  let now = timezoneOffset ? moment().zone(timezoneOffset) : moment();
+  // Date after which to begin compressing data
+  let cutOff = now.subtract(isSubscribed ? 365 : 60, 'days').startOf('day');
+
+  // Keep uncompressed entries (modifies history)
+  let newHistory = _.remove(history, entry => {
+    let date = moment(entry.date);
+    return date.isSame(cutOff) || date.isAfter(cutOff);
+  });
+
+  // Date after which to begin compressing data by year
+  let monthsCutOff = cutOff.subtract(isSubscribed ? 12 : 10, 'months').startOf('day');
+  let aggregateByMonth = _.remove(history, entry => {
+    let date = moment(entry.date);
+    return date.isSame(monthsCutOff) || date.isAfter(monthsCutOff);
+  });
+  // Aggregate remaining entries by month and year
+  if (aggregateByMonth.length > 0) newHistory.unshift(..._aggregate(aggregateByMonth, 'YYYYMM'));
+  if (history.length > 0) newHistory.unshift(..._aggregate(history, 'YYYY'));
+
+  return newHistory;
+}
+
+// Preen history for users and tasks. This code runs only on the server.
+export function preenUserHistory (user, tasksByType) {
+  let isSubscribed = user.isSubscribed();
+  let timezoneOffset = user.preferences.timezoneOffset;
+  let minHistoryLength = isSubscribed ? 365 : 60;
+
+  function _processTask (task) {
+    if (task.history && task.history.length > minHistoryLength) {
+      task.history = preenHistory(task.history, isSubscribed, timezoneOffset);
+      task.markModified('history');
+    }
+  }
+
+  tasksByType.habits.forEach(_processTask);
+  tasksByType.dailys.forEach(_processTask);
+
+  if (user.history.exp.length > minHistoryLength) {
+    user.history.exp = preenHistory(user.history.exp, isSubscribed, timezoneOffset);
+    user.markModified('history.exp');
+  }
+
+  if (user.history.todos.length > minHistoryLength) {
+    user.history.todos = preenHistory(user.history.todos, isSubscribed, timezoneOffset);
+    user.markModified('history.todos');
+  }
+}
diff --git a/test/common/algos.mocha.js b/test/common/algos.mocha.js
index 8c18e3c92c..0528d62933 100644
--- a/test/common/algos.mocha.js
+++ b/test/common/algos.mocha.js
@@ -938,124 +938,6 @@ describe('Cron', () => {
     expect(beforeTasks).to.eql(afterTasks);
   });
 
-  describe('preening', () => {
-    beforeEach(function () {
-      this.clock = sinon.useFakeTimers(Date.parse('2013-11-20'), 'Date');
-    });
-    afterEach(function () {
-      return this.clock.restore();
-    });
-
-    it('should preen user history', function () {
-      let ref = beforeAfter({
-        daysAgo: 1,
-      });
-      let after = ref.after;
-
-      let history = [
-        {
-          date: '09/01/2012',
-          value: 0,
-        }, {
-          date: '10/01/2012',
-          value: 0,
-        }, {
-          date: '11/01/2012',
-          value: 2,
-        }, {
-          date: '12/01/2012',
-          value: 2,
-        }, {
-          date: '01/01/2013',
-          value: 1,
-        }, {
-          date: '01/15/2013',
-          value: 3,
-        }, {
-          date: '02/01/2013',
-          value: 2,
-        }, {
-          date: '02/15/2013',
-          value: 4,
-        }, {
-          date: '03/01/2013',
-          value: 3,
-        }, {
-          date: '03/15/2013',
-          value: 5,
-        }, {
-          date: '04/01/2013',
-          value: 4,
-        }, {
-          date: '04/15/2013',
-          value: 6,
-        }, {
-          date: '05/01/2013',
-          value: 5,
-        }, {
-          date: '05/15/2013',
-          value: 7,
-        }, {
-          date: '06/01/2013',
-          value: 6,
-        }, {
-          date: '06/15/2013',
-          value: 8,
-        }, {
-          date: '07/01/2013',
-          value: 7,
-        }, {
-          date: '07/15/2013',
-          value: 9,
-        }, {
-          date: '08/01/2013',
-          value: 8,
-        }, {
-          date: '08/15/2013',
-          value: 10,
-        }, {
-          date: '09/01/2013',
-          value: 9,
-        }, {
-          date: '09/15/2013',
-          value: 11,
-        }, {
-          date: '010/01/2013',
-          value: 10,
-        }, {
-          date: '010/15/2013',
-          value: 12,
-        }, {
-          date: '011/01/2013',
-          value: 12,
-        }, {
-          date: '011/02/2013',
-          value: 13,
-        }, {
-          date: '011/03/2013',
-          value: 14,
-        }, {
-          date: '011/04/2013',
-          value: 15,
-        },
-      ];
-
-      after.history = {
-        exp: _.cloneDeep(history),
-        todos: _.cloneDeep(history),
-      };
-      after.habits[0].history = _.cloneDeep(history);
-      after.fns.cron();
-      after.history.exp.pop();
-      after.history.todos.pop();
-      _.each([after.history.exp, after.history.todos, after.habits[0].history], function (arr) {
-        expect(_.map(arr, (x) => {
-          return x.value;
-        })).to.eql([1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15]);
-      });
-    });
-  });
-
   describe('Todos', () => {
     it('1 day missed', () => {
       let ref = beforeAfter({
diff --git a/test/common/preening.test.js b/test/common/preening.test.js
new file mode 100644
index 0000000000..f19a3c26b5
--- /dev/null
+++ b/test/common/preening.test.js
@@ -0,0 +1,70 @@
+import { preenHistory } from '../../common/script/preening';
+import moment from 'moment';
+import sinon from 'sinon'; // eslint-disable-line no-shadow
+
+function generateHistory (days) {
+  let history = [];
+  let now = Number(moment().toDate());
+
+  while (days > 0) {
+    history.push({
+      value: days,
+      date: Number(moment(now).subtract(days, 'days').toDate()),
+    });
+    days--;
+  }
+
+  return history;
+}
+
+describe('preenHistory', () => {
+  let clock;
+
+  beforeEach(() => {
+    // Replace system clocks so we can get predictable results
+    clock = sinon.useFakeTimers(Number(moment('2013-10-20').zone(0).startOf('day').toDate()), 'Date');
+  });
+  afterEach(() => {
+    return clock.restore();
+  });
+
+  it('does not modify history if all entries are more recent than cutoff (free users)', () => {
+    let h = generateHistory(60);
+    expect(preenHistory(_.cloneDeep(h), false, 0)).to.eql(h);
+  });
+
+  it('does not modify history if all entries are more recent than cutoff (subscribers)', () => {
+    let h = generateHistory(365);
+    expect(preenHistory(_.cloneDeep(h), true, 0)).to.eql(h);
+  });
+
+  it('does aggregate data in monthly entries before cutoff (free users)', () => {
+    let h = generateHistory(81); // Jumps to July
+    let preened = preenHistory(_.cloneDeep(h), false, 0);
+    expect(preened.length).to.eql(62); // Keeps 60 days + 2 entries per august and july
+  });
+
+  it('does aggregate data in monthly entries before cutoff (subscribers)', () => {
+    let h = generateHistory(396); // Jumps to September 2012
+    let preened = preenHistory(_.cloneDeep(h), true, 0);
+    expect(preened.length).to.eql(367); // Keeps 365 days + 2 entries per october and september
+  });
+
+  it('does aggregate data in monthly and yearly entries before cutoff (free users)', () => {
+    let h = generateHistory(731); // Jumps to October 21 2012
+    let preened = preenHistory(_.cloneDeep(h), false, 0);
+    expect(preened.length).to.eql(73); // Keeps 60 days + 11 montly entries and 2 yearly entry for 2011 and 2012
+  });
+
+  it('does aggregate data in monthly and yearly entries before cutoff (subscribers)', () => {
+    let h = generateHistory(1031); // Jumps to October 21 2012
+    let preened = preenHistory(_.cloneDeep(h), true, 0);
+    expect(preened.length).to.eql(380); // Keeps 365 days + 13 montly entries and 2 yearly entries for 2011 and 2010
+  });
+
+  it('correctly aggregates values', () => {
+    let h = generateHistory(63); // Compress last 3 days
+    let preened = preenHistory(_.cloneDeep(h), false, 0);
+    expect(preened[0].value).to.eql((61 + 62 + 63) / 3);
+  });
+});
diff --git a/website/src/controllers/api-v3/tasks.js b/website/src/controllers/api-v3/tasks.js
index 84ccf0a221..316a4054aa 100644
--- a/website/src/controllers/api-v3/tasks.js
+++ b/website/src/controllers/api-v3/tasks.js
@@ -10,8 +10,9 @@ import {
 import shared from '../../../../common';
 import Q from 'q';
 import _ from 'lodash';
+import moment from 'moment';
 import scoreTask from '../../../../common/script/api-v3/scoreTask';
-import { preenHistory } from '../../../../common/script/api-v3/preenHistory';
+import { preenHistory } from '../../../../common/script/api-v3/preening';
 
 let api = {};
 
@@ -313,11 +314,27 @@ api.scoreTask = {
         }).exec();
 
         chalTask.value += delta;
+
         if (chalTask.type === 'habit' || chalTask.type === 'daily') {
-          chalTask.history.push({value: chalTask.value, date: Number(new Date())});
-          // TODO 1. treat challenges as subscribed users for preening 2. it's expensive to do it at every score - how to have it happen once like for cron?
-          chalTask.history = preenHistory(user, chalTask.history);
-          chalTask.markModified('history');
+          // Add only one history entry per day
+          if (moment(chalTask.history[chalTask.history.length - 1].date).isSame(new Date(), 'day')) {
+            chalTask.history[chalTask.history.length - 1] = {
+              date: Number(new Date()),
+              value: chalTask.value,
+            };
+            chalTask.markModified(`history.${chalTask.history.length - 1}`);
+          } else {
+            chalTask.history.push({
+              date: Number(new Date()),
+              value: chalTask.value,
+            });
+
+            // Only preen task history once a day when the task is scored first
+            if (chalTask.history.length > 365) {
+              chalTask.history = preenHistory(chalTask.history, true); // true means the challenge will retain as much entries as a subscribed user
+              chalTask.markModified(`history.${chalTask.history.length - 1}`);
+            }
+          }
         }
 
         await chalTask.save();
diff --git a/website/src/middlewares/api-v3/cron.js b/website/src/middlewares/api-v3/cron.js
index 0d6d46b900..9457926233 100644
--- a/website/src/middlewares/api-v3/cron.js
+++ b/website/src/middlewares/api-v3/cron.js
@@ -6,9 +6,10 @@ import {
 import cron from '../../../../common/script/api-v3/cron';
 import common from '../../../../common';
 import Task from '../../models/task';
+import Q from 'q';
 // import Group from '../../models/group';
 
-// TODO check that it's usef everywhere
+// TODO check that it's used everywhere
 export default function cronMiddleware (req, res, next) {
   let user = res.locals.user;
   let analytics = res.analytics;
@@ -26,7 +27,7 @@ export default function cronMiddleware (req, res, next) {
       {type: {$in: ['habit', 'daily', 'reward']}},
     ],
   }).exec()
-  .then((tasks) => {
+  .then(tasks => {
     let tasksByType = {habits: [], dailys: [], todos: [], rewards: []};
     tasks.forEach(task => tasksByType[`${task.type}s`].push(task));
 
@@ -49,7 +50,15 @@ export default function cronMiddleware (req, res, next) {
     // if (ranCron) res.locals.wasModified = true; // TODO remove?
     if (!ranCron) return next();
     // TODO Group.tavernBoss(user, progress);
-    if (!quest || true /* TODO remove */) return user.save(next);
+    if (!quest || true /* TODO remove */) {
+      // Save user and tasks
+      let toSave = [user.save()];
+      tasks.forEach(task => {
+        if (task.isModified) toSave.push(task.save());
+      });
+
+      return Q.all(toSave).then(() => next()).catch(next);
+    }
 
     // If user is on a quest, roll for boss & player, or handle collections
     // FIXME this saves user, runs db updates, loads user. Is there a better way to handle this?