fix: include initial person props in $identify when group() called first (#2725)

ordehi · web-flow · commit 90a0f143e639 · 2025-12-12T09:41:58.000Z
Skip $set_once calculation for $groupidentify events since server strips
person properties from them anyway. This prevents the flag from being set
prematurely, ensuring UTM params are included with subsequent $identify.
diff --git a/.changeset/wicked-papayas-shake.md b/.changeset/wicked-papayas-shake.md
@@ -0,0 +1,5 @@
+---
+'posthog-js': patch
+---
+
+fix: include initial person props in $identify when group() called first
diff --git a/packages/browser/src/__tests__/group-before-identify-bug.test.ts b/packages/browser/src/__tests__/group-before-identify-bug.test.ts
@@ -0,0 +1,102 @@
+/**
+ * Test to verify bug: calling group() before identify() causes initial person props to be lost
+ */
+import { createPosthogInstance } from './helpers/posthog-instance'
+import { uuidv7 } from '../uuidv7'
+
+jest.mock('../utils/globals', () => {
+    const orig = jest.requireActual('../utils/globals')
+    const mockURLGetter = jest.fn()
+    const mockReferrerGetter = jest.fn()
+    return {
+        ...orig,
+        mockURLGetter,
+        mockReferrerGetter,
+        document: {
+            ...orig.document,
+            createElement: (...args: any[]) => orig.document.createElement(...args),
+            body: {},
+            get referrer() {
+                return mockReferrerGetter()
+            },
+            get URL() {
+                return mockURLGetter()
+            },
+        },
+        get location() {
+            const url = mockURLGetter()
+            return {
+                href: url,
+                toString: () => url,
+            }
+        },
+    }
+})
+
+// eslint-disable-next-line @typescript-eslint/no-require-imports
+const { mockURLGetter, mockReferrerGetter } = require('../utils/globals')
+
+describe('group before identify bug', () => {
+    beforeEach(() => {
+        mockReferrerGetter.mockReturnValue('https://referrer.com')
+        mockURLGetter.mockReturnValue('https://example.com?utm_source=linkedin&utm_campaign=test')
+    })
+
+    it('should include initial UTM params in $identify even when group() is called first', async () => {
+        const token = uuidv7()
+        const beforeSendMock = jest.fn().mockImplementation((e) => e)
+
+        const posthog = await createPosthogInstance(token, {
+            before_send: beforeSendMock,
+            person_profiles: 'identified_only',
+        })
+
+        // Simulate what Clerk does: call group() with properties before identify()
+        posthog.group('organization', 'org_123', { name: 'Acme Corp' })
+
+        // Then call identify
+        posthog.identify('user_123')
+
+        // Find the events
+        const calls = beforeSendMock.mock.calls
+        const groupIdentifyCall = calls.find((c: any) => c[0].event === '$groupidentify')
+        const identifyCall = calls.find((c: any) => c[0].event === '$identify')
+
+        console.log(
+            '$groupidentify $set_once.$initial_utm_source:',
+            groupIdentifyCall?.[0]?.$set_once?.$initial_utm_source
+        )
+        console.log('$identify $set_once.$initial_utm_source:', identifyCall?.[0]?.$set_once?.$initial_utm_source)
+
+        // THE BUG: $identify should have $set_once with initial UTM params
+        // but because group() was called first, _personProcessingSetOncePropertiesSent is already true
+        expect(identifyCall).toBeDefined()
+        expect(identifyCall[0].$set_once).toBeDefined()
+        expect(identifyCall[0].$set_once.$initial_utm_source).toEqual('linkedin')
+        expect(identifyCall[0].$set_once.$initial_utm_campaign).toEqual('test')
+    })
+
+    it('should include initial UTM params when identify() is called without group() first', async () => {
+        const token = uuidv7()
+        const beforeSendMock = jest.fn().mockImplementation((e) => e)
+
+        const posthog = await createPosthogInstance(token, {
+            before_send: beforeSendMock,
+            person_profiles: 'identified_only',
+        })
+
+        // Just call identify without group first
+        posthog.identify('user_123')
+
+        const calls = beforeSendMock.mock.calls
+        const identifyCall = calls.find((c: any) => c[0].event === '$identify')
+
+        console.log('$identify $set_once (no group):', JSON.stringify(identifyCall?.[0]?.$set_once, null, 2))
+
+        // This should work
+        expect(identifyCall).toBeDefined()
+        expect(identifyCall[0].$set_once).toBeDefined()
+        expect(identifyCall[0].$set_once.$initial_utm_source).toEqual('linkedin')
+        expect(identifyCall[0].$set_once.$initial_utm_campaign).toEqual('test')
+    })
+})
diff --git a/packages/browser/src/__tests__/personProcessing.test.ts b/packages/browser/src/__tests__/personProcessing.test.ts
@@ -563,7 +563,8 @@ describe('person processing', () => {
             expect(eventAfterGroup[0].properties.$process_person_profile).toEqual(true)
         })
 
-        it('should not send the $groupidentify event if person_processing is set to never', async () => {
+        it('should send the $groupidentify event even if person_processing is set to never', async () => {
+            // Groups are separate from person processing - $groupidentify should always be sent
             // arrange
             const { posthog, beforeSendMock } = await setup('never')
 
@@ -573,15 +574,20 @@ describe('person processing', () => {
             posthog.capture('custom event after group')
 
             // assert
+            // setGroupPropertiesForFlags still has a person processing check
             expect(mockLogger.error).toBeCalledTimes(1)
             expect(mockLogger.error).toHaveBeenCalledWith(
-                'posthog.group was called, but process_person is set to "never". This call will be ignored.'
+                'posthog.setGroupPropertiesForFlags was called, but process_person is set to "never". This call will be ignored.'
             )
 
-            expect(beforeSendMock).toBeCalledTimes(2)
+            // $groupidentify is sent (groups are independent of person processing)
+            expect(beforeSendMock).toBeCalledTimes(3)
             const eventBeforeGroup = beforeSendMock.mock.calls[0]
             expect(eventBeforeGroup[0].properties.$process_person_profile).toEqual(false)
-            const eventAfterGroup = beforeSendMock.mock.calls[1]
+            const groupIdentify = beforeSendMock.mock.calls[1]
+            expect(groupIdentify[0].event).toEqual('$groupidentify')
+            // $groupidentify doesn't set $process_person_profile since it doesn't process persons
+            const eventAfterGroup = beforeSendMock.mock.calls[2]
             expect(eventAfterGroup[0].properties.$process_person_profile).toEqual(false)
         })
     })
diff --git a/packages/browser/src/extensions/product-tours/product-tour.css b/packages/browser/src/extensions/product-tours/product-tour.css
@@ -187,7 +187,9 @@
     border-radius: 6px;
     border: none;
     cursor: pointer;
-    transition: opacity 0.15s ease, transform 0.1s ease;
+    transition:
+        opacity 0.15s ease,
+        transform 0.1s ease;
 }
 
 .ph-tour-button:hover {
@@ -225,7 +227,9 @@
     color: var(--ph-tour-text-secondary-color);
     font-size: 18px;
     line-height: 1;
-    transition: background 0.15s ease, color 0.15s ease;
+    transition:
+        background 0.15s ease,
+        color 0.15s ease;
 }
 
 .ph-tour-dismiss:hover {
diff --git a/packages/browser/src/posthog-core.ts b/packages/browser/src/posthog-core.ts
@@ -593,13 +593,10 @@ export class PostHog {
         if (this._hasBootstrappedFeatureFlags()) {
             const activeFlags = Object.keys(config.bootstrap?.featureFlags || {})
                 .filter((flag) => !!config.bootstrap?.featureFlags?.[flag])
-                .reduce(
-                    (res: Record<string, string | boolean>, key) => (
-                        (res[key] = config.bootstrap?.featureFlags?.[key] || false),
-                        res
-                    ),
-                    {}
-                )
+                .reduce((res: Record<string, string | boolean>, key) => {
+                    res[key] = config.bootstrap?.featureFlags?.[key] || false
+                    return res
+                }, {})
             const featureFlagPayloads = Object.keys(config.bootstrap?.featureFlagPayloads || {})
                 .filter((key) => activeFlags[key])
                 .reduce((res: Record<string, JsonType>, key) => {
@@ -1144,7 +1141,11 @@ export class PostHog {
         if (setProperties) {
             data.$set = options?.$set
         }
-        const setOnceProperties = this._calculate_set_once_properties(options?.$set_once)
+        // $groupidentify doesn't process person $set_once on the server, so don't mark
+        // initial person props as sent. This ensures they're included with subsequent
+        // $identify calls.
+        const markSetOnceAsSent = event_name !== '$groupidentify'
+        const setOnceProperties = this._calculate_set_once_properties(options?.$set_once, markSetOnceAsSent)
         if (setOnceProperties) {
             data.$set_once = setOnceProperties
         }
@@ -1369,8 +1370,10 @@ export class PostHog {
      * profile with mostly-accurate properties, despite earlier events not setting them. We do this by storing them in
      * persistence.
      * @param dataSetOnce
+     * @param markAsSent - if true, marks the properties as sent so they won't be included in future events.
+     *                     Set to false for events like $groupidentify where the server doesn't process person props.
      */
-    _calculate_set_once_properties(dataSetOnce?: Properties): Properties | undefined {
+    _calculate_set_once_properties(dataSetOnce?: Properties, markAsSent: boolean = true): Properties | undefined {
         if (!this.persistence || !this._hasPersonProcessing()) {
             return dataSetOnce
         }
@@ -1389,7 +1392,9 @@ export class PostHog {
             logger.error('sanitize_properties is deprecated. Use before_send instead')
             setOnceProperties = sanitize_properties(setOnceProperties, '$set_once')
         }
-        this._personProcessingSetOncePropertiesSent = true
+        if (markAsSent) {
+            this._personProcessingSetOncePropertiesSent = true
+        }
         if (isEmptyObject(setOnceProperties)) {
             return undefined
         }
@@ -2270,10 +2275,6 @@ export class PostHog {
             return
         }
 
-        if (!this._requirePersonProcessing('posthog.group')) {
-            return
-        }
-
         const existingGroups = this.getGroups()
 
         // if group key changes, remove stored group properties

-Original file line number
+Diff line change
@@ @@ -0,0 +1,5 @@ @@
 +---
 +'posthog-js': patch
 +---
++
 +fix: include initial person props in $identify when group() called first