Skip to content
Open
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
5 changes: 5 additions & 0 deletions .changeset/warm-bears-glow.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'posthog-js': patch
---

fix: send $groupidentify for new groups even when no properties are provided
3 changes: 2 additions & 1 deletion packages/browser/playwright/mocked/group-analytics.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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]))
})
Expand Down
34 changes: 30 additions & 4 deletions packages/browser/src/__tests__/posthog-core-also.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand All @@ -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', {
Expand All @@ -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(
Expand All @@ -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!)) {
Expand Down
22 changes: 16 additions & 6 deletions packages/browser/src/posthog-core.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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()
}
}
Expand Down
Loading