Skip to content

Commit

Permalink
fix: [AMP-95816] fix pageCounter bug (#720)
Browse files Browse the repository at this point in the history
  • Loading branch information
@yuhao900914
yuhao900914 authored Apr 22, 2024
1 parent e55f597 commit 8899853
Show file tree
Hide file tree
Showing 5 changed files with 380 additions and 13 deletions.
2 changes: 1 addition & 1 deletion packages/analytics-browser-test/package.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
"url": "https://github.com/amplitude/Amplitude-TypeScript/issues"
},
"dependencies": {
"@amplitude/analytics-browser": "^2.3.6-beta.0"
"@amplitude/analytics-browser": "^2.6.3-beta.0"
},
"devDependencies": {
"nock": "^13.2.4"
Expand Down
175 changes: 174 additions & 1 deletion packages/analytics-browser-test/test/index.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import * as amplitude from '@amplitude/analytics-browser';
import { default as nock } from 'nock';
import { success } from './responses';
import 'isomorphic-fetch';
import { path, SUCCESS_MESSAGE, url, uuidPattern } from './constants';
import { path, url, SUCCESS_MESSAGE, uuidPattern } from './constants';
import { LogLevel } from '@amplitude/analytics-types';
import { UUID } from '@amplitude/analytics-core';

Expand All @@ -28,9 +28,27 @@ describe('integration', () => {
const event_upload_time = '2023-01-01T12:00:00:000Z';
Date.prototype.toISOString = jest.fn(() => event_upload_time);

beforeAll(() => {
Object.defineProperty(window, 'location', {
value: {
hostname: '',
href: '',
pathname: '',
search: '',
},
writable: true,
});
});

beforeEach(() => {
client = amplitude.createInstance();
apiKey = UUID();
(window.location as any) = {
hostname: '',
href: '',
pathname: '',
search: '',
};
});

afterEach(() => {
Expand Down Expand Up @@ -1563,6 +1581,154 @@ describe('integration', () => {
});
});

test('should send page view with and clear pageCounter in new session', () => {
let payload: any = undefined;
const scope = nock(url)
.post(path, (body: Record<string, any>) => {
payload = body;
return true;
})
.reply(200, success);

const client = amplitude.createInstance();
client.init(apiKey, '[email protected]', {
defaultTracking: {
...defaultTracking,
pageViews: true,
},
sessionTimeout: 500,
flushIntervalMillis: 3000,
});

client.track('Event in first session');

const oldURL = new URL('https://www.example.com');
mockWindowLocationFromURL(oldURL);
window.history.pushState(undefined, oldURL.href);

const newURL = new URL('https://www.example.com/about');
mockWindowLocationFromURL(newURL);
window.history.pushState(undefined, newURL.href);

// Fire page view event in current session
setTimeout(() => {
const latestURL = new URL('https://www.example.com/contact');
mockWindowLocationFromURL(latestURL);
window.history.pushState(undefined, latestURL.href);
}, 500);

// Fire page view event in new session
setTimeout(() => {
const anotherURL = new URL('https://www.example.com/more');
mockWindowLocationFromURL(anotherURL);
window.history.pushState(undefined, anotherURL.href);
}, 2000);

return new Promise<void>((resolve) => {
setTimeout(() => {
expect(payload).toEqual({
api_key: apiKey,
client_upload_time: event_upload_time,
events: [
{
device_id: uuid,
event_id: 0,
event_properties: {
'[Amplitude] Page Domain': 'www.example.com',
'[Amplitude] Page Location': 'https://www.example.com/about',
'[Amplitude] Page Path': '/about',
'[Amplitude] Page Title': '',
'[Amplitude] Page URL': 'https://www.example.com/about',
'[Amplitude] Page Counter': 1,
},
event_type: '[Amplitude] Page Viewed',
insert_id: uuid,
ip: '$remote',
language: 'en-US',
library,
partner_id: undefined,
plan: undefined,
platform: 'Web',
session_id: number,
time: number,
user_agent: userAgent,
user_id: '[email protected]',
},
{
device_id: uuid,
event_id: 1,
event_type: 'Event in first session',
insert_id: uuid,
ip: '$remote',
language: 'en-US',
library,
partner_id: undefined,
plan: undefined,
platform: 'Web',
session_id: number,
time: number,
user_agent: userAgent,
user_id: '[email protected]',
},
{
device_id: uuid,
event_id: 2,
event_properties: {
'[Amplitude] Page Domain': 'www.example.com',
'[Amplitude] Page Location': 'https://www.example.com/contact',
'[Amplitude] Page Path': '/contact',
'[Amplitude] Page Title': '',
'[Amplitude] Page URL': 'https://www.example.com/contact',
'[Amplitude] Page Counter': 2,
},
event_type: '[Amplitude] Page Viewed',
insert_id: uuid,
ip: '$remote',
language: 'en-US',
library,
partner_id: undefined,
plan: undefined,
platform: 'Web',
session_id: number,
time: number,
user_agent: userAgent,
user_id: '[email protected]',
},
{
device_id: uuid,
event_id: 3,
event_properties: {
'[Amplitude] Page Domain': 'www.example.com',
'[Amplitude] Page Location': 'https://www.example.com/more',
'[Amplitude] Page Path': '/more',
'[Amplitude] Page Title': '',
'[Amplitude] Page URL': 'https://www.example.com/more',
'[Amplitude] Page Counter': 1,
},
event_type: '[Amplitude] Page Viewed',
insert_id: uuid,
ip: '$remote',
language: 'en-US',
library,
partner_id: undefined,
plan: undefined,
platform: 'Web',
session_id: number,
time: number,
user_agent: userAgent,
user_id: '[email protected]',
},
],
options: {
min_id_length: undefined,
},
});
scope.done();
resolve();
}, 4000);
});
});

test('should send page view', () => {
let payload: any = undefined;
const scope = nock(url)
Expand Down Expand Up @@ -1873,3 +2039,10 @@ const setLegacyCookie = (
eventId ? eventId.toString(32) : '',
].join('.')}`;
};

const mockWindowLocationFromURL = (url: URL) => {
window.location.href = url.toString();
window.location.search = url.search;
window.location.hostname = url.hostname;
window.location.pathname = url.pathname;
};
145 changes: 145 additions & 0 deletions packages/analytics-browser/playground/amplitude.js
View file

Large diffs are not rendered by default.

23 changes: 16 additions & 7 deletions packages/plugin-page-view-tracking-browser/src/page-view-tracking.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,17 +12,19 @@ import { BASE_CAMPAIGN } from '@amplitude/analytics-client-common';
import { CreatePageViewTrackingPlugin, Options } from './typings/page-view-tracking';
import { omitUndefined } from './utils';

export const defaultPageViewEvent = '[Amplitude] Page Viewed';

export const pageViewTrackingPlugin: CreatePageViewTrackingPlugin = (options: Options = {}) => {
let amplitude: BrowserClient | undefined;
const globalScope = getGlobalScope();
let loggerProvider: Logger | undefined = undefined;
let pushState: undefined | ((data: any, unused: string, url?: string | URL | null) => void);
let localConfig: BrowserConfig;
const { trackOn, trackHistoryChanges, eventType = defaultPageViewEvent } = options;

const createPageViewEvent = async (): Promise<Event> => {
localConfig.pageCounter = !localConfig.pageCounter ? 1 : localConfig.pageCounter + 1;
return {
event_type: options.eventType ?? '[Amplitude] Page Viewed',
event_type: eventType,
event_properties: {
...(await getCampaignParams()),
'[Amplitude] Page Domain':
Expand All @@ -34,21 +36,19 @@ export const pageViewTrackingPlugin: CreatePageViewTrackingPlugin = (options: Op
'[Amplitude] Page Title': /* istanbul ignore next */ (typeof document !== 'undefined' && document.title) || '',
'[Amplitude] Page URL':
/* istanbul ignore next */ (typeof location !== 'undefined' && location.href.split('?')[0]) || '',
'[Amplitude] Page Counter': localConfig.pageCounter,
},
};
};

const shouldTrackOnPageLoad = () =>
typeof options.trackOn === 'undefined' || (typeof options.trackOn === 'function' && options.trackOn());
const shouldTrackOnPageLoad = () => typeof trackOn === 'undefined' || (typeof trackOn === 'function' && trackOn());

/* istanbul ignore next */
let previousURL: string | null = typeof location !== 'undefined' ? location.href : null;

const trackHistoryPageView = async (): Promise<void> => {
const newURL = location.href;
const shouldTrackPageView =
shouldTrackHistoryPageView(options.trackHistoryChanges, newURL, previousURL || '') && shouldTrackOnPageLoad();
shouldTrackHistoryPageView(trackHistoryChanges, newURL, previousURL || '') && shouldTrackOnPageLoad();
// Note: Update `previousURL` in the same clock tick as `shouldTrackHistoryPageView()`
// This was previously done after `amplitude?.track(await createPageViewEvent());` and
// causes a concurrency issue where app triggers `pushState` twice with the same URL target
Expand Down Expand Up @@ -106,7 +106,7 @@ export const pageViewTrackingPlugin: CreatePageViewTrackingPlugin = (options: Op
},

execute: async (event: Event) => {
if (options.trackOn === 'attribution' && isCampaignEvent(event)) {
if (trackOn === 'attribution' && isCampaignEvent(event)) {
/* istanbul ignore next */ // loggerProvider should be defined by the time execute is invoked
loggerProvider?.log('Enriching campaign event to page view event with campaign parameters');
const pageViewEvent = await createPageViewEvent();
Expand All @@ -116,6 +116,15 @@ export const pageViewTrackingPlugin: CreatePageViewTrackingPlugin = (options: Op
...pageViewEvent.event_properties,
};
}

// Update the pageCounter for the page view event
if (localConfig && event.event_type === eventType) {
localConfig.pageCounter = !localConfig.pageCounter ? 1 : localConfig.pageCounter + 1;
event.event_properties = {
...event.event_properties,
'[Amplitude] Page Counter': localConfig.pageCounter,
};
}
return event;
},

Expand Down
48 changes: 44 additions & 4 deletions packages/plugin-page-view-tracking-browser/test/page-view-tracking.test.ts
View file
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { createInstance } from '@amplitude/analytics-browser';
import { Logger, UUID } from '@amplitude/analytics-core';
import { BrowserConfig, LogLevel } from '@amplitude/analytics-types';
import { pageViewTrackingPlugin, shouldTrackHistoryPageView } from '../src/page-view-tracking';
import { defaultPageViewEvent, pageViewTrackingPlugin, shouldTrackHistoryPageView } from '../src/page-view-tracking';
import { CookieStorage, FetchTransport } from '@amplitude/analytics-client-common';

describe('pageViewTrackingPlugin', () => {
Expand All @@ -17,7 +17,6 @@ describe('pageViewTrackingPlugin', () => {
serverUrl: undefined,
transportProvider: new FetchTransport(),
useBatch: false,

cookieOptions: {
domain: '.amplitude.com',
expiration: 365,
Expand Down Expand Up @@ -95,7 +94,6 @@ describe('pageViewTrackingPlugin', () => {
'[Amplitude] Page Path': newURL.pathname,
'[Amplitude] Page Title': '',
'[Amplitude] Page URL': newURL.toString(),
'[Amplitude] Page Counter': 2,
},
event_type: '[Amplitude] Page Viewed',
});
Expand Down Expand Up @@ -138,7 +136,6 @@ describe('pageViewTrackingPlugin', () => {
'[Amplitude] Page Path': pathname,
'[Amplitude] Page Title': '',
'[Amplitude] Page URL': `https://${hostname}${pathname}`,
'[Amplitude] Page Counter': 1,
utm_source: 'google',
utm_medium: 'cpc',
utm_campaign: 'brand',
Expand Down Expand Up @@ -296,6 +293,49 @@ describe('pageViewTrackingPlugin', () => {
const event = await plugin.execute?.(sentEvent);
expect(event).toBe(sentEvent);
});

test('should set the pageCounter', async () => {
const config = { ...mockConfig };
config.pageCounter = 0;
const amplitude = createInstance();
jest.spyOn(amplitude, 'track').mockReturnValue({
promise: Promise.resolve({
code: 200,
message: '',
event: {
event_type: defaultPageViewEvent,
},
}),
});

const plugin = pageViewTrackingPlugin();
await plugin.setup?.(config, amplitude);
await plugin.execute?.({ event_type: defaultPageViewEvent });
expect(config.pageCounter).toBe(1);

await plugin.execute?.({ event_type: defaultPageViewEvent });
expect(config.pageCounter).toBe(2);
});

test('should not set the pageCounter', async () => {
const config = { ...mockConfig };
config.pageCounter = 0;
const amplitude = createInstance();
jest.spyOn(amplitude, 'track').mockReturnValue({
promise: Promise.resolve({
code: 200,
message: '',
event: {
event_type: defaultPageViewEvent,
},
}),
});

const plugin = pageViewTrackingPlugin();
await plugin.setup?.(config, amplitude);
await plugin.execute?.({ event_type: 'other event' });
expect(config.pageCounter).toBe(0);
});
});

describe('teardown', () => {
Expand Down

0 comments on commit 8899853

Please sign in to comment.