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

Overwrite direct entry referrer information if campaign referrer is found in later request. #14273

Merged
merged 13 commits into from May 10, 2019

Conversation

diosmosis
Copy link
Member

Noticed in one user's logs, the campaign information came in the second request. @mattab suggested that direct entry referrers should be overwritten by campaign information if supplied in a later request.

@diosmosis diosmosis added RFC Indicates the issue is a request for comments where the author is looking for feedback. Needs Review PRs that need a code review labels Mar 26, 2019
@diosmosis diosmosis added this to the 3.10.0 milestone Mar 26, 2019
plugins/Referrers/Columns/Keyword.php Outdated Show resolved Hide resolved
@mattab
Copy link
Member

mattab commented Mar 26, 2019

Feedback:

  • When the Matomo admin didn't specify the website URLs alias for the Website (in Matomo), then Matomo will think that a visitor viewing Example.com/Page2 coming from Example.com/Page1 is coming from a Website Referrer Example.com/Page1. So it will mistakenly overwrite direct entry with the current website URL as referrer. So wondering if in:
    $this->typeReferrerAnalyzed = Common::REFERRER_TYPE_WEBSITE;
    $this->nameReferrerAnalyzed = Common::mb_strtolower($this->referrerHost);
    $urlsByHost = $this->getCachedUrlsByHostAndIdSite();
    $directEntry = new SiteUrls();
    $path = $directEntry->getPathMatchingUrl($this->referrerUrlParse, $urlsByHost);
    if (!empty($path) && $path !== '/') {
    $this->nameReferrerAnalyzed .= rtrim($path, '/');
    }

    we would want to also look at the current Page URL and "Add to the Site URLs alias" the current page url hostname?

@diosmosis
Copy link
Member Author

diosmosis commented Mar 26, 2019

@mattab Is example.com the main URL for the site? Or an alias URL?

@mattab
Copy link
Member

mattab commented Mar 27, 2019

example.com would be the main URL (or even alias URL) but which wasn't specified in Website URLs field.

@diosmosis
Copy link
Member Author

@mattab I see there's no required main URL field anymore. So you mean to add hostname(url) to the list of website URLs, makes sense.

@diosmosis diosmosis removed the RFC Indicates the issue is a request for comments where the author is looking for feedback. label Mar 27, 2019
@mattab
Copy link
Member

mattab commented Mar 28, 2019

Looks good to me (quick look). Looking forward to the tests 👍

@diosmosis
Copy link
Member Author

Added integration tests, system tests will need to be fixed, ready for a review.

Copy link
Member

@mattab mattab left a comment

Choose a reason for hiding this comment

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

  • be great to also update any other system tests impacted by this and check they are not regression

plugins/Referrers/Columns/Base.php Outdated Show resolved Hide resolved
plugins/Referrers/Columns/Base.php Outdated Show resolved Hide resolved
plugins/Referrers/Columns/Campaign.php Show resolved Hide resolved
@diosmosis diosmosis merged commit c6eb523 into 3.x-dev May 10, 2019
@diosmosis diosmosis deleted the direct-entry-to-campaign branch May 10, 2019 08:51
@mattab mattab added the Bug For errors / faults / flaws / inconsistencies etc. label Jun 29, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug For errors / faults / flaws / inconsistencies etc. Needs Review PRs that need a code review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants