Skip to content

Conversation

deammer
Copy link

@deammer deammer commented Apr 1, 2025

Summary

This fixes an issue with identifying users in certain contexts, namely across subdomains. See this comment for more information.

Resolves #1010.

Testing Plan

  • Was this tested locally? ➡️ It was tested successfully on real live traffic.

The testing scenario:

  1. customer lands on www.website.com
    a. mParticle initializes on the subdomain
    b. mParticle starts a session and stores it in local storage
  2. customer is sent to login.website.com to log in
    a. mParticle initializes on the subdomain
    b. mParticle starts a session and stores it in local storage
  3. customer logs in
    a. mParticle.login() is called
    b. a cookie containing the customerid is stored
    c. user is thus identified on login.website.com
  4. customer is sent back to www.website.com
    a. mParticle initializes on the subdomain
    b. mParticle does not start a new session because there's already one in local storage

Before this fix: users would not be identified.

With this fix: users are consistently identified.

@deammer
Copy link
Author

deammer commented Apr 1, 2025

@alexs-mparticle tagging you here as requested!

Copy link
Collaborator

@alexs-mparticle alexs-mparticle left a comment

Choose a reason for hiding this comment

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

This PR is a great start. Thanks for opening it. My main request is for you to write some tests around the new feature to make sure any dependent functionality isn't affected.

Copy link

@deammer
Copy link
Author

deammer commented Apr 16, 2025

Hey @alexs-mparticle thank you for the review 🙏 I've made the changes you requested except for writing tests, which looks pretty involved based on a brief look I took. Is there any chance you or your team could take care of that?

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.

2 participants