refactor to checkAndGetSessionAndWindowId

Author: rcmarron

src/__tests__/extensions/sessionrecording.js

@@ -26,7 +26,7 @@ describe('SessionRecording', () => {
         persistence: { register: jest.fn() },
         _captureMetrics: { incr: jest.fn() },
         sessionManager: {
-            getSessionAndWindowId: jest.fn().mockImplementation(() => given.incomingSessionAndWindowId),
+            checkAndGetSessionAndWindowId: jest.fn().mockImplementation(() => given.incomingSessionAndWindowId),
         },
         _addCaptureHook: jest.fn(),
     }))
@@ -321,17 +321,20 @@ describe('SessionRecording', () => {
                 expect(window.rrwebRecord.takeFullSnapshot).not.toHaveBeenCalled()
             })
 
-            it('it calls getSessionAndWindowId with shouldExtendExistingSessionOrTriggerNewOne as false if it not a user interaction', () => {
+            it('it calls checkAndGetSessionAndWindowId with readOnly as true if it not a user interaction', () => {
                 _emit(NON_USER_GENERATED_EVENT)
-                expect(given.posthog.sessionManager.getSessionAndWindowId).toHaveBeenCalledWith(false, undefined)
+                expect(given.posthog.sessionManager.checkAndGetSessionAndWindowId).toHaveBeenCalledWith(true, undefined)
             })
 
-            it('it calls getSessionAndWindowId with shouldExtendExistingSessionOrTriggerNewOne as true if it is a user interaction', () => {
+            it('it calls checkAndGetSessionAndWindowId with readOnly as false if it is a user interaction', () => {
                 _emit({
                     event: 123,
                     type: INCREMENTAL_SNAPSHOT_EVENT_TYPE,
                 })
-                expect(given.posthog.sessionManager.getSessionAndWindowId).toHaveBeenCalledWith(true, undefined)
+                expect(given.posthog.sessionManager.checkAndGetSessionAndWindowId).toHaveBeenCalledWith(
+                    false,
+                    undefined
+                )
             })
         })
     })

src/__tests__/posthog-core.js

@@ -396,7 +396,7 @@ describe('_calculate_event_properties()', () => {
             properties: () => ({ distinct_id: 'abc', persistent: 'prop' }),
         },
         sessionManager: {
-            getSessionAndWindowId: jest.fn().mockReturnValue({
+            checkAndGetSessionAndWindowId: jest.fn().mockReturnValue({
                 windowId: 'windowId',
                 sessionId: 'sessionId',
             }),
@@ -444,7 +444,7 @@ describe('_calculate_event_properties()', () => {
             event: 'prop',
             distinct_id: 'abc',
         })
-        expect(given.overrides.sessionManager.getSessionAndWindowId).not.toHaveBeenCalled()
+        expect(given.overrides.sessionManager.checkAndGetSessionAndWindowId).not.toHaveBeenCalled()
     })
 
     it('calls sanitize_properties', () => {

src/__tests__/sessionid.js

@@ -7,9 +7,7 @@ jest.mock('../utils')
 jest.mock('../storage')
 
 describe('Session ID manager', () => {
-    given('subject', () =>
-        given.sessionIdManager.getSessionAndWindowId(given.shouldExtendExistingSessionOrTriggerNewOne, given.timestamp)
-    )
+    given('subject', () => given.sessionIdManager.checkAndGetSessionAndWindowId(given.readOnly, given.timestamp))
     given('sessionIdManager', () => new SessionIdManager(given.config, given.persistence))
 
     given('persistence', () => ({
@@ -42,8 +40,8 @@ describe('Session ID manager', () => {
             expect(sessionStore.set).toHaveBeenCalledWith('ph_persistance-name_window_id', 'newUUID')
         })
 
-        it('generates an initial session id and window id, and saves them even if shouldExtendExistingSessionOrTriggerNewOne is false', () => {
-            given('shouldExtendExistingSessionOrTriggerNewOne', () => false)
+        it('generates an initial session id and window id, and saves them even if readOnly is true', () => {
+            given('readOnly', () => true)
             expect(given.subject).toMatchObject({
                 windowId: 'newUUID',
                 sessionId: 'newUUID',
@@ -67,10 +65,10 @@ describe('Session ID manager', () => {
             expect(given.persistence.register).toHaveBeenCalledWith({ [SESSION_ID]: [given.timestamp, 'oldSessionID'] })
         })
 
-        it('reuses old ids and does not update the session timestamp if  > 30m pass and shouldExtendExistingSessionOrTriggerNewOne is false', () => {
+        it('reuses old ids and does not update the session timestamp if  > 30m pass and readOnly is true', () => {
             const old_timestamp = 1602107460000
             given('storedSessionIdData', () => [old_timestamp, 'oldSessionID'])
-            given('shouldExtendExistingSessionOrTriggerNewOne', () => false)
+            given('readOnly', () => true)
 
             expect(given.subject).toEqual({
                 windowId: 'oldWindowID',

src/extensions/sessionrecording.js

@@ -98,12 +98,11 @@ export class SessionRecording {
         // Some recording events are triggered by non-user events (e.g. "X minutes ago" text updating on the screen).
         // We don't want to extend the session or trigger a new session in these cases. These events are designated by event
         // type -> incremental update, and source -> mutation.
-        const isUserInteraction = !(
+        const isNotUserInteraction =
             event.type === INCREMENTAL_SNAPSHOT_EVENT_TYPE && event.data?.source === MUTATION_SOURCE_TYPE
-        )
 
-        const { windowId, sessionId } = this.instance.sessionManager.getSessionAndWindowId(
-            isUserInteraction,
+        const { windowId, sessionId } = this.instance.sessionManager.checkAndGetSessionAndWindowId(
+            isNotUserInteraction,
             event.timestamp
         )
 

src/posthog-core.js

@@ -657,7 +657,7 @@ PostHogLib.prototype._calculate_event_properties = function (event_name, event_p
     }
 
     if (this.sessionManager) {
-        const { sessionId, windowId } = this.sessionManager.getSessionAndWindowId()
+        const { sessionId, windowId } = this.sessionManager.checkAndGetSessionAndWindowId()
         properties['$session_id'] = sessionId
         properties['$window_id'] = windowId
     }
@@ -1499,7 +1499,7 @@ PostHogLib.prototype.sentry_integration = function (_posthog, organization, proj
                 event.tags['PostHog Recording URL'] =
                     _posthog.config.api_host +
                     '/recordings/#sessionRecordingId=' +
-                    _posthog.sessionManager.getSessionAndWindowId(false).sessionId
+                    _posthog.sessionManager.checkAndGetSessionAndWindowId(true).sessionId
             }
             let data = {
                 $sentry_event_id: event.event_id,

src/sessionid.js

@@ -55,7 +55,7 @@ export class SessionIdManager {
         return this.persistence['props'][SESSION_ID] || [0, null]
     }
 
-    // Resets the session id by setting it to null. On the subsequent call to getSessionAndWindowId,
+    // Resets the session id by setting it to null. On the subsequent call to checkAndGetSessionAndWindowId,
     // new ids will be generated.
     resetSessionId() {
         this._setSessionId(null, null)
@@ -68,34 +68,29 @@ export class SessionIdManager {
      * sessionId and windowId when appropriate by doing the following:
      *
      * 1. If the sessionId or windowId is not set, it will generate a new one and store it.
-     * 2. If the shouldExtendExistingSessionOrTriggerNewOne param is set, it will:
+     * 2. If the readOnly param is set to false, it will:
      *    a. Check if it has been > SESSION_CHANGE_THRESHOLD since the last call with this flag set.
      *       If so, it will generate a new sessionId and store it.
      *    b. Update the timestamp stored with the sessionId to ensure the current session is extended
      *       for the appropriate amount of time.
      *
-     * @param {boolean} shouldExtendExistingSessionOrTriggerNewOne (optional) Defaults to True. Should be set to False when the call to the function should be read only (e.g. being called for non-user generated events)
+     * @param {boolean} readOnly (optional) Defaults to False. Should be set to True when the call to the function should not extend or cycle the session (e.g. being called for non-user generated events)
      * @param {Number} timestamp (optional) Defaults to the current time. The timestamp to be stored with the sessionId (used when determining if a new sessionId should be generated)
      */
-    getSessionAndWindowId(shouldExtendExistingSessionOrTriggerNewOne = true, timestamp = null) {
+    checkAndGetSessionAndWindowId(readOnly = false, timestamp = null) {
         timestamp = timestamp || new Date().getTime()
 
         let [lastTimestamp, sessionId] = this._getSessionId()
         let windowId = this._getWindowId()
 
-        if (
-            !sessionId ||
-            (shouldExtendExistingSessionOrTriggerNewOne &&
-                Math.abs(timestamp - lastTimestamp) > SESSION_CHANGE_THRESHOLD)
-        ) {
+        if (!sessionId || (!readOnly && Math.abs(timestamp - lastTimestamp) > SESSION_CHANGE_THRESHOLD)) {
             sessionId = _.UUID()
             windowId = _.UUID()
         } else if (!windowId) {
             windowId = _.UUID()
         }
 
-        const newTimestamp =
-            lastTimestamp === 0 || shouldExtendExistingSessionOrTriggerNewOne ? timestamp : lastTimestamp
+        const newTimestamp = lastTimestamp === 0 || !readOnly ? timestamp : lastTimestamp
 
         this._setWindowId(windowId)
         this._setSessionId(sessionId, newTimestamp)