From 55db0a4a4b066b5d103f0a8433de9f84e1c1b6db Mon Sep 17 00:00:00 2001
From: Matteo Pagliazzi <matteopagliazzi@gmail.com>
Date: Tue, 12 Jan 2016 17:27:06 +0100
Subject: [PATCH 01/16] add routes to get a single user, get members for a
 group, get members invited to a group

---
 common/locales/en/api-v3.json                |   1 +
 website/src/controllers/api-v3/challenges.js |   2 +-
 website/src/controllers/api-v3/chat.js       |   2 +-
 website/src/controllers/api-v3/members.js    | 157 +++++++++++++++++++
 website/src/models/group.js                  |   1 -
 website/src/models/user.js                   |  10 +-
 6 files changed, 169 insertions(+), 4 deletions(-)
 create mode 100644 website/src/controllers/api-v3/members.js

diff --git a/common/locales/en/api-v3.json b/common/locales/en/api-v3.json
index c934ad7446..c87946e717 100644
--- a/common/locales/en/api-v3.json
+++ b/common/locales/en/api-v3.json
@@ -15,6 +15,7 @@
   "cantDetachFb": "Account lacks another authentication method, can't detach Facebook.",
   "onlySocialAttachLocal": "Local auth can only be added to a social account.",
   "invalidReqParams": "Invalid request parameters.",
+  "memberIdRequired": "\"member\" must be a valid UUID.",
   "taskIdRequired": "\"taskId\" must be a valid UUID.",
   "taskNotFound": "Task not found.",
   "invalidTaskType": "Task type must be one of \"habit\", \"daily\", \"todo\", \"reward\".",
diff --git a/website/src/controllers/api-v3/challenges.js b/website/src/controllers/api-v3/challenges.js
index fa3a71b818..7eaeb940d5 100644
--- a/website/src/controllers/api-v3/challenges.js
+++ b/website/src/controllers/api-v3/challenges.js
@@ -167,7 +167,7 @@ function _closeChal (challenge, broken = {}) {
     Tasks.Task.remove({'challenge.id': challenge._id, userId: {$exists: false}}).exec(),
     // Set the challenge tag to non-challenge status and remove the challenge from the user's challenges
     User.update({
-      challenges: {$in: [challenge._id]},
+      challenges: challenge._id,
       'tags._id': challenge._id,
     }, {
       $set: {'tags.$.challenge': false},
diff --git a/website/src/controllers/api-v3/chat.js b/website/src/controllers/api-v3/chat.js
index 48dc2e440d..7c2d42bf62 100644
--- a/website/src/controllers/api-v3/chat.js
+++ b/website/src/controllers/api-v3/chat.js
@@ -7,7 +7,7 @@ import {
 } from '../../libs/api-v3/errors';
 import _ from 'lodash';
 import { sendTxn } from '../../libs/api-v3/email';
-import nconf    from 'nconf';
+import nconf from 'nconf';
 
 let api = {};
 
diff --git a/website/src/controllers/api-v3/members.js b/website/src/controllers/api-v3/members.js
new file mode 100644
index 0000000000..7bc719d7d4
--- /dev/null
+++ b/website/src/controllers/api-v3/members.js
@@ -0,0 +1,157 @@
+import { authWithHeaders } from '../../middlewares/api-v3/auth';
+import cron from '../../middlewares/api-v3/cron';
+import {
+  model as User,
+  publicFields as memberFields,
+  nameFields,
+} from '../../models/user';
+import { model as Group } from '../../models/group';
+import {
+  NotFound,
+} from '../../libs/api-v3/errors';
+
+let api = {};
+
+// TODO allow only to select nameFields instead of all publicFields?
+/**
+ * @api {get} /members/:memberId Get a member profile
+ * @apiVersion 3.0.0
+ * @apiName GetMember
+ * @apiGroup Member
+ *
+ * @apiParam {UUID} memberId The member's id
+ *
+ * @apiSuccess {object} member The member object
+ */
+api.getMember = {
+  method: 'GET',
+  url: '/members/:memberId',
+  middlewares: [authWithHeaders(), cron],
+  async handler (req, res) {
+    req.checkParams('memberId', res.t('memberIdRequired')).notEmpty().isUUID();
+
+    let validationErrors = req.validationErrors();
+    if (validationErrors) throw validationErrors;
+
+    let memberId = req.params.memberId;
+
+    let member = await User
+      .findById(memberId)
+      .select(memberFields)
+      .exec();
+
+    if (!member) throw new NotFound(res.t('userWithIDNotFound', {userId: memberId}));
+
+    res.respond(200, member);
+  },
+};
+
+// TODO allow to get more members' fields (the same as in api.getMember) for parties?
+/**
+ * @api {get} /groups/:groupId/members Get members for a groups with a limit of 30 member per request. To get all members run requests against this routes (updating the lastId query parameter) until you get less than 30 results.
+ * @apiVersion 3.0.0
+ * @apiName GetMembersForGroup
+ * @apiGroup Member
+ *
+ * @apiParam {UUID} groupId The group id
+ * @apiParam {UUID} lastId Query parameter to specify the last member returned in a previous request to this route and get the next batch of results
+ * @apiParam {boolean} includeAllPublicFields Query parameter avalaible only when fetching a party. If === `true` then all public fields for members will be returned (liek when making a request for a single member)
+ *
+ * @apiSuccess {array} members An array of members, sorted by _id
+ */
+api.getMembersForGroup = {
+  method: 'GET',
+  url: '/groups/:groupId/members',
+  middlewares: [authWithHeaders(), cron],
+  async handler (req, res) {
+    req.checkParams('groupId', res.t('groupIdRequired')).notEmpty();
+    req.checkQuery('lastId').optional().notEmpty().isUUID();
+
+    let validationErrors = req.validationErrors();
+    if (validationErrors) throw validationErrors;
+
+    let groupId = req.params.groupId;
+    let lastId = req.query.lastId;
+    let user = res.locals.user;
+
+    let group = await Group.getGroup(user, groupId, '_id type');
+    if (!group) throw new NotFound(res.t('groupNotFound'));
+
+    let query = {};
+    let fields = nameFields;
+
+    if (group.type === 'guild') {
+      query.guilds = group._id;
+    } else {
+      query['party._id'] = group._id; // group._id and not groupId because groupId could be === 'party'
+
+      if (req.query.includeAllPublicFields === 'true') {
+        fields = memberFields;
+      }
+    }
+
+    if (lastId) query._id = {$gt: lastId};
+
+    let members = await User
+      .find(query)
+      .sortBy({_id: 1})
+      .limit(30)
+      .select(fields)
+      .exec();
+
+    res.respond(200, members);
+  },
+};
+
+// TODO very similar to getInvitesForGroup might be worth abstracting some logic
+/**
+ * @api {get} /groups/:groupId/invites Get invites for a groups with a limit of 30 member per request. To get all invites run requests against this routes (updating the lastId query parameter) until you get less than 30 results.
+ * @apiVersion 3.0.0
+ * @apiName GetInvitesForGroup
+ * @apiGroup Member
+ *
+ * @apiParam {UUID} groupId The group id
+ * @apiParam {UUID} lastId Query parameter to specify the last invite returned in a previous request to this route and get the next batch of results
+ *
+ * @apiSuccess {array} invites An array of invites, sorted by _id
+ */
+api.getInvitesForGroup = {
+  method: 'GET',
+  url: '/groups/:groupId/invites',
+  middlewares: [authWithHeaders(), cron],
+  async handler (req, res) {
+    req.checkParams('groupId', res.t('groupIdRequired')).notEmpty();
+    req.checkQuery('lastId').optional().notEmpty().isUUID();
+
+    let validationErrors = req.validationErrors();
+    if (validationErrors) throw validationErrors;
+
+    let groupId = req.params.groupId;
+    let lastId = req.query.lastId;
+    let user = res.locals.user;
+
+    let group = await Group.getGroup(user, groupId, '_id type');
+    if (!group) throw new NotFound(res.t('groupNotFound'));
+
+    let query = {};
+
+    if (group.type === 'guild') {
+      query['invitations.guilds.id'] = group._id;
+    } else {
+      query['invitations.party.id'] = group._id; // group._id and not groupId because groupId could be === 'party'
+    }
+
+    if (lastId) query._id = {$gt: lastId};
+
+    let invites = await User
+      .find(query)
+      .sortBy({_id: 1})
+      .limit(30)
+      .select(nameFields)
+      .exec();
+
+    res.respond(200, invites);
+  },
+};
+
+export default api;
diff --git a/website/src/models/group.js b/website/src/models/group.js
index 27ca80e73f..4770c5803d 100644
--- a/website/src/models/group.js
+++ b/website/src/models/group.js
@@ -125,7 +125,6 @@ schema.post('remove', function postRemoveGroup (group) {
   firebase.deleteGroup(group._id);
 });
 
-// TODO populate (invites too), isMember?
 schema.statics.getGroup = function getGroup (user, groupId, fields, optionalMembership) {
   let query;
 
diff --git a/website/src/models/user.js b/website/src/models/user.js
index 8dff4bcafc..0738d48e02 100644
--- a/website/src/models/user.js
+++ b/website/src/models/user.js
@@ -338,7 +338,6 @@ export let schema = new Schema({
     orderAscending: {type: String, default: 'ascending'},
     quest: {
       key: String,
-      // TODO why are we storing quest progress here too and not only on party object?
       progress: {
         up: {type: Number, default: 0},
         down: {type: Number, default: 0},
@@ -489,6 +488,15 @@ schema.plugin(baseModel, {
   },
 });
 
+// A list of publicly accessible fields (not everything from preferences because there are also a lot of settings tha should remain private)
+// TODO is all party data meant to be public?
+export let publicFields = `preferences.size preferences.hair preferences.skin preferences.shirt
+  preferences.costume preferences.sleep preferences.background profile stats achievements party
+  backer contributor auth.timestamps items`;
+
+// The minimum amount of data needed when populating multiple users
+export let nameFields = `profile.name`;
+
 schema.post('init', function postInitUser (doc) {
   shared.wrap(doc);
 });

From 12705932e3b591a2d4afd1ff1f14fd63399b473a Mon Sep 17 00:00:00 2001
From: Matteo Pagliazzi <matteopagliazzi@gmail.com>
Date: Tue, 12 Jan 2016 18:00:03 +0100
Subject: [PATCH 02/16] abstract common logic in members controller

---
 website/src/controllers/api-v3/members.js | 107 +++++++++-------------
 1 file changed, 45 insertions(+), 62 deletions(-)

diff --git a/website/src/controllers/api-v3/members.js b/website/src/controllers/api-v3/members.js
index 7bc719d7d4..3d36a63cda 100644
--- a/website/src/controllers/api-v3/members.js
+++ b/website/src/controllers/api-v3/members.js
@@ -12,7 +12,6 @@ import {
 
 let api = {};
 
-// TODO allow only to select nameFields instead of all publicFields?
 /**
  * @api {get} /members/:memberId Get a member profile
  * @apiVersion 3.0.0
@@ -46,24 +45,14 @@ api.getMember = {
   },
 };
 
-// TODO allow to get more members' fields (the same as in api.getMember) for parties?
-/**
- * @api {get} /groups/:groupId/members Get members for a groups with a limit of 30 member per request. To get all members run requests against this routes (updating the lastId query parameter) until you get less than 30 results.
- * @apiVersion 3.0.0
- * @apiName GetMembersForGroup
- * @apiGroup Member
- *
- * @apiParam {UUID} groupId The group id
- * @apiParam {UUID} lastId Query parameter to specify the last member returned in a previous request to this route and get the next batch of results
- * @apiParam {boolean} includeAllPublicFields Query parameter avalaible only when fetching a party. If === `true` then all public fields for members will be returned (liek when making a request for a single member)
- *
- * @apiSuccess {array} members An array of members, sorted by _id
- */
-api.getMembersForGroup = {
-  method: 'GET',
-  url: '/groups/:groupId/members',
-  middlewares: [authWithHeaders(), cron],
-  async handler (req, res) {
+// Return a request handler for getMembersForGroup / getInvitesForGroup
+// type is `invites` or `members`
+function handleGetMembersInvitesForGroup (type) {
+  if (type !== 'members' && type !== 'invites') {
+    throw new Error('Type must be "invites" or "members"');
+  }
+
+  return async function getMembersOrInvitesForGroup (req, res) {
     req.checkParams('groupId', res.t('groupIdRequired')).notEmpty();
     req.checkQuery('lastId').optional().notEmpty().isUUID();
 
@@ -80,30 +69,56 @@ api.getMembersForGroup = {
     let query = {};
     let fields = nameFields;
 
-    if (group.type === 'guild') {
-      query.guilds = group._id;
-    } else {
-      query['party._id'] = group._id; // group._id and not groupId because groupId could be === 'party'
+    if (type === 'members') {
+      if (group.type === 'guild') {
+        query.guilds = group._id;
+      } else {
+        query['party._id'] = group._id; // group._id and not groupId because groupId could be === 'party'
 
-      if (req.query.includeAllPublicFields === 'true') {
-        fields = memberFields;
+        if (req.query.includeAllPublicFields === 'true') {
+          fields = memberFields;
+        }
+      }
+    } else {
+      if (group.type === 'guild') { // eslint-disable-line no-lonely-if
+        query['invitations.guilds.id'] = group._id;
+      } else {
+        query['invitations.party.id'] = group._id; // group._id and not groupId because groupId could be === 'party'
       }
     }
 
     if (lastId) query._id = {$gt: lastId};
 
-    let members = await User
+    let users = await User
       .find(query)
       .sortBy({_id: 1})
       .limit(30)
       .select(fields)
       .exec();
 
-    res.respond(200, members);
-  },
+    res.respond(200, users);
+  };
+}
+
+/**
+ * @api {get} /groups/:groupId/members Get members for a groups with a limit of 30 member per request. To get all members run requests against this routes (updating the lastId query parameter) until you get less than 30 results.
+ * @apiVersion 3.0.0
+ * @apiName GetMembersForGroup
+ * @apiGroup Member
+ *
+ * @apiParam {UUID} groupId The group id
+ * @apiParam {UUID} lastId Query parameter to specify the last member returned in a previous request to this route and get the next batch of results
+ * @apiParam {boolean} includeAllPublicFields Query parameter avalaible only when fetching a party. If === `true` then all public fields for members will be returned (liek when making a request for a single member)
+ *
+ * @apiSuccess {array} members An array of members, sorted by _id
+ */
+api.getMembersForGroup = {
+  method: 'GET',
+  url: '/groups/:groupId/members',
+  middlewares: [authWithHeaders(), cron],
+  handler: handleGetMembersInvitesForGroup('members'),
 };
 
-// TODO very similar to getInvitesForGroup might be worth abstracting some logic
 /**
  * @api {get} /groups/:groupId/invites Get invites for a groups with a limit of 30 member per request. To get all invites run requests against this routes (updating the lastId query parameter) until you get less than 30 results.
  * @apiVersion 3.0.0
@@ -119,39 +134,7 @@ api.getInvitesForGroup = {
   method: 'GET',
   url: '/groups/:groupId/invites',
   middlewares: [authWithHeaders(), cron],
-  async handler (req, res) {
-    req.checkParams('groupId', res.t('groupIdRequired')).notEmpty();
-    req.checkQuery('lastId').optional().notEmpty().isUUID();
-
-    let validationErrors = req.validationErrors();
-    if (validationErrors) throw validationErrors;
-
-    let groupId = req.params.groupId;
-    let lastId = req.query.lastId;
-    let user = res.locals.user;
-
-    let group = await Group.getGroup(user, groupId, '_id type');
-    if (!group) throw new NotFound(res.t('groupNotFound'));
-
-    let query = {};
-
-    if (group.type === 'guild') {
-      query['invitations.guilds.id'] = group._id;
-    } else {
-      query['invitations.party.id'] = group._id; // group._id and not groupId because groupId could be === 'party'
-    }
-
-    if (lastId) query._id = {$gt: lastId};
-
-    let invites = await User
-      .find(query)
-      .sortBy({_id: 1})
-      .limit(30)
-      .select(nameFields)
-      .exec();
-
-    res.respond(200, invites);
-  },
+  handler: handleGetMembersInvitesForGroup('invites'),
 };
 
 export default api;

From 7c15472fab0d83ff10b8514e6ac6d21c895704cc Mon Sep 17 00:00:00 2001
From: Matteo Pagliazzi <matteopagliazzi@gmail.com>
Date: Wed, 13 Jan 2016 18:29:02 +0100
Subject: [PATCH 03/16] wip on members controllers

---
 website/src/controllers/api-v3/members.js | 67 ++++++++++++++++++++++-
 1 file changed, 65 insertions(+), 2 deletions(-)

diff --git a/website/src/controllers/api-v3/members.js b/website/src/controllers/api-v3/members.js
index 3d36a63cda..9ac5d826f3 100644
--- a/website/src/controllers/api-v3/members.js
+++ b/website/src/controllers/api-v3/members.js
@@ -101,7 +101,7 @@ function handleGetMembersInvitesForGroup (type) {
 }
 
 /**
- * @api {get} /groups/:groupId/members Get members for a groups with a limit of 30 member per request. To get all members run requests against this routes (updating the lastId query parameter) until you get less than 30 results.
+ * @api {get} /groups/:groupId/members Get members for a group with a limit of 30 member per request. To get all members run requests against this routes (updating the lastId query parameter) until you get less than 30 results.
  * @apiVersion 3.0.0
  * @apiName GetMembersForGroup
  * @apiGroup Member
@@ -120,7 +120,7 @@ api.getMembersForGroup = {
 };
 
 /**
- * @api {get} /groups/:groupId/invites Get invites for a groups with a limit of 30 member per request. To get all invites run requests against this routes (updating the lastId query parameter) until you get less than 30 results.
+ * @api {get} /groups/:groupId/invites Get invites for a group with a limit of 30 member per request. To get all invites run requests against this routes (updating the lastId query parameter) until you get less than 30 results.
  * @apiVersion 3.0.0
  * @apiName GetInvitesForGroup
  * @apiGroup Member
@@ -137,4 +137,67 @@ api.getInvitesForGroup = {
   handler: handleGetMembersInvitesForGroup('invites'),
 };
 
+/**
+ * @api {get} /challenges/:challengeId/members Get members for a challenge with a limit of 30 member per request. To get all members run requests against this routes (updating the lastId query parameter) until you get less than 30 results.
+ * @apiVersion 3.0.0
+ * @apiName GetMembersForChallenge
+ * @apiGroup Member
+ *
+ * @apiParam {UUID} challengeId The challenge id
+ * @apiParam {UUID} lastId Query parameter to specify the last member returned in a previous request to this route and get the next batch of results
+ *
+ * @apiSuccess {array} members An array of members, sorted by _id
+ */
+api.getMembersForChallenge = {
+  method: 'GET',
+  url: '/challenges/:challengeId/members',
+  middlewares: [authWithHeaders(), cron],
+  async handler (req, res) {
+    req.checkParams('challenge', res.t('challengeIdRequired')).notEmpty();
+    req.checkQuery('lastId').optional().notEmpty().isUUID();
+
+    let validationErrors = req.validationErrors();
+    if (validationErrors) throw validationErrors;
+
+    let challengeId = req.params.challengeId;
+    let lastId = req.query.lastId;
+    let user = res.locals.user;
+
+    let challenge = await Challenge.findById(challengeId).exec();
+
+    let query = {};
+    let fields = nameFields;
+
+    if (type === 'members') {
+      if (group.type === 'guild') {
+        query.guilds = group._id;
+      } else {
+        query['party._id'] = group._id; // group._id and not groupId because groupId could be === 'party'
+
+        if (req.query.includeAllPublicFields === 'true') {
+          fields = memberFields;
+        }
+      }
+    } else {
+      if (group.type === 'guild') { // eslint-disable-line no-lonely-if
+        query['invitations.guilds.id'] = group._id;
+      } else {
+        query['invitations.party.id'] = group._id; // group._id and not groupId because groupId could be === 'party'
+      }
+    }
+
+    if (lastId) query._id = {$gt: lastId};
+
+    let users = await User
+      .find(query)
+      .sortBy({_id: 1})
+      .limit(30)
+      .select(fields)
+      .exec();
+
+    res.respond(200, users);
+  }
+};
+
+
 export default api;

From 50a85337a7fb9d6407e1efdad2ebc349834abc0a Mon Sep 17 00:00:00 2001
From: Matteo Pagliazzi <matteopagliazzi@gmail.com>
Date: Fri, 15 Jan 2016 15:29:50 +0100
Subject: [PATCH 04/16] better access control for challenges

---
 website/src/controllers/api-v3/challenges.js | 14 ++++++--------
 website/src/models/challenge.js              | 18 ++++++++++++++++++
 2 files changed, 24 insertions(+), 8 deletions(-)

diff --git a/website/src/controllers/api-v3/challenges.js b/website/src/controllers/api-v3/challenges.js
index 8bec5eae6e..321f3c66c5 100644
--- a/website/src/controllers/api-v3/challenges.js
+++ b/website/src/controllers/api-v3/challenges.js
@@ -100,9 +100,9 @@ api.getChallenges = {
   async handler (req, res) {
     let user = res.locals.user;
 
-    let groups = user.guilds || [];
+    let groups = user.guilds.slice(0); // slice is used to clone the array so we don't modify it directly
     if (user.party._id) groups.push(user.party._id);
-    groups.push('habitrpg'); // Public challenges
+    groups.push('habitrpg'); // tavern challenges
 
     let challenges = await Challenge.find({
       $or: [
@@ -143,11 +143,9 @@ api.getChallenge = {
     let user = res.local.user;
     let challengeId = req.params.challengeId;
 
-    let challenge = await Challenge.findOne({_id: challengeId}).exec(); // TODO populate
+    let challenge = await Challenge.findById(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
-    // TODO support challenges in groups I'm a member of
-    if (!challenge || (user.challenges.indexOf(challengeId) === -1 && challenge.leader !== user._id && !user.contributor.admin)) { // eslint-disable-line no-extra-parens
+    if (!challenge || !challenge.hasAccess(user)) {
       throw new NotFound(res.t('challengeNotFound'));
     }
 
@@ -233,7 +231,7 @@ api.deleteChallenge = {
 
     let challenge = await Challenge.findOne({_id: req.params.challengeId}).exec();
     if (!challenge) throw new NotFound(res.t('challengeNotFound'));
-    if (challenge.leader !== user._id && !user.contributor.admin) throw new NotAuthorized(res.t('onlyLeaderDeleteChal'));
+    if (!challenge.canModify(user)) throw new NotAuthorized(res.t('onlyLeaderDeleteChal'));
 
     res.respond(200, {});
     // Close channel in background
@@ -264,7 +262,7 @@ api.selectChallengeWinner = {
 
     let challenge = await Challenge.findOne({_id: req.params.challengeId}).exec();
     if (!challenge) throw new NotFound(res.t('challengeNotFound'));
-    if (challenge.leader !== user._id && !user.contributor.admin) throw new NotAuthorized(res.t('onlyLeaderDeleteChal'));
+    if (!challenge.canModify(user)) throw new NotAuthorized(res.t('onlyLeaderDeleteChal'));
 
     let winner = await User.findOne({_id: req.params.winnerId}).exec();
     if (!winner || winner.challenges.indexOf(challenge._id) === -1) throw new NotFound(res.t('winnerNotFound', {userId: req.parama.winnerId}));
diff --git a/website/src/models/challenge.js b/website/src/models/challenge.js
index b675a7833e..fcfb5cc1e3 100644
--- a/website/src/models/challenge.js
+++ b/website/src/models/challenge.js
@@ -31,6 +31,24 @@ schema.plugin(baseModel, {
   noSet: ['_id', 'memberCount', 'challengeCount', 'tasksOrder'],
 });
 
+// Return true if user has access to the challenge
+schema.methods.hasAccess = function hasAccessToChallenge (user) {
+  let userGroups = user.guilds.slice(0);
+  if (user.party._id) userGroups.push(user.party._id);
+  userGroups.push('habitrpg'); // tavern challenges
+  return this.leader === user._id || user.contributor.admin || userGroups.indexOf(this.groupId) !== -1;
+};
+
+// Return true if user is a member of the challenge
+schema.methods.isMember = function isChallengeMember (user) {
+  return user.challenges.indexOf(this._id) !== -1;
+};
+
+// Return true if the user can modify (close, selectWinner, ...) the challenge
+schema.methods.canModify = function canModifyChallenge (user) {
+  return user.contributor.admin || this.leader === user._id;
+};
+
 // 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

From 3eb9d4b0989f9d18fedc9e6fafb252cef623c37a Mon Sep 17 00:00:00 2001
From: Matteo Pagliazzi <matteopagliazzi@gmail.com>
Date: Fri, 15 Jan 2016 15:30:13 +0100
Subject: [PATCH 05/16] members controller: support challenges

---
 website/src/controllers/api-v3/members.js | 86 ++++++++---------------
 1 file changed, 28 insertions(+), 58 deletions(-)

diff --git a/website/src/controllers/api-v3/members.js b/website/src/controllers/api-v3/members.js
index 9ac5d826f3..f464fe830c 100644
--- a/website/src/controllers/api-v3/members.js
+++ b/website/src/controllers/api-v3/members.js
@@ -6,6 +6,7 @@ import {
   nameFields,
 } from '../../models/user';
 import { model as Group } from '../../models/group';
+import { model as Challenge } from '../../models/challenge';
 import {
   NotFound,
 } from '../../libs/api-v3/errors';
@@ -45,31 +46,45 @@ api.getMember = {
   },
 };
 
-// Return a request handler for getMembersForGroup / getInvitesForGroup
+// Return a request handler for getMembersForGroup / getInvitesForGroup / getMembersForChallenge
 // type is `invites` or `members`
-function handleGetMembersInvitesForGroup (type) {
-  if (type !== 'members' && type !== 'invites') {
-    throw new Error('Type must be "invites" or "members"');
+function _getMembersForItem (type) {
+  if (['group-members', 'group-invites', 'challenge-members'].indexOf(type) === -1) {
+    throw new Error('Type must be one of "group-members", "group-invites", "challenge-members"');
   }
 
-  return async function getMembersOrInvitesForGroup (req, res) {
-    req.checkParams('groupId', res.t('groupIdRequired')).notEmpty();
+  return async function handleGetMembersForItem (req, res) {
+    if (type === 'challenge-members') {
+      req.checkParams('challengeId', res.t('challengeIdRequired')).notEmpty().isUUID();
+    } else {
+      req.checkParams('groupId', res.t('groupIdRequired')).notEmpty();
+    }
     req.checkQuery('lastId').optional().notEmpty().isUUID();
 
     let validationErrors = req.validationErrors();
     if (validationErrors) throw validationErrors;
 
     let groupId = req.params.groupId;
+    let challengeId = req.params.challengeId;
     let lastId = req.query.lastId;
     let user = res.locals.user;
+    let challenge;
+    let group;
 
-    let group = await Group.getGroup(user, groupId, '_id type');
-    if (!group) throw new NotFound(res.t('groupNotFound'));
+    if (type === 'challenge-members') {
+      challenge = await Challenge.findById(challengeId).select('_id type leader').exec();
+      if (!challenge || !challenge.hasAccess(user)) throw new NotFound(res.t('groupNotFound'));
+    } else {
+      group = await Group.getGroup(user, groupId, '_id type');
+      if (!group) throw new NotFound(res.t('groupNotFound'));
+    }
 
     let query = {};
     let fields = nameFields;
 
-    if (type === 'members') {
+    if (type === 'challenge-members') {
+      query.challenges = challenge._id;
+    } else if (type === 'group-members') {
       if (group.type === 'guild') {
         query.guilds = group._id;
       } else {
@@ -79,7 +94,7 @@ function handleGetMembersInvitesForGroup (type) {
           fields = memberFields;
         }
       }
-    } else {
+    } else if (type === 'group-invites') {
       if (group.type === 'guild') { // eslint-disable-line no-lonely-if
         query['invitations.guilds.id'] = group._id;
       } else {
@@ -116,7 +131,7 @@ api.getMembersForGroup = {
   method: 'GET',
   url: '/groups/:groupId/members',
   middlewares: [authWithHeaders(), cron],
-  handler: handleGetMembersInvitesForGroup('members'),
+  handler: _getMembersForItem('group-members'),
 };
 
 /**
@@ -134,7 +149,7 @@ api.getInvitesForGroup = {
   method: 'GET',
   url: '/groups/:groupId/invites',
   middlewares: [authWithHeaders(), cron],
-  handler: handleGetMembersInvitesForGroup('invites'),
+  handler: _getMembersForItem('group-invites'),
 };
 
 /**
@@ -152,52 +167,7 @@ api.getMembersForChallenge = {
   method: 'GET',
   url: '/challenges/:challengeId/members',
   middlewares: [authWithHeaders(), cron],
-  async handler (req, res) {
-    req.checkParams('challenge', res.t('challengeIdRequired')).notEmpty();
-    req.checkQuery('lastId').optional().notEmpty().isUUID();
-
-    let validationErrors = req.validationErrors();
-    if (validationErrors) throw validationErrors;
-
-    let challengeId = req.params.challengeId;
-    let lastId = req.query.lastId;
-    let user = res.locals.user;
-
-    let challenge = await Challenge.findById(challengeId).exec();
-
-    let query = {};
-    let fields = nameFields;
-
-    if (type === 'members') {
-      if (group.type === 'guild') {
-        query.guilds = group._id;
-      } else {
-        query['party._id'] = group._id; // group._id and not groupId because groupId could be === 'party'
-
-        if (req.query.includeAllPublicFields === 'true') {
-          fields = memberFields;
-        }
-      }
-    } else {
-      if (group.type === 'guild') { // eslint-disable-line no-lonely-if
-        query['invitations.guilds.id'] = group._id;
-      } else {
-        query['invitations.party.id'] = group._id; // group._id and not groupId because groupId could be === 'party'
-      }
-    }
-
-    if (lastId) query._id = {$gt: lastId};
-
-    let users = await User
-      .find(query)
-      .sortBy({_id: 1})
-      .limit(30)
-      .select(fields)
-      .exec();
-
-    res.respond(200, users);
-  }
+  handler: _getMembersForItem('challenge-members'),
 };
 
-
 export default api;

From b0caf71641384ab9f771d69b501e2562d7387561 Mon Sep 17 00:00:00 2001
From: Matteo Pagliazzi <matteopagliazzi@gmail.com>
Date: Fri, 15 Jan 2016 18:54:28 +0100
Subject: [PATCH 06/16] fix several busg with tasks and challenges

---
 website/src/controllers/api-v3/challenges.js | 12 +++++++++---
 website/src/controllers/api-v3/tasks.js      |  4 ++--
 2 files changed, 11 insertions(+), 5 deletions(-)

diff --git a/website/src/controllers/api-v3/challenges.js b/website/src/controllers/api-v3/challenges.js
index 321f3c66c5..f98cac0302 100644
--- a/website/src/controllers/api-v3/challenges.js
+++ b/website/src/controllers/api-v3/challenges.js
@@ -77,9 +77,15 @@ api.createChallenge = {
     req.body.official = user.contributor.admin && req.body.official;
     let challenge = new Challenge(Challenge.sanitize(req.body));
 
-    let results = await Q.all(challenge.save(), group.save());
-    let savedChal = results[0];
+    // First validate challenge so we don't save group if it's invalid (only runs sync validators)
+    let challengeValidationErrors = challenge.validateSync();
+    if (challengeValidationErrors) throw challengeValidationErrors;
 
+    let results = await Q.all([challenge.save({
+      validateBeforeSave: false, // already validate
+    }), group.save()]);
+
+    let savedChal = results[0];
     await savedChal.syncToUser(user); // (it also saves the user)
     res.respond(201, savedChal);
   },
@@ -140,7 +146,7 @@ api.getChallenge = {
     let validationErrors = req.validationErrors();
     if (validationErrors) throw validationErrors;
 
-    let user = res.local.user;
+    let user = res.locals.user;
     let challengeId = req.params.challengeId;
 
     let challenge = await Challenge.findById(challengeId).exec();
diff --git a/website/src/controllers/api-v3/tasks.js b/website/src/controllers/api-v3/tasks.js
index cee1ba4527..c2498b2a94 100644
--- a/website/src/controllers/api-v3/tasks.js
+++ b/website/src/controllers/api-v3/tasks.js
@@ -92,7 +92,7 @@ api.createChallengeTasks = {
     let reqValidationErrors = req.validationErrors();
     if (reqValidationErrors) throw reqValidationErrors;
 
-    let user = res.local.user;
+    let user = res.locals.user;
     let challengeId = req.params.challengeId;
 
     let challenge = await Challenge.findOne({_id: challengeId}).exec();
@@ -194,7 +194,7 @@ api.getChallengeTasks = {
     let validationErrors = req.validationErrors();
     if (validationErrors) throw validationErrors;
 
-    let user = res.local.user;
+    let user = res.locals.user;
     let challengeId = req.params.challengeId;
 
     let challenge = await Challenge.findOne({_id: challengeId}).select('leader').exec();

From 2ee75c1ad34f652e41dfd4ad1b7e180354c72bec Mon Sep 17 00:00:00 2001
From: Matteo Pagliazzi <matteopagliazzi@gmail.com>
Date: Fri, 15 Jan 2016 19:44:45 +0100
Subject: [PATCH 07/16] add tests for getMember

---
 .../members/GET-members_id.test.js            | 41 +++++++++++++++++++
 .../v3/integration/tags/GET-tags_id.test.js   |  2 +
 website/src/controllers/api-v3/members.js     |  3 +-
 website/src/models/user.js                    | 12 +++---
 4 files changed, 52 insertions(+), 6 deletions(-)
 create mode 100644 test/api/v3/integration/members/GET-members_id.test.js

diff --git a/test/api/v3/integration/members/GET-members_id.test.js b/test/api/v3/integration/members/GET-members_id.test.js
new file mode 100644
index 0000000000..25c7600c80
--- /dev/null
+++ b/test/api/v3/integration/members/GET-members_id.test.js
@@ -0,0 +1,41 @@
+import {
+  generateUser,
+  translate as t,
+} from '../../../../helpers/api-v3-integration.helper';
+import { v4 as generateUUID } from 'uuid';
+
+describe('GET /members/:memberId', () => {
+  let user;
+
+  before(async () => {
+    user = await generateUser();
+  });
+
+  it('returns a member public data only', async () => {
+    let member = await generateUser({ // make sure user has all the fields that can be returned by the getMember call
+      contributor: {level: 1},
+      backer: {tier: 3},
+      preferences: {
+        costume: false,
+        background: 'volcano',
+      },
+    });
+    let memberRes = await user.get(`/members/${member._id}`);
+    expect(memberRes).to.have.all.keys([ // works as: object has all and only these keys
+      '_id', 'preferences', 'profile', 'stats', 'achievements', 'party',
+      'backer', 'contributor', 'auth', 'items',
+    ]);
+    expect(Object.keys(memberRes.auth)).to.eql(['timestamps']);
+    expect(Object.keys(memberRes.preferences).sort()).to.eql(['size', 'hair', 'skin', 'shirt',
+      'costume', 'sleep', 'background'].sort());
+  });
+
+  it('handles non-existing members', async () => {
+    let dummyId = generateUUID();
+    await expect(user.get(`/members/${dummyId}`)).to.eventually.be.rejected.and.eql({
+      code: 404,
+      error: 'NotFound',
+      message: t('userWithIDNotFound', {userId: dummyId}),
+    });
+  });
+});
diff --git a/test/api/v3/integration/tags/GET-tags_id.test.js b/test/api/v3/integration/tags/GET-tags_id.test.js
index 1189c2af24..c46b5b8801 100644
--- a/test/api/v3/integration/tags/GET-tags_id.test.js
+++ b/test/api/v3/integration/tags/GET-tags_id.test.js
@@ -15,4 +15,6 @@ describe('GET /tags/:tagId', () => {
 
     expect(tag).to.deep.equal(createdTag);
   });
+
+  it('handles non-existing tags');
 });
diff --git a/website/src/controllers/api-v3/members.js b/website/src/controllers/api-v3/members.js
index f464fe830c..6eef598050 100644
--- a/website/src/controllers/api-v3/members.js
+++ b/website/src/controllers/api-v3/members.js
@@ -42,7 +42,8 @@ api.getMember = {
 
     if (!member) throw new NotFound(res.t('userWithIDNotFound', {userId: memberId}));
 
-    res.respond(200, member);
+    // manually call toJSON with minimize: true so empty paths aren't returned
+    res.respond(200, member.toJSON({minimize: true}));
   },
 };
 
diff --git a/website/src/models/user.js b/website/src/models/user.js
index fcad88c5a6..8942a8afd2 100644
--- a/website/src/models/user.js
+++ b/website/src/models/user.js
@@ -472,18 +472,20 @@ export let schema = new Schema({
   },
 }, {
   strict: true,
-  minimize: false, // So empty objects are returned
+  minimize: false, // So empty objects are returned TODO make sure it's in every model
 });
 
 schema.plugin(baseModel, {
-  // TODO revisit a lot of things are missing
-  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'],
+  // 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
+  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 (doc) {
     // FIXME? Is this a reference to `doc.filters` or just disabled code? Remove?
     // TODO this works?
-    doc.filters = {};
-    doc._tmp = this._tmp; // be sure to send down drop notifs
+    // doc.filters = {};
+    // doc._tmp = this._tmp; // be sure to send down drop notifs
 
     return doc;
   },

From ad41037f597c296930ce8cce11e418f4b5f32799 Mon Sep 17 00:00:00 2001
From: Matteo Pagliazzi <matteopagliazzi@gmail.com>
Date: Fri, 15 Jan 2016 21:47:20 +0100
Subject: [PATCH 08/16] finish tests for members controller

---
 .../GET-challenges_challengeId_members.test   |  0
 .../groups/GET-groups_groupId_invites.test.js |  0
 .../groups/GET-groups_groupId_members.test.js | 95 +++++++++++++++++++
 ...in.js => POST-groups_groupId_join.test.js} |  0
 .../members/GET-members_id.test.js            |  8 ++
 website/src/controllers/api-v3/members.js     |  7 +-
 6 files changed, 107 insertions(+), 3 deletions(-)
 create mode 100644 test/api/v3/integration/challenges/GET-challenges_challengeId_members.test
 create mode 100644 test/api/v3/integration/groups/GET-groups_groupId_invites.test.js
 create mode 100644 test/api/v3/integration/groups/GET-groups_groupId_members.test.js
 rename test/api/v3/integration/groups/{POST-groups_groupId_join.js => POST-groups_groupId_join.test.js} (100%)

diff --git a/test/api/v3/integration/challenges/GET-challenges_challengeId_members.test b/test/api/v3/integration/challenges/GET-challenges_challengeId_members.test
new file mode 100644
index 0000000000..e69de29bb2
diff --git a/test/api/v3/integration/groups/GET-groups_groupId_invites.test.js b/test/api/v3/integration/groups/GET-groups_groupId_invites.test.js
new file mode 100644
index 0000000000..e69de29bb2
diff --git a/test/api/v3/integration/groups/GET-groups_groupId_members.test.js b/test/api/v3/integration/groups/GET-groups_groupId_members.test.js
new file mode 100644
index 0000000000..d7ad8ac769
--- /dev/null
+++ b/test/api/v3/integration/groups/GET-groups_groupId_members.test.js
@@ -0,0 +1,95 @@
+import {
+  generateUser,
+  generateGroup,
+  translate as t,
+} from '../../../../helpers/api-v3-integration.helper';
+import { v4 as generateUUID } from 'uuid';
+
+describe('GET /groups/:groupId/members', () => {
+  let user;
+
+  beforeEach(async () => {
+    user = await generateUser();
+  });
+
+  it('validates optional req.query.lastId to be an UUID', async () => {
+    await expect(user.get(`/groups/groupId/members?lastId=invalidUUID`)).to.eventually.be.rejected.and.eql({
+      code: 400,
+      error: 'BadRequest',
+      message: t('invalidReqParams'),
+    });
+  });
+
+  it('fails if group doesn\'t exists', async () => {
+    await expect(user.get(`/groups/${generateUUID()}/members`)).to.eventually.be.rejected.and.eql({
+      code: 404,
+      error: 'NotFound',
+      message: t('groupNotFound'),
+    });
+  });
+
+  it('fails if user doesn\'t have access to the group', async () => {
+    let group = await generateGroup(user, {type: 'party', name: generateUUID()});
+    let anotherUser = await generateUser();
+    await expect(anotherUser.get(`/groups/${group._id}/members`)).to.eventually.be.rejected.and.eql({
+      code: 404,
+      error: 'NotFound',
+      message: t('groupNotFound'),
+    });
+  });
+
+  it('works when passing party as req.params.groupId', async () => {
+    await generateGroup(user, {type: 'party', name: generateUUID()});
+    let res = await user.get(`/groups/party/members`);
+    expect(res).to.be.an('array');
+    expect(res.length).to.equal(1);
+    expect(res[0]).to.eql({
+      _id: user._id,
+      profile: {name: user.profile.name},
+    });
+  });
+
+  it('populates only some fields', async () => {
+    await generateGroup(user, {type: 'party', name: generateUUID()});
+    let res = await user.get(`/groups/party/members`);
+    expect(res[0]).to.have.all.keys(['_id', 'profile']);
+    expect(res[0].profile).to.have.all.keys(['name']);
+  });
+
+  it('returns only first 30 members', async () => {
+    let group = await generateGroup(user, {type: 'party', name: generateUUID()});
+
+    let usersToGenerate = [];
+    for (let i = 0; i < 31; i++) {
+      usersToGenerate.push(generateUser({party: {_id: group._id}}));
+    }
+    await Promise.all(usersToGenerate);
+
+    let res = await user.get(`/groups/party/members`);
+    expect(res.length).to.equal(30);
+    res.forEach(member => {
+      expect(member).to.have.all.keys(['_id', 'profile']);
+      expect(member.profile).to.have.all.keys(['name']);
+    });
+  });
+
+  it('supports using req.query.lastId to get more members', async () => {
+    let leader = await generateUser({balance: 4});
+    let group = await generateGroup(leader, {type: 'guild', privacy: 'public', name: generateUUID()});
+
+    let usersToGenerate = [];
+    for (let i = 0; i < 57; i++) {
+      usersToGenerate.push(generateUser({guilds: [group._id]}));
+    }
+    let generatedUsers = await Promise.all(usersToGenerate); // Group has 59 members (1 is the leader)
+    let expectedIds = [leader._id].concat(generatedUsers.map(generatedUser => generatedUser._id));
+
+    let res = await user.get(`/groups/${group._id}/members`);
+    expect(res.length).to.equal(30);
+    let res2 = await user.get(`/groups/${group._id}/members?lastId=${res[res.length - 1]._id}`);
+    expect(res2.length).to.equal(28);
+
+    let resIds = res.concat(res2).map(member => member._id);
+    expect(resIds).to.eql(expectedIds.sort())
+  });
+});
diff --git a/test/api/v3/integration/groups/POST-groups_groupId_join.js b/test/api/v3/integration/groups/POST-groups_groupId_join.test.js
similarity index 100%
rename from test/api/v3/integration/groups/POST-groups_groupId_join.js
rename to test/api/v3/integration/groups/POST-groups_groupId_join.test.js
diff --git a/test/api/v3/integration/members/GET-members_id.test.js b/test/api/v3/integration/members/GET-members_id.test.js
index 25c7600c80..9b4c734ea9 100644
--- a/test/api/v3/integration/members/GET-members_id.test.js
+++ b/test/api/v3/integration/members/GET-members_id.test.js
@@ -11,6 +11,14 @@ describe('GET /members/:memberId', () => {
     user = await generateUser();
   });
 
+  it('validates req.params.memberId', async () => {
+    await expect(user.get(`/members/invalidUUID`)).to.eventually.be.rejected.and.eql({
+      code: 400,
+      error: 'BadRequest',
+      message: t('invalidReqParams'),
+    });
+  });
+
   it('returns a member public data only', async () => {
     let member = await generateUser({ // make sure user has all the fields that can be returned by the getMember call
       contributor: {level: 1},
diff --git a/website/src/controllers/api-v3/members.js b/website/src/controllers/api-v3/members.js
index 6eef598050..3f5ca5ac9d 100644
--- a/website/src/controllers/api-v3/members.js
+++ b/website/src/controllers/api-v3/members.js
@@ -105,14 +105,15 @@ function _getMembersForItem (type) {
 
     if (lastId) query._id = {$gt: lastId};
 
-    let users = await User
+    let members = await User
       .find(query)
-      .sortBy({_id: 1})
+      .sort({_id: 1})
       .limit(30)
       .select(fields)
       .exec();
 
-    res.respond(200, users);
+    // manually call toJSON with minimize: true so empty paths aren't returned
+    res.respond(200, members.map(member => member.toJSON({minimize: true})));
   };
 }
 

From 96c523493a4e9b03c1c684410bf347d691290b1b Mon Sep 17 00:00:00 2001
From: Matteo Pagliazzi <matteopagliazzi@gmail.com>
Date: Fri, 15 Jan 2016 22:08:51 +0100
Subject: [PATCH 09/16] fix missing semicolon

---
 .../v3/integration/groups/GET-groups_groupId_members.test.js    | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/test/api/v3/integration/groups/GET-groups_groupId_members.test.js b/test/api/v3/integration/groups/GET-groups_groupId_members.test.js
index d7ad8ac769..15193db012 100644
--- a/test/api/v3/integration/groups/GET-groups_groupId_members.test.js
+++ b/test/api/v3/integration/groups/GET-groups_groupId_members.test.js
@@ -90,6 +90,6 @@ describe('GET /groups/:groupId/members', () => {
     expect(res2.length).to.equal(28);
 
     let resIds = res.concat(res2).map(member => member._id);
-    expect(resIds).to.eql(expectedIds.sort())
+    expect(resIds).to.eql(expectedIds.sort());
   });
 });

From d7d63ad229c01f04c38617125901ca102485770d Mon Sep 17 00:00:00 2001
From: Matteo Pagliazzi <matteopagliazzi@gmail.com>
Date: Sat, 16 Jan 2016 15:10:18 +0100
Subject: [PATCH 10/16] populate group.leader

---
 test/helpers/api-v3-integration.helper.js    |  3 +--
 website/src/controllers/api-v3/challenges.js |  2 +-
 website/src/controllers/api-v3/chat.js       |  8 +++----
 website/src/controllers/api-v3/groups.js     | 17 ++++++++-------
 website/src/controllers/api-v3/members.js    |  2 +-
 website/src/models/group.js                  | 22 +++++++++++++-------
 6 files changed, 30 insertions(+), 24 deletions(-)

diff --git a/test/helpers/api-v3-integration.helper.js b/test/helpers/api-v3-integration.helper.js
index 530b275fb6..a5d48478cd 100644
--- a/test/helpers/api-v3-integration.helper.js
+++ b/test/helpers/api-v3-integration.helper.js
@@ -201,11 +201,10 @@ export function resetHabiticaDB () {
         groups.insertOne({
           _id: 'habitrpg',
           chat: [],
-          leader: '9',
+          leader: '9', // TODO change this
           name: 'HabitRPG',
           type: 'guild',
           privacy: 'public',
-          members: [],
         }, (insertErr) => {
           if (insertErr) return reject(insertErr);
 
diff --git a/website/src/controllers/api-v3/challenges.js b/website/src/controllers/api-v3/challenges.js
index f98cac0302..ed8440ff1f 100644
--- a/website/src/controllers/api-v3/challenges.js
+++ b/website/src/controllers/api-v3/challenges.js
@@ -38,7 +38,7 @@ api.createChallenge = {
     let groupId = req.body.groupId;
     let prize = req.body.prize;
 
-    let group = await Group.getGroup(user, groupId, '-chat');
+    let group = await Group.getGroup({user, groupId, fields: '-chat'});
     if (!group) throw new NotFound(res.t('groupNotFound'));
 
     if (group.leaderOnly && group.leaderOnly.challenges && group.leader !== user._id) {
diff --git a/website/src/controllers/api-v3/chat.js b/website/src/controllers/api-v3/chat.js
index 7c2d42bf62..212e064441 100644
--- a/website/src/controllers/api-v3/chat.js
+++ b/website/src/controllers/api-v3/chat.js
@@ -33,7 +33,7 @@ api.getChat = {
     let validationErrors = req.validationErrors();
     if (validationErrors) throw validationErrors;
 
-    let group = await Group.getGroup(user, req.params.groupId, 'chat');
+    let group = await Group.getGroup({user, groupId: req.params.groupId, fields: 'chat'});
     if (!group) throw new NotFound(res.t('groupNotFound'));
 
     res.respond(200, group.chat);
@@ -67,7 +67,7 @@ api.postChat = {
     let validationErrors = req.validationErrors();
     if (validationErrors) throw validationErrors;
 
-    let group = await Group.getGroup(user, groupId);
+    let group = await Group.getGroup({user, groupId});
 
     if (!group) throw new NotFound(res.t('groupNotFound'));
     if (group.type !== 'party' && user.flags.chatRevoked) {
@@ -118,7 +118,7 @@ api.likeChat = {
     let validationErrors = req.validationErrors();
     if (validationErrors) throw validationErrors;
 
-    let group = await Group.getGroup(user, groupId);
+    let group = await Group.getGroup({user, groupId});
     if (!group) throw new NotFound(res.t('groupNotFound'));
 
     let message = _.find(group.chat, {id: req.params.chatId});
@@ -165,7 +165,7 @@ api.flagChat = {
     let validationErrors = req.validationErrors();
     if (validationErrors) throw validationErrors;
 
-    let group = await Group.getGroup(user, groupId);
+    let group = await Group.getGroup({user, groupId});
     if (!group) throw new NotFound(res.t('groupNotFound'));
     let message = _.find(group.chat, {id: req.params.chatId});
 
diff --git a/website/src/controllers/api-v3/groups.js b/website/src/controllers/api-v3/groups.js
index 8203e00b1b..d7d3eedbd3 100644
--- a/website/src/controllers/api-v3/groups.js
+++ b/website/src/controllers/api-v3/groups.js
@@ -93,7 +93,7 @@ api.getGroups = {
     types.forEach(type => {
       switch (type) {
         case 'party':
-          queries.push(Group.getGroup(user, 'party', groupFields));
+          queries.push(Group.getGroup({user, groupId: 'party', fields: groupFields, populateLeader: true}));
           break;
         case 'privateGuilds':
           queries.push(Group.find({
@@ -109,7 +109,7 @@ api.getGroups = {
           }).select(groupFields).sort(sort).exec()); // TODO use lean?
           break;
         case 'tavern':
-          queries.push(Group.getGroup(user, 'habitrpg', groupFields));
+          queries.push(Group.getGroup({user, groupId: 'habitrpg', fields: groupFields, populateLeader: true}));
           break;
       }
     });
@@ -149,7 +149,7 @@ api.getGroup = {
     let validationErrors = req.validationErrors();
     if (validationErrors) throw validationErrors;
 
-    let group = await Group.getGroup(user, req.params.groupId);
+    let group = await Group.getGroup({user, groupId: req.params.groupId, populateLeader: true});
     if (!group) throw new NotFound(res.t('groupNotFound'));
 
     res.respond(200, group);
@@ -178,7 +178,7 @@ api.updateGroup = {
     let validationErrors = req.validationErrors();
     if (validationErrors) throw validationErrors;
 
-    let group = await Group.getGroup(user, req.params.groupId);
+    let group = await Group.getGroup({user, groupId: req.params.groupId});
     if (!group) throw new NotFound(res.t('groupNotFound'));
 
     if (group.leader !== user._id) throw new NotAuthorized(res.t('messageGroupOnlyLeaderCanUpdate'));
@@ -214,7 +214,8 @@ api.joinGroup = {
     let validationErrors = req.validationErrors();
     if (validationErrors) throw validationErrors;
 
-    let group = await Group.getGroup(user, req.params.groupId, '-chat', true); // Do not fetch chat and work even if the user is not yet a member of the group
+     // Do not fetch chat and work even if the user is not yet a member of the group
+    let group = await Group.getGroup({user, groupId: req.params.groupId, fields: '-chat', optionalMembership: true}); // Do not fetch chat and work even if the user is not yet a member of the group
     if (!group) throw new NotFound(res.t('groupNotFound'));
 
     let isUserInvited = false;
@@ -288,7 +289,7 @@ api.leaveGroup = {
     let validationErrors = req.validationErrors();
     if (validationErrors) throw validationErrors;
 
-    let group = await Group.getGroup(user, req.params.groupId, '-chat'); // Do not fetch chat
+    let group = await Group.getGroup({user, groupId: req.params.groupId, fields: '-chat'}); // Do not fetch chat
     if (!group) throw new NotFound(res.t('groupNotFound'));
 
     // During quests, checke wheter user can leave
@@ -344,7 +345,7 @@ api.removeGroupMember = {
     let validationErrors = req.validationErrors();
     if (validationErrors) throw validationErrors;
 
-    let group = await Group.getGroup(user, req.params.groupId, '-chat'); // Do not fetch chat
+    let group = await Group.getGroup({user, groupId: req.params.groupId, fields: '-chat'}); // Do not fetch chat
     if (!group) throw new NotFound(res.t('groupNotFound'));
 
     let uuid = req.query.memberId;
@@ -523,7 +524,7 @@ api.inviteToGroup = {
     let validationErrors = req.validationErrors();
     if (validationErrors) throw validationErrors;
 
-    let group = await Group.getGroup(user, req.params.groupId, '-chat'); // Do not fetch chat TODO other fields too?
+    let group = await Group.getGroup({user, groupId: req.params.groupId, fields: '-chat'}); // Do not fetch chat TODO other fields too?
     if (!group) throw new NotFound(res.t('groupNotFound'));
 
     let uuids = req.body.uuids;
diff --git a/website/src/controllers/api-v3/members.js b/website/src/controllers/api-v3/members.js
index 3f5ca5ac9d..0b5638a95a 100644
--- a/website/src/controllers/api-v3/members.js
+++ b/website/src/controllers/api-v3/members.js
@@ -76,7 +76,7 @@ function _getMembersForItem (type) {
       challenge = await Challenge.findById(challengeId).select('_id type leader').exec();
       if (!challenge || !challenge.hasAccess(user)) throw new NotFound(res.t('groupNotFound'));
     } else {
-      group = await Group.getGroup(user, groupId, '_id type');
+      group = await Group.getGroup({user, groupId, fields: '_id type'});
       if (!group) throw new NotFound(res.t('groupNotFound'));
     }
 
diff --git a/website/src/models/group.js b/website/src/models/group.js
index 94072bfb79..169d977d08 100644
--- a/website/src/models/group.js
+++ b/website/src/models/group.js
@@ -1,5 +1,8 @@
 import mongoose from 'mongoose';
-import { model as User} from './user';
+import {
+  model as User,
+  nameFields,
+} from './user';
 import shared from '../../../common';
 import _  from 'lodash';
 import { model as Challenge} from './challenge';
@@ -40,7 +43,6 @@ export let schema = new Schema({
   balance: {type: Number, default: 0},
   logo: String,
   leaderMessage: String,
-  // challenges: [{type: String, validate: [validator.isUUID, 'Invalid uuid.'], ref: 'Challenge'}], // TODO do we need this? could depend on back-ref instead (Challenge.find({group:GID}))
   quest: {
     key: String,
     active: {type: Boolean, default: false},
@@ -57,6 +59,7 @@ export let schema = new Schema({
     // 'Accept', the quest begins. If a false user waits too long, probably a good sign to prod them or boot them.
     // TODO when booting user, remove from .joined and check again if we can now start the quest
     // TODO as long as quests are party only we can keep it here
+    // TODO are we sure we need this type of default for this to work?
     members: {type: Schema.Types.Mixed, default: () => {
       return {};
     }},
@@ -125,7 +128,8 @@ schema.post('remove', function postRemoveGroup (group) {
   firebase.deleteGroup(group._id);
 });
 
-schema.statics.getGroup = function getGroup (user, groupId, fields, optionalMembership) {
+schema.statics.getGroup = function getGroup (options = {}) {
+  let {user, groupId, fields, optionalMembership = false, populateLeader = false} = options;
   let query;
 
   // When optionalMembership is true it's not required for the user to be a member of the group
@@ -141,7 +145,8 @@ schema.statics.getGroup = function getGroup (user, groupId, fields, optionalMemb
 
   let mQuery = this.findOne(query);
   if (fields) mQuery.select(fields);
-  return mQuery.exec(); // TODO catch errors here?
+  if (populateLeader === true) mQuery.populate('leader', nameFields);
+  return mQuery.exec();
   // TODO purge chat flags info? in tojson?
 };
 
@@ -503,6 +508,7 @@ schema.methods.leave = function leaveGroup (user, keep) {
   });
 };
 
+export const INVITES_LIMIT = 100;
 export let model = mongoose.model('Group', schema);
 
 // initialize tavern if !exists (fresh installs)
@@ -511,12 +517,12 @@ model.count({_id: 'habitrpg'}, (err, ct) => {
   if (ct > 0) return;
 
   new model({ // eslint-disable-line babel/new-cap
-    _id: 'habitrpg', // TODO hmm this will probably break everything
+    _id: 'habitrpg',
     leader: '9', // TODO change this user id
     name: 'HabitRPG',
     type: 'guild',
     privacy: 'public',
-  }).save();
+  }).save({
+    validateBeforeSave: false, // _id = 'habitrpg' would not be valid otherwise
+  }); // TODO catch/log?
 });
-
-export const INVITES_LIMIT = 100;

From f447af19aeef7915adb141e6a5d5a88b1411e3de Mon Sep 17 00:00:00 2001
From: Matteo Pagliazzi <matteopagliazzi@gmail.com>
Date: Sat, 16 Jan 2016 16:18:06 +0100
Subject: [PATCH 11/16] add tests for getting challenge members and fix a lot
 of bugs

---
 common/locales/en/api-v3.json                     |  1 +
 .../GET-challenges_challengeId_members.test       |  0
 website/src/controllers/api-v3/challenges.js      |  9 +++++----
 website/src/controllers/api-v3/members.js         |  6 ++++--
 website/src/models/challenge.js                   | 15 +++++++++++----
 website/src/models/group.js                       | 11 +++++++++++
 6 files changed, 32 insertions(+), 10 deletions(-)
 delete mode 100644 test/api/v3/integration/challenges/GET-challenges_challengeId_members.test

diff --git a/common/locales/en/api-v3.json b/common/locales/en/api-v3.json
index d7239af5c4..2d00b19192 100644
--- a/common/locales/en/api-v3.json
+++ b/common/locales/en/api-v3.json
@@ -37,6 +37,7 @@
   "onlyLeaderCanRemoveMember": "Only group leader can remove a member!",
   "memberCannotRemoveYourself": "You cannot remove yourself!",
   "groupMemberNotFound": "User not found among group's members",
+  "mustBeGroupMember": "Must be member of the group.",
   "keepOrRemoveAll": "req.query.keep must be either \"keep-all\" or \"remove-all\"",
   "keepOrRemove": "req.query.keep must be either \"keep\" or \"remove\"",
   "canOnlyInviteEmailUuid": "Can only invite using uuids or emails.",
diff --git a/test/api/v3/integration/challenges/GET-challenges_challengeId_members.test b/test/api/v3/integration/challenges/GET-challenges_challengeId_members.test
deleted file mode 100644
index e69de29bb2..0000000000
diff --git a/website/src/controllers/api-v3/challenges.js b/website/src/controllers/api-v3/challenges.js
index ed8440ff1f..da7a7e8afe 100644
--- a/website/src/controllers/api-v3/challenges.js
+++ b/website/src/controllers/api-v3/challenges.js
@@ -38,8 +38,9 @@ api.createChallenge = {
     let groupId = req.body.groupId;
     let prize = req.body.prize;
 
-    let group = await Group.getGroup({user, groupId, fields: '-chat'});
+    let group = await Group.getGroup({user, groupId, fields: '-chat', mustBeMember: true});
     if (!group) throw new NotFound(res.t('groupNotFound'));
+    if (!group.isMember(user)) throw new NotAuthorized(res.t('mustBeGroupMember'));
 
     if (group.leaderOnly && group.leaderOnly.challenges && group.leader !== user._id) {
       throw new NotAuthorized(res.t('onlyGroupLeaderChal'));
@@ -150,10 +151,10 @@ api.getChallenge = {
     let challengeId = req.params.challengeId;
 
     let challenge = await Challenge.findById(challengeId).exec();
+    if (!challenge) throw new NotFound(res.t('challengeNotFound'));
 
-    if (!challenge || !challenge.hasAccess(user)) {
-      throw new NotFound(res.t('challengeNotFound'));
-    }
+    let group = await Group.getGroup({user, groupId: challenge.groupId, fields: '_id type privacy'});
+    if (!group || !challenge.canView(user, group)) throw new NotFound(res.t('challengeNotFound'));
 
     res.respond(200, challenge);
   },
diff --git a/website/src/controllers/api-v3/members.js b/website/src/controllers/api-v3/members.js
index 0b5638a95a..5e2cb33d35 100644
--- a/website/src/controllers/api-v3/members.js
+++ b/website/src/controllers/api-v3/members.js
@@ -73,8 +73,10 @@ function _getMembersForItem (type) {
     let group;
 
     if (type === 'challenge-members') {
-      challenge = await Challenge.findById(challengeId).select('_id type leader').exec();
-      if (!challenge || !challenge.hasAccess(user)) throw new NotFound(res.t('groupNotFound'));
+      challenge = await Challenge.findById(challengeId).select('_id type leader groupId').exec();
+      if (!challenge) throw new NotFound(res.t('challengeNotFound'));
+      group = await Group.getGroup({user, groupId: challenge.groupId, fields: '_id type privacy'});
+      if (!group || !challenge.canView(user, group)) throw new NotFound(res.t('challengeNotFound'));
     } else {
       group = await Group.getGroup({user, groupId, fields: '_id type'});
       if (!group) throw new NotFound(res.t('groupNotFound'));
diff --git a/website/src/models/challenge.js b/website/src/models/challenge.js
index fcfb5cc1e3..a19ecd3b60 100644
--- a/website/src/models/challenge.js
+++ b/website/src/models/challenge.js
@@ -20,7 +20,7 @@ let schema = new Schema({
     rewards: [{type: String, ref: 'Task'}],
   },
   leader: {type: String, ref: 'User', validate: [validator.isUUID, 'Invalid uuid.'], required: true},
-  groupId: {type: String, ref: 'Group', validate: [validator.isUUID, 'Invalid uuid.'], required: true},
+  groupId: {type: String, ref: 'Group', validate: [validator.isUUID, 'Invalid uuid.'], required: true}, // TODO no update, no set?
   timestamp: {type: Date, default: Date.now, required: true}, // TODO what is this? use timestamps from plugin? not settable?
   memberCount: {type: Number, default: 0},
   challengeCount: {type: Number, default: 0},
@@ -31,7 +31,7 @@ schema.plugin(baseModel, {
   noSet: ['_id', 'memberCount', 'challengeCount', 'tasksOrder'],
 });
 
-// Return true if user has access to the challenge
+// Returns true if user has access to the challenge (can join)
 schema.methods.hasAccess = function hasAccessToChallenge (user) {
   let userGroups = user.guilds.slice(0);
   if (user.party._id) userGroups.push(user.party._id);
@@ -39,12 +39,19 @@ schema.methods.hasAccess = function hasAccessToChallenge (user) {
   return this.leader === user._id || user.contributor.admin || userGroups.indexOf(this.groupId) !== -1;
 };
 
-// Return true if user is a member of the challenge
+// Returns true if user can view the challenge
+// Different from hasAccess because challenges of public guilds can be viewed by everyone
+schema.methods.canView = function canViewChallenge (user, group) {
+  if (group.type === 'guild' && group.privacy === 'public') return true;
+  return this.hasAccess(user);
+};
+
+// Returns true if user is a member of the challenge
 schema.methods.isMember = function isChallengeMember (user) {
   return user.challenges.indexOf(this._id) !== -1;
 };
 
-// Return true if the user can modify (close, selectWinner, ...) the challenge
+// Returns true if the user can modify (close, selectWinner, ...) the challenge
 schema.methods.canModify = function canModifyChallenge (user) {
   return user.contributor.admin || this.leader === user._id;
 };
diff --git a/website/src/models/group.js b/website/src/models/group.js
index 169d977d08..85396fc693 100644
--- a/website/src/models/group.js
+++ b/website/src/models/group.js
@@ -150,6 +150,17 @@ schema.statics.getGroup = function getGroup (options = {}) {
   // TODO purge chat flags info? in tojson?
 };
 
+// Return true if user is a member of the group
+schema.methods.isMember = function isGroupMember (user) {
+  if (this._id === 'habitrpg') {
+    return true; // everyone is considered part of the tavern
+  } else if (this.type === 'party') {
+    return user.party._id === this._id ? true : false;
+  } else { // guilds
+    return user.guilds.indexOf(this._id) !== -1;
+  }
+};
+
 export function chatDefaults (msg, user) {
   let message = {
     id: shared.uuid(),

From 4e5c4e99531eb102d00ff26f0aec2f968d144c57 Mon Sep 17 00:00:00 2001
From: Matteo Pagliazzi <matteopagliazzi@gmail.com>
Date: Sat, 16 Jan 2016 16:18:29 +0100
Subject: [PATCH 12/16] add missing test file

---
 ...GET-challenges_challengeId_members.test.js | 125 ++++++++++++++++++
 1 file changed, 125 insertions(+)
 create mode 100644 test/api/v3/integration/challenges/GET-challenges_challengeId_members.test.js

diff --git a/test/api/v3/integration/challenges/GET-challenges_challengeId_members.test.js b/test/api/v3/integration/challenges/GET-challenges_challengeId_members.test.js
new file mode 100644
index 0000000000..318181b66e
--- /dev/null
+++ b/test/api/v3/integration/challenges/GET-challenges_challengeId_members.test.js
@@ -0,0 +1,125 @@
+import {
+  generateUser,
+  generateGroup,
+  translate as t,
+} from '../../../../helpers/api-v3-integration.helper';
+import { v4 as generateUUID } from 'uuid';
+
+describe('GET /challenges/:challengeId/members', () => {
+  let user;
+
+  beforeEach(async () => {
+    user = await generateUser();
+  });
+
+  it('validates optional req.query.lastId to be an UUID', async () => {
+    await expect(user.get(`/challenges/${generateUUID()}/members?lastId=invalidUUID`)).to.eventually.be.rejected.and.eql({
+      code: 400,
+      error: 'BadRequest',
+      message: t('invalidReqParams'),
+    });
+  });
+
+  it('fails if challenge doesn\'t exists', async () => {
+    await expect(user.get(`/challenges/${generateUUID()}/members`)).to.eventually.be.rejected.and.eql({
+      code: 404,
+      error: 'NotFound',
+      message: t('challengeNotFound'),
+    });
+  });
+
+  it('fails if user doesn\'t have access to the challenge', async () => {
+    let group = await generateGroup(user, {type: 'party', name: generateUUID()});
+    let challenge = await user.post('/challenges', {
+      name: 'test chal',
+      shortName: 'test-chal',
+      groupId: group._id,
+    });
+    let anotherUser = await generateUser();
+    await expect(anotherUser.get(`/challenges/${challenge._id}/members`)).to.eventually.be.rejected.and.eql({
+      code: 404,
+      error: 'NotFound',
+      message: t('challengeNotFound'),
+    });
+  });
+
+  it('works with challenges belonging to public guild', async () => {
+    let leader = await generateUser({balance: 4});
+    let group = await generateGroup(leader, {type: 'guild', privacy: 'public', name: generateUUID()});
+    let challenge = await leader.post('/challenges', {
+      name: 'test chal',
+      shortName: 'test-chal',
+      groupId: group._id,
+    });
+    let res = await user.get(`/challenges/${challenge._id}/members`);
+    expect(res[0]).to.eql({
+      _id: leader._id,
+      profile: {name: leader.profile.name},
+    });
+    expect(res[0]).to.have.all.keys(['_id', 'profile']);
+    expect(res[0].profile).to.have.all.keys(['name']);
+  });
+
+  it('populates only some fields', async () => {
+    let anotherUser = await generateUser({balance: 3});
+    let group = await generateGroup(anotherUser, {type: 'guild', privacy: 'public', name: generateUUID()});
+    let challenge = await anotherUser.post('/challenges', {
+      name: 'test chal',
+      shortName: 'test-chal',
+      groupId: group._id,
+    });
+    let res = await user.get(`/challenges/${challenge._id}/members`);
+    expect(res[0]).to.eql({
+      _id: anotherUser._id,
+      profile: {name: anotherUser.profile.name},
+    });
+    expect(res[0]).to.have.all.keys(['_id', 'profile']);
+    expect(res[0].profile).to.have.all.keys(['name']);
+  });
+
+  it('returns only first 30 members', async () => {
+    let group = await generateGroup(user, {type: 'party', name: generateUUID()});
+    let challenge = await user.post('/challenges', {
+      name: 'test chal',
+      shortName: 'test-chal',
+      groupId: group._id,
+    });
+
+    let usersToGenerate = [];
+    for (let i = 0; i < 31; i++) {
+      usersToGenerate.push(generateUser({challenges: [challenge._id]}));
+    }
+    await Promise.all(usersToGenerate);
+
+    let res = await user.get(`/challenges/${challenge._id}/members`);
+    expect(res.length).to.equal(30);
+    res.forEach(member => {
+      expect(member).to.have.all.keys(['_id', 'profile']);
+      expect(member.profile).to.have.all.keys(['name']);
+    });
+  });
+
+  it('supports using req.query.lastId to get more members', async () => {
+    let group = await generateGroup(user, {type: 'party', name: generateUUID()});
+    let challenge = await user.post('/challenges', {
+      name: 'test chal',
+      shortName: 'test-chal',
+      groupId: group._id,
+    });
+
+    let usersToGenerate = [];
+    for (let i = 0; i < 57; i++) {
+      usersToGenerate.push(generateUser({challenges: [challenge._id]}));
+    }
+    let generatedUsers = await Promise.all(usersToGenerate); // Group has 59 members (1 is the leader)
+    let expectedIds = [user._id].concat(generatedUsers.map(generatedUser => generatedUser._id));
+
+    let res = await user.get(`/challenges/${challenge._id}/members`);
+    expect(res.length).to.equal(30);
+    let res2 = await user.get(`/challenges/${challenge._id}/members?lastId=${res[res.length - 1]._id}`);
+    expect(res2.length).to.equal(28);
+
+    let resIds = res.concat(res2).map(member => member._id);
+    expect(resIds).to.eql(expectedIds.sort());
+  });
+});

From a59da8607baa095d5659c0df7972d24818142617 Mon Sep 17 00:00:00 2001
From: Matteo Pagliazzi <matteopagliazzi@gmail.com>
Date: Sat, 16 Jan 2016 16:32:46 +0100
Subject: [PATCH 13/16] add tests for getting invites to a group

---
 .../groups/GET-groups_groupId_invites.test.js | 101 ++++++++++++++++++
 1 file changed, 101 insertions(+)

diff --git a/test/api/v3/integration/groups/GET-groups_groupId_invites.test.js b/test/api/v3/integration/groups/GET-groups_groupId_invites.test.js
index e69de29bb2..480808521e 100644
--- a/test/api/v3/integration/groups/GET-groups_groupId_invites.test.js
+++ b/test/api/v3/integration/groups/GET-groups_groupId_invites.test.js
@@ -0,0 +1,101 @@
+import {
+  generateUser,
+  generateGroup,
+  translate as t,
+} from '../../../../helpers/api-v3-integration.helper';
+import { v4 as generateUUID } from 'uuid';
+
+describe('GET /groups/:groupId/invites', () => {
+  let user;
+
+  beforeEach(async () => {
+    user = await generateUser();
+  });
+
+  it('validates optional req.query.lastId to be an UUID', async () => {
+    await expect(user.get(`/groups/groupId/invites?lastId=invalidUUID`)).to.eventually.be.rejected.and.eql({
+      code: 400,
+      error: 'BadRequest',
+      message: t('invalidReqParams'),
+    });
+  });
+
+  it('fails if group doesn\'t exists', async () => {
+    await expect(user.get(`/groups/${generateUUID()}/invites`)).to.eventually.be.rejected.and.eql({
+      code: 404,
+      error: 'NotFound',
+      message: t('groupNotFound'),
+    });
+  });
+
+  it('fails if user doesn\'t have access to the group', async () => {
+    let group = await generateGroup(user, {type: 'party', name: generateUUID()});
+    let anotherUser = await generateUser();
+    await expect(anotherUser.get(`/groups/${group._id}/invites`)).to.eventually.be.rejected.and.eql({
+      code: 404,
+      error: 'NotFound',
+      message: t('groupNotFound'),
+    });
+  });
+
+  it('works when passing party as req.params.groupId', async () => {
+    let group = await generateGroup(user, {type: 'party', name: generateUUID()});
+    let invited = await generateUser();
+    await user.post(`/groups/${group._id}/invite`, {uuids: [invited._id]});
+    let res = await user.get(`/groups/party/invites`);
+
+    expect(res).to.be.an('array');
+    expect(res.length).to.equal(1);
+    expect(res[0]).to.eql({
+      _id: invited._id,
+      profile: {name: invited.profile.name},
+    });
+  });
+
+  it('populates only some fields', async () => {
+    let group = await generateGroup(user, {type: 'party', name: generateUUID()});
+    let invited = await generateUser();
+    await user.post(`/groups/${group._id}/invite`, {uuids: [invited._id]});
+    let res = await user.get(`/groups/party/invites`);
+    expect(res[0]).to.have.all.keys(['_id', 'profile']);
+    expect(res[0].profile).to.have.all.keys(['name']);
+  });
+
+  it('returns only first 30 invites', async () => {
+    let group = await generateGroup(user, {type: 'party', name: generateUUID()});
+    let invitesToGenerate = [];
+    for (let i = 0; i < 31; i++) {
+      let invited = await generateUser();
+      invitesToGenerate.push(invited);
+      await user.post(`/groups/${group._id}/invite`, {uuids: [invited._id]});
+    }
+    await Promise.all(invitesToGenerate);
+    let res = await user.get(`/groups/party/invites`);
+    expect(res.length).to.equal(30);
+    res.forEach(member => {
+      expect(member).to.have.all.keys(['_id', 'profile']);
+      expect(member.profile).to.have.all.keys(['name']);
+    });
+  });
+
+  it('supports using req.query.lastId to get more invites', async () => {
+    let leader = await generateUser({balance: 4});
+    let group = await generateGroup(leader, {type: 'guild', privacy: 'public', name: generateUUID()});
+
+    let invitesToGenerate = [];
+    for (let i = 0; i < 32; i++) {
+      invitesToGenerate.push(await generateUser());
+    }
+    let generatedInvites = await Promise.all(invitesToGenerate); // Group has 32 invites
+    let expectedIds = generatedInvites.map(generatedInvite => generatedInvite._id);
+    await user.post(`/groups/${group._id}/invite`, {uuids: expectedIds});
+
+    let res = await user.get(`/groups/${group._id}/invites`);
+    expect(res.length).to.equal(30);
+    let res2 = await user.get(`/groups/${group._id}/invites?lastId=${res[res.length - 1]._id}`);
+    expect(res2.length).to.equal(2);
+
+    let resIds = res.concat(res2).map(invite => invite._id);
+    expect(resIds).to.eql(expectedIds.sort());
+  });
+});

From f8f591e521260751ace4dd3a327f2483d5b4f569 Mon Sep 17 00:00:00 2001
From: Matteo Pagliazzi <matteopagliazzi@gmail.com>
Date: Sat, 16 Jan 2016 16:37:21 +0100
Subject: [PATCH 14/16] do not use wait inside for loop

---
 .../groups/GET-groups_groupId_invites.test.js          | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/test/api/v3/integration/groups/GET-groups_groupId_invites.test.js b/test/api/v3/integration/groups/GET-groups_groupId_invites.test.js
index 480808521e..d1604a4d84 100644
--- a/test/api/v3/integration/groups/GET-groups_groupId_invites.test.js
+++ b/test/api/v3/integration/groups/GET-groups_groupId_invites.test.js
@@ -65,11 +65,11 @@ describe('GET /groups/:groupId/invites', () => {
     let group = await generateGroup(user, {type: 'party', name: generateUUID()});
     let invitesToGenerate = [];
     for (let i = 0; i < 31; i++) {
-      let invited = await generateUser();
-      invitesToGenerate.push(invited);
-      await user.post(`/groups/${group._id}/invite`, {uuids: [invited._id]});
+      invitesToGenerate.push(generateUser());
     }
-    await Promise.all(invitesToGenerate);
+    let generatedInvites = await Promise.all(invitesToGenerate);
+    await user.post(`/groups/${group._id}/invite`, {uuids: generatedInvites.map(invite => invite._id)});
+
     let res = await user.get(`/groups/party/invites`);
     expect(res.length).to.equal(30);
     res.forEach(member => {
@@ -84,7 +84,7 @@ describe('GET /groups/:groupId/invites', () => {
 
     let invitesToGenerate = [];
     for (let i = 0; i < 32; i++) {
-      invitesToGenerate.push(await generateUser());
+      invitesToGenerate.push(generateUser());
     }
     let generatedInvites = await Promise.all(invitesToGenerate); // Group has 32 invites
     let expectedIds = generatedInvites.map(generatedInvite => generatedInvite._id);

From 8de8ca7e18eb35150ee591935b70d8ae6a92ca76 Mon Sep 17 00:00:00 2001
From: Matteo Pagliazzi <matteopagliazzi@gmail.com>
Date: Sun, 17 Jan 2016 12:01:57 +0100
Subject: [PATCH 15/16] add getChallengeMemberProgress route and misc fixes

---
 common/locales/en/api-v3.json                |  1 +
 website/src/controllers/api-v3/challenges.js | 57 +++++++++++++++++++-
 website/src/models/challenge.js              |  4 +-
 3 files changed, 59 insertions(+), 3 deletions(-)

diff --git a/common/locales/en/api-v3.json b/common/locales/en/api-v3.json
index 2d00b19192..d16b388769 100644
--- a/common/locales/en/api-v3.json
+++ b/common/locales/en/api-v3.json
@@ -37,6 +37,7 @@
   "onlyLeaderCanRemoveMember": "Only group leader can remove a member!",
   "memberCannotRemoveYourself": "You cannot remove yourself!",
   "groupMemberNotFound": "User not found among group's members",
+  "challengeMemberNotFound": "User not found among challenge's members",
   "mustBeGroupMember": "Must be member of the group.",
   "keepOrRemoveAll": "req.query.keep must be either \"keep-all\" or \"remove-all\"",
   "keepOrRemove": "req.query.keep must be either \"keep\" or \"remove\"",
diff --git a/website/src/controllers/api-v3/challenges.js b/website/src/controllers/api-v3/challenges.js
index da7a7e8afe..d0e54667af 100644
--- a/website/src/controllers/api-v3/challenges.js
+++ b/website/src/controllers/api-v3/challenges.js
@@ -2,7 +2,10 @@ import { authWithHeaders } from '../../middlewares/api-v3/auth';
 import cron from '../../middlewares/api-v3/cron';
 import { model as Challenge } from '../../models/challenge';
 import { model as Group } from '../../models/group';
-import { model as User } from '../../models/user';
+import {
+  model as User,
+  nameFields,
+} from '../../models/user';
 import {
   NotFound,
   NotAuthorized,
@@ -135,6 +138,8 @@ api.getChallenges = {
  * @apiName GetChallenge
  * @apiGroup Challenge
  *
+ * @apiParam {UUID} challengeId The challenge _id
+ *
  * @apiSuccess {object} challenge The challenge object
  */
 api.getChallenge = {
@@ -160,6 +165,56 @@ api.getChallenge = {
   },
 };
 
+/**
+ * @api {get} /challenges/:challengeId/members/:memberId Get a challenge member progress
+ * @apiVersion 3.0.0
+ * @apiName GetChallenge
+ * @apiGroup Challenge
+ *
+ * @apiParam {UUID} challengeId The challenge _id
+ * @apiParam {UUID} member The member _id
+ *
+ * @apiSuccess {object} member Return an object with member _id, profile.name and a tasks object with the challenge tasks for the member
+ */
+api.getChallengeMemberProgress = {
+  method: 'GET',
+  url: '/challenges/:challengeId/members/:memberId',
+  middlewares: [authWithHeaders(), cron],
+  async handler (req, res) {
+    req.checkQuery('challengeId', res.t('challengeIdRequired')).notEmpty().isUUID();
+    req.checkQuery('memberId', res.t('memberIdRequired')).notEmpty().isUUID();
+
+    let validationErrors = req.validationErrors();
+    if (validationErrors) throw validationErrors;
+
+    let user = res.locals.user;
+    let challengeId = req.params.challengeId;
+    let memberId = req.params.memberId;
+
+    let member = await User.findById(memberId).select(`${nameFields} challenges`).exec();
+    if (!member) throw new NotFound(res.t('userWithIDNotFound', {userId: memberId}));
+
+    let challenge = await Challenge.findById(challengeId).exec();
+    if (!challenge) throw new NotFound(res.t('challengeNotFound'));
+
+    let group = await Group.getGroup({user, groupId: challenge.groupId, fields: '_id type privacy'});
+    if (!group || !challenge.canView(user, group)) throw new NotFound(res.t('challengeNotFound'));
+    if (!challenge.isMember(member)) throw new NotFound(res.t('challengeMemberNotFound'));
+
+    let chalTasks = Tasks.Task.find({
+      userId: memberId,
+      'challenge.id': challengeId,
+    })
+    .select('-tags') // We don't want to return the tags publicly TODO same for other data?
+    .exec();
+
+    // manually call toJSON with minimize: true so empty paths aren't returned
+    let response = member.toJSON({minimize: true});
+    response.tasks = chalTasks.map(chalTask => chalTask.toJSON({minimize: true}));
+    res.respond(200, response);
+  },
+};
+
 // TODO everything here should be moved to a worker
 // actually even for a worker it's probably just to big and will kill mongo
 function _closeChal (challenge, broken = {}) {
diff --git a/website/src/models/challenge.js b/website/src/models/challenge.js
index a19ecd3b60..112926ef0c 100644
--- a/website/src/models/challenge.js
+++ b/website/src/models/challenge.js
@@ -23,7 +23,6 @@ let schema = new Schema({
   groupId: {type: String, ref: 'Group', validate: [validator.isUUID, 'Invalid uuid.'], required: true}, // TODO no update, no set?
   timestamp: {type: Date, default: Date.now, required: true}, // TODO what is this? use timestamps from plugin? not settable?
   memberCount: {type: Number, default: 0},
-  challengeCount: {type: Number, default: 0},
   prize: {type: Number, default: 0, min: 0}, // TODO no update?
 });
 
@@ -36,12 +35,13 @@ schema.methods.hasAccess = function hasAccessToChallenge (user) {
   let userGroups = user.guilds.slice(0);
   if (user.party._id) userGroups.push(user.party._id);
   userGroups.push('habitrpg'); // tavern challenges
-  return this.leader === user._id || user.contributor.admin || userGroups.indexOf(this.groupId) !== -1;
+  return this.leader === user._id || userGroups.indexOf(this.groupId) !== -1;
 };
 
 // Returns true if user can view the challenge
 // Different from hasAccess because challenges of public guilds can be viewed by everyone
 schema.methods.canView = function canViewChallenge (user, group) {
+  if (user.contributor.admin) return true;
   if (group.type === 'guild' && group.privacy === 'public') return true;
   return this.hasAccess(user);
 };

From ec7ed9c90e0fcad36c59e35c4b54564fd04cafa0 Mon Sep 17 00:00:00 2001
From: Matteo Pagliazzi <matteopagliazzi@gmail.com>
Date: Sun, 17 Jan 2016 18:31:03 +0100
Subject: [PATCH 16/16] add tests for getChallengeMemberProgress route and
 several bug fixes

---
 ...enges_challengeId_members_memberId.test.js | 126 ++++++++++++++++++
 website/src/controllers/api-v3/challenges.js  |  55 +-------
 website/src/controllers/api-v3/members.js     |  52 ++++++++
 website/src/controllers/api-v3/tasks.js       |   6 +-
 website/src/models/challenge.js               |   4 +-
 5 files changed, 184 insertions(+), 59 deletions(-)
 create mode 100644 test/api/v3/integration/challenges/GET-challenges_challengeId_members_memberId.test.js

diff --git a/test/api/v3/integration/challenges/GET-challenges_challengeId_members_memberId.test.js b/test/api/v3/integration/challenges/GET-challenges_challengeId_members_memberId.test.js
new file mode 100644
index 0000000000..e91fef8563
--- /dev/null
+++ b/test/api/v3/integration/challenges/GET-challenges_challengeId_members_memberId.test.js
@@ -0,0 +1,126 @@
+import {
+  generateUser,
+  generateGroup,
+  translate as t,
+} from '../../../../helpers/api-v3-integration.helper';
+import { v4 as generateUUID } from 'uuid';
+
+describe('GET /challenges/:challengeId/members/:memberId', () => {
+  let user;
+
+  beforeEach(async () => {
+    user = await generateUser();
+  });
+
+  it('validates req.params.memberId to be an UUID', async () => {
+    await expect(user.get(`/challenges/invalidUUID/members/${generateUUID()}`)).to.eventually.be.rejected.and.eql({
+      code: 400,
+      error: 'BadRequest',
+      message: t('invalidReqParams'),
+    });
+  });
+
+  it('validates req.params.memberId to be an UUID', async () => {
+    await expect(user.get(`/challenges/${generateUUID()}/members/invalidUUID`)).to.eventually.be.rejected.and.eql({
+      code: 400,
+      error: 'BadRequest',
+      message: t('invalidReqParams'),
+    });
+  });
+
+  it('fails if member doesn\'t exists', async () => {
+    let userId = generateUUID();
+    await expect(user.get(`/challenges/${generateUUID()}/members/${userId}`)).to.eventually.be.rejected.and.eql({
+      code: 404,
+      error: 'NotFound',
+      message: t('userWithIDNotFound', {userId}),
+    });
+  });
+
+  it('fails if challenge doesn\'t exists', async () => {
+    let member = await generateUser();
+    await expect(user.get(`/challenges/${generateUUID()}/members/${member._id}`)).to.eventually.be.rejected.and.eql({
+      code: 404,
+      error: 'NotFound',
+      message: t('challengeNotFound'),
+    });
+  });
+
+  it('fails if user doesn\'t have access to the challenge', async () => {
+    let group = await generateGroup(user, {type: 'party', name: generateUUID()});
+    let challenge = await user.post('/challenges', {
+      name: 'test chal',
+      shortName: 'test-chal',
+      groupId: group._id,
+    });
+    let anotherUser = await generateUser();
+    let member = await generateUser();
+    await expect(anotherUser.get(`/challenges/${challenge._id}/members/${member._id}`)).to.eventually.be.rejected.and.eql({
+      code: 404,
+      error: 'NotFound',
+      message: t('challengeNotFound'),
+    });
+  });
+
+  it('fails if member is not part of the challenge', async () => {
+    let group = await generateGroup(user, {type: 'party', name: generateUUID()});
+    let challenge = await user.post('/challenges', {
+      name: 'test chal',
+      shortName: 'test-chal',
+      groupId: group._id,
+    });
+    let member = await generateUser();
+    await expect(user.get(`/challenges/${challenge._id}/members/${member._id}`)).to.eventually.be.rejected.and.eql({
+      code: 404,
+      error: 'NotFound',
+      message: t('challengeMemberNotFound'),
+    });
+  });
+
+  it('works with challenges belonging to a public guild', async () => {
+    let groupLeader = await generateUser({balance: 4});
+    let group = await generateGroup(groupLeader, {type: 'guild', privacy: 'public', name: generateUUID()});
+    let challenge = await groupLeader.post('/challenges', {
+      name: 'test chal',
+      shortName: 'test-chal',
+      groupId: group._id,
+    });
+    let taskText = 'Test Text';
+    await groupLeader.post(`/tasks/challenge/${challenge._id}`, [{type: 'habit', text: taskText}]);
+
+    let memberProgress = await user.get(`/challenges/${challenge._id}/members/${groupLeader._id}`);
+    expect(memberProgress).to.have.all.keys(['_id', 'profile', 'tasks']);
+    expect(memberProgress.profile).to.have.all.keys(['name']);
+    expect(memberProgress.tasks.length).to.equal(1);
+  });
+
+  it('returns the member tasks for the challenges', async () => {
+    let group = await generateGroup(user, {type: 'party', name: generateUUID()});
+    let challenge = await user.post('/challenges', {
+      name: 'test chal',
+      shortName: 'test-chal',
+      groupId: group._id,
+    });
+    await user.post(`/tasks/challenge/${challenge._id}`, [{type: 'habit', text: 'Test Text'}]);
+
+    let memberProgress = await user.get(`/challenges/${challenge._id}/members/${user._id}`);
+    let chalTasks = await user.get(`/tasks/challenge/${challenge._id}`);
+    expect(memberProgress.tasks.length).to.equal(chalTasks.length);
+    expect(memberProgress.tasks[0].challenge.id).to.equal(challenge._id);
+    expect(memberProgress.tasks[0].challenge.taskId).to.equal(chalTasks[0]._id);
+  });
+
+  it('returns the tasks without the tags', async () => {
+    let group = await generateGroup(user, {type: 'party', name: generateUUID()});
+    let challenge = await user.post('/challenges', {
+      name: 'test chal',
+      shortName: 'test-chal',
+      groupId: group._id,
+    });
+    let taskText = 'Test Text';
+    await user.post(`/tasks/challenge/${challenge._id}`, [{type: 'habit', text: taskText}]);
+
+    let memberProgress = await user.get(`/challenges/${challenge._id}/members/${user._id}`);
+    expect(memberProgress.tasks[0]).not.to.have.key('tags');
+  });
+});
diff --git a/website/src/controllers/api-v3/challenges.js b/website/src/controllers/api-v3/challenges.js
index d0e54667af..879b13b433 100644
--- a/website/src/controllers/api-v3/challenges.js
+++ b/website/src/controllers/api-v3/challenges.js
@@ -2,10 +2,7 @@ import { authWithHeaders } from '../../middlewares/api-v3/auth';
 import cron from '../../middlewares/api-v3/cron';
 import { model as Challenge } from '../../models/challenge';
 import { model as Group } from '../../models/group';
-import {
-  model as User,
-  nameFields,
-} from '../../models/user';
+import { model as User } from '../../models/user';
 import {
   NotFound,
   NotAuthorized,
@@ -165,56 +162,6 @@ api.getChallenge = {
   },
 };
 
-/**
- * @api {get} /challenges/:challengeId/members/:memberId Get a challenge member progress
- * @apiVersion 3.0.0
- * @apiName GetChallenge
- * @apiGroup Challenge
- *
- * @apiParam {UUID} challengeId The challenge _id
- * @apiParam {UUID} member The member _id
- *
- * @apiSuccess {object} member Return an object with member _id, profile.name and a tasks object with the challenge tasks for the member
- */
-api.getChallengeMemberProgress = {
-  method: 'GET',
-  url: '/challenges/:challengeId/members/:memberId',
-  middlewares: [authWithHeaders(), cron],
-  async handler (req, res) {
-    req.checkQuery('challengeId', res.t('challengeIdRequired')).notEmpty().isUUID();
-    req.checkQuery('memberId', res.t('memberIdRequired')).notEmpty().isUUID();
-
-    let validationErrors = req.validationErrors();
-    if (validationErrors) throw validationErrors;
-
-    let user = res.locals.user;
-    let challengeId = req.params.challengeId;
-    let memberId = req.params.memberId;
-
-    let member = await User.findById(memberId).select(`${nameFields} challenges`).exec();
-    if (!member) throw new NotFound(res.t('userWithIDNotFound', {userId: memberId}));
-
-    let challenge = await Challenge.findById(challengeId).exec();
-    if (!challenge) throw new NotFound(res.t('challengeNotFound'));
-
-    let group = await Group.getGroup({user, groupId: challenge.groupId, fields: '_id type privacy'});
-    if (!group || !challenge.canView(user, group)) throw new NotFound(res.t('challengeNotFound'));
-    if (!challenge.isMember(member)) throw new NotFound(res.t('challengeMemberNotFound'));
-
-    let chalTasks = Tasks.Task.find({
-      userId: memberId,
-      'challenge.id': challengeId,
-    })
-    .select('-tags') // We don't want to return the tags publicly TODO same for other data?
-    .exec();
-
-    // manually call toJSON with minimize: true so empty paths aren't returned
-    let response = member.toJSON({minimize: true});
-    response.tasks = chalTasks.map(chalTask => chalTask.toJSON({minimize: true}));
-    res.respond(200, response);
-  },
-};
-
 // TODO everything here should be moved to a worker
 // actually even for a worker it's probably just to big and will kill mongo
 function _closeChal (challenge, broken = {}) {
diff --git a/website/src/controllers/api-v3/members.js b/website/src/controllers/api-v3/members.js
index 5e2cb33d35..62df59b651 100644
--- a/website/src/controllers/api-v3/members.js
+++ b/website/src/controllers/api-v3/members.js
@@ -10,6 +10,7 @@ import { model as Challenge } from '../../models/challenge';
 import {
   NotFound,
 } from '../../libs/api-v3/errors';
+import * as Tasks from '../../models/task';
 
 let api = {};
 
@@ -174,4 +175,55 @@ api.getMembersForChallenge = {
   handler: _getMembersForItem('challenge-members'),
 };
 
+/**
+ * @api {get} /challenges/:challengeId/members/:memberId Get a challenge member progress
+ * @apiVersion 3.0.0
+ * @apiName GetChallenge
+ * @apiGroup Challenge
+ *
+ * @apiParam {UUID} challengeId The challenge _id
+ * @apiParam {UUID} member The member _id
+ *
+ * @apiSuccess {object} member Return an object with member _id, profile.name and a tasks object with the challenge tasks for the member
+ */
+api.getChallengeMemberProgress = {
+  method: 'GET',
+  url: '/challenges/:challengeId/members/:memberId',
+  middlewares: [authWithHeaders(), cron],
+  async handler (req, res) {
+    req.checkParams('challengeId', res.t('challengeIdRequired')).notEmpty().isUUID();
+    req.checkParams('memberId', res.t('memberIdRequired')).notEmpty().isUUID();
+
+    let validationErrors = req.validationErrors();
+    if (validationErrors) throw validationErrors;
+
+    let user = res.locals.user;
+    let challengeId = req.params.challengeId;
+    let memberId = req.params.memberId;
+
+    let member = await User.findById(memberId).select(`${nameFields} challenges`).exec();
+    if (!member) throw new NotFound(res.t('userWithIDNotFound', {userId: memberId}));
+
+    let challenge = await Challenge.findById(challengeId).exec();
+    if (!challenge) throw new NotFound(res.t('challengeNotFound'));
+
+    let group = await Group.getGroup({user, groupId: challenge.groupId, fields: '_id type privacy'});
+    if (!group || !challenge.canView(user, group)) throw new NotFound(res.t('challengeNotFound'));
+    if (!challenge.isMember(member)) throw new NotFound(res.t('challengeMemberNotFound'));
+
+    let chalTasks = await Tasks.Task.find({
+      userId: memberId,
+      'challenge.id': challengeId,
+    })
+    .select('-tags') // We don't want to return the tags publicly TODO same for other data?
+    .exec();
+
+    // manually call toJSON with minimize: true so empty paths aren't returned
+    let response = member.toJSON({minimize: true});
+    delete response.challenges;
+    response.tasks = chalTasks.map(chalTask => chalTask.toJSON({minimize: true}));
+    res.respond(200, response);
+  },
+};
+
 export default api;
diff --git a/website/src/controllers/api-v3/tasks.js b/website/src/controllers/api-v3/tasks.js
index c2498b2a94..e6a420b91a 100644
--- a/website/src/controllers/api-v3/tasks.js
+++ b/website/src/controllers/api-v3/tasks.js
@@ -84,10 +84,10 @@ api.createUserTasks = {
  */
 api.createChallengeTasks = {
   method: 'POST',
-  url: '/tasks/challenge/:challengeId',
+  url: '/tasks/challenge/:challengeId', // TODO should be /tasks/challengeS/:challengeId ? plural?
   middlewares: [authWithHeaders(), cron],
   async handler (req, res) {
-    req.checkQuery('challengeId', res.t('challengeIdRequired')).notEmpty().isUUID();
+    req.checkParams('challengeId', res.t('challengeIdRequired')).notEmpty().isUUID();
 
     let reqValidationErrors = req.validationErrors();
     if (reqValidationErrors) throw reqValidationErrors;
@@ -188,7 +188,7 @@ api.getChallengeTasks = {
   url: '/tasks/challenge/:challengeId',
   middlewares: [authWithHeaders(), cron],
   async handler (req, res) {
-    req.checkQuery('challengeId', res.t('challengeIdRequired')).notEmpty().isUUID();
+    req.checkParams('challengeId', res.t('challengeIdRequired')).notEmpty().isUUID();
     req.checkQuery('type', res.t('invalidTaskType')).optional().isIn(Tasks.tasksTypes);
 
     let validationErrors = req.validationErrors();
diff --git a/website/src/models/challenge.js b/website/src/models/challenge.js
index 112926ef0c..8234d0b7f3 100644
--- a/website/src/models/challenge.js
+++ b/website/src/models/challenge.js
@@ -60,7 +60,7 @@ schema.methods.canModify = function canModifyChallenge (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 what to do with updatedAt?
+  let omitAttrs = ['_id', 'userId', 'challenge', 'history', 'tags', 'completed', 'streak', 'notes']; // TODO what to do with updatedAt?
   if (t.type !== 'reward') omitAttrs.push('value');
   return _.omit(t, omitAttrs);
 }
@@ -168,7 +168,7 @@ schema.methods.addTasks = async function challengeAddTasks (tasks) {
         tasksOrderList.$each.unshift(userTask._id);
       }
 
-      toSave.push(userTask);
+      toSave.push(userTask.save());
     });
 
     // Update the user