Merge pull request #4006 from overleaf/jel-refresh-features-reason

Pass reason to refreshFeatures GitOrigin-RevId: 400e3081333239248e3408f93b0517b9bbdbc90c
2025-04-15 00:55:23 +00:00 · 2021-05-11 09:08:37 -05:00 · 2021-05-11 09:08:37 -05:00 · 5d7faa7977
commit 5d7faa7977
parent 2bf126af68
12 changed files with 58 additions and 24 deletions
--- a/services/web/app/src/Features/Institutions/InstitutionsController.js
+++ b/services/web/app/src/Features/Institutions/InstitutionsController.js
@ -67,7 +67,11 @@ var affiliateUserByReversedHostname = function (
            )
            return innerCallback(error)
          }
-          FeaturesUpdater.refreshFeatures(user._id, innerCallback)
+          FeaturesUpdater.refreshFeatures(
+            user._id,
+            'affiliate-user-by-reversed-hostname',
+            innerCallback
+          )
        }
      )
    },
--- a/services/web/app/src/Features/Institutions/InstitutionsManager.js
+++ b/services/web/app/src/Features/Institutions/InstitutionsManager.js
@ -86,7 +86,7 @@ var fetchInstitutionAndAffiliations = (institutionId, callback) =>

 var refreshFeatures = function (affiliation, callback) {
  const userId = ObjectId(affiliation.user_id)
-  FeaturesUpdater.refreshFeatures(userId, callback)
+  FeaturesUpdater.refreshFeatures(userId, 'refresh-institution-users', callback)
 }

 var refreshFeaturesAndNotify = function (affiliation, callback) {
@ -96,6 +96,7 @@ var refreshFeaturesAndNotify = function (affiliation, callback) {
      cb =>
        FeaturesUpdater.refreshFeatures(
          userId,
+          'refresh-institution-users',
          (err, features, featuresChanged) => cb(err, featuresChanged)
        ),
      (featuresChanged, cb) =>
--- a/services/web/app/src/Features/Institutions/InstitutionsReconfirmationHandler.js
+++ b/services/web/app/src/Features/Institutions/InstitutionsReconfirmationHandler.js
@ -30,7 +30,7 @@ function _validateUserIdList(userIds) {
 }

 function _refreshUser(userId, callback) {
-  FeaturesUpdater.refreshFeatures(userId, error => {
+  FeaturesUpdater.refreshFeatures(userId, 'reconfirmation-lapsed', error => {
    if (error) {
      logger.warn(`Failed to refresh features for ${userId}`, error)
      processLapsedLogger.failedToRefresh.push(userId)
--- a/services/web/app/src/Features/Referal/ReferalAllocator.js
+++ b/services/web/app/src/Features/Referal/ReferalAllocator.js
@ -40,7 +40,7 @@ module.exports = {
              })
              return callback(err)
            }
-            FeaturesUpdater.refreshFeatures(user._id, callback)
+            FeaturesUpdater.refreshFeatures(user._id, 'referral', callback)
          }
        )
      } else {
--- a/services/web/app/src/Features/Subscription/FeaturesUpdater.js
+++ b/services/web/app/src/Features/Subscription/FeaturesUpdater.js
@ -12,7 +12,7 @@ const InstitutionsFeatures = require('../Institutions/InstitutionsFeatures')
 const UserGetter = require('../User/UserGetter')

 const FeaturesUpdater = {
-  refreshFeatures(userId, callback = () => {}) {
+  refreshFeatures(userId, reason, callback = () => {}) {
    UserGetter.getUser(userId, { _id: 1, features: 1 }, (err, user) => {
      if (err) {
        return callback(err)
@ -294,16 +294,17 @@ const FeaturesUpdater = {
          { v1UserId, userId: user._id },
          '[AccountSync] updating user subscription and features'
        )
-        return FeaturesUpdater.refreshFeatures(user._id, callback)
+        return FeaturesUpdater.refreshFeatures(user._id, 'sync-v1', callback)
      }
    )
  },
 }

-const refreshFeaturesPromise = userId =>
+const refreshFeaturesPromise = (userId, reason) =>
  new Promise(function (resolve, reject) {
    FeaturesUpdater.refreshFeatures(
      userId,
+      reason,
      (error, features, featuresChanged) => {
        if (error) {
          reject(error)
--- a/services/web/app/src/Features/Subscription/SubscriptionController.js
+++ b/services/web/app/src/Features/Subscription/SubscriptionController.js
@ -509,12 +509,16 @@ module.exports = SubscriptionController = {

  refreshUserFeatures(req, res, next) {
    const { user_id } = req.params
-    return FeaturesUpdater.refreshFeatures(user_id, function (error) {
-      if (error != null) {
-        return next(error)
+    return FeaturesUpdater.refreshFeatures(
+      user_id,
+      'subscription-controller',
+      function (error) {
+        if (error != null) {
+          return next(error)
+        }
+        return res.sendStatus(200)
      }
-      return res.sendStatus(200)
-    })
+    )
  },
 }

--- a/services/web/app/src/Features/Subscription/SubscriptionUpdater.js
+++ b/services/web/app/src/Features/Subscription/SubscriptionUpdater.js
@ -90,7 +90,13 @@ const SubscriptionUpdater = {
        if (err != null) {
          return callback(err)
        }
-        async.map(memberIds, FeaturesUpdater.refreshFeatures, callback)
+        async.map(
+          memberIds,
+          function (userId, cb) {
+            FeaturesUpdater.refreshFeatures(userId, 'add-to-group', cb)
+          },
+          callback
+        )
      }
    )
  },
@ -116,7 +122,11 @@ const SubscriptionUpdater = {
        if (error) {
          return callback(error)
        }
-        FeaturesUpdater.refreshFeatures(userId, callback)
+        FeaturesUpdater.refreshFeatures(
+          userId,
+          'remove-user-from-groups',
+          callback
+        )
      })
    })
  },
@ -216,7 +226,13 @@ const SubscriptionUpdater = {
    const userIds = [subscription.admin_id].concat(
      subscription.member_ids || []
    )
-    async.mapSeries(userIds, FeaturesUpdater.refreshFeatures, callback)
+    async.mapSeries(
+      userIds,
+      function (userId, cb) {
+        FeaturesUpdater.refreshFeatures(userId, 'subscription-updater', cb)
+      },
+      callback
+    )
  },

  _createDeletedSubscription(subscription, deleterData, callback) {
--- a/services/web/app/src/Features/User/UserUpdater.js
+++ b/services/web/app/src/Features/User/UserUpdater.js
@ -193,7 +193,7 @@ async function confirmEmail(userId, email) {
  if (res.n === 0) {
    throw new Errors.NotFoundError('user id and email do no match')
  }
-  await FeaturesUpdater.promises.refreshFeatures(userId)
+  await FeaturesUpdater.promises.refreshFeatures(userId, 'confirm-email')
 }

 const UserUpdater = {
@ -308,7 +308,7 @@ const UserUpdater = {
        if (res.n === 0) {
          return callback(new Error('Cannot remove email'))
        }
-        FeaturesUpdater.refreshFeatures(userId, callback)
+        FeaturesUpdater.refreshFeatures(userId, 'remove-email', callback)
      })
    })
  },
--- a/services/web/test/acceptance/src/FeatureUpdaterTests.js
+++ b/services/web/test/acceptance/src/FeatureUpdaterTests.js
@ -17,7 +17,7 @@ before(function () {
 })

 const syncUserAndGetFeatures = function (user, callback) {
-  FeaturesUpdater.refreshFeatures(user._id, error => {
+  FeaturesUpdater.refreshFeatures(user._id, 'test', error => {
    if (error) {
      return callback(error)
    }
--- a/services/web/test/acceptance/src/SubscriptionDashboardTests.js
+++ b/services/web/test/acceptance/src/SubscriptionDashboardTests.js
@ -489,7 +489,6 @@ describe('Subscriptions', function () {
    let userHelper, v1Id
    beforeEach(async function () {
      v1Id = MockV1Api.nextV1Id()
-      console.log('v1Id=', v1Id)
      userHelper = await UserHelper.createUser({ overleaf: { id: v1Id } })
      this.user = userHelper.user
      MockV1Api.setUser(v1Id, (this.v1_user = {}))
--- a/services/web/test/unit/src/Subscription/FeaturesUpdaterTests.js
+++ b/services/web/test/unit/src/Subscription/FeaturesUpdaterTests.js
@ -59,6 +59,7 @@ describe('FeaturesUpdater', function () {
    it('should return features and featuresChanged', function () {
      this.FeaturesUpdater.refreshFeatures(
        this.user_id,
+        'test',
        (err, features, featuresChanged) => {
          expect(err).to.not.exist
          expect(features).to.exist
@ -69,7 +70,11 @@ describe('FeaturesUpdater', function () {

    describe('normally', function () {
      beforeEach(function () {
-        this.FeaturesUpdater.refreshFeatures(this.user_id, this.callback)
+        this.FeaturesUpdater.refreshFeatures(
+          this.user_id,
+          'test',
+          this.callback
+        )
      })

      it('should get the individual features', function () {
@ -157,7 +162,11 @@ describe('FeaturesUpdater', function () {
        this.FeaturesUpdater._mergeFeatures = sinon
          .stub()
          .returns({ dropbox: false })
-        this.FeaturesUpdater.refreshFeatures(this.user_id, this.callback)
+        this.FeaturesUpdater.refreshFeatures(
+          this.user_id,
+          'test',
+          this.callback
+        )
      })
      it('should fire module hook to unlink dropbox', function () {
        this.Modules.hooks.fire
--- a/services/web/test/unit/src/Subscription/SubscriptionUpdaterTests.js
+++ b/services/web/test/unit/src/Subscription/SubscriptionUpdaterTests.js
@ -225,7 +225,7 @@ describe('SubscriptionUpdater', function () {

  describe('_updateSubscriptionFromRecurly', function () {
    beforeEach(function () {
-      this.FeaturesUpdater.refreshFeatures = sinon.stub().callsArgWith(1)
+      this.FeaturesUpdater.refreshFeatures = sinon.stub().callsArgWith(2)
      this.SubscriptionUpdater.deleteSubscription = sinon.stub().yields()
    })

@ -441,7 +441,7 @@ describe('SubscriptionUpdater', function () {

  describe('addUsersToGroup', function () {
    beforeEach(function () {
-      this.FeaturesUpdater.refreshFeatures = sinon.stub().callsArgWith(1)
+      this.FeaturesUpdater.refreshFeatures = sinon.stub().callsArgWith(2)
    })

    it('should add the user ids to the group as a set', function (done) {
@ -477,7 +477,7 @@ describe('SubscriptionUpdater', function () {

  describe('removeUserFromGroups', function () {
    beforeEach(function () {
-      this.FeaturesUpdater.refreshFeatures = sinon.stub().callsArgWith(1)
+      this.FeaturesUpdater.refreshFeatures = sinon.stub().callsArgWith(2)
      this.UserGetter.getUser.yields(null, {})
      this.fakeSubscriptions = [{ _id: 'fake-id-1' }, { _id: 'fake-id-2' }]
      this.SubscriptionLocator.getMemberSubscriptions.yields(