Skip to content
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

allow optional reuse of anonymous id #229

Merged
merged 8 commits into from
Mar 3, 2025

Conversation

beradeep
Copy link
Contributor

💡 Motivation and Context

Fix #224

💚 How did you test it?

Added unit test.

📝 Checklist

  • I reviewed the submitted code.
  • I added tests to verify the changes.
  • I updated the docs if needed.
  • No breaking change or entry added to the changelog.

@marandaneto
Copy link
Member

@beradeep thanks
i will review later this week

@marandaneto
Copy link
Member

marandaneto commented Feb 26, 2025

I dont think this will work.

val anonymousId = this.anonymousId
if (anonymousId.isNotBlank()) {
props["\$anon_distinct_id"] = anonymousId
} else {
config?.logger?.log("identify called with invalid anonymousId: $anonymousId.")
}

once you identify an user, the given distinctId and the anonymousId are merged within a single user.

if you reset and keep the anonymousId, once you identify another user, now the older distinctId, the anonymousId and the new distinctId will also be merged within the same user.

Otherwise there will never be a "guest mode".
I think the approach we should take is that there's a specific anonymousId guest mode, similar to a$device_id actually, its always the same ID when the user is logged out.

for example:

val guestId = 123
// this is also the current anonymousId
posthog.identify(456)
// this should not merge user 123 and 456 (but it is), so we should not set `props["\$anon_distinct_id"] = anonymousId`
nor pass `featureFlags?.loadFeatureFlags(distinctId, anonymousId = anonymousId, groups, onFeatureFlags)` if `reuseAnonymousId` is enabled.
posthog.reset()
// now yes we don't delete the cached anonymousId, this is correct.
posthog.identify(789)

with this approach, the anonymousId will never be merged with any user, and it will always be the "guest user".

does that make sense?

@beradeep
Copy link
Contributor Author

Yes, it does make sense. I missed out on that part; we shouldn't merge all users on a device. Thanks for such a nice explanation.

@marandaneto
Copy link
Member

@beradeep i think we should not allow this as well

this.anonymousId = previousDistinctId

when the new flag is enabled, otherwise we will lose the anon id (guest mode).

@marandaneto
Copy link
Member

@beradeep looks good, i added a more detailed explanation here
can you add unit test for the last changes? right now we are only testing the reset changes.

Copy link
Member

@marandaneto marandaneto left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

missing 2 tests otherwise LGTM

@marandaneto marandaneto merged commit 103a969 into PostHog:main Mar 3, 2025
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Give an option to reuse the anonId between user changes instead of creating a new anonId every time
2 participants