diff --git a/.changeset/warm-bears-glow.md b/.changeset/warm-bears-glow.md new file mode 100644 index 0000000000..81b16a0a05 --- /dev/null +++ b/.changeset/warm-bears-glow.md @@ -0,0 +1,5 @@ +--- +'posthog-js': patch +--- + +fix: send $groupidentify for new groups even when no properties are provided diff --git a/packages/browser/playwright/mocked/group-analytics.spec.ts b/packages/browser/playwright/mocked/group-analytics.spec.ts index d973d2638a..5c73fe2d00 100644 --- a/packages/browser/playwright/mocked/group-analytics.spec.ts +++ b/packages/browser/playwright/mocked/group-analytics.spec.ts @@ -24,7 +24,8 @@ test.describe('group analytics', () => { await page.locator('[data-cy-custom-event-button]').click() const capturedEvents = await page.capturedEvents() - expect(capturedEvents).toHaveLength(3) + // 4 events: $groupidentify (from group() call), $pageview, $pageleave, custom event + expect(capturedEvents).toHaveLength(4) const hasGroups = new Set(capturedEvents.map((x) => !!x.properties.$groups)) expect(hasGroups).toEqual(new Set([true])) }) diff --git a/packages/browser/src/__tests__/posthog-core-also.test.ts b/packages/browser/src/__tests__/posthog-core-also.test.ts index 9f55df989b..3c88cb19de 100644 --- a/packages/browser/src/__tests__/posthog-core-also.test.ts +++ b/packages/browser/src/__tests__/posthog-core-also.test.ts @@ -1258,7 +1258,19 @@ describe('posthog core', () => { expect(posthog.persistence!.props['$stored_group_properties']).toEqual(undefined) }) - it('does not result in a capture call', () => { + it('sends $groupidentify for a new group even without properties', () => { + posthog.group('organization', 'org::5') + + expect(posthog.capture).toHaveBeenCalledWith('$groupidentify', { + $group_type: 'organization', + $group_key: 'org::5', + }) + }) + + it('does not send $groupidentify when group already exists with same key and no properties', () => { + posthog.group('organization', 'org::5') + jest.mocked(posthog.capture).mockClear() + posthog.group('organization', 'org::5') expect(posthog.capture).not.toHaveBeenCalled() @@ -1281,7 +1293,7 @@ describe('posthog core', () => { expect(posthog.reloadFeatureFlags).toHaveBeenCalledTimes(3) }) - it('captures $groupidentify event', () => { + it('captures $groupidentify event with $group_set when properties provided', () => { posthog.group('organization', 'org::5', { group: 'property', foo: 5 }) expect(posthog.capture).toHaveBeenCalledWith('$groupidentify', { @@ -1294,6 +1306,19 @@ describe('posthog core', () => { }) }) + it('sends $groupidentify with $group_set for an existing group when properties provided', () => { + posthog.group('organization', 'org::5') + jest.mocked(posthog.capture).mockClear() + + posthog.group('organization', 'org::5', { name: 'PostHog' }) + + expect(posthog.capture).toHaveBeenCalledWith('$groupidentify', { + $group_type: 'organization', + $group_key: 'org::5', + $group_set: { name: 'PostHog' }, + }) + }) + describe('subsequent capture calls', () => { beforeEach(() => { posthog = defaultPostHog().init( @@ -1316,9 +1341,10 @@ describe('posthog core', () => { posthog.capture('some_event', { prop: 5 }) - expect(posthog._requestQueue!.enqueue).toHaveBeenCalledTimes(1) + // 2 $groupidentify calls from group() + 1 some_event + expect(posthog._requestQueue!.enqueue).toHaveBeenCalledTimes(3) - const eventPayload = jest.mocked(posthog._requestQueue!.enqueue).mock.calls[0][0] + const eventPayload = jest.mocked(posthog._requestQueue!.enqueue).mock.calls[2][0] // need to help TS know event payload data is not an array // eslint-disable-next-line posthog-js/no-direct-array-check if (Array.isArray(eventPayload.data!)) { diff --git a/packages/browser/src/posthog-core.ts b/packages/browser/src/posthog-core.ts index 9296b2797a..8224d756fa 100644 --- a/packages/browser/src/posthog-core.ts +++ b/packages/browser/src/posthog-core.ts @@ -2540,26 +2540,36 @@ export class PostHog implements PostHogInterface { } const existingGroups = this.getGroups() + const isNewGroup = existingGroups[groupType] !== groupKey // if group key changes, remove stored group properties - if (existingGroups[groupType] !== groupKey) { + if (isNewGroup) { this.resetGroupPropertiesForFlags(groupType) } this.register({ $groups: { ...existingGroups, [groupType]: groupKey } }) - if (groupPropertiesToSet) { - this.capture(EVENT_GROUPIDENTIFY, { + // 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 || groupPropertiesToSet) { + const groupIdentifyProperties: Properties = { $group_type: groupType, $group_key: groupKey, - $group_set: groupPropertiesToSet, - }) + } + if (groupPropertiesToSet) { + groupIdentifyProperties.$group_set = groupPropertiesToSet + } + this.capture(EVENT_GROUPIDENTIFY, groupIdentifyProperties) + } + + if (groupPropertiesToSet) { this.setGroupPropertiesForFlags({ [groupType]: groupPropertiesToSet }) } // If groups change and no properties change, reload feature flags. // The property change reload case is handled in setGroupPropertiesForFlags. - if (existingGroups[groupType] !== groupKey && !groupPropertiesToSet) { + if (isNewGroup && !groupPropertiesToSet) { this.reloadFeatureFlags() } }