From f7900b474befec71a2c1b967ed2f24e4af4adfd8 Mon Sep 17 00:00:00 2001 From: Hugh O'Brien Date: Tue, 14 Sep 2021 11:43:57 +0100 Subject: [PATCH] Merge pull request #4799 from overleaf/hb-eslint-rules Re-enable some eslint rules GitOrigin-RevId: 16153adb839bb61784bb40fbc8e43da281fe090d --- services/web/.eslintrc | 10 ++--- .../Notifications/NotificationsController.js | 5 ++- .../src/Features/Project/ProjectController.js | 4 ++ .../Project/ProjectCreationHandler.js | 10 +++-- .../Features/Security/OneTimeTokenHandler.js | 6 --- .../Features/StaticPages/HomeController.js | 7 +-- .../Subscription/LimitationsManager.js | 21 --------- .../ThirdPartyDataStore/TpdsUpdateHandler.js | 3 ++ services/web/app/src/Features/V1/V1Handler.js | 8 ---- .../web/app/src/infrastructure/Modules.js | 2 +- .../file-view/components/file-view-header.js | 2 + .../preview/components/preview-error.js | 1 + .../components/preview-logs-pane-entry.js | 2 +- .../HumanReadableLogsRules.js | 2 +- .../UserAffiliationsReconfirmController.js | 2 +- services/web/frontend/js/main/learn.js | 4 ++ services/web/frontend/stories/icon.stories.js | 2 +- services/web/scripts/create_project.js | 10 +++-- services/web/scripts/translations/download.js | 3 +- .../scripts/translations/transformLocales.js | 3 +- services/web/scripts/translations/upload.js | 5 ++- .../src/ProjectDuplicateNameTests.js | 43 ++++++++++++++++--- .../test/acceptance/src/RegistrationTests.js | 27 ++++++------ .../Subscription/LimitationsManagerTests.js | 23 ++++++++++ .../Subscription/UserFeaturesUpdaterTests.js | 3 ++ .../src/infrastructure/GeoIpLookupTest.js | 10 +++++ 26 files changed, 136 insertions(+), 82 deletions(-) diff --git a/services/web/.eslintrc b/services/web/.eslintrc index e577f1067b..a25b965d36 100644 --- a/services/web/.eslintrc +++ b/services/web/.eslintrc @@ -47,9 +47,7 @@ "import/no-extraneous-dependencies": "error", "node/no-callback-literal": "off", - "node/no-deprecated-api": "off", - "node/handle-callback-err": "off", - "node/no-path-concat": "off" + "node/no-deprecated-api": "off" }, "overrides": [ // NOTE: changing paths may require updating them in the Makefile too. @@ -118,6 +116,9 @@ "ExposedSettings": true }, "rules": { + "react/jsx-no-target-blank": ["error", { + "allowReferrer": true + }], // Prevent usage of legacy string refs "react/no-string-refs": "error", @@ -127,9 +128,6 @@ "children": "never" }], - // Allow target="_blank" in JSX - "react/jsx-no-target-blank": "off", - // Don't import React for JSX; the JSX runtime is added by a Babel plugin "react/react-in-jsx-scope": "off", "react/jsx-uses-react": "off", diff --git a/services/web/app/src/Features/Notifications/NotificationsController.js b/services/web/app/src/Features/Notifications/NotificationsController.js index 50c43c150e..7c71c037be 100644 --- a/services/web/app/src/Features/Notifications/NotificationsController.js +++ b/services/web/app/src/Features/Notifications/NotificationsController.js @@ -3,11 +3,14 @@ const SessionManager = require('../Authentication/SessionManager') const _ = require('underscore') module.exports = { - getAllUnreadNotifications(req, res) { + getAllUnreadNotifications(req, res, next) { const userId = SessionManager.getLoggedInUserId(req.session) NotificationsHandler.getUserNotifications( userId, function (err, unreadNotifications) { + if (err) { + return next(err) + } unreadNotifications = _.map( unreadNotifications, function (notification) { diff --git a/services/web/app/src/Features/Project/ProjectController.js b/services/web/app/src/Features/Project/ProjectController.js index 93a08b9701..2c09f8e642 100644 --- a/services/web/app/src/Features/Project/ProjectController.js +++ b/services/web/app/src/Features/Project/ProjectController.js @@ -428,6 +428,10 @@ const ProjectController = { 'allInReconfirmNotificationPeriodsForUser', fullEmails, (error, results) => { + if (error != null) { + return cb(error) + } + // Module.hooks.fire accepts multiple methods // and does async.series const allInReconfirmNotificationPeriods = diff --git a/services/web/app/src/Features/Project/ProjectCreationHandler.js b/services/web/app/src/Features/Project/ProjectCreationHandler.js index 8b28d09923..af5823d388 100644 --- a/services/web/app/src/Features/Project/ProjectCreationHandler.js +++ b/services/web/app/src/Features/Project/ProjectCreationHandler.js @@ -103,8 +103,9 @@ async function _addExampleProjectFiles(ownerId, projectName, project) { ownerId ) - const frogPath = path.resolve( - __dirname + '/../../../templates/project_files/example-project/frog.jpg' + const frogPath = path.join( + __dirname, + '/../../../templates/project_files/example-project/frog.jpg' ) await ProjectEntityUpdateHandler.promises.addFile( project._id, @@ -175,8 +176,9 @@ async function _createRootDoc(project, ownerId, docLines) { async function _buildTemplate(templateName, userId, projectName) { const user = await User.findById(userId, 'first_name last_name') - const templatePath = path.resolve( - __dirname + `/../../../templates/project_files/${templateName}` + const templatePath = path.join( + __dirname, + `/../../../templates/project_files/${templateName}` ) const template = fs.readFileSync(templatePath) const data = { diff --git a/services/web/app/src/Features/Security/OneTimeTokenHandler.js b/services/web/app/src/Features/Security/OneTimeTokenHandler.js index 44684edf89..2767fe9ccd 100644 --- a/services/web/app/src/Features/Security/OneTimeTokenHandler.js +++ b/services/web/app/src/Features/Security/OneTimeTokenHandler.js @@ -11,9 +11,6 @@ const OneTimeTokenHandler = { if (!options) { options = {} } - if (!callback) { - callback = function (error, data) {} - } if (typeof options === 'function') { callback = options options = {} @@ -40,9 +37,6 @@ const OneTimeTokenHandler = { }, getValueFromTokenAndExpire(use, token, callback) { - if (!callback) { - callback = function (error, data) {} - } const now = new Date() db.tokens.findOneAndUpdate( { diff --git a/services/web/app/src/Features/StaticPages/HomeController.js b/services/web/app/src/Features/StaticPages/HomeController.js index afc7d9638d..1db71bc746 100644 --- a/services/web/app/src/Features/StaticPages/HomeController.js +++ b/services/web/app/src/Features/StaticPages/HomeController.js @@ -1,7 +1,6 @@ /* eslint-disable node/handle-callback-err, max-len, - no-path-concat, no-unused-vars, node/no-deprecated-api, */ @@ -23,7 +22,7 @@ const ErrorController = require('../Errors/ErrorController') const SessionManager = require('../Authentication/SessionManager') const homepageExists = fs.existsSync( - Path.resolve(__dirname + '/../../../views/external/home/v2.pug') + Path.join(__dirname, '/../../../views/external/home/v2.pug') ) module.exports = HomeController = { @@ -52,9 +51,7 @@ module.exports = HomeController = { if (next == null) { next = function (error) {} } - const path = Path.resolve( - __dirname + `/../../../views/external/${page}.pug` - ) + const path = Path.join(__dirname, `/../../../views/external/${page}.pug`) return fs.exists(path, function (exists) { // No error in this callback - old method in Node.js! if (exists) { diff --git a/services/web/app/src/Features/Subscription/LimitationsManager.js b/services/web/app/src/Features/Subscription/LimitationsManager.js index 6d91798bc3..1d4ee7959c 100644 --- a/services/web/app/src/Features/Subscription/LimitationsManager.js +++ b/services/web/app/src/Features/Subscription/LimitationsManager.js @@ -38,9 +38,6 @@ const LimitationsManager = { }, canAddXCollaborators(projectId, numberOfNewCollaborators, callback) { - if (!callback) { - callback = function (error, allowed) {} - } this.allowedNumberOfCollaboratorsInProject( projectId, (error, allowedNumber) => { @@ -77,9 +74,6 @@ const LimitationsManager = { }, hasPaidSubscription(user, callback) { - if (!callback) { - callback = function (err, hasSubscriptionOrIsMember) {} - } this.userHasV2Subscription(user, (err, hasSubscription, subscription) => { if (err) { return callback(err) @@ -112,9 +106,6 @@ const LimitationsManager = { }, userHasV2Subscription(user, callback) { - if (!callback) { - callback = function (err, hasSubscription, subscription) {} - } SubscriptionLocator.getUsersSubscription( user._id, function (err, subscription) { @@ -136,9 +127,6 @@ const LimitationsManager = { }, userHasV1OrV2Subscription(user, callback) { - if (!callback) { - callback = function (err, hasSubscription) {} - } this.userHasV2Subscription(user, (err, hasV2Subscription) => { if (err) { return callback(err) @@ -159,9 +147,6 @@ const LimitationsManager = { }, userIsMemberOfGroupSubscription(user, callback) { - if (!callback) { - callback = function (error, isMember, subscriptions) {} - } SubscriptionLocator.getMemberSubscriptions( user._id, function (err, subscriptions) { @@ -177,9 +162,6 @@ const LimitationsManager = { }, userHasV1Subscription(user, callback) { - if (!callback) { - callback = function (error, hasV1Subscription) {} - } V1SubscriptionManager.getSubscriptionsFromV1( user._id, function (err, v1Subscription) { @@ -201,9 +183,6 @@ const LimitationsManager = { }, hasGroupMembersLimitReached(subscriptionId, callback) { - if (!callback) { - callback = function (err, limitReached, subscription) {} - } SubscriptionLocator.getSubscription( subscriptionId, function (err, subscription) { diff --git a/services/web/app/src/Features/ThirdPartyDataStore/TpdsUpdateHandler.js b/services/web/app/src/Features/ThirdPartyDataStore/TpdsUpdateHandler.js index 7cf8f7a29d..ab48b297a2 100644 --- a/services/web/app/src/Features/ThirdPartyDataStore/TpdsUpdateHandler.js +++ b/services/web/app/src/Features/ThirdPartyDataStore/TpdsUpdateHandler.js @@ -33,6 +33,9 @@ function newUpdate(userId, projectName, path, updateRequest, source, callback) { ) } FileTypeManager.shouldIgnore(path, (err, shouldIgnore) => { + if (err) { + return callback(err) + } if (shouldIgnore) { return callback() } diff --git a/services/web/app/src/Features/V1/V1Handler.js b/services/web/app/src/Features/V1/V1Handler.js index e5e33f51dd..beeb40bdec 100644 --- a/services/web/app/src/Features/V1/V1Handler.js +++ b/services/web/app/src/Features/V1/V1Handler.js @@ -1,6 +1,5 @@ /* eslint-disable camelcase, - node/handle-callback-err, max-len, no-unused-vars, */ @@ -21,9 +20,6 @@ const logger = require('logger-sharelatex') module.exports = V1Handler = { authWithV1(email, password, callback) { - if (callback == null) { - callback = function (err, isValid, v1Profile) {} - } return V1Api.request( { method: 'POST', @@ -64,10 +60,6 @@ module.exports = V1Handler = { }, doPasswordReset(v1_user_id, password, callback) { - if (callback == null) { - callback = function (err, created) {} - } - return V1Api.request( { method: 'POST', diff --git a/services/web/app/src/infrastructure/Modules.js b/services/web/app/src/infrastructure/Modules.js index a010fd39f9..0c3ad5a5a4 100644 --- a/services/web/app/src/infrastructure/Modules.js +++ b/services/web/app/src/infrastructure/Modules.js @@ -5,7 +5,7 @@ const async = require('async') const { promisify } = require('util') const Settings = require('@overleaf/settings') -const MODULE_BASE_PATH = Path.resolve(__dirname + '/../../../modules') +const MODULE_BASE_PATH = Path.join(__dirname, '/../../../modules') const _modules = [] const _hooks = {} diff --git a/services/web/frontend/js/features/file-view/components/file-view-header.js b/services/web/frontend/js/features/file-view/components/file-view-header.js index 5187439d38..78430d1794 100644 --- a/services/web/frontend/js/features/file-view/components/file-view-header.js +++ b/services/web/frontend/js/features/file-view/components/file-view-header.js @@ -213,6 +213,7 @@ function ProjectFilePathProvider({ file }) { , ] } @@ -252,6 +253,7 @@ function ProjectOutputFileProvider({ file }) { , ] } diff --git a/services/web/frontend/js/features/preview/components/preview-error.js b/services/web/frontend/js/features/preview/components/preview-error.js index 3536554e61..c0932328fe 100644 --- a/services/web/frontend/js/features/preview/components/preview-error.js +++ b/services/web/frontend/js/features/preview/components/preview-error.js @@ -47,6 +47,7 @@ function PreviewError({ name }) { {t('learn_how_to_make_documents_compile_quickly')} diff --git a/services/web/frontend/js/features/preview/components/preview-logs-pane-entry.js b/services/web/frontend/js/features/preview/components/preview-logs-pane-entry.js index a646b62d01..1333a4c90d 100644 --- a/services/web/frontend/js/features/preview/components/preview-logs-pane-entry.js +++ b/services/web/frontend/js/features/preview/components/preview-logs-pane-entry.js @@ -224,7 +224,7 @@ function PreviewLogEntryContent({ ) : null} {extraInfoURL ? (
- + {t('log_hint_extra_info')}
diff --git a/services/web/frontend/js/ide/human-readable-logs/HumanReadableLogsRules.js b/services/web/frontend/js/ide/human-readable-logs/HumanReadableLogsRules.js index c2e0df507d..c5ba227cbe 100644 --- a/services/web/frontend/js/ide/human-readable-logs/HumanReadableLogsRules.js +++ b/services/web/frontend/js/ide/human-readable-logs/HumanReadableLogsRules.js @@ -4,7 +4,7 @@ import PropTypes from 'prop-types' function WikiLink({ url, children, skipPlainRendering }) { if (window.wikiEnabled) { return ( - + {children} ) diff --git a/services/web/frontend/js/main/affiliations/controllers/UserAffiliationsReconfirmController.js b/services/web/frontend/js/main/affiliations/controllers/UserAffiliationsReconfirmController.js index e50be864ff..2bd45f9f39 100644 --- a/services/web/frontend/js/main/affiliations/controllers/UserAffiliationsReconfirmController.js +++ b/services/web/frontend/js/main/affiliations/controllers/UserAffiliationsReconfirmController.js @@ -38,7 +38,7 @@ export default App.controller( .then(() => { $scope.reconfirm[email].reconfirmationSent = true }) - .catch(error => { + .catch(_ => { $scope.ui.hasError = true }) .finally(() => ($scope.ui.isMakingRequest = false)) diff --git a/services/web/frontend/js/main/learn.js b/services/web/frontend/js/main/learn.js index 9d36ea541d..aed5296c18 100644 --- a/services/web/frontend/js/main/learn.js +++ b/services/web/frontend/js/main/learn.js @@ -81,6 +81,10 @@ App.controller( hitsPerPage: $scope.config_hits_per_page, }, function (err, response) { + if (err) { + $scope.searchError = err + } + $scope.processingSearch = false if (response.hits.length === 0) { updateHits([]) diff --git a/services/web/frontend/stories/icon.stories.js b/services/web/frontend/stories/icon.stories.js index 87d199e980..8fd7219857 100644 --- a/services/web/frontend/stories/icon.stories.js +++ b/services/web/frontend/stories/icon.stories.js @@ -8,7 +8,7 @@ export const Type = args => { Font Awesome icons diff --git a/services/web/scripts/create_project.js b/services/web/scripts/create_project.js index 86cda94764..f03396f8a2 100644 --- a/services/web/scripts/create_project.js +++ b/services/web/scripts/create_project.js @@ -65,8 +65,9 @@ async function _addDefaultExampleProjectFiles(ownerId, projectName, project) { ownerId ) - const frogPath = path.resolve( - __dirname + '/../app/templates/project_files/example-project/frog.jpg' + const frogPath = path.join( + __dirname, + '/../app/templates/project_files/example-project/frog.jpg' ) await ProjectEntityUpdateHandler.promises.addFile( project._id, @@ -81,8 +82,9 @@ async function _addDefaultExampleProjectFiles(ownerId, projectName, project) { async function _buildTemplate(templateName, userId, projectName) { const user = await User.findById(userId, 'first_name last_name') - const templatePath = path.resolve( - __dirname + `/../app/templates/project_files/${templateName}` + const templatePath = path.join( + __dirname, + `/../app/templates/project_files/${templateName}` ) const template = fs.readFileSync(templatePath) const data = { diff --git a/services/web/scripts/translations/download.js b/services/web/scripts/translations/download.js index ac79d5d86d..6f6cf0fa84 100644 --- a/services/web/scripts/translations/download.js +++ b/services/web/scripts/translations/download.js @@ -1,3 +1,4 @@ +const path = require('path') const { promises: fs } = require('fs') const oneSky = require('@brainly/onesky-utils') const sanitizeHtml = require('sanitize-html') @@ -35,7 +36,7 @@ async function run() { } await fs.writeFile( - `${__dirname}/../../locales/${code}.json`, + path.join(__dirname, `/../../locales/${code}.json`), JSON.stringify(lang.translation, null, 2) + '\n' ) } diff --git a/services/web/scripts/translations/transformLocales.js b/services/web/scripts/translations/transformLocales.js index 467c7e9044..b02c358033 100644 --- a/services/web/scripts/translations/transformLocales.js +++ b/services/web/scripts/translations/transformLocales.js @@ -1,3 +1,4 @@ +const Path = require('path') const fs = require('fs') const LANGUAGES = [ @@ -38,7 +39,7 @@ function transformLocales(mapping, transformLocale) { }) fs.writeFileSync( - `${__dirname}/../../locales/${language}.json`, + Path.join(__dirname, `/../../locales/${language}.json`), JSON.stringify(translatedLocales, null, 2) + '\n' ) }) diff --git a/services/web/scripts/translations/upload.js b/services/web/scripts/translations/upload.js index 8ad10f8a38..29b6e7d8dd 100644 --- a/services/web/scripts/translations/upload.js +++ b/services/web/scripts/translations/upload.js @@ -1,3 +1,4 @@ +const Path = require('path') const { promises: fs } = require('fs') const { promisify } = require('util') const oneSky = require('@brainly/onesky-utils') @@ -12,7 +13,9 @@ async function uploadLocales() { fileName: 'en-US.json', language: 'en-GB', format: 'HIERARCHICAL_JSON', - content: await fs.readFile(`${__dirname}/../../locales/en.json`), + content: await fs.readFile( + Path.join(__dirname, '/../../locales/en.json') + ), keepStrings: false, // deprecate locales that no longer exist in en.json }) ) diff --git a/services/web/test/acceptance/src/ProjectDuplicateNameTests.js b/services/web/test/acceptance/src/ProjectDuplicateNameTests.js index be9e8f513e..55057d1136 100644 --- a/services/web/test/acceptance/src/ProjectDuplicateNameTests.js +++ b/services/web/test/acceptance/src/ProjectDuplicateNameTests.js @@ -30,11 +30,10 @@ describe('ProjectDuplicateNames', function () { 'example-project', { template: 'example' }, (error, projectId) => { - if (error) { - throw error - } + expect(error).to.not.exist this.example_project_id = projectId this.owner.getProject(projectId, (error, project) => { + expect(error).to.not.exist this.project = project this.mainTexDoc = _.find( project.rootFolder[0].docs, @@ -59,6 +58,7 @@ describe('ProjectDuplicateNames', function () { }, }, (err, res, body) => { + expect(err).to.not.exist this.testFolderId = body._id done() } @@ -95,6 +95,7 @@ describe('ProjectDuplicateNames', function () { }, }, (err, res, body) => { + expect(err).to.not.exist this.res = res done() } @@ -117,6 +118,7 @@ describe('ProjectDuplicateNames', function () { }, }, (err, res, body) => { + expect(err).to.not.exist this.res = res done() } @@ -141,6 +143,7 @@ describe('ProjectDuplicateNames', function () { }, }, (err, res, body) => { + expect(err).to.not.exist this.res = res done() } @@ -163,6 +166,7 @@ describe('ProjectDuplicateNames', function () { }, }, (err, res, body) => { + expect(err).to.not.exist this.res = res done() } @@ -187,7 +191,7 @@ describe('ProjectDuplicateNames', function () { formData: { qqfile: { value: fs.createReadStream( - Path.resolve(__dirname + '/../files/1pixel.png') + Path.join(__dirname, '/../files/1pixel.png') ), options: { filename: 'frog.jpg', @@ -197,6 +201,7 @@ describe('ProjectDuplicateNames', function () { }, }, (err, res, body) => { + expect(err).to.not.exist this.body = body // update the image id because we have replaced the file this.imageFile._id = this.body.entity_id @@ -224,6 +229,7 @@ describe('ProjectDuplicateNames', function () { }, }, (err, res, body) => { + expect(err).to.not.exist this.res = res done() } @@ -246,6 +252,7 @@ describe('ProjectDuplicateNames', function () { }, }, (err, res, body) => { + expect(err).to.not.exist this.res = res done() } @@ -270,7 +277,7 @@ describe('ProjectDuplicateNames', function () { formData: { qqfile: { value: fs.createReadStream( - Path.resolve(__dirname + '/../files/1pixel.png') + Path.join(__dirname, '/../files/1pixel.png') ), options: { filename: 'testfolder', @@ -280,6 +287,7 @@ describe('ProjectDuplicateNames', function () { }, }, (err, res, body) => { + expect(err).to.not.exist this.body = body done() } @@ -303,6 +311,7 @@ describe('ProjectDuplicateNames', function () { }, }, (err, res, body) => { + expect(err).to.not.exist this.res = res done() } @@ -324,6 +333,7 @@ describe('ProjectDuplicateNames', function () { }, }, (err, res, body) => { + expect(err).to.not.exist this.res = res done() } @@ -345,6 +355,7 @@ describe('ProjectDuplicateNames', function () { }, }, (err, res, body) => { + expect(err).to.not.exist this.res = res done() } @@ -368,6 +379,7 @@ describe('ProjectDuplicateNames', function () { }, }, (err, res, body) => { + expect(err).to.not.exist this.res = res done() } @@ -389,6 +401,7 @@ describe('ProjectDuplicateNames', function () { }, }, (err, res, body) => { + expect(err).to.not.exist this.res = res done() } @@ -410,6 +423,7 @@ describe('ProjectDuplicateNames', function () { }, }, (err, res, body) => { + expect(err).to.not.exist this.res = res done() } @@ -433,6 +447,7 @@ describe('ProjectDuplicateNames', function () { }, }, (err, res, body) => { + expect(err).to.not.exist this.res = res done() } @@ -454,6 +469,7 @@ describe('ProjectDuplicateNames', function () { }, }, (err, res, body) => { + expect(err).to.not.exist this.res = res done() } @@ -475,6 +491,7 @@ describe('ProjectDuplicateNames', function () { }, }, (err, res, body) => { + expect(err).to.not.exist this.res = res done() } @@ -498,6 +515,9 @@ describe('ProjectDuplicateNames', function () { }, }, (err, res, body) => { + if (err) { + throw err + } this.owner.request.post( { uri: `/project/${this.example_project_id}/doc`, @@ -507,6 +527,9 @@ describe('ProjectDuplicateNames', function () { }, }, (err, res, body) => { + if (err) { + throw err + } this.owner.request.post( { uri: `/project/${this.example_project_id}/folder`, @@ -516,6 +539,9 @@ describe('ProjectDuplicateNames', function () { }, }, (err, res, body) => { + if (err) { + throw err + } this.subFolderId = body._id this.owner.request.post( { @@ -526,6 +552,9 @@ describe('ProjectDuplicateNames', function () { }, }, (err, res, body) => { + if (err) { + throw err + } this.otherFolderId = body._id done() } @@ -548,6 +577,7 @@ describe('ProjectDuplicateNames', function () { }, }, (err, res, body) => { + expect(err).to.not.exist this.res = res done() } @@ -569,6 +599,7 @@ describe('ProjectDuplicateNames', function () { }, }, (err, res, body) => { + expect(err).to.not.exist this.res = res done() } @@ -590,6 +621,7 @@ describe('ProjectDuplicateNames', function () { }, }, (err, res, body) => { + expect(err).to.not.exist this.res = res done() } @@ -611,6 +643,7 @@ describe('ProjectDuplicateNames', function () { }, }, (err, res, body) => { + expect(err).to.not.exist this.res = res done() } diff --git a/services/web/test/acceptance/src/RegistrationTests.js b/services/web/test/acceptance/src/RegistrationTests.js index 34e97c2108..9aaff82f9f 100644 --- a/services/web/test/acceptance/src/RegistrationTests.js +++ b/services/web/test/acceptance/src/RegistrationTests.js @@ -1,7 +1,3 @@ -/* eslint-disable - handle-callback-err -*/ - const { expect } = require('chai') const async = require('async') const metrics = require('./helpers/metrics') @@ -13,9 +9,6 @@ const Features = require('../../../app/src/infrastructure/Features') // Expectations const expectProjectAccess = function (user, projectId, callback) { // should have access to project - if (callback == null) { - callback = function (err, result) {} - } user.openProject(projectId, err => { expect(err).to.be.oneOf([null, undefined]) return callback() @@ -24,9 +17,6 @@ const expectProjectAccess = function (user, projectId, callback) { const expectNoProjectAccess = function (user, projectId, callback) { // should not have access to project page - if (callback == null) { - callback = function (err, result) {} - } user.openProject(projectId, err => { expect(err).to.be.instanceof(Error) return callback() @@ -40,9 +30,6 @@ const tryLoginThroughRegistrationForm = function ( password, callback ) { - if (callback == null) { - callback = function (err, response, body) {} - } user.getCsrfToken(err => { if (err != null) { return callback(err) @@ -234,7 +221,9 @@ describe('Registration', function () { it('should register with the csrf token', function (done) { this.user.request.get('/login', (err, res, body) => { + expect(err).to.not.exist this.user.getCsrfToken(error => { + expect(error).to.not.exist this.user.request.post( { url: '/register', @@ -247,7 +236,7 @@ describe('Registration', function () { }, }, (error, response, body) => { - expect(err != null).to.equal(false) + expect(error).to.not.exist expect(response.statusCode).to.equal(200) return done() } @@ -258,7 +247,9 @@ describe('Registration', function () { it('should fail with no csrf token', function (done) { this.user.request.get('/login', (err, res, body) => { + expect(err).to.not.exist this.user.getCsrfToken(error => { + expect(error).to.not.exist this.user.request.post( { url: '/register', @@ -271,6 +262,7 @@ describe('Registration', function () { }, }, (error, response, body) => { + expect(error).to.not.exist expect(response.statusCode).to.equal(403) return done() } @@ -281,9 +273,12 @@ describe('Registration', function () { it('should fail with a stale csrf token', function (done) { this.user.request.get('/login', (err, res, body) => { + expect(err).to.not.exist this.user.getCsrfToken(error => { + expect(error).to.not.exist const oldCsrfToken = this.user.csrfToken this.user.logout(err => { + expect(err).to.not.exist this.user.request.post( { url: '/register', @@ -296,6 +291,7 @@ describe('Registration', function () { }, }, (error, response, body) => { + expect(error).to.not.exist expect(response.statusCode).to.equal(403) return done() } @@ -359,9 +355,11 @@ describe('Registration', function () { it('should not allow sign in with secondary email', function (done) { const secondaryEmail = 'acceptance-test-secondary@example.com' this.user1.addEmail(secondaryEmail, err => { + expect(err).to.not.exist this.user1.loginWith(secondaryEmail, err => { expect(err != null).to.equal(false) this.user1.isLoggedIn((err, isLoggedIn) => { + expect(err).to.not.exist expect(isLoggedIn).to.equal(false) return done() }) @@ -396,6 +394,7 @@ describe('Registration', function () { this.user1.email, 'totally_not_the_right_password', (err, response, body) => { + expect(err).to.not.exist expect(body.redir != null).to.equal(false) expect(body.message != null).to.equal(true) expect(body.message).to.have.all.keys('type', 'text') diff --git a/services/web/test/unit/src/Subscription/LimitationsManagerTests.js b/services/web/test/unit/src/Subscription/LimitationsManagerTests.js index 9be6b47803..f70af9b305 100644 --- a/services/web/test/unit/src/Subscription/LimitationsManagerTests.js +++ b/services/web/test/unit/src/Subscription/LimitationsManagerTests.js @@ -1,4 +1,5 @@ const SandboxedModule = require('sandboxed-module') +const assert = require('assert') const sinon = require('sinon') const modulePath = require('path').join( __dirname, @@ -374,6 +375,7 @@ describe('LimitationsManager', function () { this.LimitationsManager.userHasV2Subscription( this.user, (err, hasSubscription) => { + assert.equal(err, null) hasSubscription.should.equal(true) done() } @@ -386,6 +388,7 @@ describe('LimitationsManager', function () { this.LimitationsManager.userHasV2Subscription( this.user, (err, hasSubscription) => { + assert.equal(err, null) hasSubscription.should.equal(false) done() } @@ -397,6 +400,7 @@ describe('LimitationsManager', function () { this.LimitationsManager.userHasV2Subscription( this.user, (err, hasSubscription) => { + assert.equal(err, null) hasSubscription.should.equal(false) done() } @@ -413,6 +417,7 @@ describe('LimitationsManager', function () { this.LimitationsManager.userHasV2Subscription( this.user, (err, hasSubOrIsGroupMember, subscription) => { + assert.equal(err, null) subscription.should.deep.equal(stubbedSubscription) done() } @@ -433,6 +438,7 @@ describe('LimitationsManager', function () { this.LimitationsManager.userHasV2Subscription( this.user, (err, hasSubscription, subscription) => { + assert.equal(err, null) hasSubscription.should.equal(true) done() } @@ -443,6 +449,7 @@ describe('LimitationsManager', function () { this.LimitationsManager.userHasV2Subscription( this.user, (err, hasSubscription, subscription) => { + assert.equal(err, null) subscription.should.deep.equal(this.fakeSubscription) done() } @@ -461,6 +468,7 @@ describe('LimitationsManager', function () { this.LimitationsManager.userIsMemberOfGroupSubscription( this.user, (err, isMember) => { + assert.equal(err, null) isMember.should.equal(false) done() } @@ -477,6 +485,7 @@ describe('LimitationsManager', function () { this.LimitationsManager.userIsMemberOfGroupSubscription( this.user, (err, isMember, retSubscriptions) => { + assert.equal(err, null) isMember.should.equal(true) retSubscriptions.should.equal(subscriptions) done() @@ -505,6 +514,7 @@ describe('LimitationsManager', function () { this.LimitationsManager.hasPaidSubscription( this.user, (err, hasSubOrIsGroupMember) => { + assert.equal(err, null) hasSubOrIsGroupMember.should.equal(true) done() } @@ -518,6 +528,7 @@ describe('LimitationsManager', function () { this.LimitationsManager.hasPaidSubscription( this.user, (err, hasSubOrIsGroupMember) => { + assert.equal(err, null) hasSubOrIsGroupMember.should.equal(true) done() } @@ -531,6 +542,7 @@ describe('LimitationsManager', function () { this.LimitationsManager.hasPaidSubscription( this.user, (err, hasSubOrIsGroupMember) => { + assert.equal(err, null) hasSubOrIsGroupMember.should.equal(true) done() } @@ -541,6 +553,7 @@ describe('LimitationsManager', function () { this.LimitationsManager.hasPaidSubscription( this.user, (err, hasSubOrIsGroupMember) => { + assert.equal(err, null) hasSubOrIsGroupMember.should.equal(false) done() } @@ -551,6 +564,7 @@ describe('LimitationsManager', function () { this.LimitationsManager.userHasSubscriptionOrIsGroupMember( this.user, (err, hasSubOrIsGroupMember) => { + assert.equal(err, null) hasSubOrIsGroupMember.should.equal(false) done() } @@ -575,6 +589,7 @@ describe('LimitationsManager', function () { this.LimitationsManager.userHasV1OrV2Subscription( this.user, (err, hasSub) => { + assert.equal(err, null) hasSub.should.equal(true) done() } @@ -588,6 +603,7 @@ describe('LimitationsManager', function () { this.LimitationsManager.userHasV1OrV2Subscription( this.user, (err, hasSub) => { + assert.equal(err, null) hasSub.should.equal(true) done() } @@ -598,6 +614,7 @@ describe('LimitationsManager', function () { this.LimitationsManager.userHasV1OrV2Subscription( this.user, (err, hasSub) => { + assert.equal(err, null) hasSub.should.equal(false) done() } @@ -626,6 +643,7 @@ describe('LimitationsManager', function () { this.LimitationsManager.hasGroupMembersLimitReached( this.subscriptionId, (err, limitReached) => { + assert.equal(err, null) limitReached.should.equal(true) done() } @@ -642,6 +660,7 @@ describe('LimitationsManager', function () { this.LimitationsManager.hasGroupMembersLimitReached( this.subscriptionId, (err, limitReached) => { + assert.equal(err, null) limitReached.should.equal(false) done() } @@ -658,6 +677,7 @@ describe('LimitationsManager', function () { this.LimitationsManager.hasGroupMembersLimitReached( this.subscriptionId, (err, limitReached) => { + assert.equal(err, null) limitReached.should.equal(true) done() } @@ -673,6 +693,7 @@ describe('LimitationsManager', function () { this.LimitationsManager.userHasV1Subscription( this.user, (error, result) => { + assert.equal(error, null) this.V1SubscriptionManager.getSubscriptionsFromV1 .calledWith(this.userId) .should.equal(true) @@ -689,6 +710,7 @@ describe('LimitationsManager', function () { this.LimitationsManager.userHasV1Subscription( this.user, (error, result) => { + assert.equal(error, null) this.V1SubscriptionManager.getSubscriptionsFromV1 .calledWith(this.userId) .should.equal(true) @@ -705,6 +727,7 @@ describe('LimitationsManager', function () { this.LimitationsManager.userHasV1Subscription( this.user, (error, result) => { + assert.equal(error, null) this.V1SubscriptionManager.getSubscriptionsFromV1 .calledWith(this.userId) .should.equal(true) diff --git a/services/web/test/unit/src/Subscription/UserFeaturesUpdaterTests.js b/services/web/test/unit/src/Subscription/UserFeaturesUpdaterTests.js index 83a485d52d..905f3ddf1f 100644 --- a/services/web/test/unit/src/Subscription/UserFeaturesUpdaterTests.js +++ b/services/web/test/unit/src/Subscription/UserFeaturesUpdaterTests.js @@ -1,5 +1,6 @@ const SandboxedModule = require('sandboxed-module') const { expect } = require('chai') +const assert = require('assert') const sinon = require('sinon') const modulePath = '../../../../app/src/Features/Subscription/UserFeaturesUpdater' @@ -46,6 +47,7 @@ describe('UserFeaturesUpdater', function () { (err, features) => { const updateArgs = this.User.findByIdAndUpdate.lastCall.args expect(updateArgs[0]).to.deep.equal(userId) + assert.equal(err, null) expect(Object.keys(updateArgs[1]).length).to.equal(3) expect(updateArgs[1]['features.versioning']).to.equal( update.versioning @@ -69,6 +71,7 @@ describe('UserFeaturesUpdater', function () { userId, update, (err, featuresChanged) => { + assert.equal(err, null) const updateArgs = this.User.findByIdAndUpdate.lastCall.args expect(updateArgs[0]).to.equal(userId) expect(Object.keys(updateArgs[1]).length).to.equal(2) diff --git a/services/web/test/unit/src/infrastructure/GeoIpLookupTest.js b/services/web/test/unit/src/infrastructure/GeoIpLookupTest.js index 473546e92f..2e2b6f392a 100644 --- a/services/web/test/unit/src/infrastructure/GeoIpLookupTest.js +++ b/services/web/test/unit/src/infrastructure/GeoIpLookupTest.js @@ -61,6 +61,7 @@ describe('GeoIpLookup', function () { describe('callback', function () { it('should request the details using the ip', function (done) { this.GeoIpLookup.getDetails(this.ipAddress, err => { + assert.equal(err, null) this.request.get .calledWith({ url: this.settings.apis.geoIpLookup.url + '/' + this.ipAddress, @@ -74,6 +75,7 @@ describe('GeoIpLookup', function () { it('should return the ip details', function (done) { this.GeoIpLookup.getDetails(this.ipAddress, (err, returnedDetails) => { + assert.equal(err, null) assert.deepEqual(returnedDetails, this.stubbedResponse) done() }) @@ -83,6 +85,7 @@ describe('GeoIpLookup', function () { this.GeoIpLookup.getDetails( ` ${this.ipAddress} 123.123.123.123 234.234.234.234`, err => { + assert.equal(err, null) this.request.get .calledWith({ url: this.settings.apis.geoIpLookup.url + '/' + this.ipAddress, @@ -137,6 +140,7 @@ describe('GeoIpLookup', function () { this.GeoIpLookup.getCurrencyCode( this.ipAddress, (err, currencyCode) => { + assert.equal(err, null) currencyCode.should.equal('GBP') done() } @@ -149,6 +153,7 @@ describe('GeoIpLookup', function () { this.GeoIpLookup.getCurrencyCode( this.ipAddress, (err, currencyCode) => { + assert.equal(err, null) currencyCode.should.equal('GBP') done() } @@ -161,6 +166,7 @@ describe('GeoIpLookup', function () { this.GeoIpLookup.getCurrencyCode( this.ipAddress, (err, currencyCode) => { + assert.equal(err, null) currencyCode.should.equal('USD') done() } @@ -173,6 +179,7 @@ describe('GeoIpLookup', function () { this.GeoIpLookup.getCurrencyCode( this.ipAddress, (err, currencyCode) => { + assert.equal(err, null) currencyCode.should.equal('EUR') done() } @@ -184,6 +191,7 @@ describe('GeoIpLookup', function () { this.GeoIpLookup.getCurrencyCode( this.ipAddress, (err, currencyCode) => { + assert.equal(err, null) currencyCode.should.equal('USD') done() } @@ -195,6 +203,7 @@ describe('GeoIpLookup', function () { this.GeoIpLookup.getCurrencyCode( this.ipAddress, (err, currencyCode) => { + assert.equal(err, null) currencyCode.should.equal('USD') done() } @@ -207,6 +216,7 @@ describe('GeoIpLookup', function () { this.GeoIpLookup.getCurrencyCode( this.ipAddress, (err, currencyCode) => { + assert.equal(err, null) currencyCode.should.equal('USD') done() }