From 4b1babd4ea634ab12c54e9a9aea7db0e8a500941 Mon Sep 17 00:00:00 2001 From: Jimmy Domagala-Tang Date: Mon, 3 Feb 2025 11:32:56 -0500 Subject: [PATCH] Merge pull request #22965 from overleaf/jdt-async-institution-feats Async await institution features utility GitOrigin-RevId: ef24a189aab46d065925405a795709c94ff3d0b3 --- .../Institutions/InstitutionsFeatures.js | 69 +++++++------------ .../Institutions/InstitutionsFeaturesTests.js | 42 ++++++----- 2 files changed, 49 insertions(+), 62 deletions(-) diff --git a/services/web/app/src/Features/Institutions/InstitutionsFeatures.js b/services/web/app/src/Features/Institutions/InstitutionsFeatures.js index ab5125114e..0b33fe15d9 100644 --- a/services/web/app/src/Features/Institutions/InstitutionsFeatures.js +++ b/services/web/app/src/Features/Institutions/InstitutionsFeatures.js @@ -1,49 +1,32 @@ -let InstitutionsFeatures +const { callbackifyAll } = require('@overleaf/promise-utils') const UserGetter = require('../User/UserGetter') const PlansLocator = require('../Subscription/PlansLocator') const Settings = require('@overleaf/settings') -const { promisifyAll } = require('@overleaf/promise-utils') -module.exports = InstitutionsFeatures = { - getInstitutionsFeatures(userId, callback) { - InstitutionsFeatures.getInstitutionsPlan( - userId, - function (error, planCode) { - if (error) { - return callback(error) - } - const plan = planCode && PlansLocator.findLocalPlanInSettings(planCode) - const features = plan && plan.features - callback(null, features || {}) - } - ) - }, - - getInstitutionsPlan(userId, callback) { - InstitutionsFeatures.hasLicence(userId, function (error, hasLicence) { - if (error) { - return callback(error) - } - if (!hasLicence) { - return callback(null, null) - } - callback(null, Settings.institutionPlanCode) - }) - }, - - hasLicence(userId, callback) { - UserGetter.getUserFullEmails(userId, function (error, emailsData) { - if (error) { - return callback(error) - } - - const hasLicence = emailsData.some( - emailData => emailData.emailHasInstitutionLicence - ) - - callback(null, hasLicence) - }) - }, +async function getInstitutionsFeatures(userId) { + const planCode = await getInstitutionsPlan(userId) + const plan = planCode && PlansLocator.findLocalPlanInSettings(planCode) + const features = plan && plan.features + return features || {} } -module.exports.promises = promisifyAll(module.exports) +async function getInstitutionsPlan(userId) { + if (await hasLicence(userId)) { + return Settings.institutionPlanCode + } + return null +} + +async function hasLicence(userId) { + const emailsData = await UserGetter.promises.getUserFullEmails(userId) + return emailsData.some(emailData => emailData.emailHasInstitutionLicence) +} +const InstitutionsFeatures = { + getInstitutionsFeatures, + getInstitutionsPlan, + hasLicence, +} +module.exports = { + promises: InstitutionsFeatures, + ...callbackifyAll(InstitutionsFeatures), +} diff --git a/services/web/test/unit/src/Institutions/InstitutionsFeaturesTests.js b/services/web/test/unit/src/Institutions/InstitutionsFeaturesTests.js index deaa07a0b3..69def3076f 100644 --- a/services/web/test/unit/src/Institutions/InstitutionsFeaturesTests.js +++ b/services/web/test/unit/src/Institutions/InstitutionsFeaturesTests.js @@ -21,7 +21,9 @@ const modulePath = require('path').join( describe('InstitutionsFeatures', function () { beforeEach(function () { - this.UserGetter = { getUserFullEmails: sinon.stub() } + this.UserGetter = { + promises: { getUserFullEmails: sinon.stub().resolves([]) }, + } this.PlansLocator = { findLocalPlanInSettings: sinon.stub() } this.institutionPlanCode = 'institution_plan_code' this.InstitutionsFeatures = SandboxedModule.require(modulePath, { @@ -33,13 +35,14 @@ describe('InstitutionsFeatures', function () { }, }, }) - + this.emailDataWithLicense = [{ emailHasInstitutionLicence: true }] + this.emailDataWithoutLicense = [{ emailHasInstitutionLicence: false }] return (this.userId = '12345abcde') }) describe('hasLicence', function () { it('should handle error', function (done) { - this.UserGetter.getUserFullEmails.yields(new Error('Nope')) + this.UserGetter.promises.getUserFullEmails.rejects(new Error('Nope')) return this.InstitutionsFeatures.hasLicence( this.userId, (error, hasLicence) => { @@ -50,8 +53,9 @@ describe('InstitutionsFeatures', function () { }) it('should return false if user has no paid affiliations', function (done) { - const emailData = [{ emailHasInstitutionLicence: false }] - this.UserGetter.getUserFullEmails.yields(null, emailData) + this.UserGetter.promises.getUserFullEmails.resolves( + this.emailDataWithoutLicense + ) return this.InstitutionsFeatures.hasLicence( this.userId, (error, hasLicence) => { @@ -67,7 +71,7 @@ describe('InstitutionsFeatures', function () { { emailHasInstitutionLicence: true }, { emailHasInstitutionLicence: false }, ] - this.UserGetter.getUserFullEmails.yields(null, emailData) + this.UserGetter.promises.getUserFullEmails.resolves(emailData) return this.InstitutionsFeatures.hasLicence( this.userId, (error, hasLicence) => { @@ -81,7 +85,6 @@ describe('InstitutionsFeatures', function () { describe('getInstitutionsFeatures', function () { beforeEach(function () { - this.InstitutionsFeatures.getInstitutionsPlan = sinon.stub() this.testFeatures = { features: { institution: 'all' } } return this.PlansLocator.findLocalPlanInSettings .withArgs(this.institutionPlanCode) @@ -89,7 +92,7 @@ describe('InstitutionsFeatures', function () { }) it('should handle error', function (done) { - this.InstitutionsFeatures.getInstitutionsPlan.yields(new Error('Nope')) + this.UserGetter.promises.getUserFullEmails.rejects(new Error('Nope')) return this.InstitutionsFeatures.getInstitutionsFeatures( this.userId, (error, features) => { @@ -100,7 +103,9 @@ describe('InstitutionsFeatures', function () { }) it('should return no feaures if user has no plan code', function (done) { - this.InstitutionsFeatures.getInstitutionsPlan.yields(null, null) + this.UserGetter.promises.getUserFullEmails.resolves( + this.emailDataWithoutLicense + ) return this.InstitutionsFeatures.getInstitutionsFeatures( this.userId, (error, features) => { @@ -112,9 +117,8 @@ describe('InstitutionsFeatures', function () { }) it('should return feaures if user has affiliations plan code', function (done) { - this.InstitutionsFeatures.getInstitutionsPlan.yields( - null, - this.institutionPlanCode + this.UserGetter.promises.getUserFullEmails.resolves( + this.emailDataWithLicense ) return this.InstitutionsFeatures.getInstitutionsFeatures( this.userId, @@ -128,12 +132,8 @@ describe('InstitutionsFeatures', function () { }) describe('getInstitutionsPlan', function () { - beforeEach(function () { - return (this.InstitutionsFeatures.hasLicence = sinon.stub()) - }) - it('should handle error', function (done) { - this.InstitutionsFeatures.hasLicence.yields(new Error('Nope')) + this.UserGetter.promises.getUserFullEmails.rejects(new Error('Nope')) return this.InstitutionsFeatures.getInstitutionsPlan( this.userId, error => { @@ -144,7 +144,9 @@ describe('InstitutionsFeatures', function () { }) it('should return no plan if user has no licence', function (done) { - this.InstitutionsFeatures.hasLicence.yields(null, false) + this.UserGetter.promises.getUserFullEmails.resolves( + this.emailDataWithoutLicense + ) return this.InstitutionsFeatures.getInstitutionsPlan( this.userId, (error, plan) => { @@ -156,7 +158,9 @@ describe('InstitutionsFeatures', function () { }) it('should return plan if user has licence', function (done) { - this.InstitutionsFeatures.hasLicence.yields(null, true) + this.UserGetter.promises.getUserFullEmails.resolves( + this.emailDataWithLicense + ) return this.InstitutionsFeatures.getInstitutionsPlan( this.userId, (error, plan) => {