diff --git a/test/api/v3/unit/models/challenge.test.js b/test/api/v3/unit/models/challenge.test.js index 152e7a5111..e3577a0e2a 100644 --- a/test/api/v3/unit/models/challenge.test.js +++ b/test/api/v3/unit/models/challenge.test.js @@ -62,7 +62,7 @@ describe('Challenge Model', () => { each(tasksToTest, (taskValue, taskType) => { context(`${taskType}`, () => { beforeEach(async() => { - task = new Tasks[`${taskType}`](Tasks.Task.sanitizeCreate(taskValue)); + task = new Tasks[`${taskType}`](Tasks.Task.sanitize(taskValue)); task.challenge.id = challenge._id; await task.save(); }); diff --git a/test/api/v3/unit/models/task.test.js b/test/api/v3/unit/models/task.test.js index 5cef0fb10f..8a889a2145 100644 --- a/test/api/v3/unit/models/task.test.js +++ b/test/api/v3/unit/models/task.test.js @@ -54,7 +54,7 @@ describe('Task Model', () => { each(tasksToTest, (taskValue, taskType) => { context(`${taskType}`, () => { beforeEach(async() => { - task = new Tasks[`${taskType}`](Tasks.Task.sanitizeCreate(taskValue)); + task = new Tasks[`${taskType}`](Tasks.Task.sanitize(taskValue)); task.challenge.id = challenge._id; task.history = generateHistory(396); await task.save(); diff --git a/website/src/libs/api-v3/analyticsService.js b/website/src/libs/api-v3/analyticsService.js index ae3ec9f507..4979327b48 100644 --- a/website/src/libs/api-v3/analyticsService.js +++ b/website/src/libs/api-v3/analyticsService.js @@ -210,7 +210,6 @@ let _sendPurchaseDataToGoogle = (data) => { }); }; -// TODO log errors... function track (eventType, data) { return Q.all([ _sendDataToAmplitude(eventType, data), diff --git a/website/src/libs/api-v3/encryption.js b/website/src/libs/api-v3/encryption.js index 0f5f9d83dd..390c59234e 100644 --- a/website/src/libs/api-v3/encryption.js +++ b/website/src/libs/api-v3/encryption.js @@ -4,7 +4,6 @@ import { } from 'crypto'; import nconf from 'nconf'; -// TODO check this is secure const algorithm = 'aes-256-ctr'; const SESSION_SECRET = nconf.get('SESSION_SECRET'); @@ -22,4 +21,4 @@ export function decrypt (text) { dec += decipher.final('utf8'); return dec; -} \ No newline at end of file +} diff --git a/website/src/libs/api-v3/i18n.js b/website/src/libs/api-v3/i18n.js index acb9751082..0486cce3f6 100644 --- a/website/src/libs/api-v3/i18n.js +++ b/website/src/libs/api-v3/i18n.js @@ -104,15 +104,3 @@ export let multipleVersionsLanguages = { 'zh-tw': 'zh_TW', }, }; - -// Export en strings only, temporary solution for mobile -// This is copied from middlewares/locals#t() -// TODO review if this can be removed since the old mobile app is no longer active -// stringName and vars are the allowed parameters -export function enTranslations (...args) { - let language = _.find(availableLanguages, {code: 'en'}); - - // language.momentLang = ((!isStaticPage && i18n.momentLangs[language.code]) || undefined); - args.push(language.code); - return shared.i18n.t(...args); -} diff --git a/website/src/libs/api-v3/logger.js b/website/src/libs/api-v3/logger.js index 993ee44e9d..82df8b4827 100644 --- a/website/src/libs/api-v3/logger.js +++ b/website/src/libs/api-v3/logger.js @@ -20,4 +20,15 @@ if (IS_PROD) { }); } +// Logs unhandled promises errors +// when no catch is attached to a promise a unhandledRejection event will be triggered +process.on('unhandledRejection', function handlePromiseRejection (reason, promise) { + let stack = reason.stack || reason.message || reason; + + logger.error(stack, { + promise, + fullError: reason, + }); +}); + module.exports = logger; diff --git a/website/src/libs/api-v3/pushNotifications.js b/website/src/libs/api-v3/pushNotifications.js index edd10c3d2f..426c86f5c3 100644 --- a/website/src/libs/api-v3/pushNotifications.js +++ b/website/src/libs/api-v3/pushNotifications.js @@ -9,7 +9,8 @@ let gcm = GCM_API_KEY ? pushNotify.gcm({ retries: 3, }) : undefined; -// TODO log +// TODO review and test this file when push notifications are added back + if (gcm) { gcm.on('transmitted', (/* result, message, registrationId */) => { // console.info("transmitted", result, message, registrationId); @@ -24,7 +25,6 @@ if (gcm) { }); } -// TODO test module.exports = function sendNotification (user, title, message, timeToLive = 15) { // TODO need investigation: // https://github.com/HabitRPG/habitrpg/issues/5252 @@ -50,7 +50,6 @@ module.exports = function sendNotification (user, title, message, timeToLive = 1 break; case 'ios': - // TODO implement break; } }); diff --git a/website/src/libs/api-v3/setupPassport.js b/website/src/libs/api-v3/setupPassport.js index ea5c55a4c2..dd9fdeaaa2 100644 --- a/website/src/libs/api-v3/setupPassport.js +++ b/website/src/libs/api-v3/setupPassport.js @@ -14,7 +14,7 @@ const FacebookStrategy = passportFacebook.Strategy; passport.serializeUser((user, done) => done(null, user)); passport.deserializeUser((obj, done) => done(null, obj)); -// TODO +// TODO remove? // This auth strategy is no longer used. It's just kept around for auth.js#loginFacebook() (passport._strategies.facebook.userProfile) // The proper fix would be to move to a general OAuth module simply to verify accessTokens passport.use(new FacebookStrategy({ diff --git a/website/src/middlewares/api-v3/auth.js b/website/src/middlewares/api-v3/auth.js index ab948626cb..7f0fab0e07 100644 --- a/website/src/middlewares/api-v3/auth.js +++ b/website/src/middlewares/api-v3/auth.js @@ -40,7 +40,6 @@ export function authWithHeaders (optional = false) { } // Authenticate a request through a valid session -// TODO should use json web token export function authWithSession (req, res, next) { let userId = req.session.userId; diff --git a/website/src/middlewares/api-v3/domain.js b/website/src/middlewares/api-v3/domain.js index fe9d6cd462..fb5e28d352 100644 --- a/website/src/middlewares/api-v3/domain.js +++ b/website/src/middlewares/api-v3/domain.js @@ -1,6 +1,3 @@ -// TODO in api-v2 this module also checked memory usage every x minutes and -// threw an error in case of low memory avalible (possible memory leak) -// it's yet to be decided whether to keep it or not import domainMiddleware from 'domain-middleware'; module.exports = function implementDomainMiddleware (server, mongoose) { diff --git a/website/src/middlewares/api-v3/errorHandler.js b/website/src/middlewares/api-v3/errorHandler.js index 401da18627..f3f7aa5401 100644 --- a/website/src/middlewares/api-v3/errorHandler.js +++ b/website/src/middlewares/api-v3/errorHandler.js @@ -32,8 +32,6 @@ module.exports = function errorHandler (err, req, res, next) { // eslint-disable responseErr.message = err.message; } - // TODO make mongoose and express-validator errors more recognizable - // Handle errors by express-validator if (Array.isArray(err) && err[0].param && err[0].msg) { responseErr = new BadRequest(res.t('invalidReqParams')); diff --git a/website/src/middlewares/api-v3/index.js b/website/src/middlewares/api-v3/index.js index 7fd6c7a259..bc764997b4 100644 --- a/website/src/middlewares/api-v3/index.js +++ b/website/src/middlewares/api-v3/index.js @@ -49,7 +49,7 @@ module.exports = function attachMiddlewares (app, server) { extended: true, // Uses 'qs' library as old connect middleware })); app.use(bodyParser.json()); - app.use(methodOverride()); // TODO still needed in 2016? + app.use(methodOverride()); app.use(cookieSession({ name: 'connect:sess', // Used to keep backward compatibility with Express 3 cookies diff --git a/website/src/middlewares/api-v3/locals.js b/website/src/middlewares/api-v3/locals.js index 6630902b75..e6e06e3b16 100644 --- a/website/src/middlewares/api-v3/locals.js +++ b/website/src/middlewares/api-v3/locals.js @@ -12,7 +12,6 @@ import { mods } from '../../models/user'; // To avoid stringifying more data then we need, // items from `env` used on the client will have to be specified in this array -// TODO where is this used? const CLIENT_VARS = ['language', 'isStaticPage', 'availableLanguages', 'translations', 'FACEBOOK_KEY', 'NODE_ENV', 'BASE_URL', 'GA_ID', 'AMAZON_PAYMENTS', 'STRIPE_PUB_KEY', 'AMPLITUDE_KEY', diff --git a/website/src/models/challenge.js b/website/src/models/challenge.js index f19b6da9a7..4e6826c4f7 100644 --- a/website/src/models/challenge.js +++ b/website/src/models/challenge.js @@ -114,7 +114,7 @@ schema.methods.syncToUser = async function syncChallengeToUser (user) { let matchingTask = _.find(userTasks, userTask => userTask.challenge.taskId === chalTask._id); if (!matchingTask) { // If the task is new, create it - matchingTask = new Tasks[chalTask.type](Tasks.Task.sanitizeCreate(_syncableAttrs(chalTask))); + matchingTask = new Tasks[chalTask.type](Tasks.Task.sanitize(_syncableAttrs(chalTask))); matchingTask.challenge = {taskId: chalTask._id, id: challenge._id}; matchingTask.userId = user._id; user.tasksOrder[`${chalTask.type}s`].push(matchingTask._id); @@ -152,13 +152,14 @@ schema.methods.addTasks = async function challengeAddTasks (tasks) { let membersIds = await _fetchMembersIds(challenge._id); // Sync each user sequentially + // TODO are we sure it's the best solution? for (let memberId of membersIds) { let updateTasksOrderQ = {$push: {}}; let toSave = []; - // TODO eslint complaints about ahving a function inside a loop -> make sure it works + // 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.sanitizeCreate(_syncableAttrs(chalTask))); + let userTask = new Tasks[chalTask.type](Tasks.Task.sanitize(_syncableAttrs(chalTask))); userTask.challenge = {taskId: chalTask._id, id: challenge._id}; userTask.userId = memberId; @@ -192,7 +193,6 @@ schema.methods.updateTask = async function challengeUpdateTask (task) { updateCmd.$set[key] = syncableAttrs[key]; } - // TODO reveiw // Updating instead of loading and saving for performances, risks becoming a problem if we introduce more complexity in tasks await Tasks.Task.update({ userId: {$exists: true}, diff --git a/website/src/models/group.js b/website/src/models/group.js index 99a12ce908..4afc114527 100644 --- a/website/src/models/group.js +++ b/website/src/models/group.js @@ -212,13 +212,13 @@ schema.methods.removeGroupInvitations = async function removeGroupInvitations () let group = this; let usersToRemoveInvitationsFrom = await User.find({ - // TODO id -> _id ? [`invitations.${group.type}${group.type === 'guild' ? 's' : ''}.id`]: group._id, }).exec(); let userUpdates = usersToRemoveInvitationsFrom.map(user => { if (group.type === 'party') { - user.invitations.party = {}; // TODO mark modified + user.invitations.party = {}; + this.markModified('invitations.party'); } else { removeFromArray(user.invitations.guilds, { id: group._id }); } @@ -395,7 +395,6 @@ function _cleanQuestProgress (merge) { return clean; } -// TODO move to User.cleanQuestProgress? schema.statics.cleanQuestProgress = _cleanQuestProgress; // returns a clean object for group.quest @@ -619,7 +618,6 @@ schema.statics.tavernBoss = async function tavernBoss (user, progress) { _.assign(tavernQuest, tavern.quest.toObject()); return tavern.save(); } - // TODO catch }; schema.methods.leave = async function leaveGroup (user, keep = 'keep-all') { diff --git a/website/src/models/tag.js b/website/src/models/tag.js index 78c1b7884e..dadc84efc2 100644 --- a/website/src/models/tag.js +++ b/website/src/models/tag.js @@ -5,7 +5,7 @@ let Schema = mongoose.Schema; export let schema = new Schema({ name: {type: String, required: true}, - challenge: {type: String}, // TODO validate + challenge: {type: String}, }, { minimize: true, // So empty objects are returned strict: true, diff --git a/website/src/models/task.js b/website/src/models/task.js index 79e617a77d..100f8ebe3b 100644 --- a/website/src/models/task.js +++ b/website/src/models/task.js @@ -57,7 +57,7 @@ export let TaskSchema = new Schema({ TaskSchema.plugin(baseModel, { noSet: ['challenge', 'userId', 'completed', 'history', 'streak', 'dateCompleted', 'completed'], sanitizeTransform (taskObj) { - if (taskObj.type !== 'reward') { // value should be settable directly only for rewards + if (taskObj.type && taskObj.type !== 'reward') { // value should be settable directly only for rewards delete taskObj.value; } diff --git a/website/src/models/user.js b/website/src/models/user.js index cd420fd5b7..0086167f2e 100644 --- a/website/src/models/user.js +++ b/website/src/models/user.js @@ -49,7 +49,6 @@ export let schema = new Schema({ // We want to know *every* time an object updates. Mongoose uses __v to designate when an object contains arrays which // have been updated (http://goo.gl/gQLz41), but we want *every* update _v: { type: Number, default: 0 }, - // TODO give all this a default of 0? achievements: { originalUser: Boolean, habitSurveys: Number, @@ -99,8 +98,11 @@ export let schema = new Schema({ contributor: { // 1-9, see https://trello.com/c/wkFzONhE/277-contributor-gear https://github.com/HabitRPG/habitrpg/issues/3801 - // TODO validate - level: Number, + level: { + type: Number, + min: 0, + max: 9, + }, admin: Boolean, sudo: Boolean, // Artisan, Friend, Blacksmith, etc @@ -119,7 +121,6 @@ export let schema = new Schema({ purchased: { ads: {type: Boolean, default: false}, // eg, {skeleton: true, pumpkin: true, eb052b: true} - // TODO dictionary skin: {type: Schema.Types.Mixed, default: () => { return {}; }}, @@ -421,7 +422,7 @@ export let schema = new Schema({ displayInviteToPartyWhenPartyIs1: {type: Boolean, default: true}, webhooks: {type: Schema.Types.Mixed, default: () => { return {}; - }}, // TODO array? and proper controller... unless VersionError becomes problematic + }}, // For the following fields make sure to use strict comparison when searching for falsey values (=== false) // As users who didn't login after these were introduced may have them undefined/null emailNotifications: { @@ -541,7 +542,6 @@ 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`; @@ -823,6 +823,4 @@ mongoose.model('User') .then((foundMods) => { // Using push to maintain the reference to mods mods.push(...foundMods); - }, (err) => { // TODO replace with .catch which for some reason was throwing an error - throw err; // TODO ? - }); + }); // In case of failure we don't want this to crash the whole server