From a6cb3c87cd55ab2278a0eeb82f3da3d432e46223 Mon Sep 17 00:00:00 2001 From: diosmosis Date: Thu, 11 Dec 2014 20:42:04 -0800 Subject: [PATCH] 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. --- core/Tracker/Visit.php | 2 +- plugins/Referrers/Columns/Base.php | 7 +- plugins/Referrers/Columns/Campaign.php | 4 +- plugins/Referrers/Columns/Website.php | 4 +- ...sCustomVariablesCampaignsNotHeuristics.php | 52 ++++++++++++ .../PHPUnit/Integration/Tracker/VisitTest.php | 85 +++++++++++++++++++ ...gnTracking__Referrers.getCampaigns_day.xml | 56 ++++++++++-- ...ignTracking__Referrers.getWebsites_day.xml | 75 +++++++++++++++- ...ampaignTracking__VisitsSummary.get_day.xml | 16 ++-- 9 files changed, 275 insertions(+), 26 deletions(-) diff --git a/core/Tracker/Visit.php b/core/Tracker/Visit.php index c019a1acf88..2530dc774b6 100644 --- a/core/Tracker/Visit.php +++ b/core/Tracker/Visit.php @@ -616,7 +616,7 @@ protected function insertNewVisit($visit) * @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 a721dc94219..5e8c186af88 100644 --- a/plugins/Referrers/Columns/Base.php +++ b/plugins/Referrers/Columns/Base.php @@ -407,7 +407,7 @@ protected function setCampaignValuesToLowercase($type, &$name, &$keyword) } } - 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 @@ protected function hasReferrerColumnChanged(Visitor $visitor, $information, $inf 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 53f86e97eba..df648fbd34d 100644 --- a/plugins/Referrers/Columns/Campaign.php +++ b/plugins/Referrers/Columns/Campaign.php @@ -52,8 +52,8 @@ public function shouldForceNewVisit(Request $request, Visitor $visitor, Action $ $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 7d4903c902a..a806b8e8987 100644 --- a/plugins/Referrers/Columns/Website.php +++ b/plugins/Referrers/Columns/Website.php @@ -43,8 +43,8 @@ public function shouldForceNewVisit(Request $request, Visitor $visitor, Action $ $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 3c73972a7a9..b4adc3dbd17 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 @@ public function tearDown() { } + 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 @@ private function trackVisits() $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 @@ private function trackVisits() $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 3a516d1c53b..eb5fd28d781 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 @@ public function testIsVisitor_userAgentIsKnownBot() $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 0ad33169a4c..8adb115a5f4 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 @@ - 3 - 3 - 3 + 4 + 4 + 4 0 1 1084 - 3 + 4 1 @@ -21,13 +21,13 @@ - 1 - 1 - 1 + 2 + 2 + 2 0 1 1084 - 1 + 2 1 @@ -145,6 +145,46 @@ + + + 1 + 1 + 0 + 0 + 0 + 3 + 1 + + + 1 + 1 + 24 + + + 1 + 24 + + + + 1 + 1 + 0 + 0 + 0 + 3 + 1 + + + 1 + 1 + 24 + + + 1 + 24 + + + 1 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 c234bed59e9..51ea81cd041 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 @@ - \ No newline at end of file + + + + 1 + 1 + 1 + 0 + 1 + 0 + 1 + 0 + + + + 1 + 1 + 1 + 0 + 1 + 0 + 1 + 0 + + + + + + 1 + 1 + 2 + 0 + 2 + 361 + 0 + 0 + + + + 1 + 1 + 2 + 0 + 2 + 361 + 0 + 0 + + + + + + 1 + 1 + 1 + 0 + 1 + 0 + 1 + 0 + + + + 1 + 1 + 1 + 0 + 1 + 0 + 1 + 0 + + + + \ 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 78c6610886d..3e911d8174b 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 @@ 3 0 - 8 - 7 - 2 - 8 - 1084 - 1 - 100% + 15 + 14 + 3 + 14 + 1448 + 2 + 93% 0.9 - 136 + 97 \ No newline at end of file