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.

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: Add customization to add all person profile properties as setPersonPropertiesForFlags #1517

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
16 changes: 14 additions & 2 deletions playground/nextjs/pages/index.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
/* eslint-disable no-console */
import { useActiveFeatureFlags, usePostHog } from 'posthog-js/react'
import React, { useEffect, useState } from 'react'
import { PERSON_PROCESSING_MODE, cookieConsentGiven } from '@/src/posthog'
import { useEffect, useState } from 'react'
import { cookieConsentGiven, PERSON_PROCESSING_MODE } from '@/src/posthog'
import { setAllPersonProfilePropertiesAsPersonPropertiesForFlags } from 'posthog-js/lib/src/customizations/setAllPersonProfilePropertiesAsPersonPropertiesForFlags'
import { STORED_PERSON_PROPERTIES_KEY } from '../../../src/constants'

export default function Home() {
const posthog = usePostHog()
Expand Down Expand Up @@ -40,6 +43,15 @@ export default function Home() {
<a className="Button" data-attr="autocapture-button" href="#">
<span>Autocapture a &gt; span</span>
</a>
<button
onClick={() => {
console.log(posthog.persistence?.props[STORED_PERSON_PROPERTIES_KEY])
setAllPersonProfilePropertiesAsPersonPropertiesForFlags(posthog as any)
console.log(posthog.persistence?.props[STORED_PERSON_PROPERTIES_KEY])
}}
>
SetPersonPropertiesForFlags
</button>
<a href={'https://www.google.com'}>External link</a>
{isClient && typeof window !== 'undefined' && process.env.NEXT_PUBLIC_CROSSDOMAIN && (
<a
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,88 @@
import { uuidv7 } from '../../uuidv7'
import { createPosthogInstance } from '../helpers/posthog-instance'
import { setAllPersonProfilePropertiesAsPersonPropertiesForFlags } from '../../customizations/setAllPersonProfilePropertiesAsPersonPropertiesForFlags'
import { STORED_PERSON_PROPERTIES_KEY } from '../../constants'

jest.mock('../../utils/globals', () => {
const orig = jest.requireActual('../../utils/globals')
const mockURLGetter = jest.fn()
const mockReferrerGetter = jest.fn()
return {
...orig,
mockURLGetter,
mockReferrerGetter,
userAgent: 'Mozilla/5.0 (Android 4.4; Mobile; rv:41.0) Gecko/41.0 Firefox/41.0',
navigator: {
vendor: '',
},
document: {
...orig.document,
createElement: (...args: any[]) => orig.document.createElement(...args),
get referrer() {
return mockReferrerGetter()
},
get URL() {
return mockURLGetter()
},
},
get location() {
const url = mockURLGetter()
return {
href: url,
toString: () => url,
}
},
}
})

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

describe('setAllPersonPropertiesForFlags', () => {
beforeEach(() => {
mockReferrerGetter.mockReturnValue('https://referrer.com')
mockURLGetter.mockReturnValue('https://example.com?utm_source=foo')
})

it('should called setPersonPropertiesForFlags with all saved properties that are used for person properties', async () => {
// arrange
const token = uuidv7()
const posthog = await createPosthogInstance(token)

// act
setAllPersonProfilePropertiesAsPersonPropertiesForFlags(posthog)

// assert
expect(posthog.persistence?.props[STORED_PERSON_PROPERTIES_KEY]).toMatchInlineSnapshot(`
Object {
"$browser": "Mobile Safari",
"$browser_version": null,
"$current_url": "https://example.com?utm_source=foo",
"$device_type": "Mobile",
"$os": "Android",
"$os_version": "4.4.0",
"$referrer": "https://referrer.com",
"$referring_domain": "referrer.com",
"dclid": null,
"fbclid": null,
"gad_source": null,
"gbraid": null,
"gclid": null,
"gclsrc": null,
"igshid": null,
"li_fat_id": null,
"mc_cid": null,
"msclkid": null,
"rdt_cid": null,
"ttclid": null,
"twclid": null,
"utm_campaign": null,
"utm_content": null,
"utm_medium": null,
"utm_source": "foo",
"utm_term": null,
"wbraid": null,
}
`)
})
})
4 changes: 2 additions & 2 deletions src/config.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
import { version } from '../package.json'
import packageInfo from '../package.json'

Copy link
Member Author

Choose a reason for hiding this comment

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

I got a warning about this, so I fixed it as a drive-by

Should not import the named export 'version' (imported as 'version') from default-exporting module (only default export is available soon)

// overridden in posthog-core,
// e.g. Config.DEBUG = Config.DEBUG || instance.config.debug
const Config = {
DEBUG: false,
LIB_VERSION: version,
LIB_VERSION: packageInfo.version,
}

export default Config
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
import { PostHog } from '../posthog-core'
import { CAMPAIGN_PARAMS, EVENT_TO_PERSON_PROPERTIES, Info } from '../utils/event-utils'
import { each, extend, includes } from '../utils'

export const setAllPersonProfilePropertiesAsPersonPropertiesForFlags = (posthog: PostHog): void => {
Copy link
Member

Choose a reason for hiding this comment

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

could also do with a comment IMHO, why would I want to do this?

const allProperties = extend({}, Info.properties(), Info.campaignParams(), Info.referrerInfo())
const personProperties: Record<string, string> = {}
each(allProperties, function (v, k: string) {
if (includes(CAMPAIGN_PARAMS, k) || includes(EVENT_TO_PERSON_PROPERTIES, k)) {
personProperties[k] = v
}
})

posthog.setPersonPropertiesForFlags(personProperties)
}
19 changes: 19 additions & 0 deletions src/utils/event-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,25 @@ export const CAMPAIGN_PARAMS = [
'rdt_cid', // reddit
]

export const EVENT_TO_PERSON_PROPERTIES = [
Copy link
Member

Choose a reason for hiding this comment

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

I think this could do with a comment or a better name...

I think it's saying "these are event properties that we will copy to person properties"
but I don't know for sure 🙈

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't disagree but it's copied directly from plugin server with that name, I'll add a comment though!

// mobile params
'$app_build',
'$app_name',
'$app_namespace',
'$app_version',
Comment on lines +36 to +39
Copy link
Member

Choose a reason for hiding this comment

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

There are many more params than that such as $device_manufacturer, $device_model, etc, I wonder if this list is exhaustive to only what this SDK specifically collects?

Copy link
Member Author

Choose a reason for hiding this comment

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

Those other ones aren't converted into person properties though, or are they? This list comes straight from plugin-server.

Copy link
Member

Choose a reason for hiding this comment

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

I didn't check so the plugin-server should be up to date and all good.

// web params
'$browser',
'$browser_version',
'$device_type',
'$current_url',
'$pathname',
'$os',
'$os_name', // $os_name is a special case, it's treated as an alias of $os!
'$os_version',
'$referring_domain',
'$referrer',
]

export const Info = {
campaignParams: function (customParams?: string[]): Record<string, string> {
if (!document) {
Expand Down
Loading