Skip to content

Commit

Permalink
fix: fix based on comment
Browse files Browse the repository at this point in the history
  • Loading branch information
Alyssa.Yu authored and Alyssa.Yu committed Aug 22, 2023
1 parent 6b0725a commit ffd08bf
Show file tree
Hide file tree
Showing 6 changed files with 40 additions and 11 deletions.
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 _isNewSession = 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
_isNewSession
) {
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
7 changes: 7 additions & 0 deletions packages/analytics-client-common/src/session.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
export const isNewSession = (sessionTimeout: number, lastEventTime?: number): boolean => {
const _currentTime = Date.now();
const _lastEventTime = lastEventTime || Date.now();
const timeSinceLastEvent = _currentTime - _lastEventTime;

return timeSinceLastEvent > sessionTimeout;
};
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 a new session for the first event', () => {
const _isNewSession = isNewSession(sessionTimeout, undefined);

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

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

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

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

expect(_isNewSession).toBe(false);
});
});
3 changes: 1 addition & 2 deletions packages/plugin-web-attribution-browser/src/helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,7 @@ export const isNewCampaign = (
}

//In the same session, no referrer should not override or unset any persisting query params
const isNoReferrer = !referring_domain;
if (isNewSession && isNoReferrer) {
if (isNewSession && !referring_domain) {
return false;
}

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,13 +27,9 @@ export const webAttributionPlugin: CreateWebAttributionPlugin = function (option
storage.get(storageKey),
]);

const currentTime = Date.now();
const lastEventTime = config.lastEventTime || Date.now();
const timeSinceLastEvent = currentTime - lastEventTime;
const _isNewSession = isNewSession(config.sessionTimeout, config.lastEventTime);

const isNewSession = timeSinceLastEvent > config.sessionTimeout;

if (isNewCampaign(currentCampaign, previousCampaign, pluginConfig, isNewSession)) {
if (isNewCampaign(currentCampaign, previousCampaign, pluginConfig, _isNewSession)) {
if (pluginConfig.resetSessionOnNewCampaign) {
amplitude.setSessionId(Date.now());
config.loggerProvider.log('Created a new session for new campaign.');
Expand Down

0 comments on commit ffd08bf

Please sign in to comment.