@znerol opened this Pull Request on October 3rd 2017 Contributor

Replicates the user-signout mechanism from the PHP tracker in order to fix #7556

@znerol commented on October 3rd 2017 Contributor

Tests pass except for the screenshot thing. Is that a signal or noise?

@1u commented on October 9th 2017

nice!

@mattab commented on October 16th 2017 Owner

Tests pass except for the screenshot thing. Is that a signal or noise?

it's noise

@mattab commented on January 24th 2018 Owner

Thanks for creating this Pull request @znerol :+1:

Feedback:

  • This PR is solving a very important use case for which we currently don't have a good solution: logging out users in SPA (currently the User Id stays in subsequent pageviews in SPA, even after logout..)
  • rename setNewVisitorId: since User ID logout use case in Single Page Apps is the main use case for this method, maybe we call it resetUserId (or unsetUserId, or even logoutUserId) if anyone has some thoughts?
  • in the setUserId method, let's not call the new method when user id is false. It's best to force users to call resetUserId separately when needed.

@znerol feel free to make the changes, otherwise we will likely work on this next month.

@znerol commented on January 25th 2018 Contributor

Update PR, I've opted for resetUserId. It not only clears the user id but also generates a new visitor id. unsetUserId does not really capture that bit. Also logoutUserId could be misleading, we are not talking about privilege levels here.

@znerol commented on January 25th 2018 Contributor

Should I squash the commits? (Except for the minify piwik.js one.)

@mattab commented on January 26th 2018 Owner

Thanks for updating the PR! fyi: It's not needed to squash commits as when we merge it will squash them.

Code review

  • left a comment.
  • feel free to add the new feature to CHANGELOG.md but we'll do it otherwise
  • Looks good to me otherwise! Maybe @tsteur you can take a quick look?

after merge will need to update the documentation in: https://developer.matomo.org/api-reference/tracking-javascript

This Pull Request was closed on February 1st 2018
Powered by GitHub Issue Mirror