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

feat: remove old UUID code #755

Merged
merged 6 commits into from
Aug 9, 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
136 changes: 59 additions & 77 deletions cypress/e2e/identify.cy.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,93 +10,75 @@ function setup(initOptions) {
}

describe('identify()', () => {
describe('with OG uuid config', () => {
beforeEach(() => {
setup({ uuid_version: 'og' })
})

it('uses the OG uuid format when configured', () => {
cy.posthog().invoke('capture', '$pageview')
cy.phCaptures({ full: true }).then((events) => {
let deviceIds = new Set(events.map((e) => e.properties['$device_id']))
expect(deviceIds.size).to.eql(1)
const [deviceId] = deviceIds
expect(deviceId.length).to.be.greaterThan(51)
})
})
beforeEach(() => {
setup()
})

describe('with default config', () => {
beforeEach(() => {
setup()
})

it('uses the v7 uuid format by default', () => {
cy.posthog().invoke('capture', 'an-anonymous-event')
cy.phCaptures({ full: true }).then((events) => {
cy.log(events)
let deviceIds = new Set(events.map((e) => e.properties['$device_id']))
expect(deviceIds.size).to.eql(1)
const [deviceId] = deviceIds
expect(deviceId.length).to.be.eql(36)
})
it('uses the v7 uuid format', () => {
cy.posthog().invoke('capture', 'an-anonymous-event')
cy.phCaptures({ full: true }).then((events) => {
cy.log(events)
let deviceIds = new Set(events.map((e) => e.properties['$device_id']))
expect(deviceIds.size).to.eql(1)
const [deviceId] = deviceIds
expect(deviceId.length).to.be.eql(36)
})
})

it('opt_out_capturing() does not fail after identify()', () => {
cy.posthog().invoke('identify', 'some-id')
cy.posthog().invoke('opt_out_capturing')
})
it('opt_out_capturing() does not fail after identify()', () => {
cy.posthog().invoke('identify', 'some-id')
cy.posthog().invoke('opt_out_capturing')
})

it('merges people as expected when reset is called', () => {
cy.posthog().invoke('capture', 'an-anonymous-event')
cy.posthog().invoke('identify', 'first-identify') // test identify merges with previous events after init
cy.posthog().invoke('capture', 'an-identified-event')
cy.posthog().invoke('identify', 'second-identify-should-not-be-merged') // test identify is not sent after previous identify
cy.posthog().invoke('capture', 'another-identified-event') // but does change the distinct id
cy.posthog().invoke('reset')
cy.posthog().invoke('capture', 'an-anonymous-event')
cy.posthog().invoke('identify', 'third-identify')
cy.posthog().invoke('capture', 'an-identified-event')
it('merges people as expected when reset is called', () => {
cy.posthog().invoke('capture', 'an-anonymous-event')
cy.posthog().invoke('identify', 'first-identify') // test identify merges with previous events after init
cy.posthog().invoke('capture', 'an-identified-event')
cy.posthog().invoke('identify', 'second-identify-should-not-be-merged') // test identify is not sent after previous identify
cy.posthog().invoke('capture', 'another-identified-event') // but does change the distinct id
cy.posthog().invoke('reset')
cy.posthog().invoke('capture', 'an-anonymous-event')
cy.posthog().invoke('identify', 'third-identify')
cy.posthog().invoke('capture', 'an-identified-event')

cy.phCaptures({ full: true }).then((events) => {
const eventsSeen = events.map((e) => e.event)
cy.phCaptures({ full: true }).then((events) => {
const eventsSeen = events.map((e) => e.event)

expect(eventsSeen.filter((e) => e === '$identify').length).to.eq(2)
expect(eventsSeen.filter((e) => e === '$identify').length).to.eq(2)

expect(eventsSeen).to.deep.eq([
'$pageview',
'an-anonymous-event',
'$identify',
'an-identified-event',
'another-identified-event',
'an-anonymous-event',
'$identify',
'an-identified-event',
])
expect(eventsSeen).to.deep.eq([
'$pageview',
'an-anonymous-event',
'$identify',
'an-identified-event',
'another-identified-event',
'an-anonymous-event',
'$identify',
'an-identified-event',
])

expect(new Set(events.map((e) => e.properties['$device_id'])).size).to.eql(1)
expect(new Set(events.map((e) => e.properties['$device_id'])).size).to.eql(1)

// the first two events share a distinct id
expect(events[0].properties.distinct_id).to.eql(events[1].properties.distinct_id)
// then first identify is called and sends that distinct id as its anon to merge
expect(events[2].properties.distinct_id).to.eql('first-identify')
expect(events[2].properties['$anon_distinct_id']).to.eql(events[0].properties.distinct_id)
// and an event is sent with that distinct id
expect(events[3].properties.distinct_id).to.eql('first-identify')
// then second identify is called and is ignored but does change the distinct id
expect(events[4].event).to.eql('another-identified-event')
expect(events[4].properties.distinct_id).to.eql('second-identify-should-not-be-merged')
// then reset is called and the next event has a new distinct id
expect(events[5].event).to.eql('an-anonymous-event')
expect(events[5].properties.distinct_id)
.not.to.eql('first-identify')
.and.not.to.eql('second-identify-should-not-be-merged')
// then an identify merges that distinct id with the new distinct id
expect(events[6].properties.distinct_id).to.eql('third-identify')
expect(events[6].properties['$anon_distinct_id']).to.eql(events[5].properties.distinct_id)
// then a final identified event includes that identified distinct id
expect(events[7].properties.distinct_id).to.eql('third-identify')
})
// the first two events share a distinct id
expect(events[0].properties.distinct_id).to.eql(events[1].properties.distinct_id)
// then first identify is called and sends that distinct id as its anon to merge
expect(events[2].properties.distinct_id).to.eql('first-identify')
expect(events[2].properties['$anon_distinct_id']).to.eql(events[0].properties.distinct_id)
// and an event is sent with that distinct id
expect(events[3].properties.distinct_id).to.eql('first-identify')
// then second identify is called and is ignored but does change the distinct id
expect(events[4].event).to.eql('another-identified-event')
expect(events[4].properties.distinct_id).to.eql('second-identify-should-not-be-merged')
// then reset is called and the next event has a new distinct id
expect(events[5].event).to.eql('an-anonymous-event')
expect(events[5].properties.distinct_id)
.not.to.eql('first-identify')
.and.not.to.eql('second-identify-should-not-be-merged')
// then an identify merges that distinct id with the new distinct id
expect(events[6].properties.distinct_id).to.eql('third-identify')
expect(events[6].properties['$anon_distinct_id']).to.eql(events[5].properties.distinct_id)
// then a final identified event includes that identified distinct id
expect(events[7].properties.distinct_id).to.eql('third-identify')
})
})
})
9 changes: 4 additions & 5 deletions src/__tests__/extensions/sessionrecording.js
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
import { _UUID, loadScript } from '../../utils'
import { loadScript } from '../../utils'
import {
SessionRecording,
RECORDING_IDLE_ACTIVITY_TIMEOUT_MS,
RECORDING_MAX_EVENT_SIZE,
SessionRecording,
} from '../../extensions/sessionrecording'
import {
PostHogPersistence,
Expand Down Expand Up @@ -73,7 +73,6 @@ describe('SessionRecording', () => {
given('$session_recording_recorder_version_server_side', () => undefined)
given('disabled', () => false)
given('__loaded_recorder_version', () => undefined)
given('uuidFn', () => _UUID('v7'))

beforeEach(() => {
window.rrwebRecord = jest.fn()
Expand Down Expand Up @@ -554,7 +553,7 @@ describe('SessionRecording', () => {
beforeEach(() => {
given(
'sessionManager',
() => new SessionIdManager(given.config, new PostHogPersistence(given.config), given.uuidFn)
() => new SessionIdManager(given.config, new PostHogPersistence(given.config))
)

mockCallback = jest.fn()
Expand Down Expand Up @@ -633,7 +632,7 @@ describe('SessionRecording', () => {
beforeEach(() => {
given(
'sessionManager',
() => new SessionIdManager(given.config, new PostHogPersistence(given.config), given.uuidFn)
() => new SessionIdManager(given.config, new PostHogPersistence(given.config))
)
given.sessionRecording.startRecordingIfEnabled()
given.sessionRecording.startCaptureAndTrySendingQueuedSnapshots()
Expand Down
12 changes: 4 additions & 8 deletions src/__tests__/page-view-id.js
Original file line number Diff line number Diff line change
@@ -1,16 +1,12 @@
import { PageViewIdManager } from '../page-view-id'

jest.mock('../utils')
import { uuidv7 } from '../uuidv7'
jest.mock('../uuidv7')

describe('PageView ID manager', () => {
given('uuidFn', () => jest.fn())
given('pageViewIdManager', () => new PageViewIdManager(given.uuidFn))
given('pageViewIdManager', () => new PageViewIdManager())

beforeEach(() => {
given.uuidFn
.mockReturnValue('subsequentUUIDs')
.mockReturnValueOnce('firstUUID')
.mockReturnValueOnce('secondUUID')
uuidv7.mockReturnValue('subsequentUUIDs').mockReturnValueOnce('firstUUID').mockReturnValueOnce('secondUUID')
})

it('generates a page view id and resets page view id', () => {
Expand Down
6 changes: 3 additions & 3 deletions src/__tests__/segment.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,11 @@
* - Enrich Segment events with PostHog event properties.
*/

import { describe, it, expect, beforeEach, jest } from '@jest/globals'
import { beforeEach, describe, expect, it, jest } from '@jest/globals'

import posthog from '../loader-module'
import { PostHog } from '../posthog-core'
import { _UUID } from '../utils'
import { uuidv7 } from '../uuidv7'

describe(`Module-based loader in Node env`, () => {
let segment: any
Expand All @@ -21,7 +21,7 @@ describe(`Module-based loader in Node env`, () => {
beforeEach(() => {
jest.spyOn(posthog, '_send_request').mockReturnValue()
jest.spyOn(console, 'log').mockReturnValue()
posthogName = _UUID('v7')
posthogName = uuidv7()

// Create something that looks like the Segment Analytics 2.0 API. We
// could use the actual client, but it's a little more tricky and we'd
Expand Down
7 changes: 4 additions & 3 deletions src/__tests__/sessionid.js
Original file line number Diff line number Diff line change
@@ -1,14 +1,14 @@
import { SessionIdManager } from '../sessionid'
import { SESSION_ID } from '../posthog-persistence'
import { sessionStore } from '../storage'
import { uuidv7 } from '../uuidv7'

jest.mock('../utils')
jest.mock('../uuidv7')
jest.mock('../storage')

describe('Session ID manager', () => {
given('uuidFn', () => () => 'newUUID')
given('subject', () => given.sessionIdManager.checkAndGetSessionAndWindowId(given.readOnly, given.timestamp))
given('sessionIdManager', () => new SessionIdManager(given.config, given.persistence, given.uuidFn))
given('sessionIdManager', () => new SessionIdManager(given.config, given.persistence))

given('persistence', () => ({
props: { [SESSION_ID]: given.storedSessionIdData },
Expand All @@ -29,6 +29,7 @@ describe('Session ID manager', () => {
sessionStore.is_supported.mockReturnValue(true)
const mockDate = new Date(given.now)
jest.spyOn(global, 'Date').mockImplementation(() => mockDate)
uuidv7.mockReturnValue('newUUID')
})

describe('new session id manager', () => {
Expand Down
29 changes: 3 additions & 26 deletions src/__tests__/test-uuid.js
Original file line number Diff line number Diff line change
@@ -1,31 +1,8 @@
import { _UUID } from '../utils'
import { uuidv7 } from '../uuidv7'

describe('uuid', () => {
let originalUUIDFn = _UUID('og')
let v7UUIDFn = _UUID('v7')
let defaultUUIDFn = _UUID()

it('should be a uuid when requested', () => {
expect(v7UUIDFn()).toHaveLength(36)
})

it('by default should be the v7 format', () => {
expect(defaultUUIDFn()).toHaveLength(36)
})

it('can be set to OG format format', () => {
expect(originalUUIDFn().length).toBeGreaterThanOrEqual(52)
})

it('using window.performance for UUID still generates differing time parts of default UUID', () => {
const uuids = Array.from({ length: 1000 }, () => originalUUIDFn())

for (const uuid of uuids) {
// both the first and last value are based on time, but we want them to be different
const parts = uuid.split('-')
const first = parts[0]
const last = parts[parts.length - 1]
expect(first).not.toBe(last)
}
expect(uuidv7()).toHaveLength(36)
expect(uuidv7()).not.toEqual(uuidv7())
})
})
8 changes: 4 additions & 4 deletions src/page-view-id.ts
Original file line number Diff line number Diff line change
@@ -1,22 +1,22 @@
import { uuidv7 } from './uuidv7'

export class PageViewIdManager {
_pageViewId: string | undefined

_seenFirstPageView = false

constructor(private uuidFn: () => string) {}

onPageview(): void {
// As the first $pageview event may come after a different event,
// we only reset the ID _after_ the second $pageview event.
if (this._seenFirstPageView) {
this._pageViewId = this.uuidFn()
this._pageViewId = uuidv7()
}
this._seenFirstPageView = true
}

getPageViewId(): string {
if (!this._pageViewId) {
this._pageViewId = this.uuidFn()
this._pageViewId = uuidv7()
}

return this._pageViewId
Expand Down
Loading
Loading