Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Rename $process_person to $process_person_profile #1143

Merged
merged 1 commit into from
Apr 16, 2024
Merged

feat: Rename $process_person to $process_person_profile #1143

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 19 additions & 19 deletions src/__tests__/personProcessing.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,12 +31,12 @@ describe('person processing', () => {
mockURLGetter.mockReturnValue('https://example.com?utm_source=foo')
})

const setup = async (processPerson: 'always' | 'identified_only' | 'never' | undefined) => {
const setup = async (person_profiles: 'always' | 'identified_only' | 'never' | undefined) => {
const token = uuidv7()
const onCapture = jest.fn()
const posthog = await createPosthogInstance(token, {
_onCapture: onCapture,
person_profiles: processPerson,
person_profiles,
})
return { token, onCapture, posthog }
}
Expand Down Expand Up @@ -120,12 +120,12 @@ describe('person processing', () => {
// assert
expect(jest.mocked(logger).error).toBeCalledTimes(0)
const eventBeforeIdentify = onCapture.mock.calls[0]
expect(eventBeforeIdentify[1].properties.$process_person).toEqual(false)
expect(eventBeforeIdentify[1].properties.$process_person_profile).toEqual(false)
const identifyCall = onCapture.mock.calls[1]
expect(identifyCall[0]).toEqual('$identify')
expect(identifyCall[1].properties.$process_person).toEqual(true)
expect(identifyCall[1].properties.$process_person_profile).toEqual(true)
const eventAfterIdentify = onCapture.mock.calls[2]
expect(eventAfterIdentify[1].properties.$process_person).toEqual(true)
expect(eventAfterIdentify[1].properties.$process_person_profile).toEqual(true)
})

it('should not change $person_process if process_person is always', async () => {
Expand All @@ -139,12 +139,12 @@ describe('person processing', () => {
// assert
expect(jest.mocked(logger).error).toBeCalledTimes(0)
const eventBeforeIdentify = onCapture.mock.calls[0]
expect(eventBeforeIdentify[1].properties.$process_person).toEqual(true)
expect(eventBeforeIdentify[1].properties.$process_person_profile).toEqual(true)
const identifyCall = onCapture.mock.calls[1]
expect(identifyCall[0]).toEqual('$identify')
expect(identifyCall[1].properties.$process_person).toEqual(true)
expect(identifyCall[1].properties.$process_person_profile).toEqual(true)
const eventAfterIdentify = onCapture.mock.calls[2]
expect(eventAfterIdentify[1].properties.$process_person).toEqual(true)
expect(eventAfterIdentify[1].properties.$process_person_profile).toEqual(true)
})

it('should include initial referrer info in identify event if identified_only', async () => {
Expand Down Expand Up @@ -286,12 +286,12 @@ describe('person processing', () => {

// assert
const eventBeforeGroup = onCapture.mock.calls[0]
expect(eventBeforeGroup[1].properties.$process_person).toEqual(false)
expect(eventBeforeGroup[1].properties.$process_person_profile).toEqual(false)
const groupIdentify = onCapture.mock.calls[1]
expect(groupIdentify[0]).toEqual('$groupidentify')
expect(groupIdentify[1].properties.$process_person).toEqual(true)
expect(groupIdentify[1].properties.$process_person_profile).toEqual(true)
const eventAfterGroup = onCapture.mock.calls[2]
expect(eventAfterGroup[1].properties.$process_person).toEqual(true)
expect(eventAfterGroup[1].properties.$process_person_profile).toEqual(true)
})

it('should not send the $groupidentify event if person_processing is set to never', async () => {
Expand All @@ -311,9 +311,9 @@ describe('person processing', () => {

expect(onCapture).toBeCalledTimes(2)
const eventBeforeGroup = onCapture.mock.calls[0]
expect(eventBeforeGroup[1].properties.$process_person).toEqual(false)
expect(eventBeforeGroup[1].properties.$process_person_profile).toEqual(false)
const eventAfterGroup = onCapture.mock.calls[1]
expect(eventAfterGroup[1].properties.$process_person).toEqual(false)
expect(eventAfterGroup[1].properties.$process_person_profile).toEqual(false)
})
})

Expand Down Expand Up @@ -356,12 +356,12 @@ describe('person processing', () => {

// assert
const eventBeforeGroup = onCapture.mock.calls[0]
expect(eventBeforeGroup[1].properties.$process_person).toEqual(false)
expect(eventBeforeGroup[1].properties.$process_person_profile).toEqual(false)
const set = onCapture.mock.calls[1]
expect(set[0]).toEqual('$set')
expect(set[1].properties.$process_person).toEqual(true)
expect(set[1].properties.$process_person_profile).toEqual(true)
const eventAfterGroup = onCapture.mock.calls[2]
expect(eventAfterGroup[1].properties.$process_person).toEqual(true)
expect(eventAfterGroup[1].properties.$process_person_profile).toEqual(true)
})
})

Expand All @@ -377,12 +377,12 @@ describe('person processing', () => {

// assert
const eventBeforeGroup = onCapture.mock.calls[0]
expect(eventBeforeGroup[1].properties.$process_person).toEqual(false)
expect(eventBeforeGroup[1].properties.$process_person_profile).toEqual(false)
const alias = onCapture.mock.calls[1]
expect(alias[0]).toEqual('$create_alias')
expect(alias[1].properties.$process_person).toEqual(true)
expect(alias[1].properties.$process_person_profile).toEqual(true)
const eventAfterGroup = onCapture.mock.calls[2]
expect(eventAfterGroup[1].properties.$process_person).toEqual(true)
expect(eventAfterGroup[1].properties.$process_person_profile).toEqual(true)
})

it('should not send a $create_alias event if person processing is set to "never"', async () => {
Expand Down
14 changes: 7 additions & 7 deletions src/__tests__/posthog-core.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -406,7 +406,7 @@ describe('posthog core', () => {
$window_id: 'windowId',
$session_id: 'sessionId',
$is_identified: false,
$process_person: true,
$process_person_profile: true,
})
})

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

it("can't deny or blacklist $process_person", () => {
given('property_denylist', () => ['$process_person'])
given('property_blacklist', () => ['$process_person'])
it("can't deny or blacklist $process_person_profile", () => {
given('property_denylist', () => ['$process_person_profile'])
given('property_blacklist', () => ['$process_person_profile'])

expect(given.subject['$process_person']).toEqual(true)
expect(given.subject['$process_person_profile']).toEqual(true)
})

it('only adds token and distinct_id if event_name is $snapshot', () => {
Expand Down Expand Up @@ -467,7 +467,7 @@ describe('posthog core', () => {
expect(given.subject).toEqual({
event_name: given.event_name,
token: 'testtoken',
$process_person: true,
$process_person_profile: true,
})
})

Expand Down
2 changes: 1 addition & 1 deletion src/posthog-core.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -903,7 +903,7 @@ export class PostHog {
}

// add person processing flag as very last step, so it cannot be overridden. process_person=true is default
properties['$process_person'] = this._hasPersonProcessing()
properties['$process_person_profile'] = this._hasPersonProcessing()
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not exactly to do with this PR - but what does this mean for legacy SDKs. Will we all of a sudden not process persons because there is no $process_person_profile property?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a q for pipeline, but it should be that the absence of this property is treated as if it's true

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We default to true, the only thing that disables it is sending $process_person_profile=false


return properties
}
Expand Down
8 changes: 4 additions & 4 deletions src/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -149,10 +149,10 @@ export interface PostHogConfig {
// Let the pageview scroll stats use a custom css selector for the root element, e.g. `main`
scroll_root_selector?: string | string[]

/** You can control whether events from PostHog-js have person processing enabled with the `process_person` config setting. There are three options:
* - `process_person: 'always'` _(default)_ - we will process persons data for all events
* - `process_person: 'never'` - we won't process persons for any event. This means that anonymous users will not be merged once they sign up or login, so you lose the ability to create funnels that track users from anonymous to identified. All events (including `$identify`) will be sent with `$process_person: False`.
* - `process_person: 'identified_only'` - we will only process persons when you call `posthog.identify`, `posthog.alias`, `posthog.setPersonProperties`, `posthog.group`, `posthog.setPersonPropertiesForFlags` or `posthog.setGroupPropertiesForFlags` Anonymous users won't get person profiles.
/** You can control whether events from PostHog-js have person processing enabled with the `person_profiles` config setting. There are three options:
* - `person_profiles: 'always'` _(default)_ - we will process persons data for all events
* - `person_profiles: 'never'` - we won't process persons for any event. This means that anonymous users will not be merged once they sign up or login, so you lose the ability to create funnels that track users from anonymous to identified. All events (including `$identify`) will be sent with `$process_person_profile: False`.
* - `person_profiles: 'identified_only'` - we will only process persons when you call `posthog.identify`, `posthog.alias`, `posthog.setPersonProperties`, `posthog.group`, `posthog.setPersonPropertiesForFlags` or `posthog.setGroupPropertiesForFlags` Anonymous users won't get person profiles.
*/
person_profiles?: 'always' | 'never' | 'identified_only'
/** @deprecated - use `person_profiles` instead */
Expand Down
Loading