Truncate referrer name & keyword in Base class so the value used there matches what is in the DB. (#13573)

4 files changed, 96 insertions, 13 deletions

diff --git a/plugins/Referrers/Columns/Base.php b/plugins/Referrers/Columns/Base.php
index 9a515b2394..0abe38c8fe 100644
--- a/plugins/Referrers/Columns/Base.php
+++ b/plugins/Referrers/Columns/Base.php
@@ -124,6 +124,14 @@ abstract class Base extends VisitDimension
             'referer_url'     => $this->referrerUrl,
         );
 
+        if (!empty($referrerInformation['referer_name'])) {
+            $referrerInformation['referer_name'] = $this->truncateReferrerName($referrerInformation['referer_name']);
+        }
+
+        if (!empty($referrerInformation['referer_keyword'])) {
+            $referrerInformation['referer_keyword'] = $this->truncateReferrerKeyword($referrerInformation['referer_keyword']);
+        }
+
         return $referrerInformation;
     }
 
@@ -425,7 +433,10 @@ abstract class Base extends VisitDimension
 
         $this->detectCampaignKeywordFromReferrerUrl();
 
-        $isCurrentVisitACampaignWithSameName = Common::mb_strtolower($visitor->getVisitorColumn('referer_name')) == Common::mb_strtolower($this->nameReferrerAnalyzed);
+        $referrerNameAnalayzed = Common::mb_strtolower($this->nameReferrerAnalyzed);
+        $referrerNameAnalayzed = $this->truncateReferrerName($referrerNameAnalayzed);
+
+        $isCurrentVisitACampaignWithSameName = Common::mb_strtolower($visitor->getVisitorColumn('referer_name')) == $referrerNameAnalayzed;
         $isCurrentVisitACampaignWithSameName = $isCurrentVisitACampaignWithSameName && $visitor->getVisitorColumn('referer_type') == Common::REFERRER_TYPE_CAMPAIGN;
 
         // if we detected a campaign but there is still no keyword set, we set the keyword to the Referrer host
@@ -553,7 +564,6 @@ abstract class Base extends VisitDimension
     {
         foreach (array('referer_keyword', 'referer_name', 'referer_type') as $infoName) {
             if ($this->hasReferrerColumnChanged($visitor, $information, $infoName)) {
-                Common::printDebug("Referrers\Base::isReferrerInformationNew: detected change in $infoName.");
                 return true;
             }
         }
@@ -562,7 +572,15 @@ abstract class Base extends VisitDimension
 
     protected function hasReferrerColumnChanged(Visitor $visitor, $information, $infoName)
     {
-        return Common::mb_strtolower($visitor->getVisitorColumn($infoName)) != Common::mb_strtolower($information[$infoName]);
+        $existing = Common::mb_strtolower($visitor->getVisitorColumn($infoName));
+        $new = Common::mb_strtolower($information[$infoName]);
+
+        $result = $existing != $new;
+        if ($result) {
+            Common::printDebug("Referrers\Base::isReferrerInformationNew: detected change in $infoName ('$existing' != '$new').");
+        }
+
+        return $result;
     }
 
     protected function doesLastActionHaveSameReferrer(Visitor $visitor, $referrerType)
@@ -574,4 +592,14 @@ abstract class Base extends VisitDimension
     {
         return trim(urldecode($request->getParam($paramName)));
     }
+
+    private function truncateReferrerName($name)
+    {
+        return Common::mb_substr($name, 0, 70);
+    }
+
+    private function truncateReferrerKeyword($refererKeyword)
+    {
+        return Common::mb_substr($refererKeyword, 0, 255);
+    }
 }
diff --git a/plugins/Referrers/Columns/Keyword.php b/plugins/Referrers/Columns/Keyword.php
index e496740c66..777965023a 100644
--- a/plugins/Referrers/Columns/Keyword.php
+++ b/plugins/Referrers/Columns/Keyword.php
@@ -33,11 +33,6 @@ class Keyword extends Base
     public function onNewVisit(Request $request, Visitor $visitor, $action)
     {
         $information = $this->getReferrerInformationFromRequest($request, $visitor);
-
-        if (!empty($information['referer_keyword'])) {
-            return Common::mb_substr($information['referer_keyword'], 0, 255);
-        }
-
         return $information['referer_keyword'];
     }
 
diff --git a/plugins/Referrers/Columns/ReferrerName.php b/plugins/Referrers/Columns/ReferrerName.php
index f5989ecaf7..3420c36ea8 100644
--- a/plugins/Referrers/Columns/ReferrerName.php
+++ b/plugins/Referrers/Columns/ReferrerName.php
@@ -34,10 +34,6 @@ class ReferrerName extends Base
     public function onNewVisit(Request $request, Visitor $visitor, $action)
     {
         $information = $this->getReferrerInformationFromRequest($request, $visitor);
-
-        if (!empty($information['referer_name'])) {
-            return Common::mb_substr($information['referer_name'], 0, 70);
-        }
         return $information['referer_name'];
     }
 
diff --git a/plugins/Referrers/tests/System/ApiTest.php b/plugins/Referrers/tests/System/ApiTest.php
index e8e3a77012..463d47822b 100644
--- a/plugins/Referrers/tests/System/ApiTest.php
+++ b/plugins/Referrers/tests/System/ApiTest.php
@@ -9,6 +9,7 @@
 namespace Piwik\Plugins\Referrers\tests\System;
 
 use Piwik\API\Request;
+use Piwik\Config;
 use Piwik\DataTable;
 use Piwik\Tests\Fixtures\TwoSitesManyVisitsOverSeveralDaysWithSearchEngineReferrers;
 use Piwik\Tests\Framework\Fixture;
@@ -131,7 +132,7 @@ class ApiTest extends SystemTestCase
 
     public function test_forceNewVisit_shouldNotForceANewVisitWhenNoKeywordIsSetAndCampaignNameIsUpperCase()
     {
-        $dateTime = '2015-01-03';
+        $dateTime = '2015-01-04';
         $idSite = self::$fixture->idSite;
 
         $t = Fixture::getTracker($idSite, $dateTime . ' 00:01:02', $defaultInit = true);
@@ -154,6 +155,60 @@ class ApiTest extends SystemTestCase
         $this->assertEquals(2, $visits->getFirstRow()->getColumn('nb_actions'));
     }
 
+    public function test_forceNewVisit_shouldNotForceANewVisitWhenKeywordIsLongerThanDbColumnLength()
+    {
+        $dateTime = '2015-01-05';
+        $idSite = self::$fixture->idSite;
+        $longReferrer = 'thisisaverylongreferrerkeywordhereitisdefinitelylongerthanseventycharacterswhyitisevenlongerthantwohundredfiftyfivecharacters'
+            . 'butboyisithardtocomeupwiththingstosayhereimeantheresonlysomuchapersoncanthinkitsnotlikeimplatoimmoreofacamusbutithinkicangettotheendof'
+            . 'thiscrazylongstringohijustdid';
+
+        $t = Fixture::getTracker($idSite, $dateTime . ' 00:01:02', $defaultInit = true);
+        // track a campaign that was opened directly (w/ saved referrer cookie info)
+        $t->setUrlReferrer('http://www.google.com');
+        $t->setUrl('http://piwik.net/?pk_campaign=' . $longReferrer);
+        $t->doTrackPageView('My Title');
+
+        // navigate to same page but from different URL w/ same campaign
+        $t->setUrlReferrer('http://links.piwik.net/?pk_campaign=' . $longReferrer);
+        $t->setCustomTrackingParameter('_rcn', $longReferrer); // this parameter would be set by piwik.js from cookie / attributionInfo
+        $t->setCustomTrackingParameter('_rck', ''); // no keyword was used in previous tracking request
+        $t->setUrl('http://piwik.net/page1');
+        $t->doTrackPageView('Page 1');
+
+        /** @var DataTable $visits */
+        $visits = Request::processRequest('VisitsSummary.get', array('idSite' => 1, 'period' => 'day', 'date' => $dateTime));
+
+        $this->assertEquals(1, $visits->getFirstRow()->getColumn('nb_visits'));
+        $this->assertEquals(2, $visits->getFirstRow()->getColumn('nb_actions'));
+    }
+
+    public function test_forceNewVisit_shouldNotForceNewVisitWhenReferrerNameIsLongerThanDbColumnLength()
+    {
+        $dateTime = '2015-01-06';
+        $idSite = self::$fixture->idSite;
+        $longReferrer = 'http://www.thisisaverylongreferrerkeywordhereitisdefinitelylongerthanseventycharacters.com';
+
+        $t = Fixture::getTracker($idSite, $dateTime . ' 00:01:02', $defaultInit = true);
+        // track a campaign that was opened directly
+        $t->setUrlReferrer($longReferrer);
+        $t->setUrl('http://piwik.net/');
+        $t->doTrackPageView('My Title');
+
+        // navigate to same page but from different URL w/ same campaign
+        $t->setUrlReferrer($longReferrer);
+        $t->setCustomTrackingParameter('_rcn', ''); // this parameter would be set by piwik.js from cookie / attributionInfo
+        $t->setCustomTrackingParameter('_rck', ''); // no keyword was used in previous tracking request
+        $t->setUrl('http://piwik.net/page1');
+        $t->doTrackPageView('Page 1');
+
+        /** @var DataTable $visits */
+        $visits = Request::processRequest('VisitsSummary.get', array('idSite' => 1, 'period' => 'day', 'date' => $dateTime));
+
+        $this->assertEquals(1, $visits->getFirstRow()->getColumn('nb_visits'));
+        $this->assertEquals(2, $visits->getFirstRow()->getColumn('nb_actions'));
+    }
+
     public static function getOutputPrefix()
     {
         return '';
@@ -164,6 +219,15 @@ class ApiTest extends SystemTestCase
         return dirname(__FILE__);
     }
 
+    public static function provideContainerConfigBeforeClass()
+    {
+        return [
+            Config::class => \DI\decorate(function (Config $config) {
+                $config->Tracker['create_new_visit_when_website_referrer_changes'] = 1;
+                return $config;
+            }),
+        ];
+    }
 }
 
 ApiTest::$fixture = new TwoSitesManyVisitsOverSeveralDaysWithSearchEngineReferrers();
\ No newline at end of file