convert UserCreator to async/promises

GitOrigin-RevId: a040b9dd65aa834d1dd9e552ffc1434a17c43eae

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

@@ -1,87 +1,58 @@
-/* eslint-disable
-    handle-callback-err,
-    max-len,
-*/
-// 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
- * DS207: Consider shorter variations of null checks
- * Full docs: https://github.com/decaffeinate/decaffeinate/blob/master/docs/suggestions.md
- */
-let UserCreator
-const { User } = require('../../models/User')
 const logger = require('logger-sharelatex')
-const metrics = require('metrics-sharelatex')
+const util = require('util')
+const { User } = require('../../models/User')
 const { addAffiliation } = require('../Institutions/InstitutionsAPI')
 
-module.exports = UserCreator = {
-  createNewUser(attributes, options, callback) {
-    if (callback == null) {
-      callback = function(error, user) {}
-    }
-    if (arguments.length === 2) {
-      callback = options
-      options = {}
-    }
-    logger.log({ user: attributes }, 'creating new user')
-    const user = new User()
+async function createNewUser(attributes, options = {}) {
+  let user = new User()
 
-    const username = attributes.email.match(/^[^@]*/)
-    if (attributes.first_name == null || attributes.first_name === '') {
-      attributes.first_name = username[0]
-    }
+  if (attributes.first_name == null || attributes.first_name === '') {
+    attributes.first_name = attributes.email.split('@')[0]
+  }
 
-    for (let key in attributes) {
-      const value = attributes[key]
-      user[key] = value
-    }
+  Object.assign(user, attributes)
 
-    user.ace.syntaxValidation = true
-    if (user.featureSwitches != null) {
-      user.featureSwitches.pdfng = true
+  user.ace.syntaxValidation = true
+  if (user.featureSwitches != null) {
+    user.featureSwitches.pdfng = true
+  }
+
+  const reversedHostname = user.email
+    .split('@')[1]
+    .split('')
+    .reverse()
+    .join('')
+
+  user.emails = [
+    {
+      email: user.email,
+      createdAt: new Date(),
+      reversedHostname
     }
-    user.emails = [
-      {
-        email: user.email,
-        createdAt: new Date(),
-        reversedHostname: user.email
-          .split('@')[1]
-          .split('')
-          .reverse()
-          .join('')
-      }
-    ]
+  ]
 
-    return user.save(function(err) {
-      callback(err, user)
+  user = await user.save()
 
-      if (options != null ? options.skip_affiliation : undefined) {
-        return
+  if (!options.skip_affiliation) {
+    // There is no guarantee this will complete so we must not rely on it
+    addAffiliation(user._id, user.email, err => {
+      if (err) {
+        logger.error(
+          { userId: user._id, email: user.email },
+          "couldn't add affiliation for user on create"
+        )
       }
-      // call addaffiliation after the main callback so it runs in the
-      // background. There is no guaranty this will run so we must no rely on it
-      return addAffiliation(user._id, user.email, function(error) {
-        if (error) {
-          return logger.log(
-            { userId: user._id, email: user.email, error },
-            "couldn't add affiliation for user on create"
-          )
-        } else {
-          return logger.log(
-            { userId: user._id, email: user.email },
-            'added affiliation for user on create'
-          )
-        }
-      })
     })
   }
+
+  return user
+}
+
+const UserCreator = {
+  createNewUser: util.callbackify(createNewUser),
+  promises: {
+    createNewUser: createNewUser
+  }
 }
 
-metrics.timeAsyncMethod(
-  UserCreator,
-  'createNewUser',
-  'mongo.UserCreator',
-  logger
-)
+module.exports = UserCreator

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

@@ -1,30 +1,16 @@
-/* eslint-disable
-    handle-callback-err,
-    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 sinon = require('sinon')
+const SandboxedModule = require('sandboxed-module')
 const chai = require('chai')
-const assert = require('assert')
-const should = chai.should()
+const sinon = require('sinon')
+
+const assert = chai.assert
 const modulePath = '../../../../app/src/Features/User/UserCreator.js'
-const SandboxedModule = require('sandboxed-module')
 
 describe('UserCreator', function() {
   beforeEach(function() {
-    let Project
     const self = this
     this.user = { _id: '12390i', ace: {} }
-    this.user.save = sinon.stub().callsArgWith(0)
-    this.UserModel = Project = class Project {
+    this.user.save = sinon.stub().resolves(self.user)
+    this.UserModel = class Project {
       constructor() {
         return self.user
       }
@@ -48,107 +34,134 @@ describe('UserCreator', function() {
       }
     })
 
-    return (this.email = 'bob.oswald@gmail.com')
+    this.email = 'bob.oswald@gmail.com'
   })
 
   describe('createNewUser', function() {
-    it('should take the opts and put them in the model', function(done) {
-      const opts = {
-        email: this.email,
-        holdingAccount: true
-      }
-      return this.UserCreator.createNewUser(opts, (err, user) => {
-        assert.equal(user.email, this.email)
-        assert.equal(user.holdingAccount, true)
-        assert.equal(user.first_name, 'bob.oswald')
-        return done()
+    describe('with callbacks', function() {
+      it('should take the opts and put them in the model', function(done) {
+        const opts = {
+          email: this.email,
+          holdingAccount: true
+        }
+        this.UserCreator.createNewUser(opts, (err, user) => {
+          assert.ifError(err)
+          assert.equal(user.email, this.email)
+          assert.equal(user.holdingAccount, true)
+          assert.equal(user.first_name, 'bob.oswald')
+          done()
+        })
       })
-    })
 
-    it('should use the start of the email if the first name is empty string', function(done) {
-      const opts = {
-        email: this.email,
-        holdingAccount: true,
-        first_name: ''
-      }
-      return this.UserCreator.createNewUser(opts, (err, user) => {
-        assert.equal(user.email, this.email)
-        assert.equal(user.holdingAccount, true)
-        assert.equal(user.first_name, 'bob.oswald')
-        return done()
+      it('should use the start of the email if the first name is empty string', function(done) {
+        const opts = {
+          email: this.email,
+          holdingAccount: true,
+          first_name: ''
+        }
+        this.UserCreator.createNewUser(opts, (err, user) => {
+          assert.ifError(err)
+          assert.equal(user.email, this.email)
+          assert.equal(user.holdingAccount, true)
+          assert.equal(user.first_name, 'bob.oswald')
+          done()
+        })
       })
-    })
 
-    it('should use the first name if passed', function(done) {
-      const opts = {
-        email: this.email,
-        holdingAccount: true,
-        first_name: 'fiiirstname'
-      }
-      return this.UserCreator.createNewUser(opts, (err, user) => {
-        assert.equal(user.email, this.email)
-        assert.equal(user.holdingAccount, true)
-        assert.equal(user.first_name, 'fiiirstname')
-        return done()
+      it('should use the first name if passed', function(done) {
+        const opts = {
+          email: this.email,
+          holdingAccount: true,
+          first_name: 'fiiirstname'
+        }
+        this.UserCreator.createNewUser(opts, (err, user) => {
+          assert.ifError(err)
+          assert.equal(user.email, this.email)
+          assert.equal(user.holdingAccount, true)
+          assert.equal(user.first_name, 'fiiirstname')
+          done()
+        })
       })
-    })
 
-    it('should use the last name if passed', function(done) {
-      const opts = {
-        email: this.email,
-        holdingAccount: true,
-        last_name: 'lastNammmmeee'
-      }
-      return this.UserCreator.createNewUser(opts, (err, user) => {
-        assert.equal(user.email, this.email)
-        assert.equal(user.holdingAccount, true)
-        assert.equal(user.last_name, 'lastNammmmeee')
-        return done()
+      it('should use the last name if passed', function(done) {
+        const opts = {
+          email: this.email,
+          holdingAccount: true,
+          last_name: 'lastNammmmeee'
+        }
+        this.UserCreator.createNewUser(opts, (err, user) => {
+          assert.ifError(err)
+          assert.equal(user.email, this.email)
+          assert.equal(user.holdingAccount, true)
+          assert.equal(user.last_name, 'lastNammmmeee')
+          done()
+        })
       })
-    })
 
-    it('should set emails attribute', function(done) {
-      return this.UserCreator.createNewUser(
-        { email: this.email },
-        (err, user) => {
+      it('should set emails attribute', function(done) {
+        this.UserCreator.createNewUser({ email: this.email }, (err, user) => {
+          assert.ifError(err)
           user.email.should.equal(this.email)
           user.emails.length.should.equal(1)
           user.emails[0].email.should.equal(this.email)
           user.emails[0].createdAt.should.be.a('date')
           user.emails[0].reversedHostname.should.equal('moc.liamg')
-          return done()
-        }
-      )
-    })
+          done()
+        })
+      })
 
-    it('should add affiliation in background', function(done) {
-      return this.UserCreator.createNewUser(
-        { email: this.email },
-        (err, user) => {
-          // addaffiliation should not be called before the callback but only after
-          // a tick of the event loop
-          sinon.assert.notCalled(this.addAffiliation)
-          return process.nextTick(() => {
-            sinon.assert.calledWith(this.addAffiliation, user._id, user.email)
-            return done()
-          })
-        }
-      )
-    })
+      it('should add affiliation', 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
+          )
+          done()
+        })
+      })
 
-    it('should not add affiliation if skipping', function(done) {
-      const attributes = { email: this.email }
-      const options = { skip_affiliation: true }
-      return this.UserCreator.createNewUser(
-        attributes,
-        options,
-        (err, user) => {
-          return process.nextTick(() => {
+      it('should not add affiliation if skipping', function(done) {
+        const attributes = { email: this.email }
+        const options = { skip_affiliation: true }
+        this.UserCreator.createNewUser(attributes, options, (err, user) => {
+          assert.ifError(err)
+          process.nextTick(() => {
             sinon.assert.notCalled(this.addAffiliation)
-            return done()
+            done()
           })
+        })
+      })
+    })
+
+    describe('with promises', function() {
+      it('should take the opts and put them in the model', async function() {
+        const opts = {
+          email: this.email,
+          holdingAccount: true
         }
-      )
+        const user = await this.UserCreator.promises.createNewUser(opts)
+        assert.equal(user.email, this.email)
+        assert.equal(user.holdingAccount, true)
+        assert.equal(user.first_name, 'bob.oswald')
+      })
+
+      it('should add affiliation', async function() {
+        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)
+      })
+
+      it('should not add affiliation if skipping', async function() {
+        const attributes = { email: this.email }
+        const opts = { skip_affiliation: true }
+        await this.UserCreator.promises.createNewUser(attributes, opts)
+        sinon.assert.notCalled(this.addAffiliation)
+      })
     })
   })
 })