From cf1f3fa7e5dd4d1b56245edcdf0589e0614fd77e Mon Sep 17 00:00:00 2001 From: David <33458145+davidmcpowell@users.noreply.github.com> Date: Wed, 28 Feb 2024 11:20:26 +0000 Subject: [PATCH] Merge pull request #17206 from overleaf/dp-mongoose-callback-institutions-manager Promisify InsitutionsManager and InstitutionsManagerTests GitOrigin-RevId: 0872aabb646ee143f51a92608f91d7901723ec0c --- .../Institutions/InstitutionsManager.js | 595 ++++++++---------- .../Notifications/NotificationsBuilder.js | 3 + services/web/app/src/models/Institution.js | 6 + .../Institutions/InstitutionsManagerTests.js | 292 ++++----- 4 files changed, 405 insertions(+), 491 deletions(-) diff --git a/services/web/app/src/Features/Institutions/InstitutionsManager.js b/services/web/app/src/Features/Institutions/InstitutionsManager.js index a410314e54..f7c1fc6c52 100644 --- a/services/web/app/src/Features/Institutions/InstitutionsManager.js +++ b/services/web/app/src/Features/Institutions/InstitutionsManager.js @@ -1,15 +1,12 @@ -const async = require('async') -const { callbackify, promisify } = require('util') +const { + callbackifyAll, + promiseMapWithLimit, +} = require('@overleaf/promise-utils') const { ObjectId } = require('mongodb') const Settings = require('@overleaf/settings') const logger = require('@overleaf/logger') const { fetchJson } = require('@overleaf/fetch-utils') -const { - getInstitutionAffiliations, - getConfirmedInstitutionAffiliations, - addAffiliation, - promises: InstitutionsAPIPromises, -} = require('./InstitutionsAPI') +const InstitutionsAPI = require('./InstitutionsAPI') const FeaturesUpdater = require('../Subscription/FeaturesUpdater') const FeaturesHelper = require('../Subscription/FeaturesHelper') const UserGetter = require('../User/UserGetter') @@ -78,8 +75,50 @@ async function _checkUsersFeatures(userIds) { return result } -async function checkInstitutionUsers(institutionId, emitNonProUserIds) { - /* +const InstitutionsManager = { + async clearInstitutionNotifications(institutionId, dryRun) { + async function clear(key) { + const run = dryRun + ? NotificationsHandler.promises.previewMarkAsReadByKeyOnlyBulk + : NotificationsHandler.promises.markAsReadByKeyOnlyBulk + + return await run(key) + } + + const ipMatcherAffiliation = await clear( + `ip-matched-affiliation-${institutionId}` + ) + const featuresUpgradedByAffiliation = await clear( + `features-updated-by=${institutionId}` + ) + const redundantPersonalSubscription = await clear( + `redundant-personal-subscription-${institutionId}` + ) + + return { + ipMatcherAffiliation, + featuresUpgradedByAffiliation, + redundantPersonalSubscription, + } + }, + + async refreshInstitutionUsers(institutionId, notify) { + const refreshFunction = notify ? refreshFeaturesAndNotify : refreshFeatures + + const { institution, affiliations } = await fetchInstitutionAndAffiliations( + institutionId + ) + + for (const affiliation of affiliations) { + affiliation.institutionName = institution.name + affiliation.institutionId = institutionId + } + + await promiseMapWithLimit(ASYNC_LIMIT, affiliations, refreshFunction) + }, + + async checkInstitutionUsers(institutionId, emitNonProUserIds) { + /* v1 has affiliation data. Via getInstitutionAffiliationsCounts, v1 will send lapsed_user_ids, which includes all user types (not linked, linked and entitled, linked not entitled). @@ -88,357 +127,245 @@ async function checkInstitutionUsers(institutionId, emitNonProUserIds) { lapsed count into SSO (entitled and not) or just email users */ - const result = { - emailUsers: { - total: 0, // v1 all users - v2 all SSO users - current: 0, // v1 current - v1 SSO entitled - (v2 calculated not entitled current) - lapsed: 0, // v1 lapsed user IDs that are not in v2 SSO users - pro: { - current: 0, - lapsed: 0, - }, - nonPro: { - current: 0, - lapsed: 0, - }, - }, - ssoUsers: { - total: 0, // only v2 - current: { - entitled: 0, // only v1 - notEntitled: 0, // v2 non-entitled SSO users - v1 lapsed user IDs - }, - lapsed: 0, // v2 SSO users that are in v1 lapsed user IDs - pro: { - current: 0, - lapsed: 0, - }, - nonPro: { - current: 0, - lapsed: 0, - }, - }, - } - - const { - user_ids: userIds, // confirmed and not removed users. Includes users with lapsed reconfirmations - current_users_count: currentUsersCount, // all users not with lapsed reconfirmations - lapsed_user_ids: lapsedUserIds, // includes all user types that did not reconfirm (sso entitled, sso not entitled, email only) - with_confirmed_email: withConfirmedEmail, // same count as affiliation metrics - entitled_via_sso: entitled, // same count as affiliation metrics - } = await InstitutionsAPIPromises.getInstitutionAffiliationsCounts( - institutionId - ) - result.ssoUsers.current.entitled = entitled - - const { allSsoUsers, allSsoUsersByIds, currentNotEntitledCount } = - await _getSsoUsers(institutionId, lapsedUserIds) - result.ssoUsers.total = allSsoUsers.length - result.ssoUsers.current.notEntitled = currentNotEntitledCount - - // check if lapsed user ID an SSO user - const lapsedUsersByIds = {} - lapsedUserIds.forEach(id => { - lapsedUsersByIds[id] = true // create a map for more performant lookups - if (allSsoUsersByIds[id]) { - ++result.ssoUsers.lapsed - } else { - ++result.emailUsers.lapsed - } - }) - - result.emailUsers.current = - currentUsersCount - entitled - result.ssoUsers.current.notEntitled - result.emailUsers.total = userIds.length - allSsoUsers.length - - // compare v1 and v2 counts. - if ( - result.ssoUsers.current.notEntitled + result.emailUsers.current !== - withConfirmedEmail - ) { - result.databaseMismatch = { - withConfirmedEmail: { - v1: withConfirmedEmail, - v2: result.ssoUsers.current.notEntitled + result.emailUsers.current, - }, - } - } - - // Add Pro/NonPro status for users - // NOTE: Users not entitled via institution could have Pro via another method - const { proUserIds, nonProUserIds } = await _checkUsersFeatures(userIds) - proUserIds.forEach(id => { - const userType = lapsedUsersByIds[id] ? 'lapsed' : 'current' - if (allSsoUsersByIds[id]) { - result.ssoUsers.pro[userType]++ - } else { - result.emailUsers.pro[userType]++ - } - }) - nonProUserIds.forEach(id => { - const userType = lapsedUsersByIds[id] ? 'lapsed' : 'current' - if (allSsoUsersByIds[id]) { - result.ssoUsers.nonPro[userType]++ - } else { - result.emailUsers.nonPro[userType]++ - } - }) - if (emitNonProUserIds) { - result.nonProUserIds = nonProUserIds - } - return result -} - -const InstitutionsManager = { - clearInstitutionNotifications(institutionId, dryRun, callback) { - function clear(key, cb) { - const run = dryRun - ? NotificationsHandler.previewMarkAsReadByKeyOnlyBulk - : NotificationsHandler.markAsReadByKeyOnlyBulk - - run(key, cb) - } - - async.series( - { - ipMatcherAffiliation(cb) { - const key = `ip-matched-affiliation-${institutionId}` - clear(key, cb) + const result = { + emailUsers: { + total: 0, // v1 all users - v2 all SSO users + current: 0, // v1 current - v1 SSO entitled - (v2 calculated not entitled current) + lapsed: 0, // v1 lapsed user IDs that are not in v2 SSO users + pro: { + current: 0, + lapsed: 0, }, - featuresUpgradedByAffiliation(cb) { - const key = `features-updated-by=${institutionId}` - clear(key, cb) - }, - redundantPersonalSubscription(cb) { - const key = `redundant-personal-subscription-${institutionId}` - clear(key, cb) + nonPro: { + current: 0, + lapsed: 0, }, }, - callback + ssoUsers: { + total: 0, // only v2 + current: { + entitled: 0, // only v1 + notEntitled: 0, // v2 non-entitled SSO users - v1 lapsed user IDs + }, + lapsed: 0, // v2 SSO users that are in v1 lapsed user IDs + pro: { + current: 0, + lapsed: 0, + }, + nonPro: { + current: 0, + lapsed: 0, + }, + }, + } + + const { + user_ids: userIds, // confirmed and not removed users. Includes users with lapsed reconfirmations + current_users_count: currentUsersCount, // all users not with lapsed reconfirmations + lapsed_user_ids: lapsedUserIds, // includes all user types that did not reconfirm (sso entitled, sso not entitled, email only) + with_confirmed_email: withConfirmedEmail, // same count as affiliation metrics + entitled_via_sso: entitled, // same count as affiliation metrics + } = await InstitutionsAPI.promises.getInstitutionAffiliationsCounts( + institutionId ) - }, + result.ssoUsers.current.entitled = entitled - refreshInstitutionUsers(institutionId, notify, callback) { - const refreshFunction = notify ? refreshFeaturesAndNotify : refreshFeatures - async.waterfall( - [ - cb => fetchInstitutionAndAffiliations(institutionId, cb), - function (institution, affiliations, cb) { - for (const affiliation of affiliations) { - affiliation.institutionName = institution.name - affiliation.institutionId = institutionId - } - async.eachLimit(affiliations, ASYNC_LIMIT, refreshFunction, err => - cb(err) - ) - }, - ], - callback - ) - }, + const { allSsoUsers, allSsoUsersByIds, currentNotEntitledCount } = + await _getSsoUsers(institutionId, lapsedUserIds) + result.ssoUsers.total = allSsoUsers.length + result.ssoUsers.current.notEntitled = currentNotEntitledCount - checkInstitutionUsers: callbackify(checkInstitutionUsers), - - getInstitutionUsersSubscriptions(institutionId, callback) { - getInstitutionAffiliations(institutionId, function (error, affiliations) { - if (error) { - return callback(error) + // check if lapsed user ID an SSO user + const lapsedUsersByIds = {} + lapsedUserIds.forEach(id => { + lapsedUsersByIds[id] = true // create a map for more performant lookups + if (allSsoUsersByIds[id]) { + ++result.ssoUsers.lapsed + } else { + ++result.emailUsers.lapsed } - const userIds = affiliations.map( - affiliation => new ObjectId(affiliation.user_id) - ) - Subscription.find({ admin_id: userIds }) - .populate('admin_id', 'email') - .exec(callback) }) + + result.emailUsers.current = + currentUsersCount - entitled - result.ssoUsers.current.notEntitled + result.emailUsers.total = userIds.length - allSsoUsers.length + + // compare v1 and v2 counts. + if ( + result.ssoUsers.current.notEntitled + result.emailUsers.current !== + withConfirmedEmail + ) { + result.databaseMismatch = { + withConfirmedEmail: { + v1: withConfirmedEmail, + v2: result.ssoUsers.current.notEntitled + result.emailUsers.current, + }, + } + } + + // Add Pro/NonPro status for users + // NOTE: Users not entitled via institution could have Pro via another method + const { proUserIds, nonProUserIds } = await _checkUsersFeatures(userIds) + proUserIds.forEach(id => { + const userType = lapsedUsersByIds[id] ? 'lapsed' : 'current' + if (allSsoUsersByIds[id]) { + result.ssoUsers.pro[userType]++ + } else { + result.emailUsers.pro[userType]++ + } + }) + nonProUserIds.forEach(id => { + const userType = lapsedUsersByIds[id] ? 'lapsed' : 'current' + if (allSsoUsersByIds[id]) { + result.ssoUsers.nonPro[userType]++ + } else { + result.emailUsers.nonPro[userType]++ + } + }) + if (emitNonProUserIds) { + result.nonProUserIds = nonProUserIds + } + return result }, - affiliateUsers(hostname, callback) { + async getInstitutionUsersSubscriptions(institutionId) { + const affiliations = + await InstitutionsAPI.promises.getInstitutionAffiliations(institutionId) + + const userIds = affiliations.map( + affiliation => new ObjectId(affiliation.user_id) + ) + return await Subscription.find({ admin_id: userIds }) + .populate('admin_id', 'email') + .exec() + }, + + async affiliateUsers(hostname) { const reversedHostname = hostname.trim().split('').reverse().join('') - UserGetter.getInstitutionUsersByHostname(hostname, (error, users) => { - if (error) { - OError.tag(error, 'problem fetching users by hostname') - return callback(error) - } - async.mapLimit( - users, - ASYNC_LIMIT, - (user, innerCallback) => { - affiliateUserByReversedHostname(user, reversedHostname, innerCallback) - }, - callback - ) - }) + let users + try { + users = await UserGetter.promises.getInstitutionUsersByHostname(hostname) + } catch (error) { + OError.tag(error, 'problem fetching users by hostname') + throw error + } + + await promiseMapWithLimit(ASYNC_LIMIT, users, user => + affiliateUserByReversedHostname(user, reversedHostname) + ) }, - confirmDomain: callbackify(confirmDomain), + /** + * Enqueue a job for adding affiliations for when a domain is confirmed + */ + async confirmDomain(hostname) { + const queue = Queues.getQueue('confirm-institution-domain') + await queue.add({ hostname }) + }, + + async fetchV1Data(institution) { + const url = `${Settings.apis.v1.url}/universities/list/${institution.v1Id}` + try { + const data = await fetchJson(url, { + signal: AbortSignal.timeout(Settings.apis.v1.timeout), + }) + + institution.name = data?.name + institution.countryCode = data?.country_code + institution.departments = data?.departments + institution.portalSlug = data?.portal_slug + } catch (error) { + logger.err( + { model: 'Institution', v1Id: institution.v1Id, error }, + '[fetchV1DataError]' + ) + } + }, } -const fetchInstitutionAndAffiliations = (institutionId, callback) => - async.waterfall( - [ - cb => - Institution.findOne({ v1Id: institutionId }, (err, institution) => - cb(err, institution) - ), - (institution, cb) => - institution.fetchV1Data((err, institution) => cb(err, institution)), - (institution, cb) => - getConfirmedInstitutionAffiliations( - institutionId, - (err, affiliations) => cb(err, institution, affiliations) - ), - ], - callback - ) +const fetchInstitutionAndAffiliations = async institutionId => { + let institution = await Institution.findOne({ v1Id: institutionId }).exec() + institution = await institution.fetchV1DataPromise() -function refreshFeatures(affiliation, callback) { - const userId = new ObjectId(affiliation.user_id) - FeaturesUpdater.refreshFeatures(userId, 'refresh-institution-users', callback) -} - -function refreshFeaturesAndNotify(affiliation, callback) { - const userId = new ObjectId(affiliation.user_id) - async.waterfall( - [ - cb => - FeaturesUpdater.refreshFeatures( - userId, - 'refresh-institution-users', - (err, features, featuresChanged) => cb(err, featuresChanged) - ), - (featuresChanged, cb) => - getUserInfo(userId, (error, user, subscription) => - cb(error, user, subscription, featuresChanged) - ), - (user, subscription, featuresChanged, cb) => - notifyUser(user, affiliation, subscription, featuresChanged, cb), - ], - callback - ) -} - -const getUserInfo = (userId, callback) => - async.waterfall( - [ - cb => UserGetter.getUser(userId, { _id: 1 }, cb), - (user, cb) => - SubscriptionLocator.getUsersSubscription(user, (err, subscription) => - cb(err, user, subscription) - ), - ], - callback - ) - -const notifyUser = ( - user, - affiliation, - subscription, - featuresChanged, - callback -) => - async.parallel( - [ - function (cb) { - if (featuresChanged) { - NotificationsBuilder.featuresUpgradedByAffiliation( - affiliation, - user - ).create(cb) - } else { - cb() - } - }, - function (cb) { - if (subscription && !subscription.groupPlan) { - NotificationsBuilder.redundantPersonalSubscription( - affiliation, - user - ).create(cb) - } else { - cb() - } - }, - ], - callback - ) - -async function fetchV1Data(institution) { - const url = `${Settings.apis.v1.url}/universities/list/${institution.v1Id}` - try { - const data = await fetchJson(url, { - signal: AbortSignal.timeout(Settings.apis.v1.timeout), - }) - - institution.name = data?.name - institution.countryCode = data?.country_code - institution.departments = data?.departments - institution.portalSlug = data?.portal_slug - } catch (error) { - logger.err( - { model: 'Institution', v1Id: institution.v1Id, error }, - '[fetchV1DataError]' + const affiliations = + await InstitutionsAPI.promises.getConfirmedInstitutionAffiliations( + institutionId ) - } + + return { institution, affiliations } } -/** - * Enqueue a job for adding affiliations for when a domain is confirmed - */ -async function confirmDomain(hostname) { - const queue = Queues.getQueue('confirm-institution-domain') - await queue.add({ hostname }) +async function refreshFeatures(affiliation) { + const userId = new ObjectId(affiliation.user_id) + return await FeaturesUpdater.promises.refreshFeatures( + userId, + 'refresh-institution-users' + ) } -function affiliateUserByReversedHostname(user, reversedHostname, callback) { +async function refreshFeaturesAndNotify(affiliation) { + const userId = new ObjectId(affiliation.user_id) + const { featuresChanged } = await FeaturesUpdater.promises.refreshFeatures( + userId, + 'refresh-institution-users' + ) + const { user, subscription } = await getUserInfo(userId) + return await notifyUser(user, affiliation, subscription, featuresChanged) +} + +const getUserInfo = async userId => { + const user = await UserGetter.promises.getUser(userId, { _id: 1 }) + const subscription = await SubscriptionLocator.promises.getUsersSubscription( + user + ) + return { user, subscription } +} + +const notifyUser = async (user, affiliation, subscription, featuresChanged) => { + return await Promise.all([ + (async () => { + if (featuresChanged) { + return await NotificationsBuilder.promises + .featuresUpgradedByAffiliation(affiliation, user) + .create() + } + })(), + (async () => { + if (subscription && !subscription.groupPlan) { + return await NotificationsBuilder.promises + .redundantPersonalSubscription(affiliation, user) + .create() + } + })(), + ]) +} + +async function affiliateUserByReversedHostname(user, reversedHostname) { const matchingEmails = user.emails.filter( email => email.reversedHostname === reversedHostname ) - async.mapSeries( - matchingEmails, - (email, innerCallback) => { - addAffiliation( - user._id, - email.email, - { - confirmedAt: email.confirmedAt, - entitlement: - email.samlIdentifier && email.samlIdentifier.hasEntitlement, - }, - error => { - if (error) { - OError.tag( - error, - 'problem adding affiliation while confirming hostname' - ) - return innerCallback(error) - } - innerCallback() - } - ) - }, - err => { - if (err) { - return callback(err) - } - FeaturesUpdater.refreshFeatures( - user._id, - 'affiliate-user-by-reversed-hostname', - callback - ) + + for (const email of matchingEmails) { + try { + await InstitutionsAPI.promises.addAffiliation(user._id, email.email, { + confirmedAt: email.confirmedAt, + entitlement: + email.samlIdentifier && email.samlIdentifier.hasEntitlement, + }) + } catch (error) { + OError.tag(error, 'problem adding affiliation while confirming hostname') + throw error } + } + + await FeaturesUpdater.promises.refreshFeatures( + user._id, + 'affiliate-user-by-reversed-hostname' ) } -InstitutionsManager.promises = { - affiliateUsers: promisify(InstitutionsManager.affiliateUsers), - confirmDomain, - checkInstitutionUsers, - clearInstitutionNotifications: promisify( - InstitutionsManager.clearInstitutionNotifications - ), - fetchV1Data, +module.exports = { + ...callbackifyAll(InstitutionsManager), + promises: InstitutionsManager, } - -module.exports = InstitutionsManager diff --git a/services/web/app/src/Features/Notifications/NotificationsBuilder.js b/services/web/app/src/Features/Notifications/NotificationsBuilder.js index c19de246f8..8823022e7a 100644 --- a/services/web/app/src/Features/Notifications/NotificationsBuilder.js +++ b/services/web/app/src/Features/Notifications/NotificationsBuilder.js @@ -325,6 +325,9 @@ NotificationsBuilder.promises = { dropboxDuplicateProjectNames(userId) { return promisifyAll(dropboxDuplicateProjectNames(userId)) }, + featuresUpgradedByAffiliation: function (affiliation, user) { + return promisifyAll(featuresUpgradedByAffiliation(affiliation, user)) + }, ipMatcherAffiliation: function (userId) { return promisifyAll(ipMatcherAffiliation(userId)) }, diff --git a/services/web/app/src/models/Institution.js b/services/web/app/src/models/Institution.js index 0128f294e9..cb1de37ae5 100644 --- a/services/web/app/src/models/Institution.js +++ b/services/web/app/src/models/Institution.js @@ -4,6 +4,7 @@ const { ObjectId } = Schema const settings = require('@overleaf/settings') const logger = require('@overleaf/logger') const request = require('request') +const { promisify } = require('@overleaf/promise-utils') const InstitutionSchema = new Schema( { @@ -44,5 +45,10 @@ InstitutionSchema.method('fetchV1Data', function (callback) { ) }) +InstitutionSchema.method( + 'fetchV1DataPromise', + promisify(InstitutionSchema.methods.fetchV1Data) +) + exports.Institution = mongoose.model('Institution', InstitutionSchema) exports.InstitutionSchema = InstitutionSchema diff --git a/services/web/test/unit/src/Institutions/InstitutionsManagerTests.js b/services/web/test/unit/src/Institutions/InstitutionsManagerTests.js index 9b597676d6..db11b84e28 100644 --- a/services/web/test/unit/src/Institutions/InstitutionsManagerTests.js +++ b/services/web/test/unit/src/Institutions/InstitutionsManagerTests.js @@ -13,10 +13,6 @@ describe('InstitutionsManager', function () { beforeEach(function () { this.institutionId = 123 this.user = {} - this.getInstitutionAffiliations = sinon.stub() - this.getConfirmedInstitutionAffiliations = sinon.stub() - this.addAffiliation = sinon.stub().callsArgWith(3, null) - this.refreshFeatures = sinon.stub().yields() const lapsedUser = { _id: '657300a08a14461b3d1aac3e', features: {}, @@ -49,9 +45,8 @@ describe('InstitutionsManager', function () { ] this.UserGetter = { - getUsersByAnyConfirmedEmail: sinon.stub().yields(), - getUser: sinon.stub().yields(null, this.user), promises: { + getUser: sinon.stub().resolves(this.user), getUsers: sinon.stub().resolves(this.users), getUsersByAnyConfirmedEmail: sinon.stub().resolves(), getSsoUsersAtInstitution: (this.getSsoUsersAtInstitution = sinon @@ -59,26 +54,30 @@ describe('InstitutionsManager', function () { .resolves(this.ssoUsers)), }, } - this.creator = { create: sinon.stub().callsArg(0) } + this.creator = { create: sinon.stub().resolves() } this.NotificationsBuilder = { - featuresUpgradedByAffiliation: sinon.stub().returns(this.creator), - redundantPersonalSubscription: sinon.stub().returns(this.creator), + promises: { + featuresUpgradedByAffiliation: sinon.stub().returns(this.creator), + redundantPersonalSubscription: sinon.stub().returns(this.creator), + }, } this.SubscriptionLocator = { - getUsersSubscription: sinon.stub().callsArg(1), + promises: { + getUsersSubscription: sinon.stub().resolves(), + }, } this.institutionWithV1Data = { name: 'Wombat University' } this.institution = { - fetchV1Data: sinon - .stub() - .callsArgWith(0, null, this.institutionWithV1Data), + fetchV1DataPromise: sinon.stub().resolves(this.institutionWithV1Data), } this.InstitutionModel = { Institution: { - findOne: sinon.stub().callsArgWith(1, null, this.institution), + findOne: sinon.stub().returns({ + exec: sinon.stub().resolves(this.institution), + }), }, } - this.subscriptionExec = sinon.stub().yields() + this.subscriptionExec = sinon.stub().resolves() const SubscriptionModel = { Subscription: { find: () => ({ @@ -101,27 +100,13 @@ describe('InstitutionsManager', function () { with_confirmed_email: 2, // 1 non entitled SSO + 1 email user } - // this.InstitutionsController = SandboxedModule.require(modulePath, { - // requires: { - // '../User/UserGetter': { - // getInstitutionUsersByHostname: this.getInstitutionUsersByHostname, - // }, - // '../Institutions/InstitutionsAPI': { - // addAffiliation: this.addAffiliation, - // }, - // '../Subscription/FeaturesUpdater': { - // refreshFeatures: this.refreshFeatures, - // }, - // }, - this.InstitutionsManager = SandboxedModule.require(modulePath, { requires: { './InstitutionsAPI': { - getInstitutionAffiliations: this.getInstitutionAffiliations, - getConfirmedInstitutionAffiliations: - this.getConfirmedInstitutionAffiliations, - addAffiliation: this.addAffiliation, promises: { + addAffiliation: (this.addAffiliationPromise = sinon + .stub() + .resolves()), getInstitutionAffiliations: (this.getInstitutionAffiliationsPromise = sinon .stub() @@ -137,7 +122,11 @@ describe('InstitutionsManager', function () { }, }, '../Subscription/FeaturesUpdater': { - refreshFeatures: this.refreshFeatures, + promises: { + refreshFeatures: (this.refreshFeaturesPromise = sinon + .stub() + .resolves()), + }, }, '../Subscription/FeaturesHelper': { isFeatureSetBetter: (this.isFeatureSetBetter = sinon.stub()), @@ -172,104 +161,99 @@ describe('InstitutionsManager', function () { this.user3 = { _id: this.user3Id } this.user4 = { _id: this.user4Id } - this.UserGetter.getUser + this.UserGetter.promises.getUser .withArgs(new ObjectId(this.user1Id)) - .yields(null, this.user1) - this.UserGetter.getUser + .resolves(this.user1) + this.UserGetter.promises.getUser .withArgs(new ObjectId(this.user2Id)) - .yields(null, this.user2) - this.UserGetter.getUser + .resolves(this.user2) + this.UserGetter.promises.getUser .withArgs(new ObjectId(this.user3Id)) - .yields(null, this.user3) - this.UserGetter.getUser + .resolves(this.user3) + this.UserGetter.promises.getUser .withArgs(new ObjectId(this.user4Id)) - .yields(null, this.user4) + .resolves(this.user4) - this.SubscriptionLocator.getUsersSubscription + this.SubscriptionLocator.promises.getUsersSubscription .withArgs(this.user2) - .callsArgWith(1, null, { + .resolves({ planCode: 'pro', groupPlan: false, }) - this.SubscriptionLocator.getUsersSubscription + this.SubscriptionLocator.promises.getUsersSubscription .withArgs(this.user3) - .callsArgWith(1, null, { + .resolves({ planCode: 'collaborator_free_trial_7_days', groupPlan: false, }) - this.SubscriptionLocator.getUsersSubscription + this.SubscriptionLocator.promises.getUsersSubscription .withArgs(this.user4) - .callsArgWith(1, null, { + .resolves({ planCode: 'collaborator-annual', groupPlan: true, }) - this.refreshFeatures + this.refreshFeaturesPromise.resolves({ + newFeatures: {}, + featuresChanged: false, + }) + this.refreshFeaturesPromise .withArgs(new ObjectId(this.user1Id)) - .yields(null, {}, true) - this.getInstitutionAffiliations.yields(null, this.affiliations) - this.getConfirmedInstitutionAffiliations.yields(null, this.affiliations) - }) - - it('refresh all users Features', function (done) { - this.InstitutionsManager.refreshInstitutionUsers( - this.institutionId, - false, - error => { - expect(error).not.to.exist - sinon.assert.callCount(this.refreshFeatures, 4) - // expect no notifications - sinon.assert.notCalled( - this.NotificationsBuilder.featuresUpgradedByAffiliation - ) - sinon.assert.notCalled( - this.NotificationsBuilder.redundantPersonalSubscription - ) - done() - } + .resolves({ newFeatures: {}, featuresChanged: true }) + this.getInstitutionAffiliationsPromise.resolves(this.affiliations) + this.getConfirmedInstitutionAffiliationsPromise.resolves( + this.affiliations ) }) - it('notifies users if their features have been upgraded', function (done) { - this.InstitutionsManager.refreshInstitutionUsers( + it('refresh all users Features', async function () { + await this.InstitutionsManager.promises.refreshInstitutionUsers( this.institutionId, - true, - error => { - expect(error).not.to.exist - sinon.assert.calledOnce( - this.NotificationsBuilder.featuresUpgradedByAffiliation - ) - sinon.assert.calledWith( - this.NotificationsBuilder.featuresUpgradedByAffiliation, - this.affiliations[0], - this.user1 - ) - done() - } + false + ) + sinon.assert.callCount(this.refreshFeaturesPromise, 4) + // expect no notifications + sinon.assert.notCalled( + this.NotificationsBuilder.promises.featuresUpgradedByAffiliation + ) + sinon.assert.notCalled( + this.NotificationsBuilder.promises.redundantPersonalSubscription ) }) - it('notifies users if they have a subscription, or a trial subscription, that should be cancelled', function (done) { - this.InstitutionsManager.refreshInstitutionUsers( + it('notifies users if their features have been upgraded', async function () { + await this.InstitutionsManager.promises.refreshInstitutionUsers( this.institutionId, - true, - error => { - expect(error).not.to.exist - sinon.assert.calledTwice( - this.NotificationsBuilder.redundantPersonalSubscription - ) - sinon.assert.calledWith( - this.NotificationsBuilder.redundantPersonalSubscription, - this.affiliations[1], - this.user2 - ) - sinon.assert.calledWith( - this.NotificationsBuilder.redundantPersonalSubscription, - this.affiliations[2], - this.user3 - ) - done() - } + true + ) + sinon.assert.calledOnce( + this.NotificationsBuilder.promises.featuresUpgradedByAffiliation + ) + sinon.assert.calledWith( + this.NotificationsBuilder.promises.featuresUpgradedByAffiliation, + this.affiliations[0], + this.user1 + ) + }) + + it('notifies users if they have a subscription, or a trial subscription, that should be cancelled', async function () { + await this.InstitutionsManager.promises.refreshInstitutionUsers( + this.institutionId, + true + ) + + sinon.assert.calledTwice( + this.NotificationsBuilder.promises.redundantPersonalSubscription + ) + sinon.assert.calledWith( + this.NotificationsBuilder.promises.redundantPersonalSubscription, + this.affiliations[1], + this.user2 + ) + sinon.assert.calledWith( + this.NotificationsBuilder.promises.redundantPersonalSubscription, + this.affiliations[2], + this.user3 ) }) }) @@ -366,21 +350,17 @@ describe('InstitutionsManager', function () { }) describe('getInstitutionUsersSubscriptions', function () { - it('returns all institution users subscriptions', function (done) { + it('returns all institution users subscriptions', async function () { const stubbedUsers = [ { user_id: '123abc123abc123abc123abc' }, { user_id: '456def456def456def456def' }, { user_id: '789def789def789def789def' }, ] - this.getInstitutionAffiliations.yields(null, stubbedUsers) - this.InstitutionsManager.getInstitutionUsersSubscriptions( - this.institutionId, - (error, subscriptions) => { - expect(error).not.to.exist - sinon.assert.calledOnce(this.subscriptionExec) - done() - } + this.getInstitutionAffiliationsPromise.resolves(stubbedUsers) + await this.InstitutionsManager.promises.getInstitutionUsersSubscriptions( + this.institutionId ) + sinon.assert.calledOnce(this.subscriptionExec) }) }) @@ -423,7 +403,7 @@ describe('InstitutionsManager', function () { }, ] - this.getInstitutionUsersByHostname = sinon.stub().yields(null, [ + this.getInstitutionUsersByHostname = sinon.stub().resolves([ { _id: this.stubbedUser1._id, emails: this.stubbedUser1DecoratedEmails, @@ -433,55 +413,53 @@ describe('InstitutionsManager', function () { emails: this.stubbedUser2DecoratedEmails, }, ]) - this.UserGetter.getInstitutionUsersByHostname = + this.UserGetter.promises.getInstitutionUsersByHostname = this.getInstitutionUsersByHostname }) describe('affiliateUsers', function () { - it('should add affiliations for matching users', function (done) { - this.InstitutionsManager.affiliateUsers('mit.edu', error => { - expect(error).not.to.exist - this.getInstitutionUsersByHostname.calledOnce.should.equal(true) - this.addAffiliation.calledThrice.should.equal(true) - this.addAffiliation - .calledWithMatch( - this.stubbedUser1._id, - this.stubbedUser1.emails[0].email, - { entitlement: false } - ) - .should.equal(true) - this.addAffiliation - .calledWithMatch( - this.stubbedUser1._id, - this.stubbedUser1.emails[2].email, - { entitlement: true } - ) - .should.equal(true) - this.addAffiliation - .calledWithMatch( - this.stubbedUser2._id, - this.stubbedUser2.emails[0].email, - { entitlement: undefined } - ) - .should.equal(true) - this.refreshFeatures - .calledWith(this.stubbedUser1._id) - .should.equal(true) - this.refreshFeatures - .calledWith(this.stubbedUser2._id) - .should.equal(true) - this.refreshFeatures.should.have.been.calledTwice - done() - }) + it('should add affiliations for matching users', async function () { + await this.InstitutionsManager.promises.affiliateUsers('mit.edu') + + this.getInstitutionUsersByHostname.calledOnce.should.equal(true) + this.addAffiliationPromise.calledThrice.should.equal(true) + this.addAffiliationPromise + .calledWithMatch( + this.stubbedUser1._id, + this.stubbedUser1.emails[0].email, + { entitlement: false } + ) + .should.equal(true) + this.addAffiliationPromise + .calledWithMatch( + this.stubbedUser1._id, + this.stubbedUser1.emails[2].email, + { entitlement: true } + ) + .should.equal(true) + this.addAffiliationPromise + .calledWithMatch( + this.stubbedUser2._id, + this.stubbedUser2.emails[0].email, + { entitlement: undefined } + ) + .should.equal(true) + this.refreshFeaturesPromise + .calledWith(this.stubbedUser1._id) + .should.equal(true) + this.refreshFeaturesPromise + .calledWith(this.stubbedUser2._id) + .should.equal(true) + this.refreshFeaturesPromise.should.have.been.calledTwice }) - it('should return errors if last affiliation cannot be added', function (done) { - this.addAffiliation.onCall(2).callsArgWith(3, new Error('error')) - this.InstitutionsManager.affiliateUsers('mit.edu', error => { - expect(error).to.exist - this.getInstitutionUsersByHostname.calledOnce.should.equal(true) - done() - }) + it('should return errors if last affiliation cannot be added', async function () { + this.addAffiliationPromise.onCall(2).rejects() + await expect( + this.InstitutionsManager.promises.affiliateUsers('mit.edu') + ).to.be.rejected + + this.getInstitutionUsersByHostname.calledOnce.should.equal(true) }) }) })