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

fix: fix web attribution behavior for no referrer in the same session #554

Merged
merged 4 commits into from
Aug 24, 2023
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
6 changes: 3 additions & 3 deletions packages/analytics-browser/src/browser-client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import {
isFormInteractionTrackingEnabled,
setConnectorDeviceId,
setConnectorUserId,
isNewSession,
} from '@amplitude/analytics-client-common';
import {
BrowserClient,
Expand Down Expand Up @@ -250,14 +251,13 @@ export class AmplitudeBrowser extends AmplitudeCore implements BrowserClient {

async process(event: Event) {
const currentTime = Date.now();
const lastEventTime = this.config.lastEventTime || Date.now();
const timeSinceLastEvent = currentTime - lastEventTime;
const isEventInNewSession = isNewSession(this.config.sessionTimeout, this.config.lastEventTime);

if (
event.event_type !== DEFAULT_SESSION_START_EVENT &&
event.event_type !== DEFAULT_SESSION_END_EVENT &&
(!event.session_id || event.session_id === this.getSessionId()) &&
timeSinceLastEvent > this.config.sessionTimeout
isEventInNewSession
) {
this.setSessionId(currentTime);
}
Expand Down
1 change: 1 addition & 0 deletions packages/analytics-client-common/src/index.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
export { CampaignParser } from './attribution/campaign-parser';
export { CampaignTracker } from './attribution/campaign-tracker';
export { getQueryParams } from './query-params';
export { isNewSession } from './session';
export { getCookieName, getOldCookieName } from './cookie-name';
export { CookieStorage } from './storage/cookie';
export { FetchTransport } from './transports/fetch';
Expand Down
6 changes: 6 additions & 0 deletions packages/analytics-client-common/src/session.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
export const isNewSession = (sessionTimeout: number, lastEventTime: number = Date.now()): boolean => {
const currentTime = Date.now();
const timeSinceLastEvent = currentTime - lastEventTime;

return timeSinceLastEvent > sessionTimeout;
yuhao900914 marked this conversation as resolved.
Show resolved Hide resolved
};
25 changes: 25 additions & 0 deletions packages/analytics-client-common/test/session.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
import { isNewSession } from '../src/session';

describe('session', () => {
const sessionTimeout: number = 30 * 60 * 1000;

test('should be in a same session for undefined lastEventTime', () => {
const isEventInNewSession = isNewSession(sessionTimeout, undefined);

expect(isEventInNewSession).toBe(false);
});

test('should be a new session', () => {
const lastEventTime = Date.now() - sessionTimeout * 2;
const isEventInNewSession = isNewSession(sessionTimeout, lastEventTime);

expect(isEventInNewSession).toBe(true);
});

test('should be in a same session', () => {
const lastEventTime = Date.now();
const isEventInNewSession = isNewSession(sessionTimeout, lastEventTime);

expect(isEventInNewSession).toBe(false);
});
});
13 changes: 12 additions & 1 deletion packages/plugin-web-attribution-browser/src/helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,24 @@ const domainWithoutSubdomain = (domain: string) => {
return parts.slice(parts.length - 2, parts.length).join('.');
};

export const isNewCampaign = (current: Campaign, previous: Campaign | undefined, options: Options) => {
export const isNewCampaign = (
current: Campaign,
previous: Campaign | undefined,
options: Options,
isNewSession = true,
) => {
const { referrer, referring_domain, ...currentCampaign } = current;
const { referrer: _previous_referrer, referring_domain: prevReferringDomain, ...previousCampaign } = previous || {};

if (isExcludedReferrer(options.excludeReferrers, current.referring_domain)) {
return false;
}

yuhao900914 marked this conversation as resolved.
Show resolved Hide resolved
//In the same session, no referrer should not override or unset any persisting query params
if (!isNewSession && !referrer && previous) {
return false;
}

const hasNewCampaign = JSON.stringify(currentCampaign) !== JSON.stringify(previousCampaign);
const hasNewDomain =
domainWithoutSubdomain(referring_domain || '') !== domainWithoutSubdomain(prevReferringDomain || '');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { CampaignParser } from '@amplitude/analytics-client-common';
import { BeforePlugin, BrowserClient, BrowserConfig, Campaign, Event, Storage } from '@amplitude/analytics-types';
import { createCampaignEvent, getDefaultExcludedReferrers, getStorageKey, isNewCampaign } from './helpers';
import { CreateWebAttributionPlugin, Options } from './typings/web-attribution';
import { isNewSession } from '@amplitude/analytics-client-common';

export const webAttributionPlugin: CreateWebAttributionPlugin = function (options: Options = {}) {
const plugin: BeforePlugin = {
Expand All @@ -26,7 +27,9 @@ export const webAttributionPlugin: CreateWebAttributionPlugin = function (option
storage.get(storageKey),
]);

if (isNewCampaign(currentCampaign, previousCampaign, pluginConfig)) {
const isEventInNewSession = isNewSession(config.sessionTimeout, config.lastEventTime);

if (isNewCampaign(currentCampaign, previousCampaign, pluginConfig, isEventInNewSession)) {
if (pluginConfig.resetSessionOnNewCampaign) {
amplitude.setSessionId(Date.now());
config.loggerProvider.log('Created a new session for new campaign.');
Expand Down
13 changes: 13 additions & 0 deletions packages/plugin-web-attribution-browser/test/helpers.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,19 @@ describe('isNewCampaign', () => {
}),
).toBe(false);
});

test('should return false for no referrer in the same session', () => {
const previousCampaign = {
...BASE_CAMPAIGN,
utm_campaign: 'utm_campaign',
referring_domain: 'a.b.c.d',
};
const currentCampaign = {
...BASE_CAMPAIGN,
};

expect(isNewCampaign(currentCampaign, previousCampaign, {}, false)).toBe(false);
});
});

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