v3: misc fixes

common/locales/en/api-v3.json

@@ -77,7 +77,7 @@
   "uuidsMustBeAnArray": "UUIDs invites must be a an Array.",
   "emailsMustBeAnArray": "Email invites must be a an Array.",
   "canOnlyInviteMaxInvites": "You can only invite \"<%= maxInvites %>\" at a time",
-  "cantOnlyUnlinkChalTask": "Only challenges tasks can be unlinked.",
+  "cantOnlyUnlinkChalTask": "Only broken challenges tasks can be unlinked.",
   "onlyCreatorOrAdminCanDeleteChat": "Not authorized to delete this message!",
   "questInviteNotFound": "No quest invitation found.",
   "guildQuestsNotSupported": "Guilds cannot be invited on quests.",

package.json

@@ -24,6 +24,7 @@
     "cookie-session": "^1.2.0",
     "coupon-code": "^0.4.3",
     "csv-stringify": "^1.0.2",
+    "cwait": "^1.0.0",
     "domain-middleware": "~0.1.0",
     "estraverse": "^4.1.1",
     "express": "~4.13.3",

test/helpers/api-integration/requester.js

@@ -32,8 +32,8 @@ function _requestMaker (user, method, additionalSets = {}) {
     return new Promise((resolve, reject) => {
       let url = `http://localhost:${API_TEST_SERVER_PORT}`;
 
-      // do not prefix with api/apiVersion requests to top level routes like dataexport and payments
-      if (route.indexOf('/export') === 0 || route.indexOf('/paypal') === 0 || route.indexOf('/amazon') === 0 || route.indexOf('/stripe') === 0) {
+      // do not prefix with api/apiVersion requests to top level routes like dataexport, payments and emails
+      if (route.indexOf('/email') === 0 || route.indexOf('/export') === 0 || route.indexOf('/paypal') === 0 || route.indexOf('/amazon') === 0 || route.indexOf('/stripe') === 0) {
         url += `${route}`;
       } else {
         url += `/api/${apiVersion}${route}`;

website/server/controllers/api-v3/auth.js

@@ -229,7 +229,6 @@ function _passportFbProfile (accessToken) {
 }
 
 // Called as a callback by Facebook (or other social providers). Internal route
-// TODO move to top-level/auth?
 api.loginSocial = {
   method: 'POST',
   url: '/user/auth/social', // this isn't the most appropriate url but must be the same as v2

website/server/controllers/api-v3/tasks.js

@@ -760,7 +760,6 @@ api.removeTagFromTask = {
   },
 };
 
-// TODO this method needs some limitation, like to check if the challenge is really broken?
 /**
  * @api {post} /api/v3/tasks/unlink/:taskId Unlink a challenge task
  * @apiVersion 3.0.0
@@ -793,6 +792,7 @@ api.unlinkTask = {
 
     if (!task) throw new NotFound(res.t('taskNotFound'));
     if (!task.challenge.id) throw new BadRequest(res.t('cantOnlyUnlinkChalTask'));
+    if (!task.challenge.broken) throw new BadRequest(res.t('cantOnlyUnlinkChalTask'));
 
     if (keep === 'keep') {
       task.challenge = {};

website/server/controllers/top-level/email.js

@@ -7,7 +7,6 @@ import {
 
 let api = {};
 
-// TODO move to top-level controllers?
 /**
  * @api {get} /api/v3/email/unsubscribe Unsubscribe an email or user from email notifications
  * @apiDescription Does not require authentication

website/server/libs/api-v3/cron.js

@@ -246,7 +246,7 @@ export function cron (options = {}) {
   _.merge(progress, {down: 0, up: 0});
   progress.collect = _.transform(progress.collect, (m, v, k) => m[k] = 0);
 
-  // @TODO: Clean PMs - keep 200 for subscribers and 50 for free users. Should also be done while resting in the inn
+  // TODO: Clean PMs - keep 200 for subscribers and 50 for free users. Should also be done while resting in the inn
   // let numberOfPMs = Object.keys(user.inbox.messages).length;
   // if (numberOfPMs > maxPMs) {
   //   _(user.inbox.messages)

website/server/middlewares/api-v3/index.js

@@ -61,7 +61,8 @@ module.exports = function attachMiddlewares (app, server) {
   app.use(cookieSession({
     name: 'connect:sess', // Used to keep backward compatibility with Express 3 cookies
     secret: SESSION_SECRET,
-    httpOnly: false, // TODO this should be true for security, what about https only (secure) ?
+    httpOnly: true, // so cookies are not accessible with browser JS
+    // TODO what about https only (secure) ?
     maxAge: TWO_WEEKS,
   }));
 

website/server/models/challenge.js

@@ -13,6 +13,7 @@ import { removeFromArray } from '../libs/api-v3/collectionManipulators';
 import shared from '../../../common';
 import { sendTxn as txnEmail } from '../libs/api-v3/email';
 import sendPushNotification from '../libs/api-v3/pushNotifications';
+import cwait from 'cwait';
 
 let Schema = mongoose.Schema;
 
@@ -156,41 +157,45 @@ async function _fetchMembersIds (challengeId) {
   return (await User.find({challenges: {$in: [challengeId]}}).select('_id').lean().exec()).map(member => member._id);
 }
 
+async function _addTaskFn (challenge, tasks, memberId) {
+  let updateTasksOrderQ = {$push: {}};
+  let toSave = [];
+
+  tasks.forEach(chalTask => {
+    let userTask = new Tasks[chalTask.type](Tasks.Task.sanitize(_syncableAttrs(chalTask)));
+    userTask.challenge = {taskId: chalTask._id, id: challenge._id};
+    userTask.userId = memberId;
+
+    let tasksOrderList = updateTasksOrderQ.$push[`tasksOrder.${chalTask.type}s`];
+    if (!tasksOrderList) {
+      updateTasksOrderQ.$push[`tasksOrder.${chalTask.type}s`] = {
+        $position: 0, // unshift
+        $each: [userTask._id],
+      };
+    } else {
+      tasksOrderList.$each.unshift(userTask._id);
+    }
+
+    toSave.push(userTask.save({
+      validateBeforeSave: false, // no user data supplied
+    }));
+  });
+
+  // Update the user
+  toSave.unshift(User.update({_id: memberId}, updateTasksOrderQ).exec());
+  return await Bluebird.all(toSave);
+}
+
 // Add a new task to challenge members
 schema.methods.addTasks = async function challengeAddTasks (tasks) {
   let challenge = this;
   let membersIds = await _fetchMembersIds(challenge._id);
 
-  // Sync each user sequentially
-  // TODO are we sure it's the best solution? Use cwait
-  // use bulk ops? http://stackoverflow.com/questions/16726330/mongoose-mongodb-batch-insert
-  for (let memberId of membersIds) {
-    let updateTasksOrderQ = {$push: {}};
-    let toSave = [];
+  let queue = new cwait.TaskQueue(Bluebird, 5); // process only 5 users concurrently
 
-    // TODO eslint complaints about having a function inside a loop -> make sure it works
-    tasks.forEach(chalTask => { // eslint-disable-line no-loop-func
-      let userTask = new Tasks[chalTask.type](Tasks.Task.sanitize(_syncableAttrs(chalTask)));
-      userTask.challenge = {taskId: chalTask._id, id: challenge._id};
-      userTask.userId = memberId;
-
-      let tasksOrderList = updateTasksOrderQ.$push[`tasksOrder.${chalTask.type}s`];
-      if (!tasksOrderList) {
-        updateTasksOrderQ.$push[`tasksOrder.${chalTask.type}s`] = {
-          $position: 0, // unshift
-          $each: [userTask._id],
-        };
-      } else {
-        tasksOrderList.$each.unshift(userTask._id);
-      }
-
-      toSave.push(userTask.save());
-    });
-
-    // Update the user
-    toSave.unshift(User.update({_id: memberId}, updateTasksOrderQ).exec());
-    await Bluebird.all(toSave); // eslint-disable-line babel/no-await-in-loop
-  }
+  await Bluebird.map(membersIds, queue.wrap((memberId) => {
+    return _addTaskFn(challenge, tasks, memberId);
+  }));
 };
 
 // Sync updated task to challenge members