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(cdp): allow disabling site destinations #1563

Open
wants to merge 21 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
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
3 changes: 3 additions & 0 deletions playground/nextjs/src/posthog.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,10 @@ export const configForConsent = (): Partial<PostHogConfig> => {
export const updatePostHogConsent = (consentGiven: boolean) => {
if (consentGiven) {
localStorage.setItem('cookie_consent', 'true')
posthog.opt_in_capturing()
} else {
localStorage.removeItem('cookie_consent')
posthog.opt_out_capturing()
}

posthog.set_config(configForConsent())
Expand All @@ -49,6 +51,7 @@ if (typeof window !== 'undefined') {
session_recording: {
recordCrossOriginIframes: true,
},
opt_in_site_apps: true,
debug: true,
disable_web_experiments: false,
scroll_root_selector: ['#scroll_element', 'html'],
Expand Down
172 changes: 80 additions & 92 deletions src/__tests__/site-apps.ts
Original file line number Diff line number Diff line change
@@ -1,18 +1,12 @@
import { SiteApps } from '../site-apps'
import { PostHogPersistence } from '../posthog-persistence'
import { RequestRouter } from '../utils/request-router'
import { PostHog } from '../posthog-core'
import { DecideResponse, PostHogConfig, Properties, CaptureResult } from '../types'
import { DecideResponse, PostHogConfig, CaptureResult } from '../types'
import { assignableWindow } from '../utils/globals'
import { logger } from '../utils/logger'
import '../entrypoints/external-scripts-loader'
import { isFunction } from '../utils/type-utils'

jest.mock('../utils/logger', () => ({
logger: {
error: jest.fn(),
},
}))
import { uuidv7 } from '../uuidv7'
import { defaultPostHog } from './helpers/posthog-instance'

describe('SiteApps', () => {
let posthog: PostHog
Expand Down Expand Up @@ -44,26 +38,17 @@ describe('SiteApps', () => {
}),
}

posthog = {
config: { ...defaultConfig },
persistence: new PostHogPersistence(defaultConfig as PostHogConfig),
register: (props: Properties) => posthog.persistence!.register(props),
unregister: (key: string) => posthog.persistence!.unregister(key),
get_property: (key: string) => posthog.persistence!.props[key],
capture: jest.fn(),
_addCaptureHook: jest.fn(),
_afterDecideResponse: jest.fn(),
get_distinct_id: jest.fn().mockImplementation(() => 'distinctid'),
_send_request: jest.fn().mockImplementation(({ callback }) => callback?.({ config: {} })),
featureFlags: {
receivedFeatureFlags: jest.fn(),
setReloadingPaused: jest.fn(),
_startReloadTimer: jest.fn(),
},
requestRouter: new RequestRouter({ config: defaultConfig } as unknown as PostHog),
_hasBootstrappedFeatureFlags: jest.fn(),
getGroups: () => ({ organization: '5' }),
} as unknown as PostHog
const createPostHog = (config: Partial<PostHogConfig> = {}) => {
const posthog = defaultPostHog().init('testtoken', { ...config }, uuidv7())!
posthog.debug()
return posthog
}

posthog = createPostHog(defaultConfig)
posthog._addCaptureHook = jest.fn()
posthog.capture = jest.fn()
posthog._send_request = jest.fn().mockImplementation(({ callback }) => callback?.({ config: {} }))
logger.error = jest.fn()

siteAppsInstance = new SiteApps(posthog)
})
Expand All @@ -73,42 +58,6 @@ describe('SiteApps', () => {
})

describe('constructor', () => {
it('sets enabled to true when opt_in_site_apps is true and advanced_disable_decide is false', () => {
posthog.config = {
...defaultConfig,
opt_in_site_apps: true,
advanced_disable_decide: false,
} as PostHogConfig

siteAppsInstance = new SiteApps(posthog)

expect(siteAppsInstance.enabled).toBe(true)
})
MarconLP marked this conversation as resolved.
Show resolved Hide resolved

it('sets enabled to false when opt_in_site_apps is false', () => {
posthog.config = {
...defaultConfig,
opt_in_site_apps: false,
advanced_disable_decide: false,
} as PostHogConfig

siteAppsInstance = new SiteApps(posthog)

expect(siteAppsInstance.enabled).toBe(false)
})

it('sets enabled to false when advanced_disable_decide is true', () => {
posthog.config = {
...defaultConfig,
opt_in_site_apps: true,
advanced_disable_decide: true,
} as PostHogConfig

siteAppsInstance = new SiteApps(posthog)

expect(siteAppsInstance.enabled).toBe(false)
})

it('initializes missedInvocations, loaded, appsLoading correctly', () => {
expect(siteAppsInstance.missedInvocations).toEqual([])
expect(siteAppsInstance.loaded).toBe(false)
Expand All @@ -125,18 +74,26 @@ describe('SiteApps', () => {
})

describe('eventCollector', () => {
it('does nothing if enabled is false', () => {
siteAppsInstance.enabled = false
it('does nothing if opt_in_site_apps is false', () => {
posthog.config.opt_in_site_apps = false
siteAppsInstance.eventCollector('event_name', {} as CaptureResult)

expect(siteAppsInstance.missedInvocations.length).toBe(0)
})

it('does nothing if decide is disabled', () => {
posthog.config.opt_in_site_apps = true
posthog.config.advanced_disable_decide = true
siteAppsInstance.eventCollector('event_name', {} as CaptureResult)

expect(siteAppsInstance.missedInvocations.length).toBe(0)
})

it('collects event if enabled and loaded is false', () => {
siteAppsInstance.enabled = true
posthog.config.opt_in_site_apps = true
siteAppsInstance.loaded = false

const eventPayload = { event: 'test_event', properties: { prop1: 'value1' } } as CaptureResult
const eventPayload = { event: 'test_event', properties: { prop1: 'value1' } } as unknown as CaptureResult

jest.spyOn(siteAppsInstance, 'globalsForEvent').mockReturnValue({ some: 'globals' })

Expand All @@ -147,12 +104,12 @@ describe('SiteApps', () => {
})

it('trims missedInvocations to last 990 when exceeding 1000', () => {
siteAppsInstance.enabled = true
posthog.config.opt_in_site_apps = true
siteAppsInstance.loaded = false

siteAppsInstance.missedInvocations = new Array(1000).fill({})

const eventPayload = { event: 'test_event', properties: { prop1: 'value1' } } as CaptureResult
const eventPayload = { event: 'test_event', properties: { prop1: 'value1' } } as unknown as CaptureResult

jest.spyOn(siteAppsInstance, 'globalsForEvent').mockReturnValue({ some: 'globals' })

Expand Down Expand Up @@ -223,20 +180,22 @@ describe('SiteApps', () => {
})

describe('afterDecideResponse', () => {
it('sets loaded to true and enabled to false when response is undefined', () => {
siteAppsInstance.afterDecideResponse(undefined)
it('sets loaded to true response is undefined', () => {
const response = {
siteApps: [],
} as DecideResponse
siteAppsInstance.afterDecideResponse(response)

expect(siteAppsInstance.loaded).toBe(true)
expect(siteAppsInstance.enabled).toBe(false)
expect(siteAppsInstance._decideServerSiteAppsResponse.length).toBe(0)
})

it('loads site apps when enabled and opt_in_site_apps is true', (done) => {
posthog.config.opt_in_site_apps = true
siteAppsInstance.enabled = true
const response = {
siteApps: [
{ id: '1', url: '/site_app/1' },
{ id: '2', url: '/site_app/2' },
{ id: '1', type: 'site_app', url: '/site_app/1' },
{ id: '2', type: 'site_app', url: '/site_app/2' },
],
} as DecideResponse

Expand All @@ -255,25 +214,22 @@ describe('SiteApps', () => {
})

it('does not load site apps when enabled is false', () => {
siteAppsInstance.enabled = false
posthog.config.opt_in_site_apps = false
const response = {
siteApps: [{ id: '1', url: '/site_app/1' }],
siteApps: [{ id: '1', type: 'site_app', url: '/site_app/1' }],
} as DecideResponse

siteAppsInstance.afterDecideResponse(response)

expect(siteAppsInstance.loaded).toBe(true)
expect(siteAppsInstance.enabled).toBe(false)
expect(assignableWindow.__PosthogExtensions__?.loadSiteApp).not.toHaveBeenCalled()
})

it('clears missedInvocations when all apps are loaded', (done) => {
posthog.config.opt_in_site_apps = true
siteAppsInstance.enabled = true
siteAppsInstance.missedInvocations = [{ some: 'data' }]
const response = {
siteApps: [{ id: '1', url: '/site_app/1' }],
siteApps: [{ id: '1', type: 'site_app', url: '/site_app/1' }],
} as DecideResponse

siteAppsInstance.afterDecideResponse(response)
Expand All @@ -288,28 +244,26 @@ describe('SiteApps', () => {

it('sets assignableWindow properties for each site app', () => {
posthog.config.opt_in_site_apps = true
siteAppsInstance.enabled = true
const response = {
siteApps: [{ id: '1', url: '/site_app/1' }],
siteApps: [{ id: '8', type: 'site_app', url: '/site_app/8' }],
} as DecideResponse

siteAppsInstance.afterDecideResponse(response)

expect(assignableWindow['__$$ph_site_app_1_posthog']).toBe(posthog)
expect(typeof assignableWindow['__$$ph_site_app_1_missed_invocations']).toBe('function')
expect(typeof assignableWindow['__$$ph_site_app_1_callback']).toBe('function')
expect(assignableWindow['__$$ph_site_app_8']).toBe(posthog)
expect(typeof assignableWindow['__$$ph_site_app_8_missed_invocations']).toBe('function')
expect(typeof assignableWindow['__$$ph_site_app_8_callback']).toBe('function')
expect(assignableWindow.__PosthogExtensions__?.loadSiteApp).toHaveBeenCalledWith(
posthog,
'/site_app/1',
'/site_app/8',
expect.any(Function)
)
})

it('logs error if site apps are disabled but response contains site apps', () => {
posthog.config.opt_in_site_apps = false
siteAppsInstance.enabled = false
const response = {
siteApps: [{ id: '1', url: '/site_app/1' }],
siteApps: [{ id: '1', type: 'site_app', url: '/site_app/1' }],
} as DecideResponse

siteAppsInstance.afterDecideResponse(response)
Expand All @@ -321,7 +275,6 @@ describe('SiteApps', () => {
})

it('sets loaded to true if response.siteApps is empty', () => {
siteAppsInstance.enabled = true
posthog.config.opt_in_site_apps = true
const response = {
siteApps: [],
Expand All @@ -330,7 +283,42 @@ describe('SiteApps', () => {
siteAppsInstance.afterDecideResponse(response)

expect(siteAppsInstance.loaded).toBe(true)
expect(siteAppsInstance.enabled).toBe(false)
})

it('does not load site destinations if consent is not given', () => {
posthog.config.opt_in_site_apps = true
posthog.consent.isOptedOut = () => true
const response = {
siteApps: [
{ id: '5', type: 'site_app', url: '/site_app/5' },
{ id: '6', type: 'site_destination', url: '/site_app/6' },
],
} as DecideResponse

siteAppsInstance.afterDecideResponse(response)

expect(typeof assignableWindow['__$$ph_site_app_5_callback']).toBe('function')
expect(typeof assignableWindow['__$$ph_site_app_6_callback']).toBe('undefined')
})

it('load site destinations if consent is given at a later time', () => {
posthog.config.opt_in_site_apps = true
posthog.opt_out_capturing()
const response = {
siteApps: [
{ id: '5', type: 'site_app', url: '/site_app/5' },
{ id: '6', type: 'site_destination', url: '/site_app/6' },
],
} as DecideResponse

siteAppsInstance.afterDecideResponse(response)

posthog.opt_in_capturing()

siteAppsInstance.loadIfEnabled()

expect(typeof assignableWindow['__$$ph_site_app_5_callback']).toBe('function')
expect(typeof assignableWindow['__$$ph_site_app_6_callback']).toBe('function')
})
})
})
29 changes: 15 additions & 14 deletions src/posthog-core.ts
Original file line number Diff line number Diff line change
Expand Up @@ -308,6 +308,7 @@ export class PostHog {
this.scrollManager = new ScrollManager(this)
this.pageViewManager = new PageViewManager(this)
this.surveys = new PostHogSurveys(this)
this.siteApps = new SiteApps(this)
this.experiments = new WebExperiments(this)
this.exceptions = new PostHogExceptions(this)
this.rateLimiter = new RateLimiter(this)
Expand Down Expand Up @@ -434,7 +435,6 @@ export class PostHog {

new TracingHeaders(this).startIfEnabledOrStop()

this.siteApps = new SiteApps(this)
this.siteApps?.init()

this.sessionRecording = new SessionRecording(this)
Expand Down Expand Up @@ -808,6 +808,19 @@ export class PostHog {
return
}

// The initial campaign/referrer props need to be stored in the regular persistence, as they are there to mimic
// the person-initial props. The non-initial versions are stored in the sessionPersistence, as they are sent
// with every event and used by the session table to create session-initial props.
if (this.config.store_google) {
this.sessionPersistence.update_campaign_params()
}
if (this.config.save_referrer) {
this.sessionPersistence.update_referrer_info()
}
if (this.config.store_google || this.config.save_referrer) {
this.persistence.set_initial_person_info()
}

if (this.consent.isOptedOut()) {
return
}
Expand All @@ -834,19 +847,6 @@ export class PostHog {
// update persistence
this.sessionPersistence.update_search_keyword()

// The initial campaign/referrer props need to be stored in the regular persistence, as they are there to mimic
// the person-initial props. The non-initial versions are stored in the sessionPersistence, as they are sent
// with every event and used by the session table to create session-initial props.
if (this.config.store_google) {
this.sessionPersistence.update_campaign_params()
}
if (this.config.save_referrer) {
this.sessionPersistence.update_referrer_info()
}
if (this.config.store_google || this.config.save_referrer) {
this.persistence.set_initial_person_info()
}

const systemTime = new Date()
const timestamp = options?.timestamp || systemTime

Expand Down Expand Up @@ -1828,6 +1828,7 @@ export class PostHog {
this.autocapture?.startIfEnabled()
this.heatmaps?.startIfEnabled()
this.surveys.loadIfEnabled()
this.siteApps?.loadIfEnabled()
this._sync_opt_out_with_persistence()
}
}
Expand Down
Loading