From 4781c0bc3c6ded8d6fcd41f26c81a4993b3324e5 Mon Sep 17 00:00:00 2001 From: Jakob Ackermann Date: Fri, 11 Dec 2020 10:40:38 +0000 Subject: [PATCH] Merge pull request #3467 from overleaf/jel-async-getUserFullEmails Convert getUserFullEmails to async GitOrigin-RevId: 88e81460a7cc5703eb900e81f7cf594aeb204932 --- .../web/app/src/Features/User/UserGetter.js | 74 +++++++++---------- .../web/test/unit/src/User/UserGetterTests.js | 36 ++++----- 2 files changed, 52 insertions(+), 58 deletions(-) diff --git a/services/web/app/src/Features/User/UserGetter.js b/services/web/app/src/Features/User/UserGetter.js index 5b19c23d36..d35fb418b9 100644 --- a/services/web/app/src/Features/User/UserGetter.js +++ b/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) { - 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 || [] - ) - ) - }) - }) - }, + getUserFullEmails: callbackify(getUserFullEmails), 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 diff --git a/services/web/test/unit/src/User/UserGetterTests.js b/services/web/test/unit/src/User/UserGetterTests.js index 054fa7505a..8373b8dc67 100644 --- a/services/web/test/unit/src/User/UserGetterTests.js +++ b/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 - .stub() - .callsArgWith(2, null, this.fakeUser) + this.UserGetter.promises.getUser = sinon.stub().resolves(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.getUser + this.UserGetter.promises.getUser.called.should.equal(true) + 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 - .stub() - .callsArgWith(2, null, this.fakeUser) + this.UserGetter.promises.getUser = sinon.stub().resolves(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 - .stub() - .callsArgWith(2, null, this.fakeUser) + this.UserGetter.promises.getUser = sinon.stub().resolves(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.getUserAffiliations.callsArgWith(1, null, []) + this.UserGetter.promises.getUser = sinon.stub().resolves(this.fakeUser) + 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 - .stub() - .callsArgWith(2, null, this.fakeUser) + this.UserGetter.promises.getUser = sinon.stub().resolves(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.getUser + this.UserGetter.promises.getUser.called.should.equal(true) + this.UserGetter.promises.getUser .calledWith(this.fakeUser._id, projection) .should.equal(true) assert.deepEqual(fullEmails, [])