Skip to content

Commit

Permalink
feat: swap the default to identified_only (#1468)
Browse files Browse the repository at this point in the history
Co-authored-by: Robbie Coomber <[email protected]>
  • Loading branch information
raquelmsmith and robbie-c authored Oct 17, 2024
1 parent 646b0f2 commit 387d96e
Show file tree
Hide file tree
Showing 5 changed files with 154 additions and 11 deletions.
106 changes: 103 additions & 3 deletions src/__tests__/personProcessing.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { createPosthogInstance } from './helpers/posthog-instance'
import { uuidv7 } from '../uuidv7'
import { logger } from '../utils/logger'
import { INITIAL_CAMPAIGN_PARAMS, INITIAL_REFERRER_INFO } from '../constants'
import { DecideResponse } from '../types'

jest.mock('../utils/logger')

Expand Down Expand Up @@ -71,18 +72,23 @@ describe('person processing', () => {
mockURLGetter.mockReturnValue('https://example.com?utm_source=foo')
})

const setup = async (person_profiles: 'always' | 'identified_only' | 'never' | undefined, token?: string) => {
const setup = async (
person_profiles: 'always' | 'identified_only' | 'never' | undefined,
token?: string,
persistence_name?: string
) => {
token = token || uuidv7()
const onCapture = jest.fn()
const posthog = await createPosthogInstance(token, {
_onCapture: onCapture,
person_profiles,
persistence_name,
})
return { token, onCapture, posthog }
}

describe('init', () => {
it("should default to 'always' person_profiles", async () => {
it("should default to 'identified_only' person_profiles", async () => {
// arrange
const token = uuidv7()

Expand All @@ -92,7 +98,7 @@ describe('person processing', () => {
})

// assert
expect(posthog.config.person_profiles).toEqual('always')
expect(posthog.config.person_profiles).toEqual('identified_only')
})
it('should read person_profiles from init config', async () => {
// arrange
Expand Down Expand Up @@ -582,4 +588,98 @@ describe('person processing', () => {
expect(onCapture.mock.calls[2][1].properties.$process_person_profile).toEqual(false)
})
})

describe('persistence', () => {
it('should remember that a user set the mode to always on a previous visit', async () => {
// arrange
const persistenceName = uuidv7()
const { posthog: posthog1, onCapture: onCapture1 } = await setup('always', undefined, persistenceName)
posthog1.capture('custom event 1')
const { posthog: posthog2, onCapture: onCapture2 } = await setup(
'identified_only',
undefined,
persistenceName
)

// act
posthog2.capture('custom event 2')

// assert
expect(onCapture1.mock.calls.length).toEqual(1)
expect(onCapture2.mock.calls.length).toEqual(1)
expect(onCapture1.mock.calls[0][1].properties.$process_person_profile).toEqual(true)
expect(onCapture2.mock.calls[0][1].properties.$process_person_profile).toEqual(true)
})

it('should work when always is set on a later visit', async () => {
// arrange
const persistenceName = uuidv7()
const { posthog: posthog1, onCapture: onCapture1 } = await setup(
'identified_only',
undefined,
persistenceName
)
posthog1.capture('custom event 1')
const { posthog: posthog2, onCapture: onCapture2 } = await setup('always', undefined, persistenceName)

// act
posthog2.capture('custom event 2')

// assert
expect(onCapture1.mock.calls.length).toEqual(1)
expect(onCapture2.mock.calls.length).toEqual(1)
expect(onCapture1.mock.calls[0][1].properties.$process_person_profile).toEqual(false)
expect(onCapture2.mock.calls[0][1].properties.$process_person_profile).toEqual(true)
})
})

describe('decide', () => {
it('should change the person mode from default when decide response is handled', async () => {
// arrange
const { posthog, onCapture } = await setup(undefined)
posthog.capture('startup page view')

// act
posthog._afterDecideResponse({ defaultIdentifiedOnly: false } as DecideResponse)
posthog.capture('custom event')

// assert
expect(onCapture.mock.calls.length).toEqual(2)
expect(onCapture.mock.calls[0][1].properties.$process_person_profile).toEqual(false)
expect(onCapture.mock.calls[1][1].properties.$process_person_profile).toEqual(true)
})

it('should NOT change the person mode from user-defined when decide response is handled', async () => {
// arrange
const { posthog, onCapture } = await setup('identified_only')
posthog.capture('startup page view')

// act
posthog._afterDecideResponse({ defaultIdentifiedOnly: false } as DecideResponse)
posthog.capture('custom event')

// assert
expect(onCapture.mock.calls.length).toEqual(2)
expect(onCapture.mock.calls[0][1].properties.$process_person_profile).toEqual(false)
expect(onCapture.mock.calls[1][1].properties.$process_person_profile).toEqual(false)
})

it('should persist when the default person mode is overridden by decide', async () => {
// arrange
const persistenceName = uuidv7()
const { posthog: posthog1, onCapture: onCapture1 } = await setup(undefined, undefined, persistenceName)

// act
posthog1._afterDecideResponse({ defaultIdentifiedOnly: false } as DecideResponse)
posthog1.capture('custom event 1')
const { posthog: posthog2, onCapture: onCapture2 } = await setup(undefined, undefined, persistenceName)
posthog2.capture('custom event 2')

// assert
expect(onCapture1.mock.calls.length).toEqual(1)
expect(onCapture2.mock.calls.length).toEqual(1)
expect(onCapture1.mock.calls[0][1].properties.$process_person_profile).toEqual(true)
expect(onCapture2.mock.calls[0][1].properties.$process_person_profile).toEqual(true)
})
})
})
4 changes: 4 additions & 0 deletions src/__tests__/posthog-core.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,7 @@ describe('posthog core', () => {
const { posthog, onCapture } = setup({
token,
persistence_name: token,
person_profiles: 'always',
})

// act
Expand All @@ -135,6 +136,7 @@ describe('posthog core', () => {
const { posthog: posthog1 } = setup({
token,
persistence_name: token,
person_profiles: 'always',
})
posthog1.capture(eventName, eventProperties)
mockReferrerGetter.mockReturnValue('https://referrer2.example.com/some/path')
Expand Down Expand Up @@ -165,6 +167,7 @@ describe('posthog core', () => {
const { posthog: posthog1 } = setup({
token,
persistence_name: token,
person_profiles: 'always',
})
posthog1.capture(eventName, eventProperties)
mockReferrerGetter.mockReturnValue('https://referrer2.example.com/some/path')
Expand Down Expand Up @@ -195,6 +198,7 @@ describe('posthog core', () => {
const { posthog, onCapture } = setup({
token,
persistence_name: token,
person_profiles: 'always',
})

// act
Expand Down
27 changes: 22 additions & 5 deletions src/__tests__/posthog-core.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import { Info } from '../utils/event-utils'
import { document, window } from '../utils/globals'
import { uuidv7 } from '../uuidv7'
import * as globals from '../utils/globals'
import { USER_STATE } from '../constants'
import { ENABLE_PERSON_PROCESSING, USER_STATE } from '../constants'
import { createPosthogInstance, defaultPostHog } from './helpers/posthog-instance'
import { logger } from '../utils/logger'
import { DecideResponse, PostHogConfig } from '../types'
Expand Down Expand Up @@ -345,6 +345,20 @@ describe('posthog core', () => {

expect(posthog.compression).toEqual('gzip-js')
})
it('uses defaultIdentifiedOnly from decide response', () => {
const posthog = posthogWith({})

posthog._afterDecideResponse({ defaultIdentifiedOnly: true } as DecideResponse)
expect(posthog.config.person_profiles).toEqual('identified_only')

posthog._afterDecideResponse({ defaultIdentifiedOnly: false } as DecideResponse)
expect(posthog.config.person_profiles).toEqual('always')
})
it('defaultIdentifiedOnly does not override person_profiles if already set', () => {
const posthog = posthogWith({ person_profiles: 'always' })
posthog._afterDecideResponse({ defaultIdentifiedOnly: true } as DecideResponse)
expect(posthog.config.person_profiles).toEqual('always')
})

it('enables compression from decide response when only one received', () => {
const posthog = posthogWith({})
Expand Down Expand Up @@ -387,6 +401,8 @@ describe('posthog core', () => {
properties: () => ({ distinct_id: 'abc', persistent: 'prop', $is_identified: false }),
remove_event_timer: jest.fn(),
get_property: () => 'anonymous',
props: {},
register: jest.fn(),
} as unknown as PostHogPersistence,
sessionPersistence: {
properties: () => ({ distinct_id: 'abc', persistent: 'prop' }),
Expand Down Expand Up @@ -425,7 +441,7 @@ describe('posthog core', () => {
$window_id: 'windowId',
$session_id: 'sessionId',
$is_identified: false,
$process_person_profile: true,
$process_person_profile: false,
})
})

Expand All @@ -447,7 +463,7 @@ describe('posthog core', () => {
$session_id: 'sessionId',
$lib_custom_api_host: 'https://custom.posthog.com',
$is_identified: false,
$process_person_profile: true,
$process_person_profile: false,
})
})

Expand All @@ -465,7 +481,7 @@ describe('posthog core', () => {
posthog._calculate_event_properties('custom_event', { event: 'prop' }, new Date())[
'$process_person_profile'
]
).toEqual(true)
).toEqual(false)
})

it('only adds token and distinct_id if event_name is $snapshot', () => {
Expand Down Expand Up @@ -496,7 +512,7 @@ describe('posthog core', () => {
expect(posthog._calculate_event_properties('custom_event', { event: 'prop' }, new Date())).toEqual({
event_name: 'custom_event',
token: 'testtoken',
$process_person_profile: true,
$process_person_profile: false,
})
})

Expand All @@ -510,6 +526,7 @@ describe('posthog core', () => {
)

posthog.persistence.get_initial_props = () => ({ initial: 'prop' })
posthog.persistence.props[ENABLE_PERSON_PROCESSING] = true // person processing is needed for $set_once
expect(posthog._calculate_set_once_properties({ key: 'prop' })).toEqual({
event_name: '$set_once',
token: undefined,
Expand Down
27 changes: 24 additions & 3 deletions src/posthog-core.ts
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,7 @@ export const defaultConfig = (): PostHogConfig => ({
bootstrap: {},
disable_compression: false,
session_idle_timeout_seconds: 30 * 60, // 30 minutes
person_profiles: 'always',
person_profiles: 'identified_only',
__add_tracing_headers: false,
})

Expand Down Expand Up @@ -271,6 +271,7 @@ export class PostHog {
decideEndpointWasHit: boolean
analyticsDefaultEndpoint: string
version = Config.LIB_VERSION
_initialPersonProfilesConfig: 'always' | 'never' | 'identified_only' | null

SentryIntegration: typeof SentryIntegration
sentryIntegration: (options?: SentryIntegrationOptions) => ReturnType<typeof sentryIntegration>
Expand All @@ -293,6 +294,7 @@ export class PostHog {
this.__loaded = false
this.analyticsDefaultEndpoint = '/e/'
this._initialPageviewCaptured = false
this._initialPersonProfilesConfig = null

this.featureFlags = new PostHogFeatureFlags(this)
this.toolbar = new Toolbar(this)
Expand Down Expand Up @@ -389,6 +391,10 @@ export class PostHog {
this.config = {} as PostHogConfig // will be set right below
this._triggered_notifs = []

if (config.person_profiles) {
this._initialPersonProfilesConfig = config.person_profiles
}

this.set_config(
extend({}, defaultConfig(), configRenames(config), {
name: name,
Expand All @@ -407,6 +413,8 @@ export class PostHog {
this.config.persistence === 'sessionStorage'
? this.persistence
: new PostHogPersistence({ ...this.config, persistence: 'sessionStorage' })

// should I store the initial person profiles config in persistence?
const initialPersistenceProps = { ...this.persistence.props }
const initialSessionProps = { ...this.sessionPersistence.props }

Expand Down Expand Up @@ -537,6 +545,14 @@ export class PostHog {
this.analyticsDefaultEndpoint = response.analytics.endpoint
}

this.set_config({
person_profiles: this._initialPersonProfilesConfig
? this._initialPersonProfilesConfig
: response['defaultIdentifiedOnly']
? 'identified_only'
: 'always',
})

this.sessionRecording?.afterDecideResponse(response)
this.autocapture?.afterDecideResponse(response)
this.heatmaps?.afterDecideResponse(response)
Expand Down Expand Up @@ -976,8 +992,13 @@ export class PostHog {
properties = sanitize_properties(properties, event_name)
}

// add person processing flag as very last step, so it cannot be overridden. process_person=true is default
properties['$process_person_profile'] = this._hasPersonProcessing()
// add person processing flag as very last step, so it cannot be overridden
const hasPersonProcessing = this._hasPersonProcessing()
properties['$process_person_profile'] = hasPersonProcessing
// if the event has person processing, ensure that all future events will too, even if the setting changes
if (hasPersonProcessing) {
this._requirePersonProcessing('_calculate_event_properties')
}

return properties
}
Expand Down
1 change: 1 addition & 0 deletions src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -405,6 +405,7 @@ export interface DecideResponse {
isAuthenticated: boolean
siteApps: { id: number; url: string }[]
heatmaps?: boolean
defaultIdentifiedOnly?: boolean
}

export type FeatureFlagsCallback = (
Expand Down

0 comments on commit 387d96e

Please sign in to comment.