Merge pull request #4799 from overleaf/hb-eslint-rules

Re-enable some eslint rules

GitOrigin-RevId: 16153adb839bb61784bb40fbc8e43da281fe090d

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",

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) {

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 =

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 = {

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(
       {

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) {

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) {

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()
           }

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',

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 = {}

services/web/frontend/js/features/file-view/components/file-view-header.js

@@ -213,6 +213,7 @@ function ProjectFilePathProvider({ file }) {
                 <a
                   href={`/project/${file.linkedFileData.source_project_id}`}
                   target="_blank"
+                  rel="noopener"
                 />,
               ]
         }
@@ -252,6 +253,7 @@ function ProjectOutputFileProvider({ file }) {
                 <a
                   href={`/project/${file.linkedFileData.source_project_id}`}
                   target="_blank"
+                  rel="noopener"
                 />,
               ]
         }

services/web/frontend/js/features/preview/components/preview-error.js

@@ -47,6 +47,7 @@ function PreviewError({ name }) {
           <a
             href="https://www.overleaf.com/learn/how-to/Why_do_I_keep_getting_the_compile_timeout_error_message%3F"
             target="_blank"
+            rel="noopener"
           >
             {t('learn_how_to_make_documents_compile_quickly')}
           </a>

services/web/frontend/js/features/preview/components/preview-logs-pane-entry.js

@@ -224,7 +224,7 @@ function PreviewLogEntryContent({
       ) : null}
       {extraInfoURL ? (
         <div className="log-entry-content-link">
-          <a href={extraInfoURL} target="_blank">
+          <a href={extraInfoURL} target="_blank" rel="noopener">
             {t('log_hint_extra_info')}
           </a>
         </div>

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 (
-      <a href={url} target="_blank">
+      <a href={url} target="_blank" rel="noopener">
         {children}
       </a>
     )

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))

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([])

services/web/frontend/stories/icon.stories.js

@@ -8,7 +8,7 @@ export const Type = args => {
         <a
           href="https://fontawesome.com/v4.7.0/icons/"
           target="_blank"
-          rel="noopener"
+          rel="noopener noreferrer"
         >
           Font Awesome icons
         </a>

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 = {

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'
       )
     }

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'
     )
   })

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
     })
   )

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()
             }

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')

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)

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)

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()
           }