Skip to content

Commit

Permalink
fix: Add warning and conversion for number distinct_id (#993)
Browse files Browse the repository at this point in the history
* Add warning and conversion for number distinct_id

* Move test to unit test
  • Loading branch information
robbie-c authored Feb 6, 2024
1 parent 6fb4a04 commit 339b7df
Show file tree
Hide file tree
Showing 3 changed files with 56 additions and 2 deletions.
4 changes: 3 additions & 1 deletion functional_tests/identify.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,8 @@ import { waitFor } from '@testing-library/dom'
import { v4 } from 'uuid'
import { getRequests } from './mock-server'
import { createPosthogInstance } from './posthog-instance'

import { logger } from '../src/utils/logger'
jest.mock('../src/utils/logger')
test('identify sends a identify event', async () => {
const token = v4()
const posthog = await createPosthogInstance(token)
Expand All @@ -24,6 +25,7 @@ test('identify sends a identify event', async () => {
})
)
)
expect(jest.mocked(logger).error).toBeCalledTimes(0)
})

test('identify sends an engage request if identify called twice with the same distinct id and with $set/$set_once', async () => {
Expand Down
39 changes: 39 additions & 0 deletions src/__tests__/identify.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
import { v4 } from 'uuid'
import { createPosthogInstance } from '../../functional_tests/posthog-instance'
import { logger } from '../utils/logger'
jest.mock('../utils/logger')

describe('identify', () => {
// Note that there are other tests for identify in posthog-core.identify.js
// These are in the old style of tests, if you are feeling helpful you could
// convert them to the new style in this file.

it('should persist the distinct_id', async () => {
// arrange
const token = v4()
const posthog = await createPosthogInstance(token)
const distinctId = '123'

// act
posthog.identify(distinctId)

// assert
expect(posthog.persistence!.properties()['$user_id']).toEqual(distinctId)
expect(jest.mocked(logger).error).toBeCalledTimes(0)
})

it('should convert a numeric distinct_id to a string', async () => {
// arrange
const token = v4()
const posthog = await createPosthogInstance(token)
const distinctIdNum = 123
const distinctIdString = '123'

// act
posthog.identify(distinctIdNum as any)

// assert
expect(posthog.persistence!.properties()['$user_id']).toEqual(distinctIdString)
expect(jest.mocked(logger).error).toBeCalledTimes(1)
})
})
15 changes: 14 additions & 1 deletion src/posthog-core.ts
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,15 @@ import { PostHogSurveys } from './posthog-surveys'
import { RateLimiter } from './rate-limiter'
import { uuidv7 } from './uuidv7'
import { SurveyCallback } from './posthog-surveys-types'
import { _isArray, _isEmptyObject, _isFunction, _isObject, _isString, _isUndefined } from './utils/type-utils'
import {
_isArray,
_isEmptyObject,
_isFunction,
_isNumber,
_isObject,
_isString,
_isUndefined,
} from './utils/type-utils'
import { _info } from './utils/event-utils'
import { logger } from './utils/logger'
import { document, userAgent } from './utils/globals'
Expand Down Expand Up @@ -1293,6 +1301,11 @@ export class PostHog {
if (!this.__loaded || !this.persistence) {
return logger.uninitializedWarning('posthog.identify')
}
if (_isNumber(new_distinct_id)) {
logger.error('The first argument to posthog.identify was a number, but it should be a string.')
new_distinct_id = (new_distinct_id as number).toString()
}

//if the new_distinct_id has not been set ignore the identify event
if (!new_distinct_id) {
logger.error('Unique user id has not been set in posthog.identify')
Expand Down

0 comments on commit 339b7df

Please sign in to comment.