diff --git a/.changeset/gentle-foxes-rest.md b/.changeset/gentle-foxes-rest.md new file mode 100644 index 0000000000..c5b10456bc --- /dev/null +++ b/.changeset/gentle-foxes-rest.md @@ -0,0 +1,5 @@ +--- +'@posthog/core': patch +--- + +fix: send $groupidentify for new groups even when no properties are provided diff --git a/packages/core/src/__tests__/posthog.featureflags.spec.ts b/packages/core/src/__tests__/posthog.featureflags.spec.ts index 4f4917f2a3..dbbf13a150 100644 --- a/packages/core/src/__tests__/posthog.featureflags.spec.ts +++ b/packages/core/src/__tests__/posthog.featureflags.spec.ts @@ -728,8 +728,19 @@ describe('PostHog Feature Flags v4', () => { it('should reload if groups are set', async () => { posthog.group('my-group', 'is-great') await waitForPromises() - expect(mocks.fetch).toHaveBeenCalledTimes(2) - expect(JSON.parse((mocks.fetch.mock.calls[1][1].body as string) || '')).toMatchObject({ + // 3 calls: 1 for flags reload, 1 for $groupidentify batch, 1 for flags decide + expect(mocks.fetch).toHaveBeenCalledTimes(3) + // The flags reload call contains the group + const flagsCall = mocks.fetch.mock.calls.find((call) => { + try { + const body = JSON.parse((call[1].body as string) || '') + return body.groups?.['my-group'] === 'is-great' + } catch { + return false + } + }) + expect(flagsCall).toBeDefined() + expect(JSON.parse((flagsCall![1].body as string) || '')).toMatchObject({ groups: { 'my-group': 'is-great' }, }) }) diff --git a/packages/core/src/__tests__/posthog.featureflags.v1.spec.ts b/packages/core/src/__tests__/posthog.featureflags.v1.spec.ts index 6e528bf612..7cbfe8ecb8 100644 --- a/packages/core/src/__tests__/posthog.featureflags.v1.spec.ts +++ b/packages/core/src/__tests__/posthog.featureflags.v1.spec.ts @@ -411,8 +411,19 @@ describe('PostHog Feature Flags v1', () => { it('should reload if groups are set', async () => { posthog.group('my-group', 'is-great') await waitForPromises() - expect(mocks.fetch).toHaveBeenCalledTimes(2) - expect(JSON.parse((mocks.fetch.mock.calls[1][1].body as string) || '')).toMatchObject({ + // 3 calls: 1 for flags reload, 1 for $groupidentify batch, 1 for flags decide + expect(mocks.fetch).toHaveBeenCalledTimes(3) + // The flags reload call contains the group + const flagsCall = mocks.fetch.mock.calls.find((call) => { + try { + const body = JSON.parse((call[1].body as string) || '') + return body.groups?.['my-group'] === 'is-great' + } catch { + return false + } + }) + expect(flagsCall).toBeDefined() + expect(JSON.parse((flagsCall![1].body as string) || '')).toMatchObject({ groups: { 'my-group': 'is-great' }, }) }) diff --git a/packages/core/src/__tests__/posthog.groups.spec.ts b/packages/core/src/__tests__/posthog.groups.spec.ts index a7fc5c5bf7..d1653acb51 100644 --- a/packages/core/src/__tests__/posthog.groups.spec.ts +++ b/packages/core/src/__tests__/posthog.groups.spec.ts @@ -64,6 +64,79 @@ describe('PostHog Core', () => { ], }) }) + + it('should call groupIdentify for a new group even without properties', async () => { + posthog.group('other', 'team') + await waitForPromises() + + expect(mocks.fetch).toHaveBeenCalledTimes(2) // 1 for flags, 1 for groupIdentify + const batchCall = mocks.fetch.mock.calls[1] + expect(batchCall[0]).toEqual('https://us.i.posthog.com/batch/') + expect(parseBody(batchCall)).toMatchObject({ + batch: [ + { + event: '$groupidentify', + distinct_id: posthog.getDistinctId(), + properties: { + $group_type: 'other', + $group_key: 'team', + }, + type: 'capture', + }, + ], + }) + }) + + it('should not call groupIdentify when group already exists with same key and no properties', async () => { + posthog.group('other', 'team') + await waitForPromises() + mocks.fetch.mockClear() + + posthog.group('other', 'team') + await waitForPromises() + + // No new fetch calls for groupIdentify (only flags reload if needed) + const groupIdentifyCalls = mocks.fetch.mock.calls.filter((call) => { + try { + const body = parseBody(call) + return body.batch?.some((e: any) => e.event === '$groupidentify') + } catch { + return false + } + }) + expect(groupIdentifyCalls).toHaveLength(0) + }) + + it('should call groupIdentify for an existing group when properties are provided', async () => { + posthog.group('other', 'team') + await waitForPromises() + mocks.fetch.mockClear() + + posthog.group('other', 'team', { name: 'My Team' }) + await waitForPromises() + + const groupIdentifyCalls = mocks.fetch.mock.calls.filter((call) => { + try { + const body = parseBody(call) + return body.batch?.some((e: any) => e.event === '$groupidentify') + } catch { + return false + } + }) + expect(groupIdentifyCalls).toHaveLength(1) + expect(parseBody(groupIdentifyCalls[0])).toMatchObject({ + batch: [ + { + event: '$groupidentify', + properties: { + $group_type: 'other', + $group_key: 'team', + $group_set: { name: 'My Team' }, + }, + }, + ], + }) + }) }) describe('groupIdentify', () => { diff --git a/packages/core/src/posthog-core.ts b/packages/core/src/posthog-core.ts index 3b231f05c4..dfce84f850 100644 --- a/packages/core/src/posthog-core.ts +++ b/packages/core/src/posthog-core.ts @@ -443,11 +443,17 @@ export abstract class PostHogCore extends PostHogCoreStateless { options?: PostHogCaptureOptions ): void { this.wrap(() => { + const existingGroups = (this.props.$groups as PostHogGroupProperties) || {} + const isNewGroup = existingGroups[groupType] !== groupKey + this.groups({ [groupType]: groupKey, }) - if (groupProperties) { + // Send $groupidentify when the group is new/changed OR when properties + // are provided. Skip only when the group already exists with the same + // key and no new properties are being set. + if (isNewGroup || groupProperties) { this.groupIdentify(groupType, groupKey, groupProperties, options) } })