Skip to content

Commit

Permalink
feat: Add initial person info to cookie when using localPlusCookieSto…
Browse files Browse the repository at this point in the history
…re (#1559)

* Add initial person info to cookie storage

* Add tests for initial person info

* Add another test to ensure that initial person properties work cross subdomain

* Add a maximum length to initial person info

* Fix tests
  • Loading branch information
robbie-c authored Nov 27, 2024
1 parent 92190d0 commit 2de8e16
Show file tree
Hide file tree
Showing 4 changed files with 105 additions and 6 deletions.
79 changes: 78 additions & 1 deletion src/__tests__/personProcessing.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ jest.mock('../utils/globals', () => {
const orig = jest.requireActual('../utils/globals')
const mockURLGetter = jest.fn()
const mockReferrerGetter = jest.fn()
let mockedCookieVal = ''
return {
...orig,
mockURLGetter,
Expand All @@ -50,6 +51,12 @@ jest.mock('../utils/globals', () => {
get URL() {
return mockURLGetter()
},
get cookie() {
return mockedCookieVal
},
set cookie(value: string) {
mockedCookieVal = value
},
},
get location() {
const url = mockURLGetter()
Expand All @@ -62,14 +69,15 @@ jest.mock('../utils/globals', () => {
})

// eslint-disable-next-line @typescript-eslint/no-require-imports
const { mockURLGetter, mockReferrerGetter } = require('../utils/globals')
const { mockURLGetter, mockReferrerGetter, document } = require('../utils/globals')

describe('person processing', () => {
const distinctId = '123'
beforeEach(() => {
console.error = jest.fn()
mockReferrerGetter.mockReturnValue('https://referrer.com')
mockURLGetter.mockReturnValue('https://example.com?utm_source=foo')
document.cookie = ''
})

const setup = async (
Expand Down Expand Up @@ -268,6 +276,75 @@ describe('person processing', () => {
})
})

it('should preserve initial referrer info across subdomain', async () => {
const persistenceName = uuidv7()

// arrange
const { posthog: posthog1, beforeSendMock: beforeSendMock1 } = await setup(
'identified_only',
undefined,
persistenceName
)
mockReferrerGetter.mockReturnValue('https://referrer1.com')
mockURLGetter.mockReturnValue('https://example1.com/pathname1?utm_source=foo1')

// act
// subdomain 1
posthog1.register({ testProp: 'foo' })
posthog1.capture('event s1')

// clear localstorage, but not cookies, to simulate changing subdomain
window.localStorage.clear()

// subdomain 2
const { posthog: posthog2, beforeSendMock: beforeSendMock2 } = await setup(
'identified_only',
undefined,
persistenceName
)

mockReferrerGetter.mockReturnValue('https://referrer2.com')
mockURLGetter.mockReturnValue('https://example2.com/pathname2?utm_source=foo2')
posthog2.capture('event s2 before identify')
posthog2.identify(distinctId)
posthog2.capture('event s2 after identify')

// assert
const eventS1 = beforeSendMock1.mock.calls[0]
const eventS2Before = beforeSendMock2.mock.calls[0]
const eventS2Identify = beforeSendMock2.mock.calls[1]
const eventS2After = beforeSendMock2.mock.calls[2]

expect(eventS1[0].$set_once).toEqual(undefined)
expect(eventS1[0].properties.testProp).toEqual('foo')

expect(eventS2Before[0].$set_once).toEqual(undefined)
// most properties are lost across subdomain, that's intentional as we don't want to save too many things in cookies
expect(eventS2Before[0].properties.testProp).toEqual(undefined)

expect(eventS2Identify[0].event).toEqual('$identify')
expect(eventS2Identify[0].$set_once).toEqual({
...INITIAL_CAMPAIGN_PARAMS_NULL,
$initial_current_url: 'https://example1.com/pathname1?utm_source=foo1',
$initial_host: 'example1.com',
$initial_pathname: '/pathname1',
$initial_referrer: 'https://referrer1.com',
$initial_referring_domain: 'referrer1.com',
$initial_utm_source: 'foo1',
})

expect(eventS2After[0].event).toEqual('event s2 after identify')
expect(eventS2After[0].$set_once).toEqual({
...INITIAL_CAMPAIGN_PARAMS_NULL,
$initial_current_url: 'https://example1.com/pathname1?utm_source=foo1',
$initial_host: 'example1.com',
$initial_pathname: '/pathname1',
$initial_referrer: 'https://referrer1.com',
$initial_referring_domain: 'referrer1.com',
$initial_utm_source: 'foo1',
})
})

it('should include initial referrer info in identify event if always', async () => {
// arrange
const { posthog, beforeSendMock } = await setup('always')
Expand Down
12 changes: 11 additions & 1 deletion src/__tests__/posthog-persistence.test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
/// <reference lib="dom" />
import { PostHogPersistence } from '../posthog-persistence'
import { SESSION_ID, USER_STATE } from '../constants'
import { INITIAL_PERSON_INFO, SESSION_ID, USER_STATE } from '../constants'
import { PostHogConfig } from '../types'
import Mock = jest.Mock
import { PostHog } from '../posthog-core'
Expand Down Expand Up @@ -160,6 +160,15 @@ describe('persistence', () => {
})}`
)

lib.register({ [INITIAL_PERSON_INFO]: { u: 'https://www.example.com', r: 'https://www.referrer.com' } })
expect(document.cookie).toContain(
`ph__posthog=${encode({
distinct_id: 'test',
$sesid: [1000, 'sid', 2000],
$initial_person_info: { u: 'https://www.example.com', r: 'https://www.referrer.com' },
})}`
)

// Clear localstorage to simulate being on a different domain
localStorage.clear()

Expand All @@ -168,6 +177,7 @@ describe('persistence', () => {
expect(newLib.props).toEqual({
distinct_id: 'test',
$sesid: [1000, 'sid', 2000],
$initial_person_info: { u: 'https://www.example.com', r: 'https://www.referrer.com' },
})
})

Expand Down
16 changes: 14 additions & 2 deletions src/storage.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,12 @@
import { extend } from './utils'
import { PersistentStore, Properties } from './types'
import { DISTINCT_ID, ENABLE_PERSON_PROCESSING, SESSION_ID, SESSION_RECORDING_IS_SAMPLED } from './constants'
import {
DISTINCT_ID,
ENABLE_PERSON_PROCESSING,
INITIAL_PERSON_INFO,
SESSION_ID,
SESSION_RECORDING_IS_SAMPLED,
} from './constants'

import { isNull, isUndefined } from './utils/type-utils'
import { logger } from './utils/logger'
Expand Down Expand Up @@ -248,7 +254,13 @@ export const localStore: PersistentStore = {
// Use localstorage for most data but still use cookie for COOKIE_PERSISTED_PROPERTIES
// This solves issues with cookies having too much data in them causing headers too large
// Also makes sure we don't have to send a ton of data to the server
const COOKIE_PERSISTED_PROPERTIES = [DISTINCT_ID, SESSION_ID, SESSION_RECORDING_IS_SAMPLED, ENABLE_PERSON_PROCESSING]
const COOKIE_PERSISTED_PROPERTIES = [
DISTINCT_ID,
SESSION_ID,
SESSION_RECORDING_IS_SAMPLED,
ENABLE_PERSON_PROCESSING,
INITIAL_PERSON_INFO,
]

export const localPlusCookieStore: PersistentStore = {
...localStore,
Expand Down
4 changes: 2 additions & 2 deletions src/utils/event-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -164,8 +164,8 @@ export const Info = {
initialPersonInfo: function (): Record<string, any> {
// we're being a bit more economical with bytes here because this is stored in the cookie
return {
r: this.referrer(),
u: location?.href,
r: this.referrer().substring(0, 1000),
u: location?.href.substring(0, 1000),
}
},

Expand Down

0 comments on commit 2de8e16

Please sign in to comment.