-
Notifications
You must be signed in to change notification settings - Fork 215
fix: include initial person props in $identify when group() called first #2725
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
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.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
2 files reviewed, no comments
|
Size Change: +100 B (0%) Total Size: 5.19 MB
ℹ️ View Unchanged
|
Changed comma operator style to explicit return block in bootstrap featureFlags reduce. This resolves CI/local prettier formatting mismatch.
rafaeelaudibert
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, I unfortunately don't know enough about this, I'll defer to @robbie-c 's review
- Remove _requirePersonProcessing from group() - groups work independently - Add markAsSent param to _calculate_set_once_properties() so $groupidentify events don't mark initial props as sent (server drops them anyway) - Still send props for future-proofing - worst case is duplicate processing, not missing data
Initial person properties lost when
group()called beforeidentify()Bug
Calling
group()with properties beforeidentify()causes initial person properties (UTM params, referrer, etc.) to be lost from the person profile.Root cause:
group()triggers_calculate_set_once_properties()which marks initial person props as "sent" via_personProcessingSetOncePropertiesSent = true. Whenidentify()is called later, it skips sending these props because the flag is already set. However, the server ignores$set_onceon$groupidentifyevents, so the initial props are never actually saved.Fix
Skip
_calculate_set_once_properties()for$groupidentifyevents, since person properties don't apply to group events.Why it's safe
The server already strips
$set_oncefrom$groupidentifyevents (plugin-server/src/utils/event.ts:180-189):The SDK was sending person properties that the server discards anyway. This fix prevents the SDK from incorrectly marking them as "sent", ensuring they're included with the actual
$identifyevent where they belong.Impact
Affects customers using
person_profiles: 'identified_only'who callgroup()beforeidentify()a common B2B SaaS pattern where org context is set before user identification.Release info Sub-libraries affected
Libraries affected
Checklist
If releasing new changes
pnpm changesetto generate a changeset file