By default userId should overwrite the visitorId #16124
Conversation
tsteur
added
Pull Request WIP
config/global.ini.php
Outdated
| ; When enabled and a userId is set, then the visitorId will be automatically set based on the userId. This allows to | ||
| ; identify the same user as the same visitor across devices. It might also create a new visit as soon as a user logs in | ||
| ; or logs out. This won't be the case when this feature is disabled and it separates the userId from the visitorId. | ||
| ; Disabling this feature can be useful for example when using the third party cookie, and thus all Matomo sites use |
There was a problem hiding this comment.
not really sure how to best explain when it would be useful. it's a quite specific case I guess
There was a problem hiding this comment.
I think its well explained 👍
| @@ -189,7 +190,8 @@ public function isVisitNew(VisitProperties $visitProperties, Request $request, $ | |||
| return true; | |||
| } | |||
|
|
|||
| if (!$this->lastUserIdWasSetAndDoesMatch($visitProperties, $request)) { | |||
| if (!TrackerConfig::getConfigValue('enable_userid_overwrites_visitorid') | |||
There was a problem hiding this comment.
I suppose this could also be a separate setting? Or not sure if that makes sense or not
There was a problem hiding this comment.
For now I would keep it simple and leave it like that.
|
@tsteur Thank you for your work 👍 Looks good in my opinion. |
|
@peterbo could you maybe also have a quick look if that would work for you? |
|
@tsteur Code looks good to me. I'll test it later today in production to make sure! :) Ty! |
tsteur
added
Needs Review
config/global.ini.php
Outdated
| ; When enabled and a userId is set, then the visitorId will be automatically set based on the userId. This allows to | ||
| ; identify the same user as the same visitor across devices. | ||
| ; Disabling this feature can be useful for example when using the third party cookie, and thus all Matomo sites use | ||
| ; the same "global" visitorId for the same device, and some Matomo sites set a userid. |
There was a problem hiding this comment.
would change 'and thus all Matomo sites use the same "global" visitorId for the same device, and some Matomo sites set a userid.' to 'where all Matomo tracked sites use the same "global" visitorId for a device and you want to see when the same user switches between devices.'
| @@ -0,0 +1,42 @@ | |||
| <?xml version="1.0" encoding="utf-8" ?> | |||
There was a problem hiding this comment.
I'm noticing these test results are exactly the same as the normal testApi() tests. I think the Fixture needs to be changed, tracking the same visits w/ the new setting set. Maybe into a new site, then running API tests for the new site.
There was a problem hiding this comment.
👍 not sure what I was thinking @diosmosis . I've created a new fixture and new tests that test this behaviour now properly
refs #15593
This PR is basically reverting https://github.com/matomo-org/matomo/pull/14360/files but having an option to basically still use that behaviour.
Once merged will need to document this behaviour.
The failing UI test https://builds-artifacts.matomo.org/matomo-org/matomo/4.x-dev/41350/UIIntegrationTest_widgetize_ecommercelog.png seems not related to this PR