Skip to content

fix: identifyRequest intake when persistent data exists #1014

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

Open
wants to merge 2 commits into
base: development
Choose a base branch
from

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.

@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