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

Backport userId to overwrite visitorId feature #16226

Merged
merged 2 commits into from Jul 21, 2020
Merged

Backport userId to overwrite visitorId feature #16226

merged 2 commits into from Jul 21, 2020

Conversation

tsteur
Copy link
Member

@tsteur tsteur commented Jul 21, 2020

refs #15593

based on #16124

not adding any extra tests as default behaviour doesn't change and we have added tests in Matomo 4 already for both config values (enabled / disabled)

@tsteur tsteur added Pull Request WIP Indicates the current pull request is still work in progress and not ready yet for a review. not-in-changelog For issues or pull requests that should not be included in our release changelog on matomo.org. labels Jul 21, 2020
@tsteur tsteur added this to the 3.14.1 milestone Jul 21, 2020
@@ -562,7 +562,18 @@ private function setIdVisitorForExistingVisit($valuesToUpdate)
// Might update the idvisitor when it was forced or overwritten for this visit
$valuesToUpdate['idvisitor'] = $this->request->getVisitorId();
}


if (TrackerConfig::getConfigValue('enable_userid_overwrites_visitorid')) {
Copy link
Member Author

Choose a reason for hiding this comment

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

technically, this if might be no longer needed since we added above if in Matomo 4. However, the if above does not yet update $this->visitProperties->setProperty('idvisitor', $binIdVisitor); and I'm not sure why this is done and if it's needed so I added the entire if block

@tsteur tsteur added Needs Review PRs that need a code review and removed Pull Request WIP Indicates the current pull request is still work in progress and not ready yet for a review. labels Jul 21, 2020
@diosmosis diosmosis merged commit fa285c1 into 3.x-dev Jul 21, 2020
@diosmosis diosmosis deleted the userid3m branch July 21, 2020 18:59
@mattab mattab mentioned this pull request Sep 24, 2020
52 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs Review PRs that need a code review not-in-changelog For issues or pull requests that should not be included in our release changelog on matomo.org.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants