Skip to content
Merged
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/gentle-foxes-rest.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@posthog/core': patch
---

fix: send $groupidentify for new groups even when no properties are provided
15 changes: 13 additions & 2 deletions packages/core/src/__tests__/posthog.featureflags.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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' },
})
})
Expand Down
15 changes: 13 additions & 2 deletions packages/core/src/__tests__/posthog.featureflags.v1.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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' },
})
})
Expand Down
73 changes: 73 additions & 0 deletions packages/core/src/__tests__/posthog.groups.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand Down
8 changes: 7 additions & 1 deletion packages/core/src/posthog-core.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
})
Expand Down
Loading