diff --git a/services/web/app/src/Features/User/UserCreator.js b/services/web/app/src/Features/User/UserCreator.js index b75a88ce6b..e82e56781f 100644 --- a/services/web/app/src/Features/User/UserCreator.js +++ b/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('') + ] + + user = await user.save() + + 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" + ) } - ] - - return user.save(function(err) { - callback(err, user) - - if (options != null ? options.skip_affiliation : undefined) { - return - } - // 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 diff --git a/services/web/test/unit/src/User/UserCreatorTests.js b/services/web/test/unit/src/User/UserCreatorTests.js index 3f00ecad35..7cfa405fd4 100644 --- a/services/web/test/unit/src/User/UserCreatorTests.js +++ b/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 chai = require('chai') -const assert = require('assert') -const should = chai.should() -const modulePath = '../../../../app/src/Features/User/UserCreator.js' const SandboxedModule = require('sandboxed-module') +const chai = require('chai') +const sinon = require('sinon') + +const assert = chai.assert +const modulePath = '../../../../app/src/Features/User/UserCreator.js' 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) + }) }) }) })