Merge pull request #1842 from overleaf/ta-remove-v1-sync

Remove v1 Sync on Features Refresh

GitOrigin-RevId: a99e0a74c4fc503bf81d0b847b3f331b3228e24f

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

@@ -85,7 +85,7 @@ var affiliateUserByReversedHostname = function(
             )
             return innerCallback(error)
           }
-          return FeaturesUpdater.refreshFeatures(user._id, true, innerCallback)
+          return FeaturesUpdater.refreshFeatures(user._id, innerCallback)
         }
       ),
     callback

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

@@ -114,7 +114,6 @@ var refreshFeatures = function(affiliation, callback) {
       cb =>
         FeaturesUpdater.refreshFeatures(
           userId,
-          true,
           (err, features, featuresChanged) => cb(err, featuresChanged)
         ),
       (featuresChanged, cb) =>

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

@@ -27,28 +27,10 @@ const InstitutionsFeatures = require('../Institutions/InstitutionsFeatures')
 const oneMonthInSeconds = 60 * 60 * 24 * 30
 
 module.exports = FeaturesUpdater = {
-  refreshFeatures(user_id, notifyV1, callback) {
-    if (notifyV1 == null) {
-      notifyV1 = true
-    }
+  refreshFeatures(user_id, callback) {
     if (callback == null) {
       callback = function(error, features, featuresChanged) {}
     }
-    if (typeof notifyV1 === 'function') {
-      callback = notifyV1
-      notifyV1 = true
-    }
-
-    if (notifyV1) {
-      V1SubscriptionManager.notifyV1OfFeaturesChange(user_id, function(error) {
-        if (error != null) {
-          return logger.err(
-            { err: error, user_id },
-            'error notifying v1 about updated features'
-          )
-        }
-      })
-    }
 
     const jobs = {
       individualFeatures(cb) {

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

@@ -58,22 +58,6 @@ module.exports = V1SubscriptionManager = {
     )
   },
 
-  notifyV1OfFeaturesChange(userId, callback) {
-    if (callback == null) {
-      callback = function(error) {}
-    }
-    return V1SubscriptionManager._v1Request(
-      userId,
-      {
-        method: 'POST',
-        url(v1Id) {
-          return `/api/v1/sharelatex/users/${v1Id}/sync`
-        }
-      },
-      callback
-    )
-  },
-
   getSubscriptionsFromV1(userId, callback) {
     if (callback == null) {
       callback = function(err, subscriptions, v1Id) {}

services/web/app/src/Features/User/UserUpdater.js

@@ -303,7 +303,7 @@ module.exports = UserUpdater = {
             new Errors.NotFoundError('user id and email do no match')
           )
         }
-        return FeaturesUpdater.refreshFeatures(userId, true, callback)
+        return FeaturesUpdater.refreshFeatures(userId, callback)
       })
     })
   },

services/web/scripts/sync_v1_subscriptions.js

@@ -1,71 +0,0 @@
-const { db } = require('../app/js/infrastructure/mongojs')
-const FeaturesUpdater = require('../app/js/Features/Subscription/FeaturesUpdater')
-const V1SubscriptionManager = require('../app/js/Features/Subscription/V1SubscriptionManager')
-const async = require('async')
-const logger = require('logger-sharelatex')
-logger.logger.level('error')
-
-const areFeaturesEqual = function(featuresA, featuresB) {
-  for (const feature in featuresA) {
-    if (featuresA[feature] !== featuresB[feature]) {
-      return false
-    }
-  }
-  return true
-}
-
-var outOfSyncUserCount = 0
-var userCount = null
-
-db.users.find(
-  {
-    'overleaf.id': { $exists: true }
-  },
-  {
-    overleaf: 1,
-    features: 1
-  },
-  function(error, users) {
-    if (error) throw error
-    console.log('USER COUNT', (userCount = users.length))
-    async.mapSeries(
-      users,
-      function(user, callback) {
-        console.log('REFRESHING IN v2', user._id)
-        FeaturesUpdater.refreshFeatures(user._id, false, function(error) {
-          if (error) console.error('ERROR', error)
-          console.log('REFRESHING IN v1', user._id)
-          V1SubscriptionManager.notifyV1OfFeaturesChange(user._id, function(
-            error
-          ) {
-            if (error) console.error('ERROR', error)
-            db.users.find(
-              {
-                _id: user._id
-              },
-              {
-                features: 1
-              },
-              function(error, [updatedUser]) {
-                if (error) throw error
-                if (areFeaturesEqual(user.features, updatedUser.features)) {
-                  console.log('UNCHANGED', user._id)
-                } else {
-                  console.log('MODIFIED', user._id)
-                  outOfSyncUserCount = outOfSyncUserCount + 1
-                }
-                callback()
-              }
-            )
-          })
-        })
-      },
-      function(error) {
-        if (error) throw error
-        console.log('FINISHED!')
-        console.log('OUT OF SYNC USERS', outOfSyncUserCount, '/', userCount)
-        process.exit()
-      }
-    )
-  }
-)

services/web/test/acceptance/src/FeatureUpdaterTests.js

@@ -30,7 +30,7 @@ const syncUserAndGetFeatures = function(user, callback) {
   if (callback == null) {
     callback = function(error, features) {}
   }
-  return FeaturesUpdater.refreshFeatures(user._id, false, function(error) {
+  return FeaturesUpdater.refreshFeatures(user._id, function(error) {
     if (error != null) {
       return callback(error)
     }
@@ -331,14 +331,5 @@ describe('FeatureUpdater.refreshFeatures', function() {
         }
       )
     }) // returns a promise
-
-    return it('should ping the v1 API end point to sync', function(done) {
-      return FeaturesUpdater.refreshFeatures(this.user._id, true, error => {
-        return setTimeout(() => {
-          expect(MockV1Api.syncUserFeatures.calledWith('42')).to.equal(true)
-          return done()
-        }, 500)
-      })
-    })
   })
 })

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

@@ -49,7 +49,7 @@ describe('InstitutionsController', function() {
       .stub()
       .callsArgWith(2, null, [this.stubbedUser1, this.stubbedUser2])
     this.addAffiliation = sinon.stub().callsArgWith(3, null)
-    this.refreshFeatures = sinon.stub().callsArgWith(2, null)
+    this.refreshFeatures = sinon.stub().yields(null)
     this.InstitutionsController = SandboxedModule.require(modulePath, {
       requires: {
         'logger-sharelatex': this.logger,
@@ -90,10 +90,10 @@ describe('InstitutionsController', function() {
           .calledWith(this.stubbedUser2._id, this.stubbedUser2.emails[0].email)
           .should.equal(true)
         this.refreshFeatures
-          .calledWith(this.stubbedUser1._id, true)
+          .calledWith(this.stubbedUser1._id)
           .should.equal(true)
         this.refreshFeatures
-          .calledWith(this.stubbedUser2._id, true)
+          .calledWith(this.stubbedUser2._id)
           .should.equal(true)
         return done()
       }

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

@@ -102,9 +102,7 @@ describe('InstitutionsManager', function() {
       this.SubscriptionLocator.getUsersSubscription
         .withArgs(this.user2)
         .callsArgWith(1, null, this.subscription)
-      this.refreshFeatures
-        .withArgs(this.user1Id)
-        .callsArgWith(2, null, {}, true)
+      this.refreshFeatures.withArgs(this.user1Id).yields(null, {}, true)
       return this.getInstitutionAffiliations.yields(null, this.affiliations)
     })
 

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

@@ -40,9 +40,6 @@ describe('FeaturesUpdater', function() {
 
   describe('refreshFeatures', function() {
     beforeEach(function() {
-      this.V1SubscriptionManager.notifyV1OfFeaturesChange = sinon
-        .stub()
-        .yields()
       this.UserFeaturesUpdater.updateFeatures = sinon.stub().yields()
       this.FeaturesUpdater._getIndividualFeatures = sinon
         .stub()
@@ -144,28 +141,6 @@ describe('FeaturesUpdater', function() {
           .calledWith(this.user_id, { merged: 'features' })
           .should.equal(true)
       })
-
-      return it('should notify v1', function() {
-        return this.V1SubscriptionManager.notifyV1OfFeaturesChange.called.should.equal(
-          true
-        )
-      })
-    })
-
-    return describe('with notifyV1 == false', function() {
-      beforeEach(function() {
-        return this.FeaturesUpdater.refreshFeatures(
-          this.user_id,
-          false,
-          this.callback
-        )
-      })
-
-      return it('should not notify v1', function() {
-        return this.V1SubscriptionManager.notifyV1OfFeaturesChange.called.should.equal(
-          false
-        )
-      })
     })
   })
 

services/web/test/unit/src/User/UserUpdaterTests.js

@@ -497,11 +497,7 @@ describe('UserUpdater', function() {
         this.newEmail,
         err => {
           should.not.exist(err)
-          sinon.assert.calledWith(
-            this.refreshFeatures,
-            this.stubbedUser._id,
-            true
-          )
+          sinon.assert.calledWith(this.refreshFeatures, this.stubbedUser._id)
           return done()
         }
       )