From 0be5d1da9c5a30af09dfdfb6a1ae3f8a07ac918a Mon Sep 17 00:00:00 2001 From: Matteo Pagliazzi Date: Sun, 15 May 2016 15:10:41 +0200 Subject: [PATCH] v3: misc fixes --- common/locales/en/api-v3.json | 2 +- package.json | 1 + test/helpers/api-integration/requester.js | 4 +- website/server/controllers/api-v3/auth.js | 1 - website/server/controllers/api-v3/tasks.js | 2 +- .../{api-v3 => top-level}/email.js | 1 - website/server/libs/api-v3/cron.js | 2 +- website/server/middlewares/api-v3/index.js | 3 +- website/server/models/challenge.js | 63 ++++++++++--------- 9 files changed, 42 insertions(+), 37 deletions(-) rename website/server/controllers/{api-v3 => top-level}/email.js (97%) diff --git a/common/locales/en/api-v3.json b/common/locales/en/api-v3.json index 7d887f7b7c..1008f699c3 100644 --- a/common/locales/en/api-v3.json +++ b/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.", diff --git a/package.json b/package.json index 5edeadd75f..b4f5a11d25 100644 --- a/package.json +++ b/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", diff --git a/test/helpers/api-integration/requester.js b/test/helpers/api-integration/requester.js index ee3adf243f..93ad503e38 100644 --- a/test/helpers/api-integration/requester.js +++ b/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}`; diff --git a/website/server/controllers/api-v3/auth.js b/website/server/controllers/api-v3/auth.js index f501ba8c50..7295cce9b7 100644 --- a/website/server/controllers/api-v3/auth.js +++ b/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 diff --git a/website/server/controllers/api-v3/tasks.js b/website/server/controllers/api-v3/tasks.js index 3b118bfe3c..a6a25fd7ff 100644 --- a/website/server/controllers/api-v3/tasks.js +++ b/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 = {}; diff --git a/website/server/controllers/api-v3/email.js b/website/server/controllers/top-level/email.js similarity index 97% rename from website/server/controllers/api-v3/email.js rename to website/server/controllers/top-level/email.js index 675518648e..b84f54c6e1 100644 --- a/website/server/controllers/api-v3/email.js +++ b/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 diff --git a/website/server/libs/api-v3/cron.js b/website/server/libs/api-v3/cron.js index e4a96ee07f..b4b16d5c6e 100644 --- a/website/server/libs/api-v3/cron.js +++ b/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) diff --git a/website/server/middlewares/api-v3/index.js b/website/server/middlewares/api-v3/index.js index 6ccac2f844..9e3d37233e 100644 --- a/website/server/middlewares/api-v3/index.js +++ b/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, })); diff --git a/website/server/models/challenge.js b/website/server/models/challenge.js index cd99fda995..b12cea1dd8 100644 --- a/website/server/models/challenge.js +++ b/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