bug/analytics reset no longer clears anonoymousId (#655)

Author: silesky

packages/browser/src/browser/__tests__/anon-id-and-reset.integration.test.ts

@@ -0,0 +1,73 @@
+import unfetch from 'unfetch'
+import { AnalyticsBrowser } from '..'
+import {
+  clearAjsBrowserStorage,
+  getAnonId,
+} from '../../test-helpers/browser-storage'
+import { createSuccess } from '../../test-helpers/factories'
+
+jest.mock('unfetch')
+const helpers = {
+  mockFetchSettingsSuccessResponse: () => {
+    return jest
+      .mocked(unfetch)
+      .mockImplementation(() => createSuccess({ integrations: {} }))
+  },
+  loadAnalytics() {
+    return AnalyticsBrowser.load({ writeKey: 'foo' })
+  },
+}
+
+beforeEach(() => {
+  helpers.mockFetchSettingsSuccessResponse()
+})
+
+describe('anonymousId', () => {
+  describe('setting implicitly', () => {
+    it('is set if an event like track is called during pre-init period', async () => {
+      const analytics = helpers.loadAnalytics()
+      const ctx = await analytics.track('foo')
+      const id = getAnonId()
+      expect(id).toBeDefined()
+      expect(ctx.event.anonymousId).toBe(id)
+    })
+
+    it('sets the global anonymousId to the anonymousId set by the most recent event', async () => {
+      const analytics = helpers.loadAnalytics()
+      const trackCtx = await analytics.track(
+        'add to cart',
+        {},
+        { anonymousId: 'foo' }
+      )
+      expect(getAnonId()).toBe('foo')
+      expect(trackCtx.event.anonymousId).toBe('foo')
+      const idCtx = await analytics.identify('john')
+      expect(getAnonId()).toBe('foo')
+      expect(idCtx.event.anonymousId).toBe('foo')
+    })
+  })
+
+  describe('reset', () => {
+    beforeEach(() => {
+      clearAjsBrowserStorage()
+    })
+
+    it('clears anonId if reset is called during pre-init period', async () => {
+      const analytics = helpers.loadAnalytics()
+      const track = analytics.track('foo')
+      const reset = analytics.reset()
+      await Promise.all([track, reset])
+      expect(getAnonId()).toBeFalsy()
+    })
+
+    it('clears anonId if reset is called after initialization is complete', async () => {
+      const [analytics] = await helpers.loadAnalytics()
+      const track = analytics.track('foo')
+      expect(typeof getAnonId()).toBe('string')
+      analytics.reset()
+      expect(getAnonId()).toBeFalsy()
+      await track
+      expect(getAnonId()).toBeFalsy()
+    })
+  })
+})

packages/browser/src/browser/__tests__/integration.test.ts

@@ -17,6 +17,7 @@ import {
 } from '../../test-helpers/test-writekeys'
 import { PriorityQueue } from '../../lib/priority-queue'
 import { getCDN, setGlobalCDNUrl } from '../../lib/parse-cdn'
+import { clearAjsBrowserStorage } from '../../test-helpers/browser-storage'
 
 // eslint-disable-next-line @typescript-eslint/no-explicit-any
 let fetchCalls: Array<any>[] = []
@@ -595,6 +596,10 @@ describe('pageview', () => {
 })
 
 describe('setAnonymousId', () => {
+  beforeEach(() => {
+    clearAjsBrowserStorage()
+  })
+
   it('calling setAnonymousId will set a new anonymousId and returns it', async () => {
     const [analytics] = await AnalyticsBrowser.load({
       writeKey,

packages/browser/src/core/analytics/__tests__/integration.test.ts

@@ -1,6 +1,9 @@
 import { PriorityQueue } from '../../../lib/priority-queue'
 import { MiddlewareParams } from '../../../plugins/middleware'
-import { retrieveStoredData } from '../../../test-helpers/retrieve-stored-data'
+import {
+  getAjsBrowserStorage,
+  clearAjsBrowserStorage,
+} from '../../../test-helpers/browser-storage'
 import { Context } from '../../context'
 import { Plugin } from '../../plugin'
 import { EventQueue } from '../../queue/event-queue'
@@ -224,20 +227,22 @@ describe('Analytics', () => {
   })
 
   describe('reset', () => {
+    beforeEach(() => {
+      clearAjsBrowserStorage()
+    })
+
     it('clears user and group data', async () => {
       const analytics = new Analytics({ writeKey: '' })
 
-      const cookieNames = ['ajs_user_id', 'ajs_anonymous_id', 'ajs_group_id']
-      const localStorageKeys = ['ajs_user_traits', 'ajs_group_properties']
-
       analytics.user().anonymousId('unknown-user')
       analytics.user().id('known-user')
       analytics.user().traits({ job: 'engineer' })
       analytics.group().id('known-group')
       analytics.group().traits({ team: 'analytics' })
 
       // Ensure all cookies/localstorage is written correctly first
-      let storedData = retrieveStoredData({ cookieNames, localStorageKeys })
+
+      let storedData = getAjsBrowserStorage()
       expect(storedData).toEqual({
         ajs_user_id: 'known-user',
         ajs_anonymous_id: 'unknown-user',
@@ -252,7 +257,7 @@ describe('Analytics', () => {
 
       // Now make sure everything was cleared on reset
       analytics.reset()
-      storedData = retrieveStoredData({ cookieNames, localStorageKeys })
+      storedData = getAjsBrowserStorage()
       expect(storedData).toEqual({})
     })
   })

packages/browser/src/core/events/__tests__/index.test.ts

@@ -322,6 +322,48 @@ describe('Event Factory', () => {
         innerProp: '👻',
       })
     })
+
+    describe.skip('anonymousId', () => {
+      // TODO: the code should be fixed so that these tests can pass -- this eventFactory does not seem to handle these edge cases well.
+      // When an event is dispatched, there are four places anonymousId can live: event.anonymousId, event.options.anonymousId, event.context.anonymousId, and the user object / localStorage.
+      // It would be good to have a source of truth
+      test('accepts an anonymousId', () => {
+        const track = factory.track('Order Completed', shoes, {
+          anonymousId: 'foo',
+        })
+        expect(track.anonymousId).toBe('foo')
+        expect(track.context?.anonymousId).toBe('foo')
+      })
+
+      test('custom passed anonymousId should set global user instance', () => {
+        const id = Math.random().toString()
+        factory.track('Order Completed', shoes, {
+          anonymousId: id,
+        })
+        expect(user.anonymousId()).toBe(id)
+      })
+
+      test('if two different anonymousIds are passed, should use one on the event', () => {
+        const track = factory.track('Order Completed', shoes, {
+          anonymousId: 'bar',
+          context: {
+            anonymousId: 'foo',
+          },
+        })
+        expect(track.context?.anonymousId).toBe('bar')
+        expect(track.anonymousId).toBe('bar')
+      })
+
+      test('should set an anonymousId passed from the context on the event', () => {
+        const track = factory.track('Order Completed', shoes, {
+          context: {
+            anonymousId: 'foo',
+          },
+        })
+        expect(track.context?.anonymousId).toBe('foo')
+        expect(track.anonymousId).toBe('foo')
+      })
+    })
   })
 
   describe('normalize', function () {

packages/browser/src/core/events/index.ts

@@ -205,6 +205,11 @@ export class EventFactory {
   }
 
   public normalize(event: SegmentEvent): SegmentEvent {
+    // set anonymousId globally if we encounter an override
+    //segment.com/docs/connections/sources/catalog/libraries/website/javascript/identity/#override-the-anonymous-id-using-the-options-object
+    event.options?.anonymousId &&
+      this.user.anonymousId(event.options.anonymousId)
+
     const integrationBooleans = Object.keys(event.integrations ?? {}).reduce(
       (integrationNames, name) => {
         return {

packages/browser/src/plugins/segmentio/normalize.ts

@@ -124,7 +124,6 @@ export function normalize(
 
   json.context = json.context ?? json.options ?? {}
   const ctx = json.context
-  const anonId = json.anonymousId
 
   delete json.options
   json.writeKey = settings?.apiKey
@@ -159,7 +158,8 @@ export function normalize(
   referrerId(query, ctx, analytics.options.disableClientPersistence ?? false)
 
   json.userId = json.userId || user.id()
-  json.anonymousId = user.anonymousId(anonId)
+  json.anonymousId = json.anonymousId || user.anonymousId()
+
   json.sentAt = new Date()
 
   const failed = analytics.queue.failedInitializations || []

packages/browser/src/test-helpers/browser-storage.ts

@@ -0,0 +1,72 @@
+import cookie from 'js-cookie'
+
+const ajsCookieNames = [
+  'ajs_user_id',
+  'ajs_anonymous_id',
+  'ajs_group_id',
+] as const
+const ajsLocalStorageKeys = ['ajs_user_traits', 'ajs_group_properties'] as const
+
+export const getAjsBrowserStorage = () => {
+  return getBrowserStorage({
+    cookieNames: ajsCookieNames,
+    localStorageKeys: ajsLocalStorageKeys,
+  })
+}
+
+export const getAnonId = () => getAjsBrowserStorage().ajs_anonymous_id
+
+export const clearAjsBrowserStorage = () => {
+  return clearBrowserStorage({
+    cookieNames: ajsCookieNames,
+    localStorageKeys: ajsLocalStorageKeys,
+  })
+}
+
+export function getBrowserStorage<
+  CookieNames extends string,
+  LSKeys extends string
+>({
+  cookieNames,
+  localStorageKeys,
+}: {
+  cookieNames: readonly CookieNames[]
+  localStorageKeys: readonly LSKeys[]
+}): Record<CookieNames | LSKeys, string | {}> {
+  const result = {} as ReturnType<typeof getBrowserStorage>
+
+  const cookies = cookie.get()
+  cookieNames.forEach((name) => {
+    if (name in cookies) {
+      result[name] = cookies[name]
+    }
+  })
+
+  localStorageKeys.forEach((key) => {
+    const value = localStorage.getItem(key)
+    if (value !== null && typeof value !== 'undefined') {
+      result[key] = JSON.parse(value)
+    }
+  })
+
+  return result
+}
+
+export function clearBrowserStorage({
+  cookieNames,
+  localStorageKeys, // if no keys are passed, the entire thing is cleared
+}: {
+  cookieNames: string[] | readonly string[]
+  localStorageKeys?: string[] | readonly string[]
+}) {
+  cookieNames.forEach((name) => {
+    cookie.remove(name)
+  })
+  if (!localStorageKeys) {
+    localStorage.clear()
+  } else {
+    localStorageKeys.forEach((key) => {
+      localStorage.removeItem(key)
+    })
+  }
+}