Skip to content

Commit

Permalink
fix: capture a $pageview event on opting in (#1372)
Browse files Browse the repository at this point in the history
  • Loading branch information
@skoob13
skoob13 authored Aug 29, 2024
1 parent e8a6451 commit d0a9ba5
Show file tree
Hide file tree
Showing 3 changed files with 143 additions and 15 deletions.
64 changes: 59 additions & 5 deletions cypress/e2e/opting-out.cy.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ describe('opting out', () => {
cy.posthog().invoke('startSessionRecording')

cy.phCaptures({ full: true }).then((captures) => {
expect((captures || []).map((c) => c.event)).to.deep.equal(['$opt_in'])
expect((captures || []).map((c) => c.event)).to.deep.equal(['$opt_in', '$pageview'])
})

assertWhetherPostHogRequestsWereCalled({
Expand Down Expand Up @@ -173,7 +173,7 @@ describe('opting out', () => {
cy.posthog().invoke('startSessionRecording', { sampling: true })

cy.phCaptures({ full: true }).then((captures) => {
expect((captures || []).map((c) => c.event)).to.deep.equal(['$opt_in'])
expect((captures || []).map((c) => c.event)).to.deep.equal(['$opt_in', '$pageview'])
})

assertWhetherPostHogRequestsWereCalled({
Expand Down Expand Up @@ -216,7 +216,7 @@ describe('opting out', () => {
cy.posthog().invoke('startSessionRecording')

cy.phCaptures({ full: true }).then((captures) => {
expect((captures || []).map((c) => c.event)).to.deep.equal(['$opt_in'])
expect((captures || []).map((c) => c.event)).to.deep.equal(['$opt_in', '$pageview'])
})

assertWhetherPostHogRequestsWereCalled({
Expand Down Expand Up @@ -272,7 +272,7 @@ describe('opting out', () => {
cy.posthog().invoke('startSessionRecording')

cy.phCaptures({ full: true }).then((captures) => {
expect((captures || []).map((c) => c.event)).to.deep.equal(['$opt_in'])
expect((captures || []).map((c) => c.event)).to.deep.equal(['$opt_in', '$pageview'])
})

assertWhetherPostHogRequestsWereCalled({
Expand Down Expand Up @@ -333,7 +333,7 @@ describe('opting out', () => {
cy.posthog().invoke('startSessionRecording')

cy.phCaptures({ full: true }).then((captures) => {
expect((captures || []).map((c) => c.event)).to.deep.equal(['$opt_in'])
expect((captures || []).map((c) => c.event)).to.deep.equal(['$opt_in', '$pageview'])
})

assertWhetherPostHogRequestsWereCalled({
Expand All @@ -359,6 +359,60 @@ describe('opting out', () => {

pollPhCaptures('$snapshot').then(assertThatRecordingStarted)
})

it('sends a $pageview event when opting in', () => {
cy.intercept('POST', '/decide/*', {
autocapture_opt_out: true,
editorParams: {},
isAuthenticated: false,
sessionRecording: {
endpoint: '/ses/',
// will never record a session with rate of 0
sampleRate: '0',
},
}).as('decide')

cy.posthogInit({
opt_out_capturing_by_default: true,
})
// Wait for the pageview timeout
cy.wait(100)
cy.phCaptures({ full: true }).then((captures) => {
expect(captures || []).to.have.length(0)
})

cy.posthog().invoke('opt_in_capturing')

cy.phCaptures({ full: true }).then((captures) => {
expect((captures || []).map((c) => c.event)).to.deep.equal(['$opt_in', '$pageview'])
})
})

it('does not send a duplicate $pageview event when opting in', () => {
cy.intercept('POST', '/decide/*', {
autocapture_opt_out: true,
editorParams: {},
isAuthenticated: false,
sessionRecording: {
endpoint: '/ses/',
// will never record a session with rate of 0
sampleRate: '0',
},
}).as('decide')

cy.posthogInit({})
// Wait for the pageview timeout
cy.wait(100)
cy.phCaptures({ full: true }).then((captures) => {
expect((captures || []).map((c) => c.event)).to.deep.equal(['$pageview'])
})

cy.posthog().invoke('opt_in_capturing')

cy.phCaptures({ full: true }).then((captures) => {
expect((captures || []).map((c) => c.event)).to.deep.equal(['$pageview', '$opt_in'])
})
})
})

describe('user opts out after start', () => {
Expand Down
71 changes: 67 additions & 4 deletions src/__tests__/consent.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -109,11 +109,74 @@ describe('consentManager', () => {
)
})

it('should not send opt in event if null or false', () => {
posthog.opt_in_capturing({ captureEventName: null })
expect(onCapture).not.toHaveBeenCalled()
it('should not send opt in event if false', () => {
posthog.opt_in_capturing({ captureEventName: false })
expect(onCapture).not.toHaveBeenCalled()
expect(onCapture).toHaveBeenCalledTimes(1)
expect(onCapture).not.toHaveBeenCalledWith('$opt_in')
expect(onCapture).lastCalledWith('$pageview', expect.anything())
})

it('should not send opt in event if false', () => {
posthog.opt_in_capturing({ captureEventName: false })
expect(onCapture).toHaveBeenCalledTimes(1)
expect(onCapture).not.toHaveBeenCalledWith('$opt_in')
expect(onCapture).lastCalledWith('$pageview', expect.anything())
})

it('should not send $pageview on opt in if capturing is disabled', () => {
posthog = createPostHog({
opt_out_capturing_by_default: true,
_onCapture: onCapture,
capture_pageview: false,
})
posthog.opt_in_capturing({ captureEventName: false })
expect(onCapture).toHaveBeenCalledTimes(0)
})

it('should not send $pageview on opt in if is has already been captured', async () => {
posthog = createPostHog({
_onCapture: onCapture,
})
// Wait for the initial $pageview to be captured
// eslint-disable-next-line compat/compat
await new Promise((r) => setTimeout(r, 10))
expect(onCapture).toHaveBeenCalledTimes(1)
expect(onCapture).lastCalledWith('$pageview', expect.anything())
posthog.opt_in_capturing()
expect(onCapture).toHaveBeenCalledTimes(2)
expect(onCapture).toHaveBeenCalledWith('$opt_in', expect.anything())
})

it('should send $pageview on opt in if is has not been captured', async () => {
// Some other tests might call setTimeout after they've passed, so creating a new instance here.
const onCapture = jest.fn()
const posthog = createPostHog({ _onCapture: onCapture })

posthog.opt_in_capturing()
expect(onCapture).toHaveBeenCalledTimes(2)
expect(onCapture).toHaveBeenCalledWith('$opt_in', expect.anything())
expect(onCapture).lastCalledWith('$pageview', expect.anything())
// Wait for the $pageview timeout to be called
// eslint-disable-next-line compat/compat
await new Promise((r) => setTimeout(r, 10))
expect(onCapture).toHaveBeenCalledTimes(2)
})

it('should not send $pageview on subsequent opt in', async () => {
// Some other tests might call setTimeout after they've passed, so creating a new instance here.
const onCapture = jest.fn()
const posthog = createPostHog({ _onCapture: onCapture })

posthog.opt_in_capturing()
expect(onCapture).toHaveBeenCalledTimes(2)
expect(onCapture).toHaveBeenCalledWith('$opt_in', expect.anything())
expect(onCapture).lastCalledWith('$pageview', expect.anything())
// Wait for the $pageview timeout to be called
// eslint-disable-next-line compat/compat
await new Promise((r) => setTimeout(r, 10))
posthog.opt_in_capturing()
expect(onCapture).toHaveBeenCalledTimes(3)
expect(onCapture).not.lastCalledWith('$pageview', expect.anything())
})
})

Expand Down
23 changes: 17 additions & 6 deletions src/posthog-core.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -260,6 +260,7 @@ export class PostHog {
sessionRecording?: SessionRecording
webPerformance = new DeprecatedWebPerformanceObserver()

_initialPageviewCaptured: boolean
_triggered_notifs: any
compression?: Compression
__request_queue: QueuedRequestOptions[]
Expand Down Expand Up @@ -287,6 +288,7 @@ export class PostHog {
this.__request_queue = []
this.__loaded = false
this.analyticsDefaultEndpoint = '/e/'
this._initialPageviewCaptured = false

this.featureFlags = new PostHogFeatureFlags(this)
this.toolbar = new Toolbar(this)
Expand Down Expand Up @@ -559,8 +561,8 @@ export class PostHog {
// NOTE: We want to fire this on the next tick as the previous implementation had this side effect
// and some clients may rely on it
setTimeout(() => {
if (document) {
this.capture('$pageview', { title: document.title }, { send_instantly: true })
if (this.consent.isOptedIn()) {
this._captureInitialPageview()
}
}, 1)
}
Expand Down Expand Up @@ -1977,12 +1979,14 @@ export class PostHog {
this.consent.optInOut(true)
this._sync_opt_out_with_persistence()

if (!isUndefined(options?.captureEventName) && !options?.captureEventName) {
// Don't capture if captureEventName is null or false
return
// Don't capture if captureEventName is null or false
if (isUndefined(options?.captureEventName) || options?.captureEventName) {
this.capture(options?.captureEventName ?? '$opt_in', options?.captureProperties, { send_instantly: true })
}

this.capture(options?.captureEventName ?? '$opt_in', options?.captureProperties, { send_instantly: true })
if (this.config.capture_pageview) {
this._captureInitialPageview()
}
}

/**
Expand Down Expand Up @@ -2050,6 +2054,13 @@ export class PostHog {
}
}

_captureInitialPageview(): void {
if (document && !this._initialPageviewCaptured) {
this._initialPageviewCaptured = true
this.capture('$pageview', { title: document.title }, { send_instantly: true })
}
}

debug(debug?: boolean): void {
if (debug === false) {
window?.console.log("You've disabled debug mode.")
Expand Down

0 comments on commit d0a9ba5

Please sign in to comment.