Explorar o código

Merge pull request #5325 from overleaf/ab-identify-fix

Skip identify job when userId and analyticsId are the same ObjectID

GitOrigin-RevId: 150ac4aab6a33887c0b5ffba8c66cb9ec83f5c88
Jakob Ackermann %!s(int64=5) %!d(string=hai) anos
pai
achega
ca8b55def3

+ 1 - 1
services/web/app/src/Features/Analytics/AnalyticsManager.js

@@ -15,7 +15,7 @@ const analyticsUserPropertiesQueue = Queues.getAnalyticsUserPropertiesQueue()
 const ONE_MINUTE_MS = 60 * 1000
 const ONE_MINUTE_MS = 60 * 1000
 
 
 function identifyUser(userId, analyticsId, isNewUser) {
 function identifyUser(userId, analyticsId, isNewUser) {
-  if (!userId || !analyticsId || userId === analyticsId) {
+  if (!userId || !analyticsId || userId.toString() === analyticsId.toString()) {
     return
     return
   }
   }
   if (_isAnalyticsDisabled() || _isSmokeTestUser(userId)) {
   if (_isAnalyticsDisabled() || _isSmokeTestUser(userId)) {

+ 25 - 3
services/web/test/unit/src/Analytics/AnalyticsManagerTests.js

@@ -4,6 +4,7 @@ const sinon = require('sinon')
 const MockRequest = require('../helpers/MockRequest')
 const MockRequest = require('../helpers/MockRequest')
 const MockResponse = require('../helpers/MockResponse')
 const MockResponse = require('../helpers/MockResponse')
 const { assert } = require('chai')
 const { assert } = require('chai')
+const { ObjectID } = require('mongodb')
 
 
 const MODULE_PATH = path.join(
 const MODULE_PATH = path.join(
   __dirname,
   __dirname,
@@ -12,7 +13,7 @@ const MODULE_PATH = path.join(
 
 
 describe('AnalyticsManager', function () {
 describe('AnalyticsManager', function () {
   beforeEach(function () {
   beforeEach(function () {
-    this.fakeUserId = '123abc'
+    this.fakeUserId = 'dbfc9438d14996f73dd172fb'
     this.analyticsId = 'ecdb935a-52f3-4f91-aebc-7a70d2ffbb55'
     this.analyticsId = 'ecdb935a-52f3-4f91-aebc-7a70d2ffbb55'
     this.Settings = {
     this.Settings = {
       analytics: { enabled: true },
       analytics: { enabled: true },
@@ -74,15 +75,36 @@ describe('AnalyticsManager', function () {
       sinon.assert.notCalled(this.analyticsEventsQueue.add)
       sinon.assert.notCalled(this.analyticsEventsQueue.add)
     })
     })
 
 
-    it('userId or analyticsId is missing', function () {
+    it('userId is missing', function () {
+      this.AnalyticsManager.identifyUser(undefined, this.analyticsId)
+      sinon.assert.notCalled(this.analyticsEventsQueue.add)
+    })
+
+    it('analyticsId is missing', function () {
       this.AnalyticsManager.identifyUser(this.fakeUserId, undefined)
       this.AnalyticsManager.identifyUser(this.fakeUserId, undefined)
       sinon.assert.notCalled(this.analyticsEventsQueue.add)
       sinon.assert.notCalled(this.analyticsEventsQueue.add)
     })
     })
 
 
-    it('userId equal analyticsId', function () {
+    it('userId equals analyticsId', function () {
       this.AnalyticsManager.identifyUser(this.fakeUserId, this.fakeUserId)
       this.AnalyticsManager.identifyUser(this.fakeUserId, this.fakeUserId)
       sinon.assert.notCalled(this.analyticsEventsQueue.add)
       sinon.assert.notCalled(this.analyticsEventsQueue.add)
     })
     })
+
+    it('Mongo userId equals string userId', function () {
+      this.AnalyticsManager.identifyUser(
+        new ObjectID(this.fakeUserId),
+        this.fakeUserId
+      )
+      sinon.assert.notCalled(this.analyticsEventsQueue.add)
+    })
+
+    it('userId and analyticsId are the same Mongo ID', function () {
+      this.AnalyticsManager.identifyUser(
+        new ObjectID(this.fakeUserId),
+        new ObjectID(this.fakeUserId)
+      )
+      sinon.assert.notCalled(this.analyticsEventsQueue.add)
+    })
   })
   })
 
 
   describe('queues the appropriate message for', function () {
   describe('queues the appropriate message for', function () {