fix: avoid multiple segment identify calls (openedx#458)

nelc · Mar 28, 2023 · 1590bd8 · 1590bd8
1 parent 0ecb72b
commit 1590bd8
Show file tree

Hide file tree

Showing 2 changed files with 59 additions and 0 deletions.
diff --git a/src/analytics/SegmentAnalyticsService.js b/src/analytics/SegmentAnalyticsService.js
@@ -157,6 +157,10 @@ class SegmentAnalyticsService {
    * @param {*} [traits]
    */
   identifyAuthenticatedUser(userId, traits) {
+    if (this.hasIdentifyBeenCalled) {
+      return;
+    }
+
     if (!userId) {
       throw new Error('UserId is required for identifyAuthenticatedUser.');
     }
@@ -175,6 +179,10 @@ class SegmentAnalyticsService {
    * @returns {Promise} Promise that will resolve once the document readyState is complete
    */
   identifyAnonymousUser(traits) { // eslint-disable-line no-unused-vars
+    if (this.hasIdentifyBeenCalled) {
+      return Promise.resolve();
+    }
+
     if (!this.segmentInitialized) {
       return Promise.resolve();
     }

diff --git a/src/analytics/interface.test.js b/src/analytics/interface.test.js
@@ -112,6 +112,30 @@ describe('Analytics', () => {
         expect(() => identifyAuthenticatedUser(null))
           .toThrowError(new Error('UserId is required for identifyAuthenticatedUser.'));
       });
+
+      it('should call segment identify once', () => {
+        const testTraits = { anything: 'Yay!' };
+        identifyAuthenticatedUser(testUserId, testTraits);
+        identifyAuthenticatedUser(testUserId, testTraits);
+
+        expect(window.analytics.identify.mock.calls.length).toBe(1);
+      });
+
+      it('should call segment identify if hasIdentifyBeenCalled is false', () => {
+        const testTraits = { anything: 'Yay!' };
+        service.hasIdentifyBeenCalled = false;
+        identifyAuthenticatedUser(testUserId, testTraits);
+
+        expect(window.analytics.identify).toHaveBeenCalled();
+      });
+
+      it('should not call segment identify if hasIdentifyBeenCalled is true', () => {
+        const testTraits = { anything: 'Yay!' };
+        service.hasIdentifyBeenCalled = true;
+        identifyAuthenticatedUser(testUserId, testTraits);
+
+        expect(window.analytics.identify).not.toHaveBeenCalled();
+      });
     });
 
     describe('analytics identifyAnonymousUser', () => {
@@ -130,6 +154,33 @@ describe('Analytics', () => {
 
         expect(window.analytics.reset.mock.calls.length).toBe(1);
       });
+
+      it('should call segment reset once', () => {
+        window.analytics.user = () => ({ id: () => 1 });
+        const testTraits = { anything: 'Yay!' };
+        identifyAnonymousUser(testTraits);
+        identifyAnonymousUser(testTraits);
+
+        expect(window.analytics.reset.mock.calls.length).toBe(1);
+      });
+
+      it('should call segment reset if hasIdentifyBeenCalled is false', () => {
+        window.analytics.user = () => ({ id: () => 2 });
+        const testTraits = { anything: 'Yay!' };
+        service.hasIdentifyBeenCalled = false;
+        identifyAnonymousUser(testTraits);
+
+        expect(window.analytics.reset).toHaveBeenCalled();
+      });
+
+      it('should not call segment reset if hasIdentifyBeenCalled is true', () => {
+        window.analytics.user = () => ({ id: () => 3 });
+        const testTraits = { anything: 'Yay!' };
+        service.hasIdentifyBeenCalled = true;
+        identifyAnonymousUser(testTraits);
+
+        expect(window.analytics.reset).not.toHaveBeenCalled();
+      });
     });
 
     function testSendPageAfterIdentify(identifyFunction) {