From f6b36994825ba242ed4156a9310fcc32ce5cbfeb Mon Sep 17 00:00:00 2001 From: Robbie Coomber Date: Fri, 9 Aug 2024 11:15:01 +0100 Subject: [PATCH 1/3] Add prev pageview duration --- src/page-view.ts | 95 +++++++++++++++++++++++++-------------------- src/posthog-core.ts | 17 ++++---- 2 files changed, 63 insertions(+), 49 deletions(-) diff --git a/src/page-view.ts b/src/page-view.ts index 372c92535..dc34a51a2 100644 --- a/src/page-view.ts +++ b/src/page-view.ts @@ -4,6 +4,7 @@ import { isUndefined } from './utils/type-utils' interface PageViewEventProperties { $prev_pageview_pathname?: string + $prev_pageview_duration?: number // seconds $prev_pageview_last_scroll?: number $prev_pageview_last_scroll_percentage?: number $prev_pageview_max_scroll?: number @@ -16,72 +17,82 @@ interface PageViewEventProperties { export class PageViewManager { _currentPath?: string + _prevPageviewTimestamp?: Date _instance: PostHog constructor(instance: PostHog) { this._instance = instance } - doPageView(): PageViewEventProperties { - const response = this._previousScrollProperties() + doPageView(timestamp: Date): PageViewEventProperties { + const response = this._previousPageViewProperties(timestamp) // On a pageview we reset the contexts this._currentPath = window?.location.pathname ?? '' this._instance.scrollManager.resetContext() + this._prevPageviewTimestamp = timestamp return response } - doPageLeave(): PageViewEventProperties { - return this._previousScrollProperties() + doPageLeave(timestamp: Date): PageViewEventProperties { + return this._previousPageViewProperties(timestamp) } - private _previousScrollProperties(): PageViewEventProperties { + private _previousPageViewProperties(timestamp: Date): PageViewEventProperties { const previousPath = this._currentPath + const previousTimestamp = this._prevPageviewTimestamp const scrollContext = this._instance.scrollManager.getContext() - if (!previousPath || !scrollContext) { - return {} - } - - let { maxScrollHeight, lastScrollY, maxScrollY, maxContentHeight, lastContentY, maxContentY } = scrollContext + let properties: PageViewEventProperties = {} + if (scrollContext) { + let { maxScrollHeight, lastScrollY, maxScrollY, maxContentHeight, lastContentY, maxContentY } = + scrollContext - if ( - isUndefined(maxScrollHeight) || - isUndefined(lastScrollY) || - isUndefined(maxScrollY) || - isUndefined(maxContentHeight) || - isUndefined(lastContentY) || - isUndefined(maxContentY) - ) { - return {} - } + if ( + !isUndefined(maxScrollHeight) && + !isUndefined(lastScrollY) && + !isUndefined(maxScrollY) && + !isUndefined(maxContentHeight) && + !isUndefined(lastContentY) && + !isUndefined(maxContentY) + ) { + // Use ceil, so that e.g. scrolling 999.5px of a 1000px page is considered 100% scrolled + maxScrollHeight = Math.ceil(maxScrollHeight) + lastScrollY = Math.ceil(lastScrollY) + maxScrollY = Math.ceil(maxScrollY) + maxContentHeight = Math.ceil(maxContentHeight) + lastContentY = Math.ceil(lastContentY) + maxContentY = Math.ceil(maxContentY) - // Use ceil, so that e.g. scrolling 999.5px of a 1000px page is considered 100% scrolled - maxScrollHeight = Math.ceil(maxScrollHeight) - lastScrollY = Math.ceil(lastScrollY) - maxScrollY = Math.ceil(maxScrollY) - maxContentHeight = Math.ceil(maxContentHeight) - lastContentY = Math.ceil(lastContentY) - maxContentY = Math.ceil(maxContentY) + // if the maximum scroll height is near 0, then the percentage is 1 + const lastScrollPercentage = maxScrollHeight <= 1 ? 1 : clamp(lastScrollY / maxScrollHeight, 0, 1) + const maxScrollPercentage = maxScrollHeight <= 1 ? 1 : clamp(maxScrollY / maxScrollHeight, 0, 1) + const lastContentPercentage = maxContentHeight <= 1 ? 1 : clamp(lastContentY / maxContentHeight, 0, 1) + const maxContentPercentage = maxContentHeight <= 1 ? 1 : clamp(maxContentY / maxContentHeight, 0, 1) - // if the maximum scroll height is near 0, then the percentage is 1 - const lastScrollPercentage = maxScrollHeight <= 1 ? 1 : clamp(lastScrollY / maxScrollHeight, 0, 1) - const maxScrollPercentage = maxScrollHeight <= 1 ? 1 : clamp(maxScrollY / maxScrollHeight, 0, 1) - const lastContentPercentage = maxContentHeight <= 1 ? 1 : clamp(lastContentY / maxContentHeight, 0, 1) - const maxContentPercentage = maxContentHeight <= 1 ? 1 : clamp(maxContentY / maxContentHeight, 0, 1) + properties = { + $prev_pageview_last_scroll: lastScrollY, + $prev_pageview_last_scroll_percentage: lastScrollPercentage, + $prev_pageview_max_scroll: maxScrollY, + $prev_pageview_max_scroll_percentage: maxScrollPercentage, + $prev_pageview_last_content: lastContentY, + $prev_pageview_last_content_percentage: lastContentPercentage, + $prev_pageview_max_content: maxContentY, + $prev_pageview_max_content_percentage: maxContentPercentage, + } + } + } - return { - $prev_pageview_pathname: previousPath, - $prev_pageview_last_scroll: lastScrollY, - $prev_pageview_last_scroll_percentage: lastScrollPercentage, - $prev_pageview_max_scroll: maxScrollY, - $prev_pageview_max_scroll_percentage: maxScrollPercentage, - $prev_pageview_last_content: lastContentY, - $prev_pageview_last_content_percentage: lastContentPercentage, - $prev_pageview_max_content: maxContentY, - $prev_pageview_max_content_percentage: maxContentPercentage, + if (previousPath) { + properties.$prev_pageview_pathname = previousPath + } + if (previousTimestamp) { + // Use seconds, for consistency with our other duration-related properties like $duration + properties.$prev_pageview_duration = (timestamp.getTime() - previousTimestamp.getTime()) / 1000 } + + return properties } } diff --git a/src/posthog-core.ts b/src/posthog-core.ts index 61346548e..13f798f0b 100644 --- a/src/posthog-core.ts +++ b/src/posthog-core.ts @@ -802,10 +802,13 @@ export class PostHog { this.persistence.set_initial_person_info() } + const systemTime = new Date() + const timestamp = options?.timestamp || systemTime + let data: CaptureResult = { uuid: uuidv7(), event: event_name, - properties: this._calculate_event_properties(event_name, properties || {}), + properties: this._calculate_event_properties(event_name, properties || {}, timestamp), } if (clientRateLimitContext) { @@ -822,10 +825,10 @@ export class PostHog { } data = _copyAndTruncateStrings(data, options?._noTruncate ? null : this.config.properties_string_max_length) - data.timestamp = options?.timestamp || new Date() + data.timestamp = timestamp if (!isUndefined(options?.timestamp)) { data.properties['$event_time_override_provided'] = true - data.properties['$event_time_override_system_time'] = new Date() + data.properties['$event_time_override_system_time'] = systemTime } // Top-level $set overriding values from the one from properties is taken from the plugin-server normalizeEvent @@ -858,7 +861,7 @@ export class PostHog { this.on('eventCaptured', (data) => callback(data.event, data)) } - _calculate_event_properties(event_name: string, event_properties: Properties): Properties { + _calculate_event_properties(event_name: string, event_properties: Properties, timestamp: Date): Properties { if (!this.persistence || !this.sessionPersistence) { return event_properties } @@ -905,9 +908,9 @@ export class PostHog { if (!this.config.disable_scroll_properties) { let performanceProperties: Record = {} if (event_name === '$pageview') { - performanceProperties = this.pageViewManager.doPageView() + performanceProperties = this.pageViewManager.doPageView(timestamp) } else if (event_name === '$pageleave') { - performanceProperties = this.pageViewManager.doPageLeave() + performanceProperties = this.pageViewManager.doPageLeave(timestamp) } properties = extend(properties, performanceProperties) } @@ -918,7 +921,7 @@ export class PostHog { // set $duration if time_event was previously called for this event if (!isUndefined(startTimestamp)) { - const duration_in_ms = new Date().getTime() - startTimestamp + const duration_in_ms = timestamp.getTime() - startTimestamp properties['$duration'] = parseFloat((duration_in_ms / 1000).toFixed(3)) } From b931f80e8faad12302bf2baf1dbf8b9106527945 Mon Sep 17 00:00:00 2001 From: Robbie Coomber Date: Fri, 9 Aug 2024 12:47:44 +0100 Subject: [PATCH 2/3] Fix segment integration --- src/extensions/segment-integration.ts | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/extensions/segment-integration.ts b/src/extensions/segment-integration.ts index 59503ee7a..4545cd3d0 100644 --- a/src/extensions/segment-integration.ts +++ b/src/extensions/segment-integration.ts @@ -81,7 +81,11 @@ const createSegmentIntegration = (posthog: PostHog): SegmentPlugin => { posthog.reloadFeatureFlags() } - const additionalProperties = posthog._calculate_event_properties(eventName, ctx.event.properties ?? {}) + const additionalProperties = posthog._calculate_event_properties( + eventName, + ctx.event.properties ?? {}, + new Date() + ) ctx.event.properties = Object.assign({}, additionalProperties, ctx.event.properties) return ctx } From b744394694edf5f1d84fdeba1022f49fe695c2d8 Mon Sep 17 00:00:00 2001 From: Robbie Coomber Date: Fri, 9 Aug 2024 12:54:07 +0100 Subject: [PATCH 3/3] Fix tests, add duration to tests --- src/__tests__/page-view.test.ts | 22 ++++++++++++++-------- src/page-view.ts | 5 +++++ 2 files changed, 19 insertions(+), 8 deletions(-) diff --git a/src/__tests__/page-view.test.ts b/src/__tests__/page-view.test.ts index 3e296200a..c7001a38f 100644 --- a/src/__tests__/page-view.test.ts +++ b/src/__tests__/page-view.test.ts @@ -11,6 +11,10 @@ jest.mock('../utils/globals', () => ({ })) describe('PageView ID manager', () => { + const firstTimestamp = new Date() + const duration = 42 + const secondTimestamp = new Date(firstTimestamp.getTime() + duration * 1000) + describe('doPageView', () => { let instance: PostHog let pageViewIdManager: PageViewManager @@ -51,12 +55,12 @@ describe('PageView ID manager', () => { }, }) - pageViewIdManager.doPageView() + pageViewIdManager.doPageView(firstTimestamp) // force the manager to update the scroll data by calling an internal method instance.scrollManager['_updateScrollData']() - const secondPageView = pageViewIdManager.doPageView() + const secondPageView = pageViewIdManager.doPageView(secondTimestamp) expect(secondPageView.$prev_pageview_last_scroll).toEqual(2000) expect(secondPageView.$prev_pageview_last_scroll_percentage).toBeCloseTo(2 / 3) expect(secondPageView.$prev_pageview_max_scroll).toEqual(2000) @@ -65,6 +69,7 @@ describe('PageView ID manager', () => { expect(secondPageView.$prev_pageview_last_content_percentage).toBeCloseTo(3 / 4) expect(secondPageView.$prev_pageview_max_content).toEqual(3000) expect(secondPageView.$prev_pageview_max_content_percentage).toBeCloseTo(3 / 4) + expect(secondPageView.$prev_pageview_duration).toEqual(duration) }) it('includes scroll position properties for a short page', () => { @@ -81,12 +86,12 @@ describe('PageView ID manager', () => { }, }) - pageViewIdManager.doPageView() + pageViewIdManager.doPageView(firstTimestamp) // force the manager to update the scroll data by calling an internal method instance.scrollManager['_updateScrollData']() - const secondPageView = pageViewIdManager.doPageView() + const secondPageView = pageViewIdManager.doPageView(secondTimestamp) expect(secondPageView.$prev_pageview_last_scroll).toEqual(0) expect(secondPageView.$prev_pageview_last_scroll_percentage).toEqual(1) expect(secondPageView.$prev_pageview_max_scroll).toEqual(0) @@ -95,22 +100,23 @@ describe('PageView ID manager', () => { expect(secondPageView.$prev_pageview_last_content_percentage).toEqual(1) expect(secondPageView.$prev_pageview_max_content).toEqual(1000) expect(secondPageView.$prev_pageview_max_content_percentage).toEqual(1) + expect(secondPageView.$prev_pageview_duration).toEqual(duration) }) it('can handle scroll updates before doPageView is called', () => { instance.scrollManager['_updateScrollData']() - const firstPageView = pageViewIdManager.doPageView() + const firstPageView = pageViewIdManager.doPageView(firstTimestamp) expect(firstPageView.$prev_pageview_last_scroll).toBeUndefined() - const secondPageView = pageViewIdManager.doPageView() + const secondPageView = pageViewIdManager.doPageView(secondTimestamp) expect(secondPageView.$prev_pageview_last_scroll).toBeDefined() }) it('should include the pathname', () => { instance.scrollManager['_updateScrollData']() - const firstPageView = pageViewIdManager.doPageView() + const firstPageView = pageViewIdManager.doPageView(firstTimestamp) expect(firstPageView.$prev_pageview_pathname).toBeUndefined() - const secondPageView = pageViewIdManager.doPageView() + const secondPageView = pageViewIdManager.doPageView(secondTimestamp) expect(secondPageView.$prev_pageview_pathname).toEqual('/pathname') }) }) diff --git a/src/page-view.ts b/src/page-view.ts index dc34a51a2..bf1952d6e 100644 --- a/src/page-view.ts +++ b/src/page-view.ts @@ -44,6 +44,11 @@ export class PageViewManager { const previousTimestamp = this._prevPageviewTimestamp const scrollContext = this._instance.scrollManager.getContext() + if (!previousTimestamp) { + // this means there was no previous pageview + return {} + } + let properties: PageViewEventProperties = {} if (scrollContext) { let { maxScrollHeight, lastScrollY, maxScrollY, maxContentHeight, lastContentY, maxContentY } =