From 21d798bca70b57c455468726e52bd9b1ea1f1d2f Mon Sep 17 00:00:00 2001
From: Matteo Pagliazzi <matteopagliazzi@gmail.com>
Date: Tue, 3 May 2016 15:49:47 +0200
Subject: [PATCH 01/12] v3: GET /groups accept a guilds type which returns all
 the guilds the user is a member of

---
 .../v3/integration/groups/GET-groups.test.js  |  8 +++++-
 website/src/controllers/api-v3/groups.js      |  2 +-
 website/src/models/group.js                   | 26 +++++++++++++------
 3 files changed, 26 insertions(+), 10 deletions(-)

diff --git a/test/api/v3/integration/groups/GET-groups.test.js b/test/api/v3/integration/groups/GET-groups.test.js
index 7e2014e87c..d076e52a18 100644
--- a/test/api/v3/integration/groups/GET-groups.test.js
+++ b/test/api/v3/integration/groups/GET-groups.test.js
@@ -9,7 +9,8 @@ import {
 
 describe('GET /groups', () => {
   let user;
-  const NUMBER_OF_PUBLIC_GUILDS = 3;
+  const NUMBER_OF_PUBLIC_GUILDS = 3; // 2 + the tavern
+  const NUMBER_OF_PUBLIC_GUILDS_USER_IS_MEMBER = 1;
   const NUMBER_OF_USERS_PRIVATE_GUILDS = 1;
   const NUMBER_OF_GROUPS_USER_CAN_VIEW = 5;
 
@@ -87,6 +88,11 @@ describe('GET /groups', () => {
       .to.eventually.have.a.lengthOf(NUMBER_OF_PUBLIC_GUILDS);
   });
 
+  it('returns all the user\'s guilds when guilds passed in as query', async () => {
+    await expect(user.get('/groups?type=guilds'))
+      .to.eventually.have.a.lengthOf(NUMBER_OF_PUBLIC_GUILDS_USER_IS_MEMBER + NUMBER_OF_USERS_PRIVATE_GUILDS);
+  });
+
   it('returns all private guilds user is a part of when privateGuilds passed in as query', async () => {
     await expect(user.get('/groups?type=privateGuilds'))
       .to.eventually.have.a.lengthOf(NUMBER_OF_USERS_PRIVATE_GUILDS);
diff --git a/website/src/controllers/api-v3/groups.js b/website/src/controllers/api-v3/groups.js
index 3f4a7aabb4..41f47a4769 100644
--- a/website/src/controllers/api-v3/groups.js
+++ b/website/src/controllers/api-v3/groups.js
@@ -79,7 +79,7 @@ api.createGroup = {
  * @apiName GetGroups
  * @apiGroup Group
  *
- * @apiParam {string} type The type of groups to retrieve. Must be a query string representing a list of values like 'tavern,party'. Possible values are party, privateGuilds, publicGuilds, tavern
+ * @apiParam {string} type The type of groups to retrieve. Must be a query string representing a list of values like 'tavern,party'. Possible values are party, guilds, privateGuilds, publicGuilds, tavern
  *
  * @apiSuccess {Array} data An array of the requested groups
  */
diff --git a/website/src/models/group.js b/website/src/models/group.js
index a36f983b8d..ae1c1318bb 100644
--- a/website/src/models/group.js
+++ b/website/src/models/group.js
@@ -149,25 +149,35 @@ schema.statics.getGroups = async function getGroups (options = {}) {
         queries.push(this.getGroup({user, groupId: 'party', fields: groupFields, populateLeader}));
         break;
       }
+      case 'guilds': {
+        let userGuildsQuery = this.find({
+          type: 'guild',
+          _id: {$in: user.guilds},
+        }).select(groupFields);
+        if (populateLeader === true) userGuildsQuery.populate('leader', nameFields);
+        userGuildsQuery.sort(sort).exec();
+        queries.push(userGuildsQuery);
+        break;
+      }
       case 'privateGuilds': {
-        let privateGroupQuery = this.find({
+        let privateGuildsQuery = this.find({
           type: 'guild',
           privacy: 'private',
           _id: {$in: user.guilds},
         }).select(groupFields);
-        if (populateLeader === true) privateGroupQuery.populate('leader', nameFields);
-        privateGroupQuery.sort(sort).exec();
-        queries.push(privateGroupQuery);
+        if (populateLeader === true) privateGuildsQuery.populate('leader', nameFields);
+        privateGuildsQuery.sort(sort).exec();
+        queries.push(privateGuildsQuery);
         break;
       }
       case 'publicGuilds': {
-        let publicGroupQuery = this.find({
+        let publicGuildsQuery = this.find({
           type: 'guild',
           privacy: 'public',
         }).select(groupFields);
-        if (populateLeader === true) publicGroupQuery.populate('leader', nameFields);
-        publicGroupQuery.sort(sort).exec();
-        queries.push(publicGroupQuery); // TODO use lean?
+        if (populateLeader === true) publicGuildsQuery.populate('leader', nameFields);
+        publicGuildsQuery.sort(sort).exec();
+        queries.push(publicGuildsQuery); // TODO use lean?
         break;
       }
       case 'tavern': {

From c218a2cbdf5a3e78e9e6a51502b274c4abca55ed Mon Sep 17 00:00:00 2001
From: Matteo Pagliazzi <matteopagliazzi@gmail.com>
Date: Tue, 3 May 2016 15:54:02 +0200
Subject: [PATCH 02/12] v3 fix apidoc broken layout

---
 website/src/controllers/api-v3/challenges.js | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/website/src/controllers/api-v3/challenges.js b/website/src/controllers/api-v3/challenges.js
index dcd0af5c66..4f738afc80 100644
--- a/website/src/controllers/api-v3/challenges.js
+++ b/website/src/controllers/api-v3/challenges.js
@@ -510,7 +510,7 @@ export async function _closeChal (challenge, broken = {}) {
  * @apiName DeleteChallenge
  * @apiGroup Challenge
  *
- * challengeId {UUID} The _id for the challenge to delete
+ * @apiParam {UUID} challengeId The _id for the challenge to delete
  *
  * @apiSuccess {object} data An empty object
  */
@@ -542,8 +542,8 @@ api.deleteChallenge = {
  * @apiName SelectChallengeWinner
  * @apiGroup Challenge
  *
- * challengeId {UUID} The _id for the challenge to close with a winner
- * winnerId {UUID} The _id of the winning user
+ * @apiParam {UUID} challengeId The _id for the challenge to close with a winner
+ * @apiParam {UUID} winnerId The _id of the winning user
  *
  * @apiSuccess {object} data An empty object
  */

From e5e4bb5823bda38c8c31e0e23f3443202cb0c972 Mon Sep 17 00:00:00 2001
From: Matteo Pagliazzi <matteopagliazzi@gmail.com>
Date: Tue, 3 May 2016 23:27:24 +0200
Subject: [PATCH 03/12] v3 migration: correctly migrate challenges tasks

---
 migrations/api_v3/challenges.js        | 21 ++++++++++++++++---
 migrations/api_v3/challengesMembers.js |  7 +++++++
 migrations/api_v3/groups.js            |  7 +++++++
 migrations/api_v3/users.js             | 28 ++++++++++++++++++++++++--
 4 files changed, 58 insertions(+), 5 deletions(-)

diff --git a/migrations/api_v3/challenges.js b/migrations/api_v3/challenges.js
index a650e26188..85a019cc7d 100644
--- a/migrations/api_v3/challenges.js
+++ b/migrations/api_v3/challenges.js
@@ -17,6 +17,7 @@ var mongoose = require('mongoose');
 var _ = require('lodash');
 var uuid = require('uuid');
 var consoleStamp = require('console-stamp');
+var fs = require('fs');
 
 // Add timestamps to console messages
 consoleStamp(console);
@@ -48,6 +49,8 @@ var BATCH_SIZE = 1000;
 var processedChallenges = 0;
 var totoalProcessedTasks = 0;
 
+var newTasksIds = {}; // a map of old id -> [new id, challengeId]
+
 // Only process challenges that fall in a interval ie -> up to 0000-4000-0000-0000
 var AFTER_CHALLENGE_ID = nconf.get('AFTER_CHALLENGE_ID');
 var BEFORE_CHALLENGE_ID = nconf.get('BEFORE_CHALLENGE_ID');
@@ -109,23 +112,33 @@ function processChallenges (afterId) {
       if (!oldChallenge.group) throw new Error('challenge.group is required');
       if (!oldChallenge.leader) throw new Error('challenge.leader is required');
 
+      delete oldChallenge.id;
+
       var newChallenge = new NewChallenge(oldChallenge);
 
       newChallenge.createdAt = createdAt;
 
       oldTasks.forEach(function (oldTask) {
-        oldTask._id = uuid.v4(); // TODO keep the old uuid unless duplicated
+        oldTask._id = uuid.v4();
         oldTask.legacyId = oldTask.id; // store the old task id
         delete oldTask.id;
 
+        oldTask.challenge = oldTask.challenge || {};
+        oldTask.challenge.id = newChallenge._id;
+
+        if (newTasksIds[oldTask.legacyId + '-' + newChallenge._id]) {
+          throw new Error('duplicate :(');
+        } else {
+          newTasksIds[oldTask.legacyId + '-' + newChallenge._id] = oldTask._id;
+        }
+
         oldTask.tags = _.map(oldTask.tags || {}, function (tagPresent, tagId) {
           return tagPresent && tagId;
         });
 
         if (!oldTask.text) oldTask.text = 'task text'; // required
 
-        oldTask.challenge = oldTask.challenge || {};
-        oldTask.challenge.id = oldChallenge._id;
+        oldTask.createdAt = oldTask.dateCreated;
 
         newChallenge.tasksOrder[`${oldTask.type}s`].push(oldTask._id);
         if (oldTask.completed) oldTask.completed = false;
@@ -155,6 +168,8 @@ function processChallenges (afterId) {
     if (lastChallenge) {
       return processChallenges(lastChallenge);
     } else {
+      console.log('Writing newTasksIds.json...')
+      fs.writeFileSync('newTasksIds.json', JSON.stringify(newTasksIds, null, 4), 'utf8');
       return console.log('Done!');
     }
   });
diff --git a/migrations/api_v3/challengesMembers.js b/migrations/api_v3/challengesMembers.js
index f5c1532660..98a42964fa 100644
--- a/migrations/api_v3/challengesMembers.js
+++ b/migrations/api_v3/challengesMembers.js
@@ -86,6 +86,13 @@ function processChallenges (afterId) {
       lastChallenge = oldChallenges[oldChallenges.length - 1]._id;
     }
 
+    // Tyler Renelle
+    oldChallenge.members.forEach(function (id, index) {
+      if (id === '9') {
+        oldChallenge.members[index] = '00000000-0000-4000-9000-000000000000';
+      }
+    });
+
     oldChallenges.forEach(function (oldChallenge) {
       promises.push(newUserCollection.updateMany({
         _id: {$in: oldChallenge.members},
diff --git a/migrations/api_v3/groups.js b/migrations/api_v3/groups.js
index 2fa465c67e..d013924cea 100644
--- a/migrations/api_v3/groups.js
+++ b/migrations/api_v3/groups.js
@@ -144,6 +144,13 @@ function processGroups (afterId) {
       }
 
       if (oldGroup.members) {
+        // Tyler Renelle
+        oldGroup.members.forEach(function (id, index) {
+          if (id === '9') {
+            oldGroup.members[index] = '00000000-0000-4000-9000-000000000000';
+          }
+        });
+
         promises.push(newUserCollection.updateMany({
           _id: {$in: oldGroup.members},
         }, updateMembers, {multi: true}));
diff --git a/migrations/api_v3/users.js b/migrations/api_v3/users.js
index ecc502025e..abf3e12e26 100644
--- a/migrations/api_v3/users.js
+++ b/migrations/api_v3/users.js
@@ -51,6 +51,12 @@ var BATCH_SIZE = 1000;
 var processedUsers = 0;
 var totoalProcessedTasks = 0;
 
+var challengeTaskWithMatchingId = 0;
+var challengeTaskNoMatchingId = 0;
+
+// Load the new tasks ids for challenges tasks
+var newTasksIds = require('./newTasksIds.json');
+
 // Only process users that fall in a interval ie up to -> 0000-4000-0000-0000
 var AFTER_USER_ID = nconf.get('AFTER_USER_ID');
 var BEFORE_USER_ID = nconf.get('BEFORE_USER_ID');
@@ -110,6 +116,8 @@ function processUsers (afterId) {
       delete oldUser.rewards;
       delete oldUser.todos;
 
+      delete oldUser.id;
+
       oldUser.tags = oldUser.tags.map(function (tag) {
         return {
           id: tag.id,
@@ -132,10 +140,24 @@ function processUsers (afterId) {
 
         oldTask.challenge = oldTask.challenge || {};
         if (oldTask.challenge.id) {
-          oldTask.challenge.taskId = oldTask.legacyId;
+          if (oldTask.challenge.broken) {
+            oldTask.challenge.taskId = oldTask.legacyId;
+          } else {
+            var newId = newTasksIds[oldTask.legacyId + '-' + oldTask.challenge.id];
+
+            // Challenges' tasks ids changed
+            if (!newId && !oldTask.challenge.broken) {
+              challengeTaskNoMatchingId++;
+              oldTask.challenge.taskId = oldTask.legacyId;
+              oldTask.challenge.broken = 'CHALLENGE_TASK_NOT_FOUND';
+            } else {
+              challengeTaskWithMatchingId++;
+              oldTask.challenge.taskId = newId;
+            }
+          }
         }
 
-        oldTask.createdAt = old.dateCreated;
+        oldTask.createdAt = oldTask.dateCreated;
 
         if (!oldTask.text) oldTask.text = 'task text'; // required
         oldTask.tags = _.map(oldTask.tags, function (tagPresent, tagId) {
@@ -179,6 +201,8 @@ function processUsers (afterId) {
     processedUsers += oldUsers.length;
 
     console.log(`Saved ${oldUsers.length} users and their tasks.`);
+    console.log('Challenges\' tasks no matching id: ', challengeTaskNoMatchingId);
+    console.log('Challenges\' tasks with matching id: ', challengeTaskWithMatchingId);
 
     if (lastUser) {
       return processUsers(lastUser);

From bb2dd8ca0800c4190b6ee27cc5f9d7877f39d48d Mon Sep 17 00:00:00 2001
From: Matteo Pagliazzi <matteopagliazzi@gmail.com>
Date: Wed, 4 May 2016 23:43:04 +0200
Subject: [PATCH 04/12] v3: remove trimming and lowercase from fields that must
 be unique

---
 website/src/models/user.js | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/website/src/models/user.js b/website/src/models/user.js
index 68bbcbf39e..4343c43d48 100644
--- a/website/src/models/user.js
+++ b/website/src/models/user.js
@@ -30,13 +30,10 @@ export let schema = new Schema({
     local: {
       email: {
         type: String,
-        trim: true,
-        lowercase: true,
         validate: [validator.isEmail, shared.i18n.t('invalidEmail')], // TODO translate error messages here, use preferences.language?
       },
       username: {
         type: String,
-        trim: true,
       },
       // Store a lowercase version of username to check for duplicates
       lowerCaseUsername: String,

From ebf3a0979fbfe29b20b196f576937b3e90f76761 Mon Sep 17 00:00:00 2001
From: Matteo Pagliazzi <matteopagliazzi@gmail.com>
Date: Thu, 5 May 2016 09:28:07 +0200
Subject: [PATCH 05/12] fix challenges members migration

---
 migrations/api_v3/challengesMembers.js | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/migrations/api_v3/challengesMembers.js b/migrations/api_v3/challengesMembers.js
index 98a42964fa..6281c129dc 100644
--- a/migrations/api_v3/challengesMembers.js
+++ b/migrations/api_v3/challengesMembers.js
@@ -86,14 +86,14 @@ function processChallenges (afterId) {
       lastChallenge = oldChallenges[oldChallenges.length - 1]._id;
     }
 
-    // Tyler Renelle
-    oldChallenge.members.forEach(function (id, index) {
-      if (id === '9') {
-        oldChallenge.members[index] = '00000000-0000-4000-9000-000000000000';
-      }
-    });
-
     oldChallenges.forEach(function (oldChallenge) {
+      // Tyler Renelle
+      oldChallenge.members.forEach(function (id, index) {
+        if (id === '9') {
+          oldChallenge.members[index] = '00000000-0000-4000-9000-000000000000';
+        }
+      });
+
       promises.push(newUserCollection.updateMany({
         _id: {$in: oldChallenge.members},
       }, {

From c94f4ef0e923e1ea582e3abc3846757cf21f6e16 Mon Sep 17 00:00:00 2001
From: Matteo Pagliazzi <matteopagliazzi@gmail.com>
Date: Thu, 5 May 2016 12:15:28 +0200
Subject: [PATCH 06/12] v3 migration: delete old completed todos

---
 migrations/api_v3/users.js | 19 +++++++++++--------
 1 file changed, 11 insertions(+), 8 deletions(-)

diff --git a/migrations/api_v3/users.js b/migrations/api_v3/users.js
index abf3e12e26..5962a4a494 100644
--- a/migrations/api_v3/users.js
+++ b/migrations/api_v3/users.js
@@ -19,6 +19,7 @@ var _ = require('lodash');
 var uuid = require('uuid');
 var consoleStamp = require('console-stamp');
 var common = require('../../common');
+var moment = require('moment');
 
 // Add timestamps to console messages
 consoleStamp(console);
@@ -61,19 +62,13 @@ var newTasksIds = require('./newTasksIds.json');
 var AFTER_USER_ID = nconf.get('AFTER_USER_ID');
 var BEFORE_USER_ID = nconf.get('BEFORE_USER_ID');
 
-/* TODO compare old and new model
-- _id 9
-- challenges
-- groups
-- invitations
-- challenges' tasks
-*/
-
 function processUsers (afterId) {
   var processedTasks = 0;
   var lastUser = null;
   var oldUsers;
 
+  var now = new Date();
+
   var query = {};
 
   if (BEFORE_USER_ID) {
@@ -131,6 +126,7 @@ function processUsers (afterId) {
       }
 
       var newUser = new NewUser(oldUser);
+      var isSubscribed = newUser.isSubscribed();
 
       oldTasks.forEach(function (oldTask) {
         oldTask._id = uuid.v4(); // create a new unique uuid
@@ -157,6 +153,13 @@ function processUsers (afterId) {
           }
         }
 
+        // Delete old completed todos
+        if (oldTask.type === 'todo' && oldTask.completed && (!oldTask.challenge.id || oldTask.challenge.broken)) {
+          if (moment(now).subtract(isSubscribed ? 90 : 30, 'days').toDate() > moment(oldTask.dateCompleted).toDate()) {
+            return;
+          }
+        }
+
         oldTask.createdAt = oldTask.dateCreated;
 
         if (!oldTask.text) oldTask.text = 'task text'; // required

From 192488cb023ed40411776482acfb6823834b5a0f Mon Sep 17 00:00:00 2001
From: Blade Barringer <blade@crookedneighbor.com>
Date: Fri, 6 May 2016 12:05:06 -0500
Subject: [PATCH 07/12] feat: Add require-again to help with unit testing

---
 package.json                                  |  1 +
 test/api/v3/unit/libs/email.test.js           | 25 ++++++++-----------
 test/api/v3/unit/libs/logger.js               |  9 +++----
 .../api/v3/unit/middlewares/analytics.test.js | 14 +++--------
 4 files changed, 20 insertions(+), 29 deletions(-)

diff --git a/package.json b/package.json
index 05d269a4ab..23c456a6c4 100644
--- a/package.json
+++ b/package.json
@@ -149,6 +149,7 @@
     "nock": "^2.17.0",
     "phantomjs": "^1.9",
     "protractor": "^3.1.1",
+    "require-again": "^1.0.1",
     "rewire": "^2.3.3",
     "rimraf": "^2.4.3",
     "shelljs": "^0.5.3",
diff --git a/test/api/v3/unit/libs/email.test.js b/test/api/v3/unit/libs/email.test.js
index a24ac46a83..91878f1f57 100644
--- a/test/api/v3/unit/libs/email.test.js
+++ b/test/api/v3/unit/libs/email.test.js
@@ -3,6 +3,7 @@ import request from 'request';
 import nconf from 'nconf';
 import nodemailer from 'nodemailer';
 import Q from 'q';
+import requireAgain from 'require-again';
 import logger from '../../../../../website/src/libs/api-v3/logger';
 
 function getUser () {
@@ -34,10 +35,6 @@ function getUser () {
 describe('emails', () => {
   let pathToEmailLib = '../../../../../website/src/libs/api-v3/email';
 
-  beforeEach(() => {
-    delete require.cache[require.resolve(pathToEmailLib)];
-  });
-
   describe('sendEmail', () => {
     it('can send an email using the default transport', () => {
       let sendMailSpy = sandbox.stub().returns(Q.defer().promise);
@@ -46,7 +43,7 @@ describe('emails', () => {
         sendMail: sendMailSpy,
       });
 
-      let attachEmail = require(pathToEmailLib);
+      let attachEmail = requireAgain(pathToEmailLib);
       attachEmail.send();
       expect(sendMailSpy).to.be.calledOnce;
     });
@@ -60,7 +57,7 @@ describe('emails', () => {
       });
       sandbox.stub(logger, 'error');
 
-      let attachEmail = require(pathToEmailLib);
+      let attachEmail = requireAgain(pathToEmailLib);
       attachEmail.send();
       expect(sendMailSpy).to.be.calledOnce;
       deferred.reject();
@@ -75,13 +72,13 @@ describe('emails', () => {
 
   describe('getUserInfo', () => {
     it('returns an empty object if no field request', () => {
-      let attachEmail = require(pathToEmailLib);
+      let attachEmail = requireAgain(pathToEmailLib);
       let getUserInfo = attachEmail.getUserInfo;
       expect(getUserInfo({}, [])).to.be.empty;
     });
 
     it('returns correct user data', () => {
-      let attachEmail = require(pathToEmailLib);
+      let attachEmail = requireAgain(pathToEmailLib);
       let getUserInfo = attachEmail.getUserInfo;
       let user = getUser();
       let data = getUserInfo(user, ['name', 'email', '_id', 'canSend']);
@@ -93,7 +90,7 @@ describe('emails', () => {
     });
 
     it('returns correct user data [facebook users]', () => {
-      let attachEmail = require(pathToEmailLib);
+      let attachEmail = requireAgain(pathToEmailLib);
       let getUserInfo = attachEmail.getUserInfo;
       let user = getUser();
       delete user.profile.name;
@@ -108,7 +105,7 @@ describe('emails', () => {
     });
 
     it('has fallbacks for missing data', () => {
-      let attachEmail = require(pathToEmailLib);
+      let attachEmail = requireAgain(pathToEmailLib);
       let getUserInfo = attachEmail.getUserInfo;
       let user = getUser();
       delete user.profile.name;
@@ -135,7 +132,7 @@ describe('emails', () => {
 
     it('can send a txn email to one recipient', () => {
       sandbox.stub(nconf, 'get').withArgs('IS_PROD').returns(true);
-      let attachEmail = require(pathToEmailLib);
+      let attachEmail = requireAgain(pathToEmailLib);
       let sendTxnEmail = attachEmail.sendTxn;
       let emailType = 'an email type';
       let mailingInfo = {
@@ -158,7 +155,7 @@ describe('emails', () => {
 
     it('does not send email if address is missing', () => {
       sandbox.stub(nconf, 'get').withArgs('IS_PROD').returns(true);
-      let attachEmail = require(pathToEmailLib);
+      let attachEmail = requireAgain(pathToEmailLib);
       let sendTxnEmail = attachEmail.sendTxn;
       let emailType = 'an email type';
       let mailingInfo = {
@@ -172,7 +169,7 @@ describe('emails', () => {
 
     it('uses getUserInfo in case of user data', () => {
       sandbox.stub(nconf, 'get').withArgs('IS_PROD').returns(true);
-      let attachEmail = require(pathToEmailLib);
+      let attachEmail = requireAgain(pathToEmailLib);
       let sendTxnEmail = attachEmail.sendTxn;
       let emailType = 'an email type';
       let mailingInfo = getUser();
@@ -190,7 +187,7 @@ describe('emails', () => {
 
     it('sends email with some default variables', () => {
       sandbox.stub(nconf, 'get').withArgs('IS_PROD').returns(true);
-      let attachEmail = require(pathToEmailLib);
+      let attachEmail = requireAgain(pathToEmailLib);
       let sendTxnEmail = attachEmail.sendTxn;
       let emailType = 'an email type';
       let mailingInfo = {
diff --git a/test/api/v3/unit/libs/logger.js b/test/api/v3/unit/libs/logger.js
index c274897377..a0f5eb011f 100644
--- a/test/api/v3/unit/libs/logger.js
+++ b/test/api/v3/unit/libs/logger.js
@@ -1,4 +1,5 @@
 import winston from 'winston';
+import requireAgain from 'require-again';
 
 /* eslint-disable global-require */
 describe('logger', () => {
@@ -7,8 +8,6 @@ describe('logger', () => {
   let errorSpy;
 
   beforeEach(() => {
-    delete require.cache[require.resolve(pathToLoggerLib)];
-
     infoSpy = sandbox.stub();
     errorSpy = sandbox.stub();
     sandbox.stub(winston, 'Logger').returns({
@@ -22,7 +21,7 @@ describe('logger', () => {
   });
 
   it('info', () => {
-    let attachLogger = require(pathToLoggerLib);
+    let attachLogger = requireAgain(pathToLoggerLib);
     attachLogger.info(1, 2, 3);
     expect(infoSpy).to.be.calledOnce;
     expect(infoSpy).to.be.calledWith(1, 2, 3);
@@ -30,14 +29,14 @@ describe('logger', () => {
 
   describe('error', () => {
     it('with custom arguments', () => {
-      let attachLogger = require(pathToLoggerLib);
+      let attachLogger = requireAgain(pathToLoggerLib);
       attachLogger.error(1, 2, 3, 4);
       expect(errorSpy).to.be.calledOnce;
       expect(errorSpy).to.be.calledWith(1, 2, 3, 4);
     });
 
     it('with error', () => {
-      let attachLogger = require(pathToLoggerLib);
+      let attachLogger = requireAgain(pathToLoggerLib);
       let errInstance = new Error('An error.');
       attachLogger.error(errInstance, {
         data: 1,
diff --git a/test/api/v3/unit/middlewares/analytics.test.js b/test/api/v3/unit/middlewares/analytics.test.js
index eb238c6fa9..2f3a0b7ff0 100644
--- a/test/api/v3/unit/middlewares/analytics.test.js
+++ b/test/api/v3/unit/middlewares/analytics.test.js
@@ -6,6 +6,7 @@ import {
 } from '../../../../helpers/api-unit.helper';
 import analyticsService from '../../../../../website/src/libs/api-v3/analyticsService';
 import nconf from 'nconf';
+import requireAgain from 'require-again';
 
 describe('analytics middleware', () => {
   let res, req, next;
@@ -17,15 +18,8 @@ describe('analytics middleware', () => {
     next = generateNext();
   });
 
-  afterEach(() => {
-    // The nconf.get('IS_PROD') occurs when the file is required
-    // Since node caches IS_PROD, we have to delete it from the cache
-    // to test prod vs non-prod behaviors
-    delete require.cache[require.resolve(pathToAnalyticsMiddleware)];
-  });
-
   it('attaches analytics object res.locals', () => {
-    let attachAnalytics = require(pathToAnalyticsMiddleware);
+    let attachAnalytics = requireAgain(pathToAnalyticsMiddleware);
 
     attachAnalytics(req, res, next);
 
@@ -34,7 +28,7 @@ describe('analytics middleware', () => {
 
   it('attaches stubbed methods for non-prod environments', () => {
     sandbox.stub(nconf, 'get').withArgs('IS_PROD').returns(false);
-    let attachAnalytics = require(pathToAnalyticsMiddleware);
+    let attachAnalytics = requireAgain(pathToAnalyticsMiddleware);
 
     attachAnalytics(req, res, next);
 
@@ -45,7 +39,7 @@ describe('analytics middleware', () => {
   it('attaches real methods for prod environments', () => {
     sandbox.stub(nconf, 'get').withArgs('IS_PROD').returns(true);
 
-    let attachAnalytics = require(pathToAnalyticsMiddleware);
+    let attachAnalytics = requireAgain(pathToAnalyticsMiddleware);
 
     attachAnalytics(req, res, next);
 

From b687a6bf9da2795af5c37b7344f1a25627f6122c Mon Sep 17 00:00:00 2001
From: Matteo Pagliazzi <matteopagliazzi@gmail.com>
Date: Fri, 6 May 2016 19:40:36 +0200
Subject: [PATCH 08/12] fix typo in v3 groups migration

---
 migrations/api_v3/groups.js | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/migrations/api_v3/groups.js b/migrations/api_v3/groups.js
index d013924cea..e67f9592fe 100644
--- a/migrations/api_v3/groups.js
+++ b/migrations/api_v3/groups.js
@@ -132,7 +132,7 @@ function processGroups (afterId) {
 
       if (!oldGroup.privacy) {
         // throw new Error('group.privacy is required');
-        group.privacy = 'private';
+        oldGroup.privacy = 'private';
       }
 
       var updateMembers = {};

From afb7d1d62716564a816817fb254b18834a5fe282 Mon Sep 17 00:00:00 2001
From: Matteo Pagliazzi <matteopagliazzi@gmail.com>
Date: Fri, 6 May 2016 20:24:53 +0200
Subject: [PATCH 09/12] v3: start cleaning up TODO comemnts

---
 website/src/controllers/api-v3/auth.js          |  2 +-
 website/src/controllers/api-v3/challenges.js    |  4 ++--
 website/src/controllers/api-v3/chat.js          |  7 +++++--
 website/src/controllers/api-v3/groups.js        |  3 +--
 website/src/controllers/api-v3/members.js       |  4 ++--
 website/src/controllers/api-v3/tasks.js         |  7 +++----
 website/src/controllers/top-level/dataexport.js |  2 +-
 website/src/libs/api-v3/cron.js                 |  2 +-
 website/src/libs/api-v3/preening.js             |  4 ++--
 website/src/libs/api-v3/pushNotifications.js    |  5 +----
 website/src/middlewares/api-v3/auth.js          |  2 +-
 website/src/middlewares/api-v3/cron.js          |  4 ++--
 website/src/middlewares/api-v3/index.js         |  1 -
 website/src/middlewares/api-v3/setupBody.js     |  2 +-
 website/src/middlewares/api-v3/v2.js            |  4 ++--
 website/src/middlewares/apiThrottle.js          |  3 +--
 website/src/middlewares/forceRefresh.js         |  2 +-
 website/src/models/challenge.js                 |  4 ++--
 website/src/models/group.js                     |  5 +++--
 website/src/models/task.js                      |  6 +++---
 website/src/models/user.js                      | 10 +++++-----
 21 files changed, 40 insertions(+), 43 deletions(-)

diff --git a/website/src/controllers/api-v3/auth.js b/website/src/controllers/api-v3/auth.js
index 4b41448f97..8616f889f0 100644
--- a/website/src/controllers/api-v3/auth.js
+++ b/website/src/controllers/api-v3/auth.js
@@ -127,7 +127,7 @@ api.registerLocal = {
       newUser = fbUser;
     } else {
       newUser = new User(newUser);
-      newUser.registeredThrough = req.headers['x-client']; // TODO is this saved somewhere?
+      newUser.registeredThrough = req.headers['x-client']; // Not saved, used to create the correct tasks based on the device used
     }
 
     // we check for partyInvite for backward compatibility
diff --git a/website/src/controllers/api-v3/challenges.js b/website/src/controllers/api-v3/challenges.js
index 4f738afc80..f7bb61971f 100644
--- a/website/src/controllers/api-v3/challenges.js
+++ b/website/src/controllers/api-v3/challenges.js
@@ -475,7 +475,7 @@ export async function _closeChal (challenge, broken = {}) {
       ]);
     }
 
-    sendPushNotification(savedWinner, shared.i18n.t('wonChallenge'), challenge.name); // TODO translate
+    sendPushNotification(savedWinner, shared.i18n.t('wonChallenge'), challenge.name);
   }
 
   // Run some operations in the background withouth blocking the thread
@@ -501,7 +501,7 @@ export async function _closeChal (challenge, broken = {}) {
     }, {multi: true}).exec(),
   ];
 
-  Q.allSettled(backgroundTasks); // TODO look if allSettled could be useful somewhere else
+  Q.all(backgroundTasks);
 }
 
 /**
diff --git a/website/src/controllers/api-v3/chat.js b/website/src/controllers/api-v3/chat.js
index 42864fdab7..d074cb4c7b 100644
--- a/website/src/controllers/api-v3/chat.js
+++ b/website/src/controllers/api-v3/chat.js
@@ -12,6 +12,7 @@ import _ from 'lodash';
 import { removeFromArray } from '../../libs/api-v3/collectionManipulators';
 import { sendTxn } from '../../libs/api-v3/email';
 import nconf from 'nconf';
+import Q from 'q';
 
 const FLAG_REPORT_EMAILS = nconf.get('FLAG_REPORT_EMAIL').split(',').map((email) => {
   return { email, canSend: true };
@@ -87,12 +88,14 @@ api.postChat = {
 
     group.sendChat(req.body.message, user);
 
+    let toSave = [group.save()];
+
     if (group.type === 'party') {
       user.party.lastMessageSeen = group.chat[0].id;
-      user.save(); // TODO why this is non-blocking? must catch?
+      toSave.push(user.save());
     }
 
-    let savedGroup = await group.save();
+    let [savedGroup] = await Q.all(toSave);
     if (chatUpdated) {
       res.respond(200, {chat: Group.toJSONCleanChat(savedGroup, user).chat});
     } else {
diff --git a/website/src/controllers/api-v3/groups.js b/website/src/controllers/api-v3/groups.js
index 41f47a4769..60d92e6ade 100644
--- a/website/src/controllers/api-v3/groups.js
+++ b/website/src/controllers/api-v3/groups.js
@@ -95,7 +95,6 @@ api.getGroups = {
     let validationErrors = req.validationErrors();
     if (validationErrors) throw validationErrors;
 
-    // TODO validate types are acceptable? probably not necessary
     let types = req.query.type.split(',');
     let groupFields = basicGroupFields.concat('description memberCount balance');
     let sort = '-memberCount';
@@ -444,7 +443,7 @@ api.removeGroupMember = {
         group.quest.leader = undefined;
       } else if (group.quest && group.quest.members) {
         // remove member from quest
-        group.quest.members[member._id] = undefined; // TODO remmeber to check these are mark modified everywhere
+        group.quest.members[member._id] = undefined;
         group.markModified('quest.members');
       }
 
diff --git a/website/src/controllers/api-v3/members.js b/website/src/controllers/api-v3/members.js
index 1a2257f5fe..e8e6149ac4 100644
--- a/website/src/controllers/api-v3/members.js
+++ b/website/src/controllers/api-v3/members.js
@@ -16,6 +16,7 @@ import {
   sendTxn as sendTxnEmail,
 } from '../../libs/api-v3/email';
 import Q from 'q';
+import sendPushNotification from '../../libs/api-v3/pushNotifications';
 
 let api = {};
 
@@ -349,8 +350,7 @@ api.transferGems = {
       ]);
     }
 
-    // TODO: Add push notifications
-    // pushNotify.sendNotify(sender, res.t('giftedGems'), res.t('giftedGemsInfo', { amount: gemAmount, name: byUsername }));
+    sendPushNotification(sender, res.t('giftedGems'), res.t('giftedGemsInfo', { amount: gemAmount, name: byUsername }));
 
     res.respond(200, {});
   },
diff --git a/website/src/controllers/api-v3/tasks.js b/website/src/controllers/api-v3/tasks.js
index d058ba1ae1..18aadca0d1 100644
--- a/website/src/controllers/api-v3/tasks.js
+++ b/website/src/controllers/api-v3/tasks.js
@@ -85,7 +85,7 @@ api.createUserTasks = {
  */
 api.createChallengeTasks = {
   method: 'POST',
-  url: '/tasks/challenge/:challengeId', // TODO should be /tasks/challengeS/:challengeId ? plural?
+  url: '/tasks/challenge/:challengeId',
   middlewares: [authWithHeaders()],
   async handler (req, res) {
     req.checkParams('challengeId', res.t('challengeIdRequired')).notEmpty().isUUID();
@@ -303,7 +303,6 @@ api.updateTask = {
     }
 
     // we have to convert task to an object because otherwise things don't get merged correctly. Bad for performances?
-    // TODO regarding comment above, make sure other models with nested fields are using this trick too
     let [updatedTaskObj] = common.ops.updateTask(task.toObject(), req);
     _.assign(task, Tasks.Task.sanitize(updatedTaskObj));
     // console.log(task.modifiedPaths(), task.toObject().repeat === tep)
@@ -360,7 +359,7 @@ api.scoreTask = {
   middlewares: [authWithHeaders()],
   async handler (req, res) {
     req.checkParams('taskId', res.t('taskIdRequired')).notEmpty().isUUID();
-    req.checkParams('direction', res.t('directionUpDown')).notEmpty().isIn(['up', 'down']); // TODO what about rewards? maybe separate route?
+    req.checkParams('direction', res.t('directionUpDown')).notEmpty().isIn(['up', 'down']);
 
     let validationErrors = req.validationErrors();
     if (validationErrors) throw validationErrors;
@@ -389,7 +388,7 @@ api.scoreTask = {
       } else if (wasCompleted && !task.completed) {
         let hasTask = removeFromArray(user.tasksOrder.todos, task._id);
         if (!hasTask) {
-          user.tasksOrder.todos.push(task._id); // TODO push at the top?
+          user.tasksOrder.todos.push(task._id);
         } // If for some reason it hadn't been removed previously don't do anything
       }
     }
diff --git a/website/src/controllers/top-level/dataexport.js b/website/src/controllers/top-level/dataexport.js
index 33e2fee5c2..5cbcd39fa7 100644
--- a/website/src/controllers/top-level/dataexport.js
+++ b/website/src/controllers/top-level/dataexport.js
@@ -171,7 +171,7 @@ api.exportUserAvatarHtml = {
     if (!member) throw new NotFound(res.t('userWithIDNotFound', {userId: memberId}));
     res.render('avatar-static', {
       title: member.profile.name,
-      env: _.defaults({member}, res.locals.habitrpg), // TODO review once static pages are done
+      env: _.defaults({member}, res.locals.habitrpg),
     });
   },
 };
diff --git a/website/src/libs/api-v3/cron.js b/website/src/libs/api-v3/cron.js
index 61a2b46c72..a8270c4bc8 100644
--- a/website/src/libs/api-v3/cron.js
+++ b/website/src/libs/api-v3/cron.js
@@ -262,7 +262,7 @@ export function cron (options = {}) {
     gaLabel: 'Cron Count',
     gaValue: user.flags.cronCount,
     uuid: user._id,
-    user, // TODO is it really necessary passing the whole user object?
+    user,
     resting: user.preferences.sleep,
     cronCount: user.flags.cronCount,
     progressUp: _.min([_progress.up, 900]),
diff --git a/website/src/libs/api-v3/preening.js b/website/src/libs/api-v3/preening.js
index ee6a201b3a..2f1f0ad308 100644
--- a/website/src/libs/api-v3/preening.js
+++ b/website/src/libs/api-v3/preening.js
@@ -30,12 +30,12 @@ Subscribers and challenges:
 - 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
+  // history = _.filter(history, historyEntry => Boolean(historyEntry)); // Filter missing entries
   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)
+  // Keep uncompressed entries (modifies history and returns removed items)
   let newHistory = _.remove(history, entry => {
     let date = moment(entry.date);
     return date.isSame(cutOff) || date.isAfter(cutOff);
diff --git a/website/src/libs/api-v3/pushNotifications.js b/website/src/libs/api-v3/pushNotifications.js
index 426c86f5c3..8354de04e4 100644
--- a/website/src/libs/api-v3/pushNotifications.js
+++ b/website/src/libs/api-v3/pushNotifications.js
@@ -26,10 +26,7 @@ if (gcm) {
 }
 
 module.exports = function sendNotification (user, title, message, timeToLive = 15) {
-  // TODO need investigation:
-  // https://github.com/HabitRPG/habitrpg/issues/5252
-
-  if (!user) throw new Error('User is required.');
+  if (!user) return;
 
   _.each(user.pushDevices, pushDevice => {
     switch (pushDevice.type) {
diff --git a/website/src/middlewares/api-v3/auth.js b/website/src/middlewares/api-v3/auth.js
index d1fcfe2e7b..21b0032714 100644
--- a/website/src/middlewares/api-v3/auth.js
+++ b/website/src/middlewares/api-v3/auth.js
@@ -5,7 +5,7 @@ import {
   model as User,
 } from '../../models/user';
 
-// TODO how to translate the strings here since getUserLanguage hasn't run yet?
+// Strins won't be translated here because getUserLanguage has not run yet
 
 // Authenticate a request through the x-api-user and x-api key header
 // If optional is true, don't error on missing authentication
diff --git a/website/src/middlewares/api-v3/cron.js b/website/src/middlewares/api-v3/cron.js
index 50c6e32548..8473db23af 100644
--- a/website/src/middlewares/api-v3/cron.js
+++ b/website/src/middlewares/api-v3/cron.js
@@ -123,12 +123,12 @@ module.exports = function cronMiddleware (req, res, next) {
         $lt: moment(now).subtract(user.isSubscribed() ? 90 : 30, 'days').toDate(),
       },
       'challenge.id': {$exists: false},
-    }).exec(); // TODO wait before returning?
+    }).exec();
 
     let ranCron = user.isModified();
     let quest = common.content.quests[user.party.quest.key];
 
-    // if (ranCron) res.locals.wasModified = true; // TODO remove?
+    // if (ranCron) res.locals.wasModified = true; // TODO remove after v2 is retired
     if (!ranCron) return next();
 
     // Group.tavernBoss(user, progress);
diff --git a/website/src/middlewares/api-v3/index.js b/website/src/middlewares/api-v3/index.js
index 54567791e3..a381ce8cd1 100644
--- a/website/src/middlewares/api-v3/index.js
+++ b/website/src/middlewares/api-v3/index.js
@@ -52,7 +52,6 @@ module.exports = function attachMiddlewares (app, server) {
   app.use(forceSSL);
   app.use(forceHabitica);
 
-  // TODO if we don't manage to move the client off $resource the limit for bodyParser.json must be increased to 1mb from 100kb (default)
   app.use(bodyParser.urlencoded({
     extended: true, // Uses 'qs' library as old connect middleware
   }));
diff --git a/website/src/middlewares/api-v3/setupBody.js b/website/src/middlewares/api-v3/setupBody.js
index 846db4162c..b3309fb2da 100644
--- a/website/src/middlewares/api-v3/setupBody.js
+++ b/website/src/middlewares/api-v3/setupBody.js
@@ -1,4 +1,4 @@
-// TODO tests?
+// TODO test this middleware
 module.exports = function setupBodyMiddleware (req, res, next) {
   req.body = req.body || {};
   next();
diff --git a/website/src/middlewares/api-v3/v2.js b/website/src/middlewares/api-v3/v2.js
index cda6a6cf38..4eb2686bdc 100644
--- a/website/src/middlewares/api-v3/v2.js
+++ b/website/src/middlewares/api-v3/v2.js
@@ -19,8 +19,8 @@ v2app.use(responseHandler);
 
 // Custom Directives
 v2app.use('/', require('../../routes/api-v2/auth'));
-v2app.use('/', require('../../routes/api-v2/coupon')); // TODO REMOVE - ONLY v3
-v2app.use('/', require('../../routes/api-v2/unsubscription')); // TODO REMOVE - ONLY v3
+// v2app.use('/', require('../../routes/api-v2/coupon')); // TODO REMOVE - ONLY v3
+// v2app.use('/', require('../../routes/api-v2/unsubscription')); // TODO REMOVE - ONLY v3
 
 require('../../routes/api-v2/swagger')(swagger, v2app);
 
diff --git a/website/src/middlewares/apiThrottle.js b/website/src/middlewares/apiThrottle.js
index 63995e410c..b392cc777e 100644
--- a/website/src/middlewares/apiThrottle.js
+++ b/website/src/middlewares/apiThrottle.js
@@ -4,10 +4,9 @@ var limiter = require('connect-ratelimit');
 var IS_PROD = nconf.get('NODE_ENV') === 'production';
 
 // TODO since Habitica runs on many different servers this module is pretty useless
-// as it will only block requests that go to the same server
+// as it will only block requests that go to the same server but anyway we should probably have a rate limiter in place
 
 module.exports = function(app) {
-  // TODO review later
   // disable the rate limiter middleware
   if (/*!IS_PROD || */true) return;
   app.use(limiter({
diff --git a/website/src/middlewares/forceRefresh.js b/website/src/middlewares/forceRefresh.js
index 577ad2f4c6..f843694790 100644
--- a/website/src/middlewares/forceRefresh.js
+++ b/website/src/middlewares/forceRefresh.js
@@ -1,4 +1,4 @@
-// TODO do we need this module?
+// TODO do we need this module anymore in v3? No
 
 module.exports.siteVersion = 1;
 
diff --git a/website/src/models/challenge.js b/website/src/models/challenge.js
index 681d238c16..caa823acc1 100644
--- a/website/src/models/challenge.js
+++ b/website/src/models/challenge.js
@@ -123,7 +123,7 @@ schema.methods.syncToUser = async function syncChallengeToUser (user) {
       user.tasksOrder[`${chalTask.type}s`].push(matchingTask._id);
     } else {
       _.merge(matchingTask, _syncableAttrs(chalTask));
-      // Make sure the task is in user.tasksOrder TODO necessary?
+      // Make sure the task is in user.tasksOrder
       let orderList = user.tasksOrder[`${chalTask.type}s`];
       if (orderList.indexOf(matchingTask._id) === -1 && (matchingTask.type !== 'todo' || !matchingTask.completed)) orderList.push(matchingTask._id);
     }
@@ -155,7 +155,7 @@ schema.methods.addTasks = async function challengeAddTasks (tasks) {
   let membersIds = await _fetchMembersIds(challenge._id);
 
   // Sync each user sequentially
-  // TODO are we sure it's the best solution?
+  // TODO are we sure it's the best solution? Use cwait
   // use bulk ops? http://stackoverflow.com/questions/16726330/mongoose-mongodb-batch-insert
   for (let memberId of membersIds) {
     let updateTasksOrderQ = {$push: {}};
diff --git a/website/src/models/group.js b/website/src/models/group.js
index ae1c1318bb..17398e4909 100644
--- a/website/src/models/group.js
+++ b/website/src/models/group.js
@@ -420,7 +420,7 @@ schema.methods.finishQuest = function finishQuest (quest) {
   let updates = {$inc: {}, $set: {}};
 
   updates.$inc[`achievements.quests.${questK}`] = 1;
-  updates.$inc['stats.gp'] = Number(quest.drop.gp); // TODO are this castings necessary?
+  updates.$inc['stats.gp'] = Number(quest.drop.gp);
   updates.$inc['stats.exp'] = Number(quest.drop.exp);
   updates.$inc._v = 1;
 
@@ -530,7 +530,8 @@ schema.statics.bossQuest = async function bossQuest (user, progress) {
   }, {multi: true}).exec();
   // Apply changes the currently cronning user locally so we don't have to reload it to get the updated state
   // TODO how to mark not modified? https://github.com/Automattic/mongoose/pull/1167
-  // must be notModified or otherwise could overwrite future changes
+  // must be notModified or otherwise could overwrite future changes: if the user is saved it'll save
+  // the modified user.stats.hp but that must not happen as the hp value has already been updated by the User.update above
   // if (down) user.stats.hp += down;
 
   // Boss slain, finish quest
diff --git a/website/src/models/task.js b/website/src/models/task.js
index 4945f121f6..c7a8dca98c 100644
--- a/website/src/models/task.js
+++ b/website/src/models/task.js
@@ -39,7 +39,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 TODO unique index?
+    taskId: {type: String, ref: 'Task', validate: [validator.isUUID, 'Invalid uuid.']}, // When not set but challenge.id defined it's the original task
     broken: {type: String, enum: ['CHALLENGE_DELETED', 'TASK_DELETED', 'UNSUBSCRIBED', 'CHALLENGE_CLOSED']},
     winner: String, // user.profile.name of the winner
   },
@@ -149,7 +149,7 @@ export let Task = mongoose.model('Task', TaskSchema);
 
 // habits and dailies shared fields
 let habitDailySchema = () => {
-  return {history: Array}; // [{date:Date, value:Number}], // this causes major performance problems TODO revisit
+  return {history: Array}; // [{date:Date, value:Number}], // this causes major performance problems
 };
 
 // dailys and todos shared fields
@@ -197,7 +197,7 @@ export let daily = Task.discriminator('daily', DailySchema);
 
 export let TodoSchema = new Schema(_.defaults({
   dateCompleted: Date,
-  // TODO we're getting parse errors, people have stored as "today" and "3/13". Need to run a migration & put this back to type: Date
+  // TODO we're getting parse errors, people have stored as "today" and "3/13". Need to run a migration & put this back to type: Date see http://stackoverflow.com/questions/1353684/detecting-an-invalid-date-date-instance-in-javascript
   date: String, // due date for todos
 }, dailyTodoSchema()), subDiscriminatorOptions);
 export let todo = Task.discriminator('todo', TodoSchema);
diff --git a/website/src/models/user.js b/website/src/models/user.js
index 4343c43d48..11d36e1e44 100644
--- a/website/src/models/user.js
+++ b/website/src/models/user.js
@@ -30,7 +30,7 @@ export let schema = new Schema({
     local: {
       email: {
         type: String,
-        validate: [validator.isEmail, shared.i18n.t('invalidEmail')], // TODO translate error messages here, use preferences.language?
+        validate: [validator.isEmail, shared.i18n.t('invalidEmail')],
       },
       username: {
         type: String,
@@ -526,14 +526,14 @@ export let schema = new Schema({
 
 schema.plugin(baseModel, {
   // TODO revisit a lot of things are missing. Given how many attributes we do have here we should white-list the ones that can be updated
-  // TODO this is a only used for creating an user, on update we use a whitelist
+  // This is not really used as updating uses a whitelist and creating only accepts specific params (password, email, username, ...)
   noSet: ['_id', 'apiToken', 'auth.blocked', 'auth.timestamps', 'lastCron', 'auth.local.hashed_password',
     'auth.local.salt', 'tasksOrder', 'tags', 'stats', 'challenges', 'guilds', 'party._id', 'party.quest',
     'invitations', 'balance', 'backer', 'contributor'],
   private: ['auth.local.hashed_password', 'auth.local.salt'],
   toJSONTransform: function userToJSON (plainObj, originalDoc) {
-    // plainObj.filters = {}; TODO Not saved
-    plainObj._tmp = originalDoc._tmp; // be sure to send down drop notifs TODO how to test?
+    // plainObj.filters = {}; TODO Not saved, remove?
+    plainObj._tmp = originalDoc._tmp; // be sure to send down drop notifs
 
     return plainObj;
   },
@@ -590,7 +590,7 @@ function _populateDefaultTasks (user, taskTypes) {
       return newTask.save();
     });
 
-    tasksToCreate.push(...tasksOfType); // TODO find better way since this creates each task individually
+    tasksToCreate.push(...tasksOfType);
   });
 
   return Q.all(tasksToCreate)

From 4e3d4c88310bea99454e701a076a05e885285fd2 Mon Sep 17 00:00:00 2001
From: Matteo Pagliazzi <matteopagliazzi@gmail.com>
Date: Sat, 7 May 2016 15:01:40 +0200
Subject: [PATCH 10/12] v3: more verbose logging in production and fix
 migration bugs

---
 migrations/api_v3/challengesMembers.js |  2 +-
 migrations/api_v3/groups.js            |  6 ++-
 migrations/api_v3/indexes.js           | 72 +++++++++++++-------------
 migrations/api_v3/users.js             |  2 +-
 website/src/libs/api-v2/logging.js     |  2 +-
 website/src/libs/api-v3/logger.js      |  5 ++
 6 files changed, 48 insertions(+), 41 deletions(-)

diff --git a/migrations/api_v3/challengesMembers.js b/migrations/api_v3/challengesMembers.js
index 6281c129dc..b529e836e2 100644
--- a/migrations/api_v3/challengesMembers.js
+++ b/migrations/api_v3/challengesMembers.js
@@ -95,7 +95,7 @@ function processChallenges (afterId) {
       });
 
       promises.push(newUserCollection.updateMany({
-        _id: {$in: oldChallenge.members},
+        _id: {$in: oldChallenge.members || []},
       }, {
         $push: {challenges: oldChallenge._id},
       }, {multi: true}));
diff --git a/migrations/api_v3/groups.js b/migrations/api_v3/groups.js
index e67f9592fe..b87c0fa8df 100644
--- a/migrations/api_v3/groups.js
+++ b/migrations/api_v3/groups.js
@@ -102,9 +102,11 @@ function processGroups (afterId) {
     }
 
     oldGroups.forEach(function (oldGroup) {
-      if ((!oldGroup.privacy || oldGroup.privacy === 'private') && (!oldGroup.members || oldGroup.members.length === 0)) return; // delete empty private groups
+      if ((!oldGroup.privacy || oldGroup.privacy === 'private') && (!oldGroup.members || oldGroup.members.length === 0)) return; // delete empty private groups TODO must also delete challenges or this won't work
+
+      oldGroup.members = oldGroup.members || [];
       oldGroup.memberCount = oldGroup.members ? oldGroup.members.length : 0;
-      oldGroup.memberCount = oldGroup.challenges ? oldGroup.challenges.length : 0;
+      oldGroup.challengeCount = oldGroup.challenges ? oldGroup.challenges.length : 0;
 
       if (!oldGroup.balance <= 0) oldGroup.balance = 0;
       if (!oldGroup.name) oldGroup.name = 'group name';
diff --git a/migrations/api_v3/indexes.js b/migrations/api_v3/indexes.js
index 4944e375ec..07aaa21db8 100644
--- a/migrations/api_v3/indexes.js
+++ b/migrations/api_v3/indexes.js
@@ -1,52 +1,52 @@
 /*
   DEFINE BEFORE MIGRATING
 
-  tasks: userId (sparse?), challenge.id (sparse), challenge.taskId (sparse), type? completed?
+  tasks: userId OK (sparse?), challenge.id OK (sparse?), challenge.taskId OK (sparse?), type? completed?
   users:
-    id & apiToken?,
-    auth.facebook.emails.value -> unique and sparse?,
-    auth.facebook.id - unique and sparse,
-    auth.local.email - unique and sparse,
-    auth.local.lowerCaseUsername,
-    auth.local.username - unique and sparse
+    id & apiToken, OK
+    auth.facebook.emails.value OK -> unique and sparse?,
+    auth.facebook.id - unique and sparse, OK
+    auth.local.email - unique and sparse, OK
+    auth.local.lowerCaseUsername, OK
+    auth.local.username - unique OK
     auth.local.username & auth.local.hashed_password?,
-    auth.timestamps.created?,
-    auth.timestamps.loggedin?,
-    backer.tier -1
+    auth.timestamps.created?, OK
+    auth.timestamps.loggedin?, OK
+    backer.tier -1 OK
     { "contributor.admin" : 1 , "contributor.level" : -1 , "backer.npc" : -1 , "profile.name" : 1}
-    { "contributor.admin" : 1.0}
-    { "contributor.level" : 1.0}
+    { "contributor.admin" : 1.0} NO, see ^
+    { "contributor.level" : 1.0} OK
     { "contributor.level" : 1.0 , "purchased.plan.customerId" : 1.0} ?
-    { "flags.lastWeeklyRecap" : 1 , "_id" : 1 , "preferences.emailNotifications.unsubscribeFromAll" : 1 , "preferences.emailNotifications.weeklyRecaps" : 1}
-    { "invitations.guilds.id" : 1}
-    { "invitations.party.id" : 1}
-    { "preferences.sleep" : 1 , "_id" : 1 , "flags.lastWeeklyRecap" : 1 , "preferences.emailNotifications.unsubscribeFromAll" : 1 , "preferences.emailNotifications.weeklyRecaps" : 1}
-    { "preferences.sleep" : 1 , "_id" : 1 , "lastCron" : 1 , "preferences.emailNotifications.importantAnnouncements" : 1 , "preferences.emailNotifications.unsubscribeFromAll" : 1 , "flags.recaptureEmailsPhase" : 1}
-    profile.name ?
-    { "purchased.plan.customerId" : 1.0}
-    { "purchased.plan.paymentMethod" : 1.0}
+    NO { "flags.lastWeeklyRecap" : 1 , "_id" : 1 , "preferences.emailNotifications.unsubscribeFromAll" : 1 , "preferences.emailNotifications.weeklyRecaps" : 1}
+    { "invitations.guilds.id" : 1} OK
+    { "invitations.party.id" : 1} OK
+    OK { "preferences.sleep" : 1 , "_id" : 1 , "flags.lastWeeklyRecap" : 1 , "preferences.emailNotifications.unsubscribeFromAll" : 1 , "preferences.emailNotifications.weeklyRecaps" : 1}
+    OK { "preferences.sleep" : 1 , "_id" : 1 , "lastCron" : 1 , "preferences.emailNotifications.importantAnnouncements" : 1 , "preferences.emailNotifications.unsubscribeFromAll" : 1 , "flags.recaptureEmailsPhase" : 1}
+    profile.name ? OK
+    { "purchased.plan.customerId" : 1.0} OK
+    { "purchased.plan.paymentMethod" : 1.0} OK
 
-    guilds
-    party.id
-    challenges
+    guilds OK
+    party.id OK
+    challenges OK
   challenges:
-    { "_id" : 1.0 , "__v" : 1.0} ?
+    { "_id" : 1.0 , "__v" : 1.0} ? NO
     { "_id" : 1.0 , "official" : -1.0 , "timestamp" : -1.0}
-    { "group" : 1.0 , "official" : -1.0 , "timestamp" : -1.0}
-    { "leader" : 1.0 , "official" : -1.0 , "timestamp" : -1.0}
-    { "members" : 1.0 , "official" : -1.0 , "timestamp" : -1.0} ?
-    { "official" : -1 , "timestamp" : -1}
+    { "group" : 1.0 , "official" : -1.0 , "timestamp" : -1.0} OK
+    { "leader" : 1.0 , "official" : -1.0 , "timestamp" : -1.0} OK
+    { "members" : 1.0 , "official" : -1.0 , "timestamp" : -1.0} ? NO
+    { "official" : -1 , "timestamp" : -1} ?
     { "official" : -1 , "timestamp" : -1, "_id": 1} ?
   groups:
-    { "_id" : 1 , "quest.key" : 1}
+    { "_id" : 1 , "quest.key" : 1} ?
     { "_id" : 1.0 , "__v" : 1.0} ?
-    { "_id" : 1.0 , "privacy" : 1.0 , "members" : 1.0} ?
-    { "members" : 1.0 , "type" : 1.0 , "memberCount" : -1.0} ?
-    { "members" : 1} ?
+    { "_id" : 1.0 , "privacy" : 1.0 , "members" : 1.0} ? NO
+    { "members" : 1.0 , "type" : 1.0 , "memberCount" : -1.0} ? NO
+    { "members" : 1} ? NO
     { "privacy" : 1.0 , "memberCount" : -1.0} ?
-    { "privacy" : 1.0} ?
+    { "privacy" : 1.0} OK
     { "type" : 1 , "privacy" : 1} ?
-    { "type" : 1.0 , "members" : 1.0} ?
-    { "type" : 1} ?
-  emailUnsubscriptions: email unique
+    { "type" : 1.0 , "members" : 1.0} ? NO
+    { "type" : 1} ? OK
+  emailUnsubscriptions: email unique OK
 */
diff --git a/migrations/api_v3/users.js b/migrations/api_v3/users.js
index 5962a4a494..be22aee5e0 100644
--- a/migrations/api_v3/users.js
+++ b/migrations/api_v3/users.js
@@ -171,7 +171,7 @@ function processUsers (afterId) {
           newUser.tasksOrder[`${oldTask.type}s`].push(oldTask._id);
         }
 
-        var allTasksFields = ['_id', 'type', 'text', 'notes', 'tags', 'value', 'priority', 'attribute', 'challenge', 'reminders'];
+        var allTasksFields = ['_id', 'type', 'text', 'notes', 'tags', 'value', 'priority', 'attribute', 'challenge', 'reminders', 'userId', 'legacyId'];
         // using mongoose models is too slow
         if (oldTask.type === 'habit') {
           oldTask = _.pick(oldTask, allTasksFields.concat(['history', 'up', 'down']));
diff --git a/website/src/libs/api-v2/logging.js b/website/src/libs/api-v2/logging.js
index f832adb6d5..9737159f43 100644
--- a/website/src/libs/api-v2/logging.js
+++ b/website/src/libs/api-v2/logging.js
@@ -22,9 +22,9 @@ if (nconf.get('LOGGLY:enabled')){
 
 if (!logger) {
   logger = new (winston.Logger)({});
+  logger.add(winston.transports.Console, {colorize:true}); // TODO remove
 
   if (nconf.get('NODE_ENV') !== 'production') {
-    logger.add(winston.transports.Console, {colorize:true});
     logger.add(winston.transports.File, {filename: 'habitrpg.log'});
   }
 }
diff --git a/website/src/libs/api-v3/logger.js b/website/src/libs/api-v3/logger.js
index cc5d03f745..a840f279dd 100644
--- a/website/src/libs/api-v3/logger.js
+++ b/website/src/libs/api-v3/logger.js
@@ -11,6 +11,11 @@ const logger = new winston.Logger();
 if (IS_PROD) {
   // TODO production logging, use loggly and new relic too
   // log errors to console too
+  logger
+    .add(winston.transports.Console, {
+      colorize: true,
+      prettyPrint: true,
+    });
 } else if (IS_TEST) {
   // Do not log anything when testing
 } else {

From 77d2d943aec442b1fd89756071f309ca87608d35 Mon Sep 17 00:00:00 2001
From: Matteo Pagliazzi <matteopagliazzi@gmail.com>
Date: Sat, 7 May 2016 17:35:13 +0200
Subject: [PATCH 11/12] v3: fix crashes when group or leader cannot be
 populated and fixes challenges migration for tavern challenges

---
 migrations/api_v3/challenges.js              |  9 +++++++++
 website/src/controllers/api-v2/challenges.js |  7 ++++---
 website/src/controllers/api-v3/challenges.js | 15 +++++++++------
 3 files changed, 22 insertions(+), 9 deletions(-)

diff --git a/migrations/api_v3/challenges.js b/migrations/api_v3/challenges.js
index 85a019cc7d..77972172dd 100644
--- a/migrations/api_v3/challenges.js
+++ b/migrations/api_v3/challenges.js
@@ -112,6 +112,15 @@ function processChallenges (afterId) {
       if (!oldChallenge.group) throw new Error('challenge.group is required');
       if (!oldChallenge.leader) throw new Error('challenge.leader is required');
 
+
+      if (oldChallenge.leader === '9') {
+        oldChallenge.leader = '00000000-0000-4000-9000-000000000000';
+      }
+
+      if (oldChallenge.group === 'habitrpg') {
+        oldChallenge.group = '00000000-0000-4000-A000-000000000000';
+      }
+
       delete oldChallenge.id;
 
       var newChallenge = new NewChallenge(oldChallenge);
diff --git a/website/src/controllers/api-v2/challenges.js b/website/src/controllers/api-v2/challenges.js
index 51f57433c7..404fa379d1 100644
--- a/website/src/controllers/api-v2/challenges.js
+++ b/website/src/controllers/api-v2/challenges.js
@@ -61,8 +61,8 @@ api.list = async function(req, res, next) {
         User.findById(chal.leader).select(nameFields).exec(),
         Group.findById(chal.group).select(basicGroupFields).exec(),
       ]).then(populatedData => {
-        resChals[index].leader = populatedData[0].toJSON({minimize: true});
-        resChals[index].group = populatedData[1].toJSON({minimize: true});
+        resChals[index].leader = populatedData[0] ? populatedData[0].toJSON({minimize: true}) : null;
+        resChals[index].group = populatedData[1] ? populatedData[1].toJSON({minimize: true}) : null;
       });
     }));
 
@@ -88,7 +88,8 @@ api.get = async function(req, res, next) {
     let group = await Group.getGroup({user, groupId: challenge.group, optionalMembership: true});
     if (!group || !challenge.canView(user, group)) return res.status(404).json({err: 'Challenge ' + req.params.cid + ' not found'});
 
-    let leaderRes = (await User.findById(challenge.leader).select('profile.name').exec()).toJSON({minimize: true});
+    let leaderRes = await User.findById(challenge.leader).select('profile.name').exec();
+    leaderRes = leaderRes ? leaderRes.toJSON({minimize: true}) : null;
 
     challenge.getTransformedData({
       populateMembers: 'profile.name',
diff --git a/website/src/controllers/api-v3/challenges.js b/website/src/controllers/api-v3/challenges.js
index f7bb61971f..b334dedc41 100644
--- a/website/src/controllers/api-v3/challenges.js
+++ b/website/src/controllers/api-v3/challenges.js
@@ -153,7 +153,8 @@ api.joinChallenge = {
       type: group.type,
       privacy: group.privacy,
     };
-    response.leader = (await User.findById(response.leader).select(nameFields).exec()).toJSON({minimize: true});
+    let chalLeader = await User.findById(response.leader).select(nameFields).exec();
+    response.leader = chalLeader ? chalLeader.toJSON({minimize: true}) : null;
 
     res.respond(200, response);
   },
@@ -233,8 +234,8 @@ api.getUserChallenges = {
         User.findById(chal.leader).select(nameFields).exec(),
         Group.findById(chal.group).select(basicGroupFields).exec(),
       ]).then(populatedData => {
-        resChals[index].leader = populatedData[0].toJSON({minimize: true});
-        resChals[index].group = populatedData[1].toJSON({minimize: true});
+        resChals[index].leader = populatedData[0] ? populatedData[0].toJSON({minimize: true}) : null;
+        resChals[index].group = populatedData[1] ? populatedData[1].toJSON({minimize: true}) : null;
       });
     }));
 
@@ -278,7 +279,7 @@ api.getGroupChallenges = {
     // Instead of populate we make a find call manually because of https://github.com/Automattic/mongoose/issues/3833
     await Q.all(resChals.map((chal, index) => {
       return User.findById(chal.leader).select(nameFields).exec().then(populatedLeader => {
-        resChals[index].leader = populatedLeader.toJSON({minimize: true});
+        resChals[index].leader = populatedLeader ? populatedLeader.toJSON({minimize: true}) : null;
       });
     }));
 
@@ -322,7 +323,8 @@ api.getChallenge = {
     let chalRes = challenge.toJSON();
     chalRes.group = group.toJSON({minimize: true});
     // Instead of populate we make a find call manually because of https://github.com/Automattic/mongoose/issues/3833
-    chalRes.leader = (await User.findById(chalRes.leader).select(nameFields).exec()).toJSON({minimize: true});
+    let chalLeader = await User.findById(chalRes.leader).select(nameFields).exec();
+    chalRes.leader = chalLeader ? chalLeader.toJSON({minimize: true}) : null;
 
     res.respond(200, chalRes);
   },
@@ -441,7 +443,8 @@ api.updateChallenge = {
       type: group.type,
       privacy: group.privacy,
     };
-    response.leader = (await User.findById(response.leader).select(nameFields).exec()).toJSON({minimize: true});
+    let chalLeader = await User.findById(response.leader).select(nameFields).exec();
+    response.leader = chalLeader ? chalLeader.toJSON({minimize: true}) : null;
     res.respond(200, response);
   },
 };

From e19130bd8c455e6c81e4afd0646cd6614f155e00 Mon Sep 17 00:00:00 2001
From: Matteo Pagliazzi <matteopagliazzi@gmail.com>
Date: Sun, 8 May 2016 15:50:38 +0200
Subject: [PATCH 12/12] fix typo custsomerId -> customerId

---
 common/script/fns/randomDrop.js | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/common/script/fns/randomDrop.js b/common/script/fns/randomDrop.js
index 102709da57..92064e62f1 100644
--- a/common/script/fns/randomDrop.js
+++ b/common/script/fns/randomDrop.js
@@ -49,7 +49,7 @@ module.exports = function randomDrop (user, modifiers, req = {}) {
     user.markModified('party.quest.progress');
   }
 
-  if (user.purchased && user.purchased.plan && user.purchased.plan.custsomerId) {
+  if (user.purchased && user.purchased.plan && user.purchased.plan.customerId) {
     dropMultiplier = 2;
   } else {
     dropMultiplier = 1;