Merge pull request #4719 from overleaf/jel-user-features-refresh

Fix featuresChanged flag

GitOrigin-RevId: 1290f9b2b43cae2f1c4912df649800453ca20344

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

@@ -1,8 +1,16 @@
 const { User } = require('../../models/User')
 
+function _featuresChanged(newFeatures, featuresBefore) {
+  for (const feature in newFeatures) {
+    if (featuresBefore[feature] !== newFeatures[feature]) {
+      return true
+    }
+  }
+  return false
+}
+
 module.exports = {
   updateFeatures(userId, features, callback) {
-    const conditions = { _id: userId }
     const update = {
       featuresUpdatedAt: new Date(),
     }
@@ -10,16 +18,24 @@ module.exports = {
       const value = features[key]
       update[`features.${key}`] = value
     }
-    User.updateOne(conditions, update, (err, result) =>
-      callback(err, features, (result ? result.nModified : 0) === 1)
-    )
+    User.findByIdAndUpdate(userId, update, (err, docBeforeUpdate) => {
+      let featuresChanged = false
+      if (docBeforeUpdate) {
+        featuresChanged = _featuresChanged(features, docBeforeUpdate.features)
+      }
+
+      return callback(err, features, featuresChanged)
+    })
   },
 
   overrideFeatures(userId, features, callback) {
-    const conditions = { _id: userId }
     const update = { features, featuresUpdatedAt: new Date() }
-    User.updateOne(conditions, update, (err, result) =>
-      callback(err, (result ? result.nModified : 0) === 1)
-    )
+    User.findByIdAndUpdate(userId, update, (err, docBeforeUpdate) => {
+      let featuresChanged = false
+      if (docBeforeUpdate) {
+        featuresChanged = _featuresChanged(features, docBeforeUpdate.features)
+      }
+      return callback(err, featuresChanged)
+    })
   },
 }

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

@@ -6,7 +6,24 @@ const modulePath =
 
 describe('UserFeaturesUpdater', function () {
   beforeEach(function () {
-    this.User = { updateOne: sinon.stub().callsArgWith(2) }
+    this.features = {
+      collaborators: 6,
+      dropbox: true,
+      github: true,
+      gitBridge: true,
+      versioning: true,
+      compileTimeout: 180,
+      compileGroup: 'priority',
+      references: true,
+      templates: true,
+      trackChanges: true,
+      referencesSearch: true,
+      zotero: true,
+      mendeley: true,
+    }
+    this.User = {
+      findByIdAndUpdate: sinon.stub().yields(null, { features: this.features }),
+    }
     this.UserFeaturesUpdater = SandboxedModule.require(modulePath, {
       requires: {
         '../../models/User': {
@@ -19,26 +36,25 @@ describe('UserFeaturesUpdater', function () {
   describe('updateFeatures', function () {
     it('should send the users features', function (done) {
       const userId = '5208dd34438842e2db000005'
-      this.features = { versioning: true, collaborators: 10 }
+      const update = {
+        versioning: true,
+        collaborators: 10,
+      }
       this.UserFeaturesUpdater.updateFeatures(
         userId,
-        this.features,
+        update,
         (err, features) => {
-          const update = {
-            'features.versioning': true,
-            'features.collaborators': 10,
-          }
-          const updateArgs = this.User.updateOne.lastCall.args
-          expect(updateArgs[0]).to.deep.equal({ _id: userId })
+          const updateArgs = this.User.findByIdAndUpdate.lastCall.args
+          expect(updateArgs[0]).to.deep.equal(userId)
           expect(Object.keys(updateArgs[1]).length).to.equal(3)
           expect(updateArgs[1]['features.versioning']).to.equal(
-            update['features.versioning']
+            update.versioning
           )
           expect(updateArgs[1]['features.collaborators']).to.equal(
-            update['features.collaborators']
+            update.collaborators
           )
           expect(updateArgs[1].featuresUpdatedAt instanceof Date).to.be.true
-          features.should.deep.equal(this.features)
+          features.should.deep.equal(update)
           done()
         }
       )
@@ -48,26 +64,17 @@ describe('UserFeaturesUpdater', function () {
   describe('overrideFeatures', function () {
     it('should send the users features', function (done) {
       const userId = '5208dd34438842e2db000005'
-      this.features = { versioning: true, collaborators: 10 }
-      this.UserFeaturesUpdater.updateFeatures(
+      const update = Object.assign({}, { mendeley: !this.features.mendeley })
+      this.UserFeaturesUpdater.overrideFeatures(
         userId,
-        this.features,
-        (err, features) => {
-          const update = {
-            'features.versioning': true,
-            'features.collaborators': 10,
-          }
-          const updateArgs = this.User.updateOne.lastCall.args
-          expect(updateArgs[0]).to.deep.equal({ _id: userId })
-          expect(Object.keys(updateArgs[1]).length).to.equal(3)
-          expect(updateArgs[1]['features.versioning']).to.equal(
-            update['features.versioning']
-          )
-          expect(updateArgs[1]['features.collaborators']).to.equal(
-            update['features.collaborators']
-          )
+        update,
+        (err, featuresChanged) => {
+          const updateArgs = this.User.findByIdAndUpdate.lastCall.args
+          expect(updateArgs[0]).to.equal(userId)
+          expect(Object.keys(updateArgs[1]).length).to.equal(2)
+          expect(updateArgs[1].features).to.deep.equal(update)
           expect(updateArgs[1].featuresUpdatedAt instanceof Date).to.be.true
-          features.should.deep.equal(this.features)
+          featuresChanged.should.equal(true)
           done()
         }
       )