Merge pull request #4287 from overleaf/jpa-no-analytics-queues-server-ce-pro

[misc] do not set up analytics queues in Server CE/Pro

GitOrigin-RevId: 61a62f0ff7f04d5206845e01c68097229a5954fd

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

@@ -1,3 +1,4 @@
+const Features = require('../../infrastructure/Features')
 const Queues = require('../../infrastructure/Queues')
 const EmailHandler = require('../Email/EmailHandler')
 const UserUpdater = require('./UserUpdater')
@@ -34,4 +35,10 @@ class UserOnboardingEmailManager {
   }
 }
 
-module.exports = new UserOnboardingEmailManager()
+class NoopManager {
+  async scheduleOnboardingEmail() {}
+}
+
+module.exports = Features.hasFeature('saas')
+  ? new UserOnboardingEmailManager()
+  : new NoopManager()

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

@@ -4,6 +4,7 @@ const {
   promises: InstitutionsAPIPromises,
 } = require('../Institutions/InstitutionsAPI')
 const AnalyticsManager = require('../Analytics/AnalyticsManager')
+const Features = require('../../infrastructure/Features')
 
 const ONE_DAY_MS = 24 * 60 * 60 * 1000
 
@@ -47,4 +48,10 @@ async function checkAffiliations(userId) {
   }
 }
 
-module.exports = new UserPostRegistrationAnalyticsManager()
+class NoopManager {
+  async schedulePostRegistrationAnalytics() {}
+}
+
+module.exports = Features.hasFeature('saas')
+  ? new UserPostRegistrationAnalyticsManager()
+  : new NoopManager()

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

@@ -1,6 +1,7 @@
 const SandboxedModule = require('sandboxed-module')
 const path = require('path')
 const sinon = require('sinon')
+const { expect } = require('chai')
 
 const MODULE_PATH = path.join(
   __dirname,
@@ -17,11 +18,10 @@ describe('UserOnboardingEmailManager', function () {
         this.queueProcessFunction = callback
       },
     }
-    const self = this
     this.Queues = {
-      getOnboardingEmailsQueue: () => {
-        return self.onboardingEmailsQueue
-      },
+      getOnboardingEmailsQueue: sinon
+        .stub()
+        .returns(this.onboardingEmailsQueue),
     }
     this.UserGetter = {
       promises: {
@@ -41,21 +41,49 @@ describe('UserOnboardingEmailManager', function () {
         updateUser: sinon.stub().resolves(),
       },
     }
+    this.Features = {
+      hasFeature: sinon.stub(),
+    }
     this.request = sinon.stub().yields()
-    this.UserOnboardingEmailManager = SandboxedModule.require(MODULE_PATH, {
-      globals: {
-        console: console,
-      },
-      requires: {
-        '../../infrastructure/Queues': this.Queues,
-        '../Email/EmailHandler': this.EmailHandler,
-        './UserGetter': this.UserGetter,
-        './UserUpdater': this.UserUpdater,
-      },
+
+    this.init = isSAAS => {
+      this.Features.hasFeature.withArgs('saas').returns(isSAAS)
+      this.UserOnboardingEmailManager = SandboxedModule.require(MODULE_PATH, {
+        globals: {
+          console: console,
+        },
+        requires: {
+          '../../infrastructure/Features': this.Features,
+          '../../infrastructure/Queues': this.Queues,
+          '../Email/EmailHandler': this.EmailHandler,
+          './UserGetter': this.UserGetter,
+          './UserUpdater': this.UserUpdater,
+        },
+      })
+    }
+  })
+
+  describe('in Server CE/Pro', function () {
+    beforeEach(function () {
+      this.init(false)
+    })
+
+    it('should not create any queue', function () {
+      expect(this.Queues.getOnboardingEmailsQueue).to.not.have.been.called
+    })
+    it('should not schedule any email', function () {
+      this.UserOnboardingEmailManager.scheduleOnboardingEmail({
+        _id: this.fakeUserId,
+      })
+      expect(this.onboardingEmailsQueue.add).to.not.have.been.called
     })
   })
 
-  describe('schedule email', function () {
+  describe('schedule email in SAAS', function () {
+    beforeEach(function () {
+      this.init(true)
+    })
+
     it('should schedule delayed job on queue', function () {
       this.UserOnboardingEmailManager.scheduleOnboardingEmail({
         _id: this.fakeUserId,

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

@@ -1,6 +1,7 @@
 const SandboxedModule = require('sandboxed-module')
 const path = require('path')
 const sinon = require('sinon')
+const { expect } = require('chai')
 
 const MODULE_PATH = path.join(
   __dirname,
@@ -16,11 +17,10 @@ describe('UserPostRegistrationAnalyticsManager', function () {
         this.queueProcessFunction = callback
       },
     }
-    const self = this
     this.Queues = {
-      getPostRegistrationAnalyticsQueue: () => {
-        return self.postRegistrationAnalyticsQueue
-      },
+      getPostRegistrationAnalyticsQueue: sinon
+        .stub()
+        .returns(this.postRegistrationAnalyticsQueue),
     }
     this.UserGetter = {
       promises: {
@@ -35,88 +35,114 @@ describe('UserPostRegistrationAnalyticsManager', function () {
     this.AnalyticsManager = {
       setUserProperty: sinon.stub().resolves(),
     }
-    this.UserPostRegistrationAnalyticsManager = SandboxedModule.require(
-      MODULE_PATH,
-      {
-        globals: {
-          console: console,
-        },
-        requires: {
-          '../../infrastructure/Queues': this.Queues,
-          './UserGetter': this.UserGetter,
-          '../Institutions/InstitutionsAPI': this.InstitutionsAPI,
-          '../Analytics/AnalyticsManager': this.AnalyticsManager,
-        },
-      }
-    )
+    this.Features = {
+      hasFeature: sinon.stub().returns(true),
+    }
+    this.init = isSAAS => {
+      this.Features.hasFeature.withArgs('saas').returns(isSAAS)
+      this.UserPostRegistrationAnalyticsManager = SandboxedModule.require(
+        MODULE_PATH,
+        {
+          requires: {
+            '../../infrastructure/Features': this.Features,
+            '../../infrastructure/Queues': this.Queues,
+            './UserGetter': this.UserGetter,
+            '../Institutions/InstitutionsAPI': this.InstitutionsAPI,
+            '../Analytics/AnalyticsManager': this.AnalyticsManager,
+          },
+        }
+      )
+    }
   })
 
-  describe('schedule jobs', function () {
+  describe('in Server CE/Pro', function () {
+    beforeEach(function () {
+      this.init(false)
+    })
+
     it('should schedule delayed job on queue', function () {
       this.UserPostRegistrationAnalyticsManager.schedulePostRegistrationAnalytics(
-        {
-          _id: this.fakeUserId,
-        }
-      )
-      sinon.assert.calledWithMatch(
-        this.postRegistrationAnalyticsQueue.add,
-        { userId: this.fakeUserId },
-        { delay: 24 * 60 * 60 * 1000 }
+        { _id: this.fakeUserId }
       )
+      expect(this.Queues.getPostRegistrationAnalyticsQueue).to.not.have.been
+        .called
+      expect(this.postRegistrationAnalyticsQueue.add).to.not.have.been.called
     })
   })
 
-  describe('process jobs', function () {
-    it('stops without errors if user is not found', async function () {
-      this.UserGetter.promises.getUser.resolves(null)
-      await this.queueProcessFunction({ data: { userId: this.fakeUserId } })
-      sinon.assert.calledWith(this.UserGetter.promises.getUser, {
-        _id: this.fakeUserId,
+  describe('in SAAS', function () {
+    beforeEach(function () {
+      this.init(true)
+    })
+    describe('schedule jobs in SAAS', function () {
+      it('should schedule delayed job on queue', function () {
+        this.UserPostRegistrationAnalyticsManager.schedulePostRegistrationAnalytics(
+          {
+            _id: this.fakeUserId,
+          }
+        )
+        sinon.assert.calledWithMatch(
+          this.postRegistrationAnalyticsQueue.add,
+          { userId: this.fakeUserId },
+          { delay: 24 * 60 * 60 * 1000 }
+        )
       })
-      sinon.assert.notCalled(this.InstitutionsAPI.promises.getUserAffiliations)
-      sinon.assert.notCalled(this.AnalyticsManager.setUserProperty)
     })
 
-    it('sets user property if user has commons account affiliationd', async function () {
-      this.InstitutionsAPI.promises.getUserAffiliations.resolves([
-        {},
-        {
-          institution: {
-            commonsAccount: true,
+    describe('process jobs', function () {
+      it('stops without errors if user is not found', async function () {
+        this.UserGetter.promises.getUser.resolves(null)
+        await this.queueProcessFunction({ data: { userId: this.fakeUserId } })
+        sinon.assert.calledWith(this.UserGetter.promises.getUser, {
+          _id: this.fakeUserId,
+        })
+        sinon.assert.notCalled(
+          this.InstitutionsAPI.promises.getUserAffiliations
+        )
+        sinon.assert.notCalled(this.AnalyticsManager.setUserProperty)
+      })
+
+      it('sets user property if user has commons account affiliationd', async function () {
+        this.InstitutionsAPI.promises.getUserAffiliations.resolves([
+          {},
+          {
+            institution: {
+              commonsAccount: true,
+            },
           },
-        },
-        {
-          institution: {
-            commonsAccount: false,
+          {
+            institution: {
+              commonsAccount: false,
+            },
           },
-        },
-      ])
-      await this.queueProcessFunction({ data: { userId: this.fakeUserId } })
-      sinon.assert.calledWith(this.UserGetter.promises.getUser, {
-        _id: this.fakeUserId,
+        ])
+        await this.queueProcessFunction({ data: { userId: this.fakeUserId } })
+        sinon.assert.calledWith(this.UserGetter.promises.getUser, {
+          _id: this.fakeUserId,
+        })
+        sinon.assert.calledWith(
+          this.InstitutionsAPI.promises.getUserAffiliations,
+          this.fakeUserId
+        )
+        sinon.assert.calledWith(
+          this.AnalyticsManager.setUserProperty,
+          this.fakeUserId,
+          'registered-from-commons-account',
+          true
+        )
       })
-      sinon.assert.calledWith(
-        this.InstitutionsAPI.promises.getUserAffiliations,
-        this.fakeUserId
-      )
-      sinon.assert.calledWith(
-        this.AnalyticsManager.setUserProperty,
-        this.fakeUserId,
-        'registered-from-commons-account',
-        true
-      )
-    })
 
-    it('does not set user property if user has no commons account affiliation', async function () {
-      this.InstitutionsAPI.promises.getUserAffiliations.resolves([
-        {
-          institution: {
-            commonsAccount: false,
+      it('does not set user property if user has no commons account affiliation', async function () {
+        this.InstitutionsAPI.promises.getUserAffiliations.resolves([
+          {
+            institution: {
+              commonsAccount: false,
+            },
           },
-        },
-      ])
-      await this.queueProcessFunction({ data: { userId: this.fakeUserId } })
-      sinon.assert.notCalled(this.AnalyticsManager.setUserProperty)
+        ])
+        await this.queueProcessFunction({ data: { userId: this.fakeUserId } })
+        sinon.assert.notCalled(this.AnalyticsManager.setUserProperty)
+      })
     })
   })
 })