Merge pull request #17206 from overleaf/dp-mongoose-callback-institutions-manager

Promisify InsitutionsManager and InstitutionsManagerTests

GitOrigin-RevId: 0872aabb646ee143f51a92608f91d7901723ec0c

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,
+    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,
+        },
       },
-      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,
-      },
-    }
-  }
+    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
+
+    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
+      }
+    })
 
-  // 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]++
+    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,
+        },
+      }
     }
-  })
-  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)
+    // 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
     }
-
-    async.series(
-      {
-        ipMatcherAffiliation(cb) {
-          const key = `ip-matched-affiliation-${institutionId}`
-          clear(key, cb)
-        },
-        featuresUpgradedByAffiliation(cb) {
-          const key = `features-updated-by=${institutionId}`
-          clear(key, cb)
-        },
-        redundantPersonalSubscription(cb) {
-          const key = `redundant-personal-subscription-${institutionId}`
-          clear(key, cb)
-        },
-      },
-      callback
-    )
+    return result
   },
 
-  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
+  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()
   },
 
-  checkInstitutionUsers: callbackify(checkInstitutionUsers),
+  async affiliateUsers(hostname) {
+    const reversedHostname = hostname.trim().split('').reverse().join('')
 
-  getInstitutionUsersSubscriptions(institutionId, callback) {
-    getInstitutionAffiliations(institutionId, function (error, affiliations) {
-      if (error) {
-        return callback(error)
-      }
-      const userIds = affiliations.map(
-        affiliation => new ObjectId(affiliation.user_id)
-      )
-      Subscription.find({ admin_id: userIds })
-        .populate('admin_id', 'email')
-        .exec(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)
+    )
   },
 
-  affiliateUsers(hostname, callback) {
-    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)
-      }
+  /**
+   * 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.mapLimit(
-        users,
-        ASYNC_LIMIT,
-        (user, innerCallback) => {
-          affiliateUserByReversedHostname(user, reversedHostname, innerCallback)
-        },
-        callback
+  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]'
       )
-    })
+    }
   },
-
-  confirmDomain: callbackify(confirmDomain),
 }
 
-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)
+  const affiliations =
+    await InstitutionsAPI.promises.getConfirmedInstitutionAffiliations(
+      institutionId
+    )
+
+  return { institution, affiliations }
 }
 
-function refreshFeaturesAndNotify(affiliation, callback) {
+async function refreshFeatures(affiliation) {
   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
+  return await FeaturesUpdater.promises.refreshFeatures(
+    userId,
+    'refresh-institution-users'
   )
 }
 
-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
+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 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
+const getUserInfo = async userId => {
+  const user = await UserGetter.promises.getUser(userId, { _id: 1 })
+  const subscription = await SubscriptionLocator.promises.getUsersSubscription(
+    user
   )
-
-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]'
-    )
-  }
+  return { user, subscription }
 }
 
-/**
- * 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 })
+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()
+      }
+    })(),
+  ])
 }
 
-function affiliateUserByReversedHostname(user, reversedHostname, callback) {
+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

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))
   },

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

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)
+        .resolves({ newFeatures: {}, featuresChanged: true })
+      this.getInstitutionAffiliationsPromise.resolves(this.affiliations)
+      this.getConfirmedInstitutionAffiliationsPromise.resolves(
+        this.affiliations
+      )
     })
 
-    it('refresh all users Features', function (done) {
-      this.InstitutionsManager.refreshInstitutionUsers(
+    it('refresh all users Features', async function () {
+      await this.InstitutionsManager.promises.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()
-        }
+        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 their features have been upgraded', 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.calledOnce(
-            this.NotificationsBuilder.featuresUpgradedByAffiliation
-          )
-          sinon.assert.calledWith(
-            this.NotificationsBuilder.featuresUpgradedByAffiliation,
-            this.affiliations[0],
-            this.user1
-          )
-          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', function (done) {
-      this.InstitutionsManager.refreshInstitutionUsers(
+    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,
-        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.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)
       })
     })
   })