Merge pull request #3741 from overleaf/ta-refresh-institution-users

Refresh Institution Users Script Changes

GitOrigin-RevId: 917287450ba30a9f8b605a2d38933a0f6e1979e6

services/web/app/src/Features/Institutions/InstitutionsManager.js

@@ -13,7 +13,8 @@ const OError = require('@overleaf/o-error')
 
 const ASYNC_LIMIT = parseInt(process.env.ASYNC_LIMIT, 10) || 5
 module.exports = {
-  upgradeInstitutionUsers(institutionId, callback) {
+  refreshInstitutionUsers(institutionId, notify, callback) {
+    const refreshFunction = notify ? refreshFeaturesAndNotify : refreshFeatures
     async.waterfall(
       [
         cb => fetchInstitutionAndAffiliations(institutionId, cb),
@@ -23,7 +24,7 @@ module.exports = {
             affiliation.institutionId = institutionId
             return affiliation
           })
-          async.eachLimit(affiliations, ASYNC_LIMIT, refreshFeatures, err =>
+          async.eachLimit(affiliations, ASYNC_LIMIT, refreshFunction, err =>
             cb(err)
           )
         }
@@ -84,6 +85,11 @@ var fetchInstitutionAndAffiliations = (institutionId, callback) =>
   )
 
 var refreshFeatures = function(affiliation, callback) {
+  const userId = ObjectId(affiliation.user_id)
+  FeaturesUpdater.refreshFeatures(userId, callback)
+}
+
+var refreshFeaturesAndNotify = function(affiliation, callback) {
   const userId = ObjectId(affiliation.user_id)
   async.waterfall(
     [

services/web/app/src/Features/Subscription/FeaturesUpdater.js

@@ -23,21 +23,25 @@ const FeaturesUpdater = {
           return callback(error)
         }
         logger.log({ userId, features }, 'updating user features')
-        UserFeaturesUpdater.updateFeatures(userId, features, err => {
-          if (err) {
-            return callback(err)
-          }
-          if (oldFeatures.dropbox === true && features.dropbox === false) {
-            logger.log({ userId }, '[FeaturesUpdater] must unlink dropbox')
-            const Modules = require('../../infrastructure/Modules')
-            Modules.hooks.fire('removeDropbox', userId, err => {
-              if (err) {
-                logger.error(err)
-              }
-            })
+        UserFeaturesUpdater.updateFeatures(
+          userId,
+          features,
+          (err, newFeatures, featuresChanged) => {
+            if (err) {
+              return callback(err)
+            }
+            if (oldFeatures.dropbox === true && features.dropbox === false) {
+              logger.log({ userId }, '[FeaturesUpdater] must unlink dropbox')
+              const Modules = require('../../infrastructure/Modules')
+              Modules.hooks.fire('removeDropbox', userId, err => {
+                if (err) {
+                  logger.error(err)
+                }
+              })
+            }
+            return callback(null, newFeatures, featuresChanged)
           }
-          return callback()
-        })
+        )
       })
     })
   },

services/web/scripts/upgrade_institution_users.js → services/web/scripts/refresh_institution_users.js

@@ -1,4 +1,5 @@
 const { waitForDb } = require('../app/src/infrastructure/mongodb')
+const minimist = require('minimist')
 const InstitutionsManager = require('../app/src/Features/Institutions/InstitutionsManager')
 
 const institutionId = parseInt(process.argv[2])
@@ -13,7 +14,19 @@ waitForDb()
   })
 
 function main() {
-  InstitutionsManager.upgradeInstitutionUsers(institutionId, function(error) {
+  const argv = minimist(process.argv.slice(2))
+  if (!argv.notify) {
+    throw new Error('Missing `notify` flag. Please use `--notify true|false`')
+  }
+  if (!argv.notify[0]) {
+    throw new Error('Empty `notify` flag. Please use `--notify true|false`')
+  }
+  const notify = argv.notify[0] === 't'
+  console.log('Running with notify =', notify)
+
+  InstitutionsManager.refreshInstitutionUsers(institutionId, notify, function(
+    error
+  ) {
     if (error) {
       console.log(error)
     } else {

services/web/test/unit/src/Institutions/InstitutionsManagerTests.js

@@ -1,14 +1,3 @@
-/* eslint-disable
-    max-len,
-    no-return-assign,
-*/
-// TODO: This file was created by bulk-decaffeinate.
-// Fix any style issues and re-enable lint.
-/*
- * decaffeinate suggestions:
- * DS102: Remove unnecessary code created because of implicit returns
- * Full docs: https://github.com/decaffeinate/decaffeinate/blob/master/docs/suggestions.md
- */
 const should = require('chai').should()
 const SandboxedModule = require('sandboxed-module')
 const path = require('path')
@@ -55,18 +44,16 @@ describe('InstitutionsManager', function() {
     }
     const SubscriptionModel = {
       Subscription: {
-        find: () => {
-          return {
-            populate: () => {
-              return { exec: this.subscriptionExec }
-            }
-          }
-        }
+        find: () => ({
+          populate: () => ({
+            exec: this.subscriptionExec
+          })
+        })
       }
     }
     this.Mongo = { ObjectId: sinon.stub().returnsArg(0) }
 
-    return (this.InstitutionsManager = SandboxedModule.require(modulePath, {
+    this.InstitutionsManager = SandboxedModule.require(modulePath, {
       globals: {
         console: console
       },
@@ -86,10 +73,10 @@ describe('InstitutionsManager', function() {
         mongodb: this.Mongo,
         '../User/SAMLIdentityManager': this.SAMLIdentityManager
       }
-    }))
+    })
   })
 
-  describe('upgradeInstitutionUsers', function() {
+  describe('refreshInstitutionUsers', function() {
     beforeEach(function() {
       this.user1Id = '123abc123abc123abc123abc'
       this.user2Id = '456def456def456def456def'
@@ -110,23 +97,33 @@ describe('InstitutionsManager', function() {
         .withArgs(this.user2)
         .callsArgWith(1, null, this.subscription)
       this.refreshFeatures.withArgs(this.user1Id).yields(null, {}, true)
-      return this.getInstitutionAffiliations.yields(null, this.affiliations)
+      this.getInstitutionAffiliations.yields(null, this.affiliations)
     })
 
     it('refresh all users Features', function(done) {
-      return this.InstitutionsManager.upgradeInstitutionUsers(
+      this.InstitutionsManager.refreshInstitutionUsers(
         this.institutionId,
+        false,
         error => {
           should.not.exist(error)
           sinon.assert.calledTwice(this.refreshFeatures)
-          return done()
+
+          // expect no notifications
+          sinon.assert.notCalled(
+            this.NotificationsBuilder.featuresUpgradedByAffiliation
+          )
+          sinon.assert.notCalled(
+            this.NotificationsBuilder.redundantPersonalSubscription
+          )
+          done()
         }
       )
     })
 
     it('notifies users if their features have been upgraded', function(done) {
-      return this.InstitutionsManager.upgradeInstitutionUsers(
+      this.InstitutionsManager.refreshInstitutionUsers(
         this.institutionId,
+        true,
         error => {
           should.not.exist(error)
           sinon.assert.calledOnce(
@@ -137,14 +134,15 @@ describe('InstitutionsManager', function() {
             this.affiliations[0],
             this.user1
           )
-          return done()
+          done()
         }
       )
     })
 
     it('notifies users if they have a subscription that should be cancelled', function(done) {
-      return this.InstitutionsManager.upgradeInstitutionUsers(
+      this.InstitutionsManager.refreshInstitutionUsers(
         this.institutionId,
+        true,
         error => {
           should.not.exist(error)
           sinon.assert.calledOnce(
@@ -155,7 +153,7 @@ describe('InstitutionsManager', function() {
             this.affiliations[1],
             this.user2
           )
-          return done()
+          done()
         }
       )
     })
@@ -183,7 +181,7 @@ describe('InstitutionsManager', function() {
       this.SAMLIdentityManager.userHasEntitlement.onCall(0).returns(true)
       this.SAMLIdentityManager.userHasEntitlement.onCall(1).returns(true)
       this.SAMLIdentityManager.userHasEntitlement.onCall(2).returns(false)
-      return this.InstitutionsManager.checkInstitutionUsers(
+      this.InstitutionsManager.checkInstitutionUsers(
         this.institutionId,
         (error, usersSummary) => {
           should.not.exist(error)
@@ -202,7 +200,7 @@ describe('InstitutionsManager', function() {
           expect(usersSummary.entitledSSOUsers.nonProUsers).to.deep.equal([
             '456def456def456def456def'
           ])
-          return done()
+          done()
         }
       )
     })
@@ -216,12 +214,12 @@ describe('InstitutionsManager', function() {
         { user_id: '789def789def789def789def' }
       ]
       this.getInstitutionAffiliations.yields(null, stubbedUsers)
-      return this.InstitutionsManager.getInstitutionUsersSubscriptions(
+      this.InstitutionsManager.getInstitutionUsersSubscriptions(
         this.institutionId,
         (error, subscriptions) => {
           should.not.exist(error)
           sinon.assert.calledOnce(this.subscriptionExec)
-          return done()
+          done()
         }
       )
     })

services/web/test/unit/src/Subscription/FeaturesUpdaterTests.js

@@ -38,7 +38,9 @@ describe('FeaturesUpdater', function() {
         _id: this.user_id,
         features: {}
       }
-      this.UserFeaturesUpdater.updateFeatures = sinon.stub().yields()
+      this.UserFeaturesUpdater.updateFeatures = sinon
+        .stub()
+        .yields(null, { some: 'features' }, true)
       this.FeaturesUpdater._getIndividualFeatures = sinon
         .stub()
         .yields(null, { individual: 'features' })
@@ -60,6 +62,18 @@ describe('FeaturesUpdater', function() {
       this.UserGetter.getUser = sinon.stub().yields(null, this.user)
       this.callback = sinon.stub()
     })
+
+    it('should return features and featuresChanged', function() {
+      this.FeaturesUpdater.refreshFeatures(
+        this.user_id,
+        (err, features, featuresChanged) => {
+          expect(err).to.not.exist
+          expect(features).to.exist
+          expect(featuresChanged).to.exist
+        }
+      )
+    })
+
     describe('normally', function() {
       beforeEach(function() {
         this.FeaturesUpdater.refreshFeatures(this.user_id, this.callback)