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 setting empty userId, refs #7402 #7518

Closed
wants to merge 2 commits into from
Closed

Allow setting empty userId, refs #7402 #7518

wants to merge 2 commits into from

Conversation

Sija
Copy link
Contributor

@Sija Sija commented Mar 24, 2015

With this change we can clear userId by calling setUserId with falsy value, like null, undefined or empty string ''.
#7402

mattab added a commit that referenced this pull request Mar 26, 2015
@mattab
Copy link
Member

mattab commented Mar 26, 2015

Hi @Sija

I'm not 100% sure yet about this change, because some users call the setUserId('') on each pageview. This may break tracking accuracy for these guys (not sure).

Maybe we could make the change only when setUserId(false) is called (with false), which would effectively reset the user id.

(I've added some unit tests as we'd need to add some tests with the PR in case you want to contribute some!)

@mattab
Copy link
Member

mattab commented Apr 8, 2015

it was suggested in #7556 that we introduce a new method eg. logoutUser (or better name) to explicitly log out users.

this is also related to request: Save User ID in cookie #7286

@mattab mattab added the Pull Request WIP Indicates the current pull request is still work in progress and not ready yet for a review. label Apr 8, 2015
@mattab mattab added this to the Short term milestone Apr 8, 2015
@mattab
Copy link
Member

mattab commented Apr 22, 2015

Hi @Sija
let's continue the discussion on how this could be done in this other issue: #7556

@mattab mattab closed this Apr 22, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Pull Request WIP Indicates the current pull request is still work in progress and not ready yet for a review.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants