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
Provide possibility to configure referrer exclusion list #19302
Conversation
6f39dd0
to
dc81b49
Compare
77030f6
to
f9d7ecc
Compare
0ed885b
to
e4d94bb
Compare
@sgiehl I haven't reviewed yet, but based on your comment in the other issue, can we add |
I'm not able to say if there might be reasons for someone to not having it excluded, but yes, I can let it be added to the list once on update or install. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've noted a couple of minor issues, but overall this seems to be working well. UI works to set the excluded referrers, the tracker code is generated with the exclusions, any excluded referrers are ignored and the tests all pass. 👍
plugins/CoreAdminHome/vue/src/JsTrackingCodeGenerator/JsTrackingCodeGenerator.vue
Outdated
Show resolved
Hide resolved
plugins/CoreAdminHome/vue/src/JsTrackingCodeGenerator/JsTrackingCodeGenerator.vue
Outdated
Show resolved
Hide resolved
@bx80 applied some fixes |
@bx80 I've created a PR for the developer docs, so the new methods will be documented. Feel free to review and merge that one. |
@sgiehl didn't check the UI, but was checking the UI tests and noticed in https://github.com/matomo-org/matomo/blob/6f76e4fdbd183f729900d5a7286d5e281ac7d22a/plugins/SitesManager/tests/UI/expected-screenshots/SitesManager_global_settings.png that it doesn't show "paypal.com" in the field - i would have expected the screenshot test to show 'paypal.com' so it doesn't regress in the future? |
Haven't added that yet, but still having that in mind. The discussion in the other issue is still ongoing and I'm not sure if adding paypal by default is that much of help |
Yes it is of much help and is needed, so let's add it so it actually fixes the issue for people 👍 |
@mattab This will only have an impact on the PHP side of tracking. The tracking code would need to be updated by everyone to also reflect that on javascript side, which would be the more important part. Also I guess this might currently not be easily possible with TagManager. |
Let's add Paypal.com to our core JS tracker code so it's not an issue for Paypal - that's a big reason of why we do this work. |
@mattab There are so many other payment providers and other services like SSOs, that might then still produce the same issue. Also having a hard coded list, might in the end cause problems for users that might expect e.g. paypal from referring users to their site. |
Let's start with the workaround and see how it goes (pragmatic) so we can move on from this and fix the high impact issues. |
Description:
This PR provides the possibility to configure a list of host excluded from referrer detection.
The list can be maintained globally, as well as enriched per site.
Referrer on that exclusion list will be ignored on PHP side and such hits will be handled as DIRECT entry.
In addition it will be possible to define ignored referrers within the javascript tracker, so they are ignored for setting the referrer cookies and also not sent with tracking requests.
refs #18612
Review