@laszlovl opened this Issue on November 5th 2017
  1. Link to your site with ?pk_campaign=Campaignname
  2. Click on an internal site link

The second action will create a new visit. Because of that, goals in that visit won't be properly attributed.

The issue is in https://github.com/piwik/piwik/blob/master/plugins/Referrers/Columns/Base.php#L369: $visitor->getVisitorColumn('referer_name') contains the lowercased value ("campaignname"), while $this->nameReferrerAnalyzed contains the original value ("Campaignname"). Because of this, the fix for #9299 won't be applied and Campaign::shouldForceNewVisit will force a new visit.

In general, this logic feels quite fragile. Is it really right to consider a changed referer_keyword value as a sign that the campaign changed, when referer_keyword is sometimes set by internal logic instead of directly from tracking attributes? If shouldForceNewVisit would only look at the name & type, the workaround code could be removed.

FWIW, I agree with #9944 that create_new_visit_when_campaign_changes is counter-intuitive and should be disabled by default.

@tsteur commented on November 5th 2017 Member

Thanks for the report @laszlovl sounds definitely like there is a bug.

I suggest to discuss the default value in #9944 . Feedback is important and very appreciated on this topic.

I can't also say much about the logic being fragile and whether keyword should be considered or not. I recommend creating a new issue for this where we can focus the discussion on this.

@mattab commented on November 19th 2017 Member

The second action will create a new visit. Because of that, goals in that visit won't be properly attributed.

I tried reproducing this issue but in my case it didn't create a new visit. The internal action/page url was added to the visit with Campaignname campaign referrer. Can you please check again @laszlovl what are the steps to reproduce the issue?

@laszlovl commented on November 19th 2017

@mattab Just checking - are you sure that you're testing with create_new_visit_when_campaign_changes enabled (e.g. the default value)? Also, not sure if it makes a difference, but my setup has the markting campaigns plugin enabled.

@mattab commented on November 19th 2017 Member

@laszlovl just tried to reproduce with the MarketingCampaignsReporting plugin enabled, and could not reproduce

@laszlovl commented on November 19th 2017

I just tried, and I could still reproduce the issue on a Piwik 3.2.0 install. Let me clarify the steps to reproduce:

  1. Create a page on your website "a.html" which contains the piwik tracking code, and a link to "b.html"
  2. Create a page on your website "b.html" which contains the piwik tracking code
  3. Create an external link to your website, for example using bit.ly, using the following url: http://piwik.domain/a.html?pk_campaign=Campaignname
  4. Open the bit.ly url in your browser and be redirected to http://piwik.domain/a.html?pk_campaign=Campaignname
  5. On the page a.html, click the link to b.html

Now your visit log will show two separate visits. Note that the issue will only occur when

  • create_new_visit_when_campaign_changes is enabled (the default), and
  • the campaign name contains a capital letter
@mattab commented on November 20th 2017 Member

I haven't yet tried these exact steps, could in the meantime could you try replace the file piwik/plugins/Referrers/Columns/Base.php
by the following content: https://raw.githubusercontent.com/piwik/piwik/003e401a31ab7fb50200bf7e058faaaaf10796db/plugins/Referrers/Columns/Base.php

and check if this helps?

If it does, then likely the bug would be in the plugin itself and will need to investigate further

@laszlovl commented on November 20th 2017

That doesn't fix the problem. To te clear, I debugged the problem down to the exact line of code as described in my original post. A crude patch like this one fixes the issue at hand, I'm just not sure if fixing it like that it would be ignoring a deeper underlying issue.

--- a/plugins/Referrers/Columns/Base.php
+++ b/plugins/Referrers/Columns/Base.php
@@ -366,7 +366,7 @@ abstract class Base extends VisitDimension


-        $isCurrentVisitACampaignWithSameName = $visitor->getVisitorColumn('referer_name') == $this->nameReferrerAnalyzed;
+        $isCurrentVisitACampaignWithSameName = strtolower($visitor->getVisitorColumn('referer_name')) == strtolower($this->nameReferrerAnalyzed);
@mattab commented on September 23rd 2018 Member

Fyi: we've created & merged a PR here: https://github.com/matomo-org/matomo/pull/13469
feel free to try it, or test the next beta which will include this fix.

@laszlovl commented on September 24th 2018

Great! That's pretty much what I've been running in production for a year (though Common::mb_strtolower instead of strtolower), so I think it'll be fine. If I notice any problems, I'll report back.

This Issue was closed on September 24th 2018
Powered by GitHub Issue Mirror