Merge pull request #2594 from overleaf/jel-flag-affiliation-when-registering

Flag unchecked affiliation when registering

GitOrigin-RevId: c9da12ceb5a1712c8c477222f967fb54eceef252

services/web/app/src/Features/User/UserCreator.js

@@ -1,7 +1,28 @@
 const logger = require('logger-sharelatex')
 const util = require('util')
 const { User } = require('../../models/User')
-const { addAffiliation } = require('../Institutions/InstitutionsAPI')
+const Features = require('../../infrastructure/Features')
+const UserUpdater = require('./UserUpdater')
+const UserGetter = require('./UserGetter')
+
+async function _addAffiliation(user) {
+  try {
+    await UserUpdater.promises.addAffiliationForNewUser(user._id, user.email)
+  } catch (error) {
+    // do not pass error back during registration
+    // errors are logged in UserUpdater and InstitutionsAPI
+  }
+
+  try {
+    user = await UserGetter.promises.getUser(user._id)
+  } catch (error) {
+    logger.error(
+      { userId: user._id, email: user.email },
+      'could not get fresh user data'
+    )
+  }
+  return user
+}
 
 async function createNewUser(attributes) {
   let user = new User()
@@ -28,6 +49,9 @@ async function createNewUser(attributes) {
     createdAt: new Date(),
     reversedHostname
   }
+  if (Features.hasFeature('affiliations')) {
+    emailData.affiliationUnchecked = true
+  }
   if (
     attributes.samlIdentifiers &&
     attributes.samlIdentifiers[0] &&
@@ -40,14 +64,9 @@ async function createNewUser(attributes) {
 
   user = await user.save()
 
-  addAffiliation(user._id, user.email, err => {
-    if (err) {
-      logger.error(
-        { userId: user._id, email: user.email },
-        "couldn't add affiliation for user on create"
-      )
-    }
-  })
+  if (Features.hasFeature('affiliations')) {
+    user = await _addAffiliation(user)
+  }
 
   return user
 }

services/web/app/src/Features/User/UserUpdater.js

@@ -10,12 +10,35 @@ const {
   addAffiliation,
   removeAffiliation
 } = require('../Institutions/InstitutionsAPI')
+const Features = require('../../infrastructure/Features')
 const FeaturesUpdater = require('../Subscription/FeaturesUpdater')
 const EmailHelper = require('../Helpers/EmailHelper')
 const Errors = require('../Errors/Errors')
 const NewsletterManager = require('../Newsletter/NewsletterManager')
 
 const UserUpdater = {
+  addAffiliationForNewUser(userId, email, callback) {
+    addAffiliation(userId, email, error => {
+      if (error) {
+        return callback(error)
+      }
+      UserUpdater.updateUser(
+        { _id: userId, 'emails.email': email },
+        { $unset: { 'emails.$.affiliationUnchecked': 1 } },
+        (error, updated) => {
+          if (error || (updated && updated.nModified === 0)) {
+            logger.error(
+              { userId, email },
+              'could not remove affiliationUnchecked flag for user on create'
+            )
+            return callback(error)
+          }
+          return callback(error, updated)
+        }
+      )
+    })
+  },
+
   updateUser(query, update, callback) {
     if (callback == null) {
       callback = () => {}
@@ -213,6 +236,11 @@ const UserUpdater = {
           'emails.$.confirmedAt': confirmedAt
         }
       }
+      if (Features.hasFeature('affiliations')) {
+        update['$unset'] = {
+          'emails.$.affiliationUnchecked': 1
+        }
+      }
       UserUpdater.updateUser(query, update, (error, res) => {
         if (error != null) {
           return callback(error)
@@ -249,6 +277,7 @@ const UserUpdater = {
 )
 
 const promises = {
+  addAffiliationForNewUser: promisify(UserUpdater.addAffiliationForNewUser),
   addEmailAddress: promisify(UserUpdater.addEmailAddress),
   confirmEmail: promisify(UserUpdater.confirmEmail),
   updateUser: promisify(UserUpdater.updateUser)

services/web/app/src/models/User.js

@@ -20,7 +20,8 @@ const UserSchema = new Schema({
         }
       },
       confirmedAt: { type: Date },
-      samlProviderId: { type: String }
+      samlProviderId: { type: String },
+      affiliationUnchecked: { type: Boolean }
     }
   ],
   first_name: { type: String, default: '' },

services/web/test/acceptance/src/RegistrationTests.js

@@ -1,25 +1,11 @@
 /* eslint-disable
-    camelcase,
-    handle-callback-err,
-    max-len,
-    no-return-assign,
-    no-unused-vars,
+    handle-callback-err
 */
-// TODO: This file was created by bulk-decaffeinate.
-// Fix any style issues and re-enable lint.
-/*
- * decaffeinate suggestions:
- * DS102: Remove unnecessary code created because of implicit returns
- * DS103: Rewrite code to no longer use __guard__
- * DS207: Consider shorter variations of null checks
- * Full docs: https://github.com/decaffeinate/decaffeinate/blob/master/docs/suggestions.md
- */
+
 const { expect } = require('chai')
 const { assert } = require('chai')
 const async = require('async')
 const User = require('./helpers/User')
-const request = require('./helpers/request')
-const settings = require('settings-sharelatex')
 const redis = require('./helpers/redis')
 const _ = require('lodash')
 const Features = require('../../../app/src/infrastructure/Features')
@@ -33,7 +19,7 @@ const expectProjectAccess = function(user, projectId, callback) {
   if (callback == null) {
     callback = function(err, result) {}
   }
-  return user.openProject(projectId, err => {
+  user.openProject(projectId, err => {
     expect(err).to.be.oneOf([null, undefined])
     return callback()
   })
@@ -44,7 +30,7 @@ const expectNoProjectAccess = function(user, projectId, callback) {
   if (callback == null) {
     callback = function(err, result) {}
   }
-  return user.openProject(projectId, err => {
+  user.openProject(projectId, err => {
     expect(err).to.be.instanceof(Error)
     return callback()
   })
@@ -60,11 +46,11 @@ const tryLoginThroughRegistrationForm = function(
   if (callback == null) {
     callback = function(err, response, body) {}
   }
-  return user.getCsrfToken(err => {
+  user.getCsrfToken(err => {
     if (err != null) {
       return callback(err)
     }
-    return user.request.post(
+    user.request.post(
       {
         url: '/register',
         json: {
@@ -82,19 +68,19 @@ describe('Registration', function() {
     beforeEach(function() {
       this.user = new User()
       this.badEmail = 'bademail@example.com'
-      return (this.badPassword = 'badpassword')
+      this.badPassword = 'badpassword'
     })
 
     it('should rate limit login attempts after 10 within two minutes', function(done) {
-      return this.user.request.get('/login', (err, res, body) => {
-        return async.timesSeries(
+      this.user.request.get('/login', (err, res, body) => {
+        async.timesSeries(
           15,
           (n, cb) => {
-            return this.user.getCsrfToken(error => {
+            this.user.getCsrfToken(error => {
               if (error != null) {
                 return cb(error)
               }
-              return this.user.request.post(
+              this.user.request.post(
                 {
                   url: '/login',
                   json: {
@@ -103,13 +89,8 @@ describe('Registration', function() {
                   }
                 },
                 (err, response, body) => {
-                  return cb(
-                    null,
-                    __guard__(
-                      body != null ? body.message : undefined,
-                      x => x.text
-                    )
-                  )
+                  const message = body && body.message && body.message.text
+                  return cb(null, message)
                 }
               )
             })
@@ -147,17 +128,17 @@ describe('Registration', function() {
     beforeEach(function() {
       this.user = new User()
       this.email = `test+${Math.random()}@example.com`
-      return (this.password = 'password11')
+      this.password = 'password11'
     })
 
     afterEach(function(done) {
-      return this.user.fullDeleteUser(this.email, done)
+      this.user.fullDeleteUser(this.email, done)
     })
 
     it('should register with the csrf token', function(done) {
-      return this.user.request.get('/login', (err, res, body) => {
-        return this.user.getCsrfToken(error => {
-          return this.user.request.post(
+      this.user.request.get('/login', (err, res, body) => {
+        this.user.getCsrfToken(error => {
+          this.user.request.post(
             {
               url: '/register',
               json: {
@@ -179,9 +160,9 @@ describe('Registration', function() {
     })
 
     it('should fail with no csrf token', function(done) {
-      return this.user.request.get('/login', (err, res, body) => {
-        return this.user.getCsrfToken(error => {
-          return this.user.request.post(
+      this.user.request.get('/login', (err, res, body) => {
+        this.user.getCsrfToken(error => {
+          this.user.request.post(
             {
               url: '/register',
               json: {
@@ -202,11 +183,11 @@ describe('Registration', function() {
     })
 
     it('should fail with a stale csrf token', function(done) {
-      return this.user.request.get('/login', (err, res, body) => {
-        return this.user.getCsrfToken(error => {
+      this.user.request.get('/login', (err, res, body) => {
+        this.user.getCsrfToken(error => {
           const oldCsrfToken = this.user.csrfToken
-          return this.user.logout(err => {
-            return this.user.request.post(
+          this.user.logout(err => {
+            this.user.request.post(
               {
                 url: '/register',
                 json: {
@@ -236,11 +217,11 @@ describe('Registration', function() {
     })
 
     beforeEach(function() {
-      return (this.user = new User())
+      this.user = new User()
     })
 
     it('Set emails attribute', function(done) {
-      return this.user.register((error, user) => {
+      this.user.register((error, user) => {
         expect(error).to.not.exist
         user.email.should.equal(this.user.email)
         user.emails.should.exist
@@ -262,7 +243,7 @@ describe('Registration', function() {
     beforeEach(function(done) {
       this.user1 = new User()
       this.user2 = new User()
-      return async.series(
+      async.series(
         [
           cb => this.user1.register(cb),
           cb =>
@@ -276,7 +257,7 @@ describe('Registration', function() {
     })
 
     it('Adds a referal when an id is supplied and the referal source is "bonus"', function(done) {
-      return this.user1.get((error, user) => {
+      this.user1.get((error, user) => {
         expect(error).to.not.exist
         user.refered_user_count.should.eql(1)
 
@@ -301,7 +282,7 @@ describe('Registration', function() {
         ],
         done
       )
-      return (this.project_id = null)
+      this.project_id = null
     })
 
     describe('[Security] Trying to register/login as another user', function() {
@@ -313,10 +294,10 @@ describe('Registration', function() {
 
       it('should not allow sign in with secondary email', function(done) {
         const secondaryEmail = 'acceptance-test-secondary@example.com'
-        return this.user1.addEmail(secondaryEmail, err => {
-          return this.user1.loginWith(secondaryEmail, err => {
+        this.user1.addEmail(secondaryEmail, err => {
+          this.user1.loginWith(secondaryEmail, err => {
             expect(err != null).to.equal(false)
-            return this.user1.isLoggedIn((err, isLoggedIn) => {
+            this.user1.isLoggedIn((err, isLoggedIn) => {
               expect(isLoggedIn).to.equal(false)
               return done()
             })
@@ -368,9 +349,3 @@ describe('Registration', function() {
     })
   })
 })
-
-function __guard__(value, transform) {
-  return typeof value !== 'undefined' && value !== null
-    ? transform(value)
-    : undefined
-}

services/web/test/unit/src/User/UserCreatorTests.js

@@ -15,9 +15,6 @@ describe('UserCreator', function() {
         return self.user
       }
     }
-
-    this.UserGetter = { getUserByMainEmail: sinon.stub() }
-    this.addAffiliation = sinon.stub().yields()
     this.UserCreator = SandboxedModule.require(modulePath, {
       globals: {
         console: console
@@ -26,11 +23,26 @@ describe('UserCreator', function() {
         '../../models/User': {
           User: this.UserModel
         },
-        'logger-sharelatex': { log: sinon.stub(), err: sinon.stub() },
+        'logger-sharelatex': {
+          error: sinon.stub()
+        },
         'metrics-sharelatex': { timeAsyncMethod() {} },
-        '../Institutions/InstitutionsAPI': {
-          addAffiliation: this.addAffiliation
-        }
+        '../../infrastructure/Features': (this.Features = {
+          hasFeature: sinon.stub().returns(false)
+        }),
+        './UserGetter': (this.UserGetter = {
+          promises: {
+            getUser: sinon.stub().resolves(this.user)
+          }
+        }),
+        './UserUpdater': (this.UserUpdater = {
+          promises: {
+            addAffiliationForNewUser: sinon
+              .stub()
+              .resolves({ n: 1, nModified: 1, ok: 1 }),
+            updateUser: sinon.stub().resolves()
+          }
+        })
       }
     })
 
@@ -110,15 +122,76 @@ describe('UserCreator', function() {
         })
       })
 
-      it('should add affiliation', function(done) {
+      it('should not add affiliation', function() {})
+
+      describe('with affiliations feature', function() {
+        let attributes, user
+        beforeEach(function() {
+          attributes = { email: this.email }
+          this.Features.hasFeature = sinon
+            .stub()
+            .withArgs('affiliations')
+            .returns(true)
+        })
+        describe('when v1 affiliations API does not return an error', function() {
+          beforeEach(function(done) {
+            this.UserCreator.createNewUser(attributes, (err, createdUser) => {
+              user = createdUser
+              assert.ifError(err)
+              done()
+            })
+          })
+          it('should flag that affiliation is unchecked', function() {
+            user.emails[0].affiliationUnchecked.should.equal(true)
+          })
+          it('should try to add affiliation to v1', function() {
+            sinon.assert.calledOnce(
+              this.UserUpdater.promises.addAffiliationForNewUser
+            )
+            sinon.assert.calledWithMatch(
+              this.UserUpdater.promises.addAffiliationForNewUser,
+              user._id,
+              this.email
+            )
+          })
+          it('should query for updated user data', function() {
+            sinon.assert.calledOnce(this.UserGetter.promises.getUser)
+          })
+        })
+        describe('when v1 affiliations API does return an error', function() {
+          beforeEach(function(done) {
+            this.UserUpdater.promises.addAffiliationForNewUser.rejects()
+            this.UserCreator.createNewUser(attributes, (error, createdUser) => {
+              user = createdUser
+              assert.ifError(error)
+              done()
+            })
+          })
+          it('should flag that affiliation is unchecked', function() {
+            user.emails[0].affiliationUnchecked.should.equal(true)
+          })
+          it('should try to add affiliation to v1', function() {
+            sinon.assert.calledOnce(
+              this.UserUpdater.promises.addAffiliationForNewUser
+            )
+            sinon.assert.calledWithMatch(
+              this.UserUpdater.promises.addAffiliationForNewUser,
+              user._id,
+              this.email
+            )
+          })
+          it('should query for updated user data', function() {
+            sinon.assert.calledOnce(this.UserGetter.promises.getUser)
+          })
+        })
+      })
+
+      it('should not add affiliation when without affiliation feature', function(done) {
         const attributes = { email: this.email }
         this.UserCreator.createNewUser(attributes, (err, user) => {
           assert.ifError(err)
-          sinon.assert.calledOnce(this.addAffiliation)
-          sinon.assert.calledWithMatch(
-            this.addAffiliation,
-            user._id,
-            this.email
+          sinon.assert.notCalled(
+            this.UserUpdater.promises.addAffiliationForNewUser
           )
           done()
         })
@@ -137,11 +210,30 @@ describe('UserCreator', function() {
         assert.equal(user.first_name, 'bob.oswald')
       })
 
-      it('should add affiliation', async function() {
+      it('should add affiliation when with affiliation feature', async function() {
+        this.Features.hasFeature = sinon
+          .stub()
+          .withArgs('affiliations')
+          .returns(true)
         const attributes = { email: this.email }
         const user = await this.UserCreator.promises.createNewUser(attributes)
-        sinon.assert.calledOnce(this.addAffiliation)
-        sinon.assert.calledWithMatch(this.addAffiliation, user._id, this.email)
+        sinon.assert.calledOnce(
+          this.UserUpdater.promises.addAffiliationForNewUser
+        )
+        sinon.assert.calledWithMatch(
+          this.UserUpdater.promises.addAffiliationForNewUser,
+          user._id,
+          this.email
+        )
+      })
+
+      it('should not add affiliation when without affiliation feature', async function() {
+        this.Features.hasFeature = sinon.stub().returns(false)
+        const attributes = { email: this.email }
+        await this.UserCreator.promises.createNewUser(attributes)
+        sinon.assert.notCalled(
+          this.UserUpdater.promises.addAffiliationForNewUser
+        )
       })
 
       it('should include SAML provider ID with email', async function() {

services/web/test/unit/src/User/UserUpdaterTests.js

@@ -1,26 +1,13 @@
-/* eslint-disable
-    max-len,
-    no-return-assign,
-    no-unused-vars,
-*/
-// TODO: This file was created by bulk-decaffeinate.
-// Fix any style issues and re-enable lint.
-/*
- * decaffeinate suggestions:
- * DS102: Remove unnecessary code created because of implicit returns
- * Full docs: https://github.com/decaffeinate/decaffeinate/blob/master/docs/suggestions.md
- */
 const should = require('chai').should()
 const SandboxedModule = require('sandboxed-module')
-const assert = require('assert')
 const path = require('path')
 const sinon = require('sinon')
 const modulePath = path.join(
   __dirname,
   '../../../../app/src/Features/User/UserUpdater'
 )
-const { expect } = require('chai')
 const tk = require('timekeeper')
+const { expect } = require('chai')
 
 describe('UserUpdater', function() {
   beforeEach(function() {
@@ -37,7 +24,7 @@ describe('UserUpdater', function() {
       ensureUniqueEmailAddress: sinon.stub()
     }
     this.logger = {
-      err: sinon.stub(),
+      error: sinon.stub(),
       log() {},
       warn: sinon.stub()
     }
@@ -60,6 +47,9 @@ describe('UserUpdater', function() {
           addAffiliation: this.addAffiliation,
           removeAffiliation: this.removeAffiliation
         },
+        '../../infrastructure/Features': (this.Features = {
+          hasFeature: sinon.stub().returns(false)
+        }),
         '../Subscription/FeaturesUpdater': {
           refreshFeatures: this.refreshFeatures
         },
@@ -82,18 +72,55 @@ describe('UserUpdater', function() {
     return tk.reset()
   })
 
+  describe('addAffiliationForNewUser', function(done) {
+    beforeEach(function() {
+      this.UserUpdater.updateUser = sinon
+        .stub()
+        .callsArgWith(2, null, { n: 1, nModified: 1, ok: 1 })
+    })
+    it('should not remove affiliationUnchecked flag if v1 returns an error', function(done) {
+      this.addAffiliation.yields(true)
+      this.UserUpdater.addAffiliationForNewUser(
+        this.stubbedUser._id,
+        this.newEmail,
+        (error, updated) => {
+          expect(error).to.exist
+          expect(updated).to.be.undefined
+          sinon.assert.notCalled(this.UserUpdater.updateUser)
+          done()
+        }
+      )
+    })
+    it('should remove affiliationUnchecked flag if v1 does not return an error', function(done) {
+      this.addAffiliation.yields()
+      this.UserUpdater.addAffiliationForNewUser(
+        this.stubbedUser._id,
+        this.newEmail,
+        (error, updated) => {
+          should.not.exist(error)
+          expect(updated).to.deep.equal({ n: 1, nModified: 1, ok: 1 })
+          sinon.assert.calledOnce(this.UserUpdater.updateUser)
+          sinon.assert.calledWithMatch(
+            this.UserUpdater.updateUser,
+            { _id: this.stubbedUser._id, 'emails.email': this.newEmail },
+            { $unset: { 'emails.$.affiliationUnchecked': 1 } }
+          )
+          done()
+        }
+      )
+    })
+  })
+
   describe('changeEmailAddress', function() {
     beforeEach(function() {
       this.UserGetter.getUserEmail.callsArgWith(1, null, this.stubbedUser.email)
       this.UserUpdater.addEmailAddress = sinon.stub().callsArgWith(2)
       this.UserUpdater.setDefaultEmailAddress = sinon.stub().yields()
-      return (this.UserUpdater.removeEmailAddress = sinon
-        .stub()
-        .callsArgWith(2))
+      this.UserUpdater.removeEmailAddress = sinon.stub().callsArgWith(2)
     })
 
     it('change email', function(done) {
-      return this.UserUpdater.changeEmailAddress(
+      this.UserUpdater.changeEmailAddress(
         this.stubbedUser._id,
         this.newEmail,
         err => {
@@ -107,30 +134,26 @@ describe('UserUpdater', function() {
           this.UserUpdater.removeEmailAddress
             .calledWith(this.stubbedUser._id, this.stubbedUser.email)
             .should.equal(true)
-          return done()
+          done()
         }
       )
     })
 
     it('validates email', function(done) {
-      return this.UserUpdater.changeEmailAddress(
-        this.stubbedUser._id,
-        'foo',
-        err => {
-          should.exist(err)
-          return done()
-        }
-      )
+      this.UserUpdater.changeEmailAddress(this.stubbedUser._id, 'foo', err => {
+        should.exist(err)
+        done()
+      })
     })
 
     it('handle error', function(done) {
       this.UserUpdater.removeEmailAddress.callsArgWith(2, new Error('nope'))
-      return this.UserUpdater.changeEmailAddress(
+      this.UserUpdater.changeEmailAddress(
         this.stubbedUser._id,
         this.newEmail,
         err => {
           should.exist(err)
-          return done()
+          done()
         }
       )
     })
@@ -139,11 +162,11 @@ describe('UserUpdater', function() {
   describe('addEmailAddress', function() {
     beforeEach(function() {
       this.UserGetter.ensureUniqueEmailAddress = sinon.stub().callsArgWith(1)
-      return (this.UserUpdater.updateUser = sinon.stub().callsArgWith(2, null))
+      this.UserUpdater.updateUser = sinon.stub().callsArgWith(2, null)
     })
 
     it('add email', function(done) {
-      return this.UserUpdater.addEmailAddress(
+      this.UserUpdater.addEmailAddress(
         this.stubbedUser._id,
         this.newEmail,
         err => {
@@ -165,7 +188,7 @@ describe('UserUpdater', function() {
               }
             })
             .should.equal(true)
-          return done()
+          done()
         }
       )
     })
@@ -176,7 +199,7 @@ describe('UserUpdater', function() {
         role: 'Prof',
         department: 'Math'
       }
-      return this.UserUpdater.addEmailAddress(
+      this.UserUpdater.addEmailAddress(
         this.stubbedUser._id,
         this.newEmail,
         affiliationOptions,
@@ -187,7 +210,7 @@ describe('UserUpdater', function() {
           args[0].should.equal(this.stubbedUser._id)
           args[1].should.equal(this.newEmail)
           args[2].should.equal(affiliationOptions)
-          return done()
+          done()
         }
       )
     })
@@ -197,52 +220,47 @@ describe('UserUpdater', function() {
         .stub()
         .callsArgWith(2, new Error('nope'))
 
-      return this.UserUpdater.addEmailAddress(
+      this.UserUpdater.addEmailAddress(
         this.stubbedUser._id,
         this.newEmail,
         err => {
           this.logger.warn.called.should.equal(true)
           should.exist(err)
-          return done()
+          done()
         }
       )
     })
 
     it('handle affiliation error', function(done) {
-      const body = { errors: 'affiliation error message' }
       this.addAffiliation.callsArgWith(3, new Error('nope'))
-      return this.UserUpdater.addEmailAddress(
+      this.UserUpdater.addEmailAddress(
         this.stubbedUser._id,
         this.newEmail,
         err => {
           should.exist(err)
           this.UserUpdater.updateUser.called.should.equal(false)
-          return done()
+          done()
         }
       )
     })
 
     it('validates email', function(done) {
-      return this.UserUpdater.addEmailAddress(
-        this.stubbedUser._id,
-        'bar',
-        err => {
-          should.exist(err)
-          return done()
-        }
-      )
+      this.UserUpdater.addEmailAddress(this.stubbedUser._id, 'bar', err => {
+        should.exist(err)
+        done()
+      })
     })
   })
 
   describe('removeEmailAddress', function() {
     beforeEach(function() {
-      return (this.UserUpdater.updateUser = sinon
+      this.UserUpdater.updateUser = sinon
         .stub()
-        .callsArgWith(2, null, { nMatched: 1 }))
+        .callsArgWith(2, null, { nMatched: 1 })
     })
 
     it('remove email', function(done) {
-      return this.UserUpdater.removeEmailAddress(
+      this.UserUpdater.removeEmailAddress(
         this.stubbedUser._id,
         this.newEmail,
         err => {
@@ -253,13 +271,13 @@ describe('UserUpdater', function() {
               { $pull: { emails: { email: this.newEmail } } }
             )
             .should.equal(true)
-          return done()
+          done()
         }
       )
     })
 
     it('remove affiliation', function(done) {
-      return this.UserUpdater.removeEmailAddress(
+      this.UserUpdater.removeEmailAddress(
         this.stubbedUser._id,
         this.newEmail,
         err => {
@@ -268,7 +286,7 @@ describe('UserUpdater', function() {
           const { args } = this.removeAffiliation.lastCall
           args[0].should.equal(this.stubbedUser._id)
           args[1].should.equal(this.newEmail)
-          return done()
+          done()
         }
       )
     })
@@ -278,12 +296,12 @@ describe('UserUpdater', function() {
         .stub()
         .callsArgWith(2, new Error('nope'))
 
-      return this.UserUpdater.removeEmailAddress(
+      this.UserUpdater.removeEmailAddress(
         this.stubbedUser._id,
         this.newEmail,
         err => {
           should.exist(err)
-          return done()
+          done()
         }
       )
     })
@@ -291,38 +309,34 @@ describe('UserUpdater', function() {
     it('handle missed update', function(done) {
       this.UserUpdater.updateUser = sinon.stub().callsArgWith(2, null, { n: 0 })
 
-      return this.UserUpdater.removeEmailAddress(
+      this.UserUpdater.removeEmailAddress(
         this.stubbedUser._id,
         this.newEmail,
         err => {
           should.exist(err)
-          return done()
+          done()
         }
       )
     })
 
     it('handle affiliation error', function(done) {
       this.removeAffiliation.callsArgWith(2, new Error('nope'))
-      return this.UserUpdater.removeEmailAddress(
+      this.UserUpdater.removeEmailAddress(
         this.stubbedUser._id,
         this.newEmail,
         err => {
           should.exist(err)
           this.UserUpdater.updateUser.called.should.equal(false)
-          return done()
+          done()
         }
       )
     })
 
     it('validates email', function(done) {
-      return this.UserUpdater.removeEmailAddress(
-        this.stubbedUser._id,
-        'baz',
-        err => {
-          should.exist(err)
-          return done()
-        }
-      )
+      this.UserUpdater.removeEmailAddress(this.stubbedUser._id, 'baz', err => {
+        should.exist(err)
+        done()
+      })
     })
   })
 
@@ -462,13 +476,11 @@ describe('UserUpdater', function() {
 
   describe('confirmEmail', function() {
     beforeEach(function() {
-      return (this.UserUpdater.updateUser = sinon
-        .stub()
-        .callsArgWith(2, null, { n: 1 }))
+      this.UserUpdater.updateUser = sinon.stub().callsArgWith(2, null, { n: 1 })
     })
 
     it('should update the email record', function(done) {
-      return this.UserUpdater.confirmEmail(
+      this.UserUpdater.confirmEmail(
         this.stubbedUser._id,
         this.newEmail,
         err => {
@@ -479,13 +491,13 @@ describe('UserUpdater', function() {
               { $set: { 'emails.$.confirmedAt': new Date() } }
             )
             .should.equal(true)
-          return done()
+          done()
         }
       )
     })
 
     it('add affiliation', function(done) {
-      return this.UserUpdater.confirmEmail(
+      this.UserUpdater.confirmEmail(
         this.stubbedUser._id,
         this.newEmail,
         err => {
@@ -497,7 +509,7 @@ describe('UserUpdater', function() {
             this.newEmail,
             { confirmedAt: new Date() }
           )
-          return done()
+          done()
         }
       )
     })
@@ -507,12 +519,12 @@ describe('UserUpdater', function() {
         .stub()
         .callsArgWith(2, new Error('nope'))
 
-      return this.UserUpdater.confirmEmail(
+      this.UserUpdater.confirmEmail(
         this.stubbedUser._id,
         this.newEmail,
         err => {
           should.exist(err)
-          return done()
+          done()
         }
       )
     })
@@ -520,44 +532,44 @@ describe('UserUpdater', function() {
     it('handle missed update', function(done) {
       this.UserUpdater.updateUser = sinon.stub().callsArgWith(2, null, { n: 0 })
 
-      return this.UserUpdater.confirmEmail(
+      this.UserUpdater.confirmEmail(
         this.stubbedUser._id,
         this.newEmail,
         err => {
           should.exist(err)
-          return done()
+          done()
         }
       )
     })
 
     it('validates email', function(done) {
-      return this.UserUpdater.confirmEmail(this.stubbedUser._id, '@', err => {
+      this.UserUpdater.confirmEmail(this.stubbedUser._id, '@', err => {
         should.exist(err)
-        return done()
+        done()
       })
     })
 
     it('handle affiliation error', function(done) {
       this.addAffiliation.callsArgWith(3, new Error('nope'))
-      return this.UserUpdater.confirmEmail(
+      this.UserUpdater.confirmEmail(
         this.stubbedUser._id,
         this.newEmail,
         err => {
           should.exist(err)
           this.UserUpdater.updateUser.called.should.equal(false)
-          return done()
+          done()
         }
       )
     })
 
     it('refresh features', function(done) {
-      return this.UserUpdater.confirmEmail(
+      this.UserUpdater.confirmEmail(
         this.stubbedUser._id,
         this.newEmail,
         err => {
           should.not.exist(err)
           sinon.assert.calledWith(this.refreshFeatures, this.stubbedUser._id)
-          return done()
+          done()
         }
       )
     })