Do not create new visit if no referrer info in current action, but referrer info present in last action. Also add integration + system tests for new behavior.

9 files changed, 275 insertions, 26 deletions

diff --git a/core/Tracker/Visit.php b/core/Tracker/Visit.php
index c019a1acf8..2530dc774b 100644
--- a/core/Tracker/Visit.php
+++ b/core/Tracker/Visit.php
@@ -616,7 +616,7 @@ class Visit implements VisitInterface
      * @param Action|null $action The current action being tracked.
      * @return bool
      */
-    private function isVisitNew(Visitor $visitor, Action $action = null)
+    public function isVisitNew(Visitor $visitor, Action $action = null)
     {
         $isLastActionInTheSameVisit = $this->isLastActionInTheSameVisit($visitor);
 
diff --git a/plugins/Referrers/Columns/Base.php b/plugins/Referrers/Columns/Base.php
index a721dc9421..5e8c186af8 100644
--- a/plugins/Referrers/Columns/Base.php
+++ b/plugins/Referrers/Columns/Base.php
@@ -407,7 +407,7 @@ abstract class Base extends VisitDimension
         }
     }
 
-    protected function hasReferrerInformationChanged(Visitor $visitor, $information)
+    protected function isReferrerInformationNew(Visitor $visitor, $information)
     {
         foreach (array('referer_keyword', 'referer_name', 'referer_type') as $infoName) {
             if ($this->hasReferrerColumnChanged($visitor, $information, $infoName)) {
@@ -422,9 +422,8 @@ abstract class Base extends VisitDimension
         return Common::mb_strtolower($visitor->getVisitorColumn($infoName)) != $information[$infoName];
     }
 
-    protected function doesLastOrCurrentActionHaveSameReferrer(Visitor $visitor, $currentInformation, $referrerType)
+    protected function doesLastActionHaveSameReferrer(Visitor $visitor, $referrerType)
     {
-        return $visitor->getVisitorColumn('referer_type') == $referrerType
-        || $currentInformation['referer_type'] == $referrerType;
+        return $visitor->getVisitorColumn('referer_type') == $referrerType;
     }
 }
\ No newline at end of file
diff --git a/plugins/Referrers/Columns/Campaign.php b/plugins/Referrers/Columns/Campaign.php
index 53f86e97eb..df648fbd34 100644
--- a/plugins/Referrers/Columns/Campaign.php
+++ b/plugins/Referrers/Columns/Campaign.php
@@ -52,8 +52,8 @@ class Campaign extends Base
 
         $information = $this->getReferrerInformationFromRequest($request);
 
-        if ($this->doesLastOrCurrentActionHaveSameReferrer($visitor, $information, Common::REFERRER_TYPE_CAMPAIGN)
-            && $this->hasReferrerInformationChanged($visitor, $information)
+        if ($information['referer_type'] == Common::REFERRER_TYPE_CAMPAIGN
+            && $this->isReferrerInformationNew($visitor, $information)
         ) {
             Common::printDebug("Existing visit detected, but creating new visit because campaign information is different than last action.");
 
diff --git a/plugins/Referrers/Columns/Website.php b/plugins/Referrers/Columns/Website.php
index 7d4903c902..a806b8e898 100644
--- a/plugins/Referrers/Columns/Website.php
+++ b/plugins/Referrers/Columns/Website.php
@@ -43,8 +43,8 @@ class Website extends Base
 
         $information = $this->getReferrerInformationFromRequest($request);
 
-        if ($this->doesLastOrCurrentActionHaveSameReferrer($visitor, $information, Common::REFERRER_TYPE_WEBSITE)
-            && $this->hasReferrerInformationChanged($visitor, $information)
+        if ($information['referer_type'] == Common::REFERRER_TYPE_WEBSITE
+            && $this->isReferrerInformationNew($visitor, $information)
         ) {
             Common::printDebug("Existing visit detected, but creating new visit because website referrer information is different than last action.");
 
diff --git a/tests/PHPUnit/Fixtures/SomeVisitsCustomVariablesCampaignsNotHeuristics.php b/tests/PHPUnit/Fixtures/SomeVisitsCustomVariablesCampaignsNotHeuristics.php
index 3c73972a7a..b4adc3dbd1 100644
--- a/tests/PHPUnit/Fixtures/SomeVisitsCustomVariablesCampaignsNotHeuristics.php
+++ b/tests/PHPUnit/Fixtures/SomeVisitsCustomVariablesCampaignsNotHeuristics.php
@@ -25,6 +25,7 @@ class SomeVisitsCustomVariablesCampaignsNotHeuristics extends Fixture
 
     public function setUp()
     {
+        $this->setPiwikEnviornmentOverrides();
         $this->setUpWebsitesAndGoals();
         $this->trackVisits();
     }
@@ -33,6 +34,14 @@ class SomeVisitsCustomVariablesCampaignsNotHeuristics extends Fixture
     {
     }
 
+    private function setPiwikEnviornmentOverrides()
+    {
+        $configOverride = $this->getTestEnvironment()->configOverride;
+        $configOverride['Tracker']['tracker_create_new_visit_when_website_referrer_changes'] = 1;
+        $this->getTestEnvironment()->configOverride = $configOverride;
+        $this->getTestEnvironment()->save();
+    }
+
     private function setUpWebsitesAndGoals()
     {
         if (!self::siteCreated($idSite = 1)) {
@@ -85,6 +94,11 @@ class SomeVisitsCustomVariablesCampaignsNotHeuristics extends Fixture
         $t3->setUrl('http://example.org/index.htm#pk_campaign=CREDITED TO GOAL PLEASE');
         self::checkResponse($t3->doTrackGoal($idGoal, 42));
 
+        // another action soon after last but with different campaign (should result in new visit)
+        $t3->setForceVisitDateTime(Date::factory($dateTime)->addHour(1.4)->getDatetime());
+        $t3->setUrl('http://example.org/index.html#pk_campaign=CREDITED TO ANOTHER GOAL');
+        self::checkResponse($t3->doTrackGoal($idGoal, 24));
+
         // visitor #4, test for blank referrer campaign keyword
         $t4 = self::getTracker($idSite, $dateTime);
         $t4->setForceVisitDateTime(Date::factory($dateTime)->addHour(3)->getDatetime());
@@ -125,6 +139,44 @@ class SomeVisitsCustomVariablesCampaignsNotHeuristics extends Fixture
         $t4->setUrlReferrer($adwords);
         $t4->setUrl('http://example.org/index.html');
         self::checkResponse($t4->doTrackPageView('Bonjour le monde'));
+
+        // test one action w/ no campaign & then one action w/ a campaign (should result in 2 visits)
+        $t4->setForceVisitDateTime(Date::factory($dateTime)->addHour(10)->getDatetime());
+        $t4->setUrlReferrer('');
+        $t4->setUrl('http://example.org/index.html');
+        self::checkResponse($t4->doTrackPageView('Hallo welt'));
+
+        $t4->setForceVisitDateTime(Date::factory($dateTime)->addHour(10.1)->getDatetime());
+        $t4->setUrl('http://example.org/index.html?utm_campaign=GA Campaign&piwik_kwd=Piwik kwd');
+        self::checkResponse($t4->doTrackPageView('¡hola mundo'));
+
+        // right after last action, visit w/ referrer website (should result in another visit)
+        $t4->setForceVisitDateTime(Date::factory($dateTime)->addHour(10.2)->getDatetime());
+        $t4->setUrlReferrer('http://myreferrerwebsite.com');
+        $t4->setUrl('http://example.org/index.html');
+        self::checkResponse($t4->doTrackPageView('Dia duit ar domhan'));
+
+        // test one action w/ no referrer website & then one action w/ referrer website (should result in 2 visits)
+        $t4->setForceVisitDateTime(Date::factory($dateTime)->addHour(11)->getDatetime());
+        $t4->setUrlReferrer('');
+        $t4->setUrl('http://example.org/index.html');
+        self::checkResponse($t4->doTrackPageView('привет мир'));
+
+        $t4->setForceVisitDateTime(Date::factory($dateTime)->addHour(11.1)->getDatetime());
+        $t4->setUrlReferrer('http://myotherreferrerwebsite.com');
+        $t4->setUrl('http://example.org/index.html');
+        self::checkResponse($t4->doTrackPageView('hallå världen'));
+
+        $t4->setForceVisitDateTime(Date::factory($dateTime)->addHour(11.2)->getDatetime()); // same referrer in next action, should result in just another action
+        $t4->setUrlReferrer('http://myotherreferrerwebsite.com');
+        $t4->setUrl('http://example.org/index.html');
+        self::checkResponse($t4->doTrackPageView('halló heimur'));
+
+        // same visitor as last w/ action soon after last action but w/ new referrer website (should result in another visit)
+        $t4->setForceVisitDateTime(Date::factory($dateTime)->addHour(11.3)->getDatetime());
+        $t4->setUrlReferrer('http://mutantregistration.com');
+        $t4->setUrl('http://example.org/index.html');
+        self::checkResponse($t4->doTrackPageView('העלא וועלט'));
     }
 
     // see updateDomainHash() in piwik.js
diff --git a/tests/PHPUnit/Integration/Tracker/VisitTest.php b/tests/PHPUnit/Integration/Tracker/VisitTest.php
index 3a516d1c53..eb5fd28d78 100644
--- a/tests/PHPUnit/Integration/Tracker/VisitTest.php
+++ b/tests/PHPUnit/Integration/Tracker/VisitTest.php
@@ -9,13 +9,18 @@
 namespace Piwik\Tests\Integration\Tracker;
 
 use Piwik\Access;
+use Piwik\Cache\PluginAwareStaticCache;
+use Piwik\Date;
 use Piwik\Network\IPUtils;
 use Piwik\Plugin\Manager;
 use Piwik\Plugins\SitesManager\API;
 use Piwik\Tests\Framework\Mock\FakeAccess;
+use Piwik\Tracker\ActionPageview;
 use Piwik\Tracker\Request;
+use Piwik\Tracker\Visit;
 use Piwik\Tracker\VisitExcluded;
 use Piwik\Tests\Framework\TestCase\IntegrationTestCase;
+use Piwik\Tracker\Visitor;
 
 /**
  * @group Core
@@ -249,6 +254,86 @@ class VisitTest extends IntegrationTestCase
             $this->assertSame($isBot, $excluded->public_isNonHumanBot(), $userAgent);
         }
     }
+
+    public function test_isVisitNew_ReturnsFalse_IfLastActionTimestampIsWithinVisitTimeLength_AndNoDimensionForcesVisit_AndVisitorKnown()
+    {
+        $this->setDimensionsWithOnNewVisit(array(false, false, false));
+
+        /** @var Visit $visit */
+        list($visit, $visitor, $action) = $this->makeVisitorAndAction(
+            $lastActionTime = '2012-01-02 08:08:34', $thisActionTime = '2012-01-02 08:12:45', $isVisitorKnown = true);
+
+        $result = $visit->isVisitNew($visitor, $action);
+
+        $this->assertFalse($result);
+    }
+
+    public function test_isVisitNew_ReturnsTrue_IfLastActionTimestampIsNotWithinVisitTimeLength_AndNoDimensionForcesVisit_AndVisitorNotKnown()
+    {
+        $this->setDimensionsWithOnNewVisit(array(false, false, false));
+
+        /** @var Visit $visit */
+        list($visit, $visitor, $action) = $this->makeVisitorAndAction($lastActionTime = '2012-01-02 08:08:34', $thisActionTime = '2012-01-02 09:12:45');
+
+        $result = $visit->isVisitNew($visitor, $action);
+
+        $this->assertTrue($result);
+    }
+
+    public function test_isVisitNew_ReturnsTrue_IfLastActionTimestampIsWithinVisitTimeLength_AndDimensionForcesVisit()
+    {
+        $this->setDimensionsWithOnNewVisit(array(false, false, true));
+
+        /** @var Visit $visit */
+        list($visit, $visitor, $action) = $this->makeVisitorAndAction($lastActionTime = '2012-01-02 08:08:34', $thisActionTime = '2012-01-02 08:12:45');
+
+        $result = $visit->isVisitNew($visitor, $action);
+
+        $this->assertTrue($result);
+    }
+
+    public  function test_isVisitNew_ReturnsTrue_IfDimensionForcesVisit_AndVisitorKnown()
+    {
+        $this->setDimensionsWithOnNewVisit(array(false, false, true));
+
+        /** @var Visit $visit */
+        list($visit, $visitor, $action) = $this->makeVisitorAndAction($lastActionTime = '2012-01-02 08:08:34', $thisActionTime = '2012-01-02 08:12:45');
+
+        $result = $visit->isVisitNew($visitor, $action);
+
+        $this->assertTrue($result);
+    }
+
+    private function makeVisitorAndAction($lastActionTimestamp, $currentActionTime, $isVisitorKnown = false)
+    {
+        $idsite = API::getInstance()->addSite("name", "http://piwik.net/");
+
+        $request = new Request(array('idsite' => $idsite));
+        $request->setCurrentTimestamp(Date::factory($currentActionTime)->getTimestamp());
+
+        $visit = new Visit();
+        $visit->setRequest($request);
+
+        $visitor = new Visitor($request, 'configid', array('visit_last_action_time' => Date::factory($lastActionTimestamp)->getTimestamp()));
+        $visitor->setIsVisitorKnown($isVisitorKnown);
+
+        $action = new ActionPageview($request);
+
+        return array($visit, $visitor, $action);
+    }
+
+    private function setDimensionsWithOnNewVisit($dimensionOnNewVisitResults)
+    {
+        $dimensions = array();
+        foreach ($dimensionOnNewVisitResults as $onNewVisitResult) {
+            $dim = $this->getMock('Piwik\\Plugin\\Dimension', array('shouldForceNewVisit', 'getColumnName'));
+            $dim->expects($this->any())->method('shouldForceNewVisit')->will($this->returnValue($onNewVisitResult));
+            $dimensions[] = $dim;
+        }
+
+        $cache = new PluginAwareStaticCache('VisitDimensions');
+        $cache->set($dimensions);
+    }
 }
 
 class VisitExcluded_public extends VisitExcluded
diff --git a/tests/PHPUnit/System/expected/test_PiwikTracker_trackForceUsingVisitId_insteadOfHeuristics_alsoTestsCampaignTracking__Referrers.getCampaigns_day.xml b/tests/PHPUnit/System/expected/test_PiwikTracker_trackForceUsingVisitId_insteadOfHeuristics_alsoTestsCampaignTracking__Referrers.getCampaigns_day.xml
index 0ad33169a4..8adb115a5f 100644
--- a/tests/PHPUnit/System/expected/test_PiwikTracker_trackForceUsingVisitId_insteadOfHeuristics_alsoTestsCampaignTracking__Referrers.getCampaigns_day.xml
+++ b/tests/PHPUnit/System/expected/test_PiwikTracker_trackForceUsingVisitId_insteadOfHeuristics_alsoTestsCampaignTracking__Referrers.getCampaigns_day.xml
@@ -2,13 +2,13 @@
 <result>
 	<row>
 		<label>ga campaign</label>
-		<nb_uniq_visitors>3</nb_uniq_visitors>
-		<nb_visits>3</nb_visits>
-		<nb_actions>3</nb_actions>
+		<nb_uniq_visitors>4</nb_uniq_visitors>
+		<nb_visits>4</nb_visits>
+		<nb_actions>4</nb_actions>
 		<nb_users>0</nb_users>
 		<max_actions>1</max_actions>
 		<sum_visit_length>1084</sum_visit_length>
-		<bounce_count>3</bounce_count>
+		<bounce_count>4</bounce_count>
 		<goals>
 			<row idgoal='1'>
 				<nb_conversions>1</nb_conversions>
@@ -21,13 +21,13 @@
 		<subtable>
 			<row>
 				<label>piwik kwd</label>
-				<nb_uniq_visitors>1</nb_uniq_visitors>
-				<nb_visits>1</nb_visits>
-				<nb_actions>1</nb_actions>
+				<nb_uniq_visitors>2</nb_uniq_visitors>
+				<nb_visits>2</nb_visits>
+				<nb_actions>2</nb_actions>
 				<nb_users>0</nb_users>
 				<max_actions>1</max_actions>
 				<sum_visit_length>1084</sum_visit_length>
-				<bounce_count>1</bounce_count>
+				<bounce_count>2</bounce_count>
 				<goals>
 					<row idgoal='1'>
 						<nb_conversions>1</nb_conversions>
@@ -146,6 +146,46 @@
 		</subtable>
 	</row>
 	<row>
+		<label>credited to another goal</label>
+		<nb_uniq_visitors>1</nb_uniq_visitors>
+		<nb_visits>1</nb_visits>
+		<nb_actions>0</nb_actions>
+		<nb_users>0</nb_users>
+		<max_actions>0</max_actions>
+		<sum_visit_length>3</sum_visit_length>
+		<bounce_count>1</bounce_count>
+		<goals>
+			<row idgoal='1'>
+				<nb_conversions>1</nb_conversions>
+				<nb_visits_converted>1</nb_visits_converted>
+				<revenue>24</revenue>
+			</row>
+		</goals>
+		<nb_conversions>1</nb_conversions>
+		<revenue>24</revenue>
+		<subtable>
+			<row>
+				<label>example.org</label>
+				<nb_uniq_visitors>1</nb_uniq_visitors>
+				<nb_visits>1</nb_visits>
+				<nb_actions>0</nb_actions>
+				<nb_users>0</nb_users>
+				<max_actions>0</max_actions>
+				<sum_visit_length>3</sum_visit_length>
+				<bounce_count>1</bounce_count>
+				<goals>
+					<row idgoal='1'>
+						<nb_conversions>1</nb_conversions>
+						<nb_visits_converted>1</nb_visits_converted>
+						<revenue>24</revenue>
+					</row>
+				</goals>
+				<nb_conversions>1</nb_conversions>
+				<revenue>24</revenue>
+			</row>
+		</subtable>
+	</row>
+	<row>
 		<label>credited to goal please</label>
 		<nb_uniq_visitors>1</nb_uniq_visitors>
 		<nb_visits>1</nb_visits>
diff --git a/tests/PHPUnit/System/expected/test_PiwikTracker_trackForceUsingVisitId_insteadOfHeuristics_alsoTestsCampaignTracking__Referrers.getWebsites_day.xml b/tests/PHPUnit/System/expected/test_PiwikTracker_trackForceUsingVisitId_insteadOfHeuristics_alsoTestsCampaignTracking__Referrers.getWebsites_day.xml
index c234bed59e..51ea81cd04 100644
--- a/tests/PHPUnit/System/expected/test_PiwikTracker_trackForceUsingVisitId_insteadOfHeuristics_alsoTestsCampaignTracking__Referrers.getWebsites_day.xml
+++ b/tests/PHPUnit/System/expected/test_PiwikTracker_trackForceUsingVisitId_insteadOfHeuristics_alsoTestsCampaignTracking__Referrers.getWebsites_day.xml
@@ -1,2 +1,75 @@
 <?xml version="1.0" encoding="utf-8" ?>
-<result />
\ No newline at end of file
+<result>
+	<row>
+		<label>mutantregistration.com</label>
+		<nb_uniq_visitors>1</nb_uniq_visitors>
+		<nb_visits>1</nb_visits>
+		<nb_actions>1</nb_actions>
+		<nb_users>0</nb_users>
+		<max_actions>1</max_actions>
+		<sum_visit_length>0</sum_visit_length>
+		<bounce_count>1</bounce_count>
+		<nb_visits_converted>0</nb_visits_converted>
+		<subtable>
+			<row>
+				<label>http://mutantregistration.com</label>
+				<nb_uniq_visitors>1</nb_uniq_visitors>
+				<nb_visits>1</nb_visits>
+				<nb_actions>1</nb_actions>
+				<nb_users>0</nb_users>
+				<max_actions>1</max_actions>
+				<sum_visit_length>0</sum_visit_length>
+				<bounce_count>1</bounce_count>
+				<nb_visits_converted>0</nb_visits_converted>
+			</row>
+		</subtable>
+	</row>
+	<row>
+		<label>myotherreferrerwebsite.com</label>
+		<nb_uniq_visitors>1</nb_uniq_visitors>
+		<nb_visits>1</nb_visits>
+		<nb_actions>2</nb_actions>
+		<nb_users>0</nb_users>
+		<max_actions>2</max_actions>
+		<sum_visit_length>361</sum_visit_length>
+		<bounce_count>0</bounce_count>
+		<nb_visits_converted>0</nb_visits_converted>
+		<subtable>
+			<row>
+				<label>http://myotherreferrerwebsite.com</label>
+				<nb_uniq_visitors>1</nb_uniq_visitors>
+				<nb_visits>1</nb_visits>
+				<nb_actions>2</nb_actions>
+				<nb_users>0</nb_users>
+				<max_actions>2</max_actions>
+				<sum_visit_length>361</sum_visit_length>
+				<bounce_count>0</bounce_count>
+				<nb_visits_converted>0</nb_visits_converted>
+			</row>
+		</subtable>
+	</row>
+	<row>
+		<label>myreferrerwebsite.com</label>
+		<nb_uniq_visitors>1</nb_uniq_visitors>
+		<nb_visits>1</nb_visits>
+		<nb_actions>1</nb_actions>
+		<nb_users>0</nb_users>
+		<max_actions>1</max_actions>
+		<sum_visit_length>0</sum_visit_length>
+		<bounce_count>1</bounce_count>
+		<nb_visits_converted>0</nb_visits_converted>
+		<subtable>
+			<row>
+				<label>http://myreferrerwebsite.com</label>
+				<nb_uniq_visitors>1</nb_uniq_visitors>
+				<nb_visits>1</nb_visits>
+				<nb_actions>1</nb_actions>
+				<nb_users>0</nb_users>
+				<max_actions>1</max_actions>
+				<sum_visit_length>0</sum_visit_length>
+				<bounce_count>1</bounce_count>
+				<nb_visits_converted>0</nb_visits_converted>
+			</row>
+		</subtable>
+	</row>
+</result>
\ No newline at end of file
diff --git a/tests/PHPUnit/System/expected/test_PiwikTracker_trackForceUsingVisitId_insteadOfHeuristics_alsoTestsCampaignTracking__VisitsSummary.get_day.xml b/tests/PHPUnit/System/expected/test_PiwikTracker_trackForceUsingVisitId_insteadOfHeuristics_alsoTestsCampaignTracking__VisitsSummary.get_day.xml
index 78c6610886..3e911d8174 100644
--- a/tests/PHPUnit/System/expected/test_PiwikTracker_trackForceUsingVisitId_insteadOfHeuristics_alsoTestsCampaignTracking__VisitsSummary.get_day.xml
+++ b/tests/PHPUnit/System/expected/test_PiwikTracker_trackForceUsingVisitId_insteadOfHeuristics_alsoTestsCampaignTracking__VisitsSummary.get_day.xml
@@ -2,13 +2,13 @@
 <result>
 	<nb_uniq_visitors>3</nb_uniq_visitors>
 	<nb_users>0</nb_users>
-	<nb_visits>8</nb_visits>
-	<nb_actions>7</nb_actions>
-	<nb_visits_converted>2</nb_visits_converted>
-	<bounce_count>8</bounce_count>
-	<sum_visit_length>1084</sum_visit_length>
-	<max_actions>1</max_actions>
-	<bounce_rate>100%</bounce_rate>
+	<nb_visits>15</nb_visits>
+	<nb_actions>14</nb_actions>
+	<nb_visits_converted>3</nb_visits_converted>
+	<bounce_count>14</bounce_count>
+	<sum_visit_length>1448</sum_visit_length>
+	<max_actions>2</max_actions>
+	<bounce_rate>93%</bounce_rate>
 	<nb_actions_per_visit>0.9</nb_actions_per_visit>
-	<avg_time_on_site>136</avg_time_on_site>
+	<avg_time_on_site>97</avg_time_on_site>
 </result>
\ No newline at end of file