Merge pull request #3467 from overleaf/jel-async-getUserFullEmails

Convert getUserFullEmails to async

GitOrigin-RevId: 88e81460a7cc5703eb900e81f7cf594aeb204932

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

@@ -1,13 +1,43 @@
+const { callbackify } = require('util')
 const { db } = require('../../infrastructure/mongodb')
 const metrics = require('@overleaf/metrics')
 const logger = require('logger-sharelatex')
 const { promisifyAll } = require('../../util/promises')
-const { getUserAffiliations } = require('../Institutions/InstitutionsAPI')
+const {
+  promises: InstitutionsAPIPromises
+} = require('../Institutions/InstitutionsAPI')
 const InstitutionsHelper = require('../Institutions/InstitutionsHelper')
 const Errors = require('../Errors/Errors')
 const Features = require('../../infrastructure/Features')
 const { normalizeQuery, normalizeMultiQuery } = require('../Helpers/Mongo')
 
+async function getUserFullEmails(userId) {
+  const user = await UserGetter.promises.getUser(userId, {
+    email: 1,
+    emails: 1,
+    samlIdentifiers: 1
+  })
+
+  if (!user) {
+    throw new Error('User not Found')
+  }
+
+  if (!Features.hasFeature('affiliations')) {
+    return decorateFullEmails(user.email, user.emails, [], [])
+  }
+
+  const affiliationsData = await InstitutionsAPIPromises.getUserAffiliations(
+    userId
+  )
+
+  return decorateFullEmails(
+    user.email,
+    user.emails || [],
+    affiliationsData,
+    user.samlIdentifiers || []
+  )
+}
+
 const UserGetter = {
   getUser(query, projection, callback) {
     if (arguments.length === 2) {
@@ -28,41 +58,7 @@ const UserGetter = {
     )
   },
 
-  getUserFullEmails(userId, callback) {
+  getUserFullEmails: callbackify(getUserFullEmails),
-    this.getUser(userId, { email: 1, emails: 1, samlIdentifiers: 1 }, function(
-      error,
-      user
-    ) {
-      if (error) {
-        return callback(error)
-      }
-      if (!user) {
-        return callback(new Error('User not Found'))
-      }
-
-      if (!Features.hasFeature('affiliations')) {
-        return callback(
-          null,
-          decorateFullEmails(user.email, user.emails, [], [])
-        )
-      }
-
-      getUserAffiliations(userId, function(error, affiliationsData) {
-        if (error) {
-          return callback(error)
-        }
-        callback(
-          null,
-          decorateFullEmails(
-            user.email,
-            user.emails || [],
-            affiliationsData,
-            user.samlIdentifiers || []
-          )
-        )
-      })
-    })
-  },
 
   getUserByMainEmail(email, projection, callback) {
     email = email.trim()
@@ -204,5 +200,9 @@ var decorateFullEmails = (
   metrics.timeAsyncMethod(UserGetter, method, 'mongo.UserGetter', logger)
 )
 
-UserGetter.promises = promisifyAll(UserGetter)
+UserGetter.promises = promisifyAll(UserGetter, {
+  without: ['getUserFullEmails']
+})
+UserGetter.promises.getUserFullEmails = getUserFullEmails
+
 module.exports = UserGetter

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

@@ -42,7 +42,7 @@ describe('UserGetter', function() {
       ObjectId
     }
     const settings = { apis: { v1: { url: 'v1.url', user: '', pass: '' } } }
-    this.getUserAffiliations = sinon.stub().callsArgWith(1, null, [])
+    this.getUserAffiliations = sinon.stub().resolves([])
 
     this.UserGetter = SandboxedModule.require(modulePath, {
       globals: {
@@ -59,7 +59,9 @@ describe('UserGetter', function() {
         },
         'settings-sharelatex': settings,
         '../Institutions/InstitutionsAPI': {
-          getUserAffiliations: this.getUserAffiliations
+          promises: {
+            getUserAffiliations: this.getUserAffiliations
+          }
         },
         '../../infrastructure/Features': {
           hasFeature: sinon.stub().returns(true)
@@ -116,16 +118,14 @@ describe('UserGetter', function() {
 
   describe('getUserFullEmails', function() {
     it('should get user', function(done) {
-      this.UserGetter.getUser = sinon
+      this.UserGetter.promises.getUser = sinon.stub().resolves(this.fakeUser)
-        .stub()
-        .callsArgWith(2, null, this.fakeUser)
       const projection = { email: 1, emails: 1, samlIdentifiers: 1 }
       this.UserGetter.getUserFullEmails(
         this.fakeUser._id,
         (error, fullEmails) => {
           expect(error).to.not.exist
-          this.UserGetter.getUser.called.should.equal(true)
+          this.UserGetter.promises.getUser.called.should.equal(true)
-          this.UserGetter.getUser
+          this.UserGetter.promises.getUser
             .calledWith(this.fakeUser._id, projection)
             .should.equal(true)
           done()
@@ -134,9 +134,7 @@ describe('UserGetter', function() {
     })
 
     it('should fetch emails data', function(done) {
-      this.UserGetter.getUser = sinon
+      this.UserGetter.promises.getUser = sinon.stub().resolves(this.fakeUser)
-        .stub()
-        .callsArgWith(2, null, this.fakeUser)
       this.UserGetter.getUserFullEmails(
         this.fakeUser._id,
         (error, fullEmails) => {
@@ -162,9 +160,7 @@ describe('UserGetter', function() {
     })
 
     it('should merge affiliation data', function(done) {
-      this.UserGetter.getUser = sinon
+      this.UserGetter.promises.getUser = sinon.stub().resolves(this.fakeUser)
-        .stub()
-        .callsArgWith(2, null, this.fakeUser)
       const affiliationsData = [
         {
           email: 'email1@foo.bar',
@@ -179,7 +175,7 @@ describe('UserGetter', function() {
           }
         }
       ]
-      this.getUserAffiliations.callsArgWith(1, null, affiliationsData)
+      this.getUserAffiliations.resolves(affiliationsData)
       this.UserGetter.getUserFullEmails(
         this.fakeUser._id,
         (error, fullEmails) => {
@@ -218,8 +214,8 @@ describe('UserGetter', function() {
       const fakeUserWithSaml = this.fakeUser
       fakeUserWithSaml.emails[0].samlProviderId = 'saml_id'
       fakeUserWithSaml.samlIdentifiers = fakeSamlIdentifiers
-      this.UserGetter.getUser = sinon.stub().yields(null, this.fakeUser)
+      this.UserGetter.promises.getUser = sinon.stub().resolves(this.fakeUser)
-      this.getUserAffiliations.callsArgWith(1, null, [])
+      this.getUserAffiliations.resolves([])
       this.UserGetter.getUserFullEmails(
         this.fakeUser._id,
         (error, fullEmails) => {
@@ -251,16 +247,14 @@ describe('UserGetter', function() {
         _id: '12390i',
         email: 'email2@foo.bar'
       }
-      this.UserGetter.getUser = sinon
+      this.UserGetter.promises.getUser = sinon.stub().resolves(this.fakeUser)
-        .stub()
-        .callsArgWith(2, null, this.fakeUser)
       const projection = { email: 1, emails: 1, samlIdentifiers: 1 }
       this.UserGetter.getUserFullEmails(
         this.fakeUser._id,
         (error, fullEmails) => {
           expect(error).to.not.exist
-          this.UserGetter.getUser.called.should.equal(true)
+          this.UserGetter.promises.getUser.called.should.equal(true)
-          this.UserGetter.getUser
+          this.UserGetter.promises.getUser
             .calledWith(this.fakeUser._id, projection)
             .should.equal(true)
           assert.deepEqual(fullEmails, [])