From 0a0a0d2cdef310e98636ef1b68fc4f9d4108b235 Mon Sep 17 00:00:00 2001 From: diosmosis Date: Tue, 9 Dec 2014 17:01:25 -0800 Subject: [PATCH 1/7] Refs #2624, add INI config options that will force a new visit if campaign/website referrer information changes between actions. Includes new tracker hook shouldForceNewVisit that dimensions can use to force a new visit during tracking. Also includes light refactoring to referrer dimensions. Testless. --- config/global.ini.php | 10 ++++ core/Columns/Dimension.php | 22 ++++++++ core/Tracker/Visit.php | 66 ++++++++++++++++++---- plugins/Referrers/Columns/Base.php | 30 +++++++++- plugins/Referrers/Columns/Campaign.php | 51 ++++++++++++++++- plugins/Referrers/Columns/Keyword.php | 5 +- plugins/Referrers/Columns/ReferrerName.php | 5 +- plugins/Referrers/Columns/ReferrerType.php | 5 +- plugins/Referrers/Columns/ReferrerUrl.php | 5 +- plugins/Referrers/Columns/Website.php | 41 +++++++++++++- 10 files changed, 207 insertions(+), 33 deletions(-) diff --git a/config/global.ini.php b/config/global.ini.php index 2bab7b130d7..b4fde4c3582 100644 --- a/config/global.ini.php +++ b/config/global.ini.php @@ -586,6 +586,16 @@ ; Includes by default the GA style campaign keyword parameter utm_term campaign_keyword_var_name = "pk_kwd,pk_keyword,piwik_kwd,utm_term" +; if set to 1, actions that contain different campaign information from the last action in the current visit will +; be treated as the start of a new visit. This will include situations when campaign information was absent before, +; but is present now (or vice versa). +tracker_create_new_visit_when_campaign_changes = 1 + +; if set to 1, actions that contain different website referrer information from the last action in the current visit +; will be treatedas the start of a new visit. This will include situations when website referrer information was +; absent before, but is present now (or vice versa). +tracker_create_new_visit_when_website_referrer_changes = 0 + ; maximum length of a Page Title or a Page URL recorded in the log_action.name table page_maximum_length = 1024; diff --git a/core/Columns/Dimension.php b/core/Columns/Dimension.php index e760a090832..0da3bcf8b3d 100644 --- a/core/Columns/Dimension.php +++ b/core/Columns/Dimension.php @@ -15,6 +15,9 @@ use Piwik\Plugin\Dimension\ConversionDimension; use Piwik\Plugin\Dimension\VisitDimension; use Piwik\Plugin\Segment; +use Piwik\Tracker\Action; +use Piwik\Tracker\Request; +use Piwik\Tracker\Visitor; use Piwik\Translate; /** @@ -166,6 +169,25 @@ public function getId() return $pluginName . '.' . $dimensionName; } + /** + * This hook is executed by the tracker when determining if an action is the start of a new visit + * or part of an existing one. Derived classes can use it to force new visits based on dimension + * data. + * + * For example, the Campaign dimension in the Referrers plugin will force a new visit if the + * campaign information for the current action is different from the last. + * + * @param Request $request The current tracker request information. + * @param Visitor $visitor The information for the currently recognized visitor. + * @param Action|null $action The current action information (if any). + * @return bool Return true to force a visit, false if otherwise. + * @api + */ + public function shouldForceNewVisit(Request $request, Visitor $visitor, Action $action = null) + { + return false; + } + /** * Gets an instance of all available visit, action and conversion dimension. * @return Dimension[] diff --git a/core/Tracker/Visit.php b/core/Tracker/Visit.php index 4065ebe15cb..869434d4cd4 100644 --- a/core/Tracker/Visit.php +++ b/core/Tracker/Visit.php @@ -152,11 +152,7 @@ public function handle() $this->visitorInfo = $visitor->getVisitorInfo(); - $isLastActionInTheSameVisit = $this->isLastActionInTheSameVisit($visitor); - - if (!$isLastActionInTheSameVisit) { - Common::printDebug("Visitor detected, but last action was more than 30 minutes ago..."); - } + $isNewVisit = $this->isVisitNew($visitor, $action); // Known visit when: // ( - the visitor has the Piwik cookie with the idcookie ID used by Piwik to match the visitor @@ -165,9 +161,7 @@ public function handle() // ) // AND // - the last page view for this visitor was less than 30 minutes ago @see isLastActionInTheSameVisit() - if ($visitor->isVisitorKnown() - && $isLastActionInTheSameVisit - ) { + if (!$isNewVisit) { $idReferrerActionUrl = $this->visitorInfo['visit_exit_idaction_url']; $idReferrerActionName = $this->visitorInfo['visit_exit_idaction_name']; @@ -203,9 +197,7 @@ public function handle() // - the visitor has the Piwik cookie but the last action was performed more than 30 min ago @see isLastActionInTheSameVisit() // - the visitor doesn't have the Piwik cookie, and couldn't be matched in @see recognizeTheVisitor() // - the visitor does have the Piwik cookie but the idcookie and idvisit found in the cookie didn't match to any existing visit in the DB - if (!$visitor->isVisitorKnown() - || !$isLastActionInTheSameVisit - ) { + if ($isNewVisit) { $this->handleNewVisit($visitor, $action, $visitIsConverted); if (!is_null($action)) { $action->record($visitor, 0, 0); @@ -550,6 +542,23 @@ private function triggerHookOnDimensions($dimensions, $hook, $visitor, $action, return $valuesToUpdate; } + private function triggerPredicateHookOnDimensions($dimensions, $hook, Visitor $visitor, Action $action = null, $returnFirstTrue = true) + { + $result = $returnFirstTrue ? false : array(); + foreach ($dimensions as $dimension) { + $value = $dimension->$hook($this->request, $visitor, $action); + + if ($returnFirstTrue) { + if ($value) { + return true; + } + } else { + $result[] = $value; + } + } + return $result; + } + protected function getAllVisitDimensions() { $dimensions = VisitDimension::getAllDimensions(); @@ -598,4 +607,39 @@ protected function insertNewVisit($visit) { return $this->getModel()->createVisit($visit); } + + /** + * Determines if the tracker if the current action should be treated as the start of a new visit or + * an action in an existing visit. + * + * @param Visitor $visitor The current visit/visitor information. + * @param Action|null $action The current action being tracked. + * @return bool + */ + private function isVisitNew(Visitor $visitor, Action $action = null) + { + $isLastActionInTheSameVisit = $this->isLastActionInTheSameVisit($visitor); + + if (!$isLastActionInTheSameVisit) { + Common::printDebug("Visitor detected, but last action was more than 30 minutes ago..."); + + return true; + } + + $shouldForceNewVisit = $this->triggerPredicateHookOnDimensions($this->getAllVisitDimensions(), 'shouldForceNewVisit', $visitor, $action); + if ($shouldForceNewVisit) { + return true; + } + + // if we should create a new visit when the referrer changes, check if referrer changed + if ($this->createNewVisitWhenWebsiteReferrerChanges + && $visitor->isReferrerInformationDifferent() + ) { + Common::printDebug("Existing visit detected, but creating new visit because referrer information is different than last action"); + + return true; + } + + return !$visitor->isVisitorKnown(); + } } diff --git a/plugins/Referrers/Columns/Base.php b/plugins/Referrers/Columns/Base.php index a69fcc54bfe..a721dc94219 100644 --- a/plugins/Referrers/Columns/Base.php +++ b/plugins/Referrers/Columns/Base.php @@ -127,6 +127,14 @@ protected function getReferrerInformation($referrerUrl, $currentUrl, $idSite) return $referrerInformation; } + protected function getReferrerInformationFromRequest(Request $request) + { + $referrerUrl = $request->getParam('urlref'); + $currentUrl = $request->getParam('url'); + + return $this->getReferrerInformation($referrerUrl, $currentUrl, $request->getIdSite()); + } + /** * Search engine detection * @return bool @@ -399,4 +407,24 @@ protected function setCampaignValuesToLowercase($type, &$name, &$keyword) } } -} + protected function hasReferrerInformationChanged(Visitor $visitor, $information) + { + foreach (array('referer_keyword', 'referer_name', 'referer_type') as $infoName) { + if ($this->hasReferrerColumnChanged($visitor, $information, $infoName)) { + return true; + } + } + return false; + } + + protected function hasReferrerColumnChanged(Visitor $visitor, $information, $infoName) + { + return Common::mb_strtolower($visitor->getVisitorColumn($infoName)) != $information[$infoName]; + } + + protected function doesLastOrCurrentActionHaveSameReferrer(Visitor $visitor, $currentInformation, $referrerType) + { + return $visitor->getVisitorColumn('referer_type') == $referrerType + || $currentInformation['referer_type'] == $referrerType; + } +} \ No newline at end of file diff --git a/plugins/Referrers/Columns/Campaign.php b/plugins/Referrers/Columns/Campaign.php index 4413cd702f6..53f86e97eba 100644 --- a/plugins/Referrers/Columns/Campaign.php +++ b/plugins/Referrers/Columns/Campaign.php @@ -8,13 +8,58 @@ */ namespace Piwik\Plugins\Referrers\Columns; -use Piwik\Columns\Dimension; +use Piwik\Common; use Piwik\Piwik; +use Piwik\Tracker\Action; +use Piwik\Tracker\Request; +use Piwik\Tracker\TrackerConfig; +use Piwik\Tracker\Visitor; -class Campaign extends Dimension +class Campaign extends Base { + /** + * Obtained from the `[Tracker] tracker_create_new_visit_when_campaign_changes` INI config option. + * If true, will create new visits when campaign name changes. + * + * @var bool + */ + protected $createNewVisitWhenCampaignChanges; + + public function __construct() + { + $this->createNewVisitWhenCampaignChanges = TrackerConfig::getConfigValue('tracker_create_new_visit_when_campaign_changes') == 1; + } + public function getName() { return Piwik::translate('Referrers_ColumnCampaign'); } -} + + /** + * If we should create a new visit when the campaign changes, check if the campaign info changed and if so + * force the tracker to create a new visit. + * + * @param Request $request + * @param Visitor $visitor + * @param Action|null $action + * @return bool + */ + public function shouldForceNewVisit(Request $request, Visitor $visitor, Action $action = null) + { + if (!$this->createNewVisitWhenCampaignChanges) { + return false; + } + + $information = $this->getReferrerInformationFromRequest($request); + + if ($this->doesLastOrCurrentActionHaveSameReferrer($visitor, $information, Common::REFERRER_TYPE_CAMPAIGN) + && $this->hasReferrerInformationChanged($visitor, $information) + ) { + Common::printDebug("Existing visit detected, but creating new visit because campaign information is different than last action."); + + return true; + } + + return false; + } +} \ No newline at end of file diff --git a/plugins/Referrers/Columns/Keyword.php b/plugins/Referrers/Columns/Keyword.php index a5025b63c21..78605c9d94a 100644 --- a/plugins/Referrers/Columns/Keyword.php +++ b/plugins/Referrers/Columns/Keyword.php @@ -41,10 +41,7 @@ public function getName() */ public function onNewVisit(Request $request, Visitor $visitor, $action) { - $referrerUrl = $request->getParam('urlref'); - $currentUrl = $request->getParam('url'); - - $information = $this->getReferrerInformation($referrerUrl, $currentUrl, $request->getIdSite()); + $information = $this->getReferrerInformationFromRequest($request); if (!empty($information['referer_keyword'])) { return substr($information['referer_keyword'], 0, 255); diff --git a/plugins/Referrers/Columns/ReferrerName.php b/plugins/Referrers/Columns/ReferrerName.php index fbd55219dcc..7bdb0732a67 100644 --- a/plugins/Referrers/Columns/ReferrerName.php +++ b/plugins/Referrers/Columns/ReferrerName.php @@ -35,10 +35,7 @@ protected function configureSegments() */ public function onNewVisit(Request $request, Visitor $visitor, $action) { - $referrerUrl = $request->getParam('urlref'); - $currentUrl = $request->getParam('url'); - - $information = $this->getReferrerInformation($referrerUrl, $currentUrl, $request->getIdSite()); + $information = $this->getReferrerInformationFromRequest($request); if (!empty($information['referer_name'])) { diff --git a/plugins/Referrers/Columns/ReferrerType.php b/plugins/Referrers/Columns/ReferrerType.php index f4e688f60e0..5303a1c360f 100644 --- a/plugins/Referrers/Columns/ReferrerType.php +++ b/plugins/Referrers/Columns/ReferrerType.php @@ -42,10 +42,7 @@ public function getName() */ public function onNewVisit(Request $request, Visitor $visitor, $action) { - $referrerUrl = $request->getParam('urlref'); - $currentUrl = $request->getParam('url'); - - $information = $this->getReferrerInformation($referrerUrl, $currentUrl, $request->getIdSite()); + $information = $this->getReferrerInformationFromRequest($request); return $information['referer_type']; } diff --git a/plugins/Referrers/Columns/ReferrerUrl.php b/plugins/Referrers/Columns/ReferrerUrl.php index 0404a132472..21c5cd5bdcc 100644 --- a/plugins/Referrers/Columns/ReferrerUrl.php +++ b/plugins/Referrers/Columns/ReferrerUrl.php @@ -35,10 +35,7 @@ protected function configureSegments() */ public function onNewVisit(Request $request, Visitor $visitor, $action) { - $referrerUrl = $request->getParam('urlref'); - $currentUrl = $request->getParam('url'); - - $information = $this->getReferrerInformation($referrerUrl, $currentUrl, $request->getIdSite()); + $information = $this->getReferrerInformationFromRequest($request); return $information['referer_url']; } diff --git a/plugins/Referrers/Columns/Website.php b/plugins/Referrers/Columns/Website.php index 53b143d6933..7d4903c902a 100644 --- a/plugins/Referrers/Columns/Website.php +++ b/plugins/Referrers/Columns/Website.php @@ -8,13 +8,50 @@ */ namespace Piwik\Plugins\Referrers\Columns; -use Piwik\Columns\Dimension; +use Piwik\Common; use Piwik\Piwik; +use Piwik\Tracker\Action; +use Piwik\Tracker\Request; +use Piwik\Tracker\TrackerConfig; +use Piwik\Tracker\Visitor; -class Website extends Dimension +class Website extends Base { + /** + * Set using the `[Tracker] tracker_create_new_visit_when_website_referrer_changes` INI config option. + * If true, will force new visits if the referrer website changes. + * + * @var bool + */ + protected $createNewVisitWhenWebsiteReferrerChanges; + + public function __construct() + { + $this->createNewVisitWhenWebsiteReferrerChanges = TrackerConfig::getConfigValue('tracker_create_new_visit_when_website_referrer_changes') == 1; + } + public function getName() { return Piwik::translate('General_Website'); } + + public function shouldForceNewVisit(Request $request, Visitor $visitor, Action $action = null) + { + if (!$this->createNewVisitWhenWebsiteReferrerChanges) { + return false; + } + + $information = $this->getReferrerInformationFromRequest($request); + + if ($this->doesLastOrCurrentActionHaveSameReferrer($visitor, $information, Common::REFERRER_TYPE_WEBSITE) + && $this->hasReferrerInformationChanged($visitor, $information) + ) { + Common::printDebug("Existing visit detected, but creating new visit because website referrer information is different than last action."); + + return true; + } + + return false; + + } } \ No newline at end of file From 7c4ffab00e7e07b79157fca61cc91d0c3b8911c0 Mon Sep 17 00:00:00 2001 From: diosmosis Date: Wed, 10 Dec 2014 15:48:19 -0800 Subject: [PATCH 2/7] Removing dead code in Tracker/Visit.php. --- core/Tracker/Visit.php | 9 --------- 1 file changed, 9 deletions(-) diff --git a/core/Tracker/Visit.php b/core/Tracker/Visit.php index 869434d4cd4..c019a1acf88 100644 --- a/core/Tracker/Visit.php +++ b/core/Tracker/Visit.php @@ -631,15 +631,6 @@ private function isVisitNew(Visitor $visitor, Action $action = null) return true; } - // if we should create a new visit when the referrer changes, check if referrer changed - if ($this->createNewVisitWhenWebsiteReferrerChanges - && $visitor->isReferrerInformationDifferent() - ) { - Common::printDebug("Existing visit detected, but creating new visit because referrer information is different than last action"); - - return true; - } - return !$visitor->isVisitorKnown(); } } From a6cb3c87cd55ab2278a0eeb82f3da3d432e46223 Mon Sep 17 00:00:00 2001 From: diosmosis Date: Thu, 11 Dec 2014 20:42:04 -0800 Subject: [PATCH 3/7] 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 From 09a15c217316139dbf07b5b428cc6f85bcc9ae81 Mon Sep 17 00:00:00 2001 From: diosmosis Date: Fri, 12 Dec 2014 15:43:16 -0800 Subject: [PATCH 4/7] Applying review changes: remove tracker_ prefix from config names, move shouldForceNewVisit to VisitDimension and simplify triggerPredicateHookOnDimensions. --- config/global.ini.php | 12 +++++----- core/Columns/Dimension.php | 22 ------------------- core/Plugin/Dimension/VisitDimension.php | 19 ++++++++++++++++ core/Tracker/Visit.php | 15 ++++--------- plugins/Referrers/Columns/Campaign.php | 2 +- plugins/Referrers/Columns/Website.php | 2 +- ...sCustomVariablesCampaignsNotHeuristics.php | 2 +- 7 files changed, 32 insertions(+), 42 deletions(-) diff --git a/config/global.ini.php b/config/global.ini.php index b4fde4c3582..65e42247b8a 100644 --- a/config/global.ini.php +++ b/config/global.ini.php @@ -586,15 +586,15 @@ ; Includes by default the GA style campaign keyword parameter utm_term campaign_keyword_var_name = "pk_kwd,pk_keyword,piwik_kwd,utm_term" -; if set to 1, actions that contain different campaign information from the last action in the current visit will +; if set to 1, actions that contain different campaign information from the visitor's in progress visit will ; be treated as the start of a new visit. This will include situations when campaign information was absent before, -; but is present now (or vice versa). -tracker_create_new_visit_when_campaign_changes = 1 +; but is present now. +create_new_visit_when_campaign_changes = 1 -; if set to 1, actions that contain different website referrer information from the last action in the current visit +; if set to 1, actions that contain different website referrer information from the visitor's in progress visit ; will be treatedas the start of a new visit. This will include situations when website referrer information was -; absent before, but is present now (or vice versa). -tracker_create_new_visit_when_website_referrer_changes = 0 +; absent before, but is present now. +create_new_visit_when_website_referrer_changes = 0 ; maximum length of a Page Title or a Page URL recorded in the log_action.name table page_maximum_length = 1024; diff --git a/core/Columns/Dimension.php b/core/Columns/Dimension.php index 0da3bcf8b3d..e760a090832 100644 --- a/core/Columns/Dimension.php +++ b/core/Columns/Dimension.php @@ -15,9 +15,6 @@ use Piwik\Plugin\Dimension\ConversionDimension; use Piwik\Plugin\Dimension\VisitDimension; use Piwik\Plugin\Segment; -use Piwik\Tracker\Action; -use Piwik\Tracker\Request; -use Piwik\Tracker\Visitor; use Piwik\Translate; /** @@ -169,25 +166,6 @@ public function getId() return $pluginName . '.' . $dimensionName; } - /** - * This hook is executed by the tracker when determining if an action is the start of a new visit - * or part of an existing one. Derived classes can use it to force new visits based on dimension - * data. - * - * For example, the Campaign dimension in the Referrers plugin will force a new visit if the - * campaign information for the current action is different from the last. - * - * @param Request $request The current tracker request information. - * @param Visitor $visitor The information for the currently recognized visitor. - * @param Action|null $action The current action information (if any). - * @return bool Return true to force a visit, false if otherwise. - * @api - */ - public function shouldForceNewVisit(Request $request, Visitor $visitor, Action $action = null) - { - return false; - } - /** * Gets an instance of all available visit, action and conversion dimension. * @return Dimension[] diff --git a/core/Plugin/Dimension/VisitDimension.php b/core/Plugin/Dimension/VisitDimension.php index 09a58554c09..b9aa3a9b2aa 100644 --- a/core/Plugin/Dimension/VisitDimension.php +++ b/core/Plugin/Dimension/VisitDimension.php @@ -269,6 +269,25 @@ public function onAnyGoalConversion(Request $request, Visitor $visitor, $action) return false; } + /** + * This hook is executed by the tracker when determining if an action is the start of a new visit + * or part of an existing one. Derived classes can use it to force new visits based on dimension + * data. + * + * For example, the Campaign dimension in the Referrers plugin will force a new visit if the + * campaign information for the current action is different from the last. + * + * @param Request $request The current tracker request information. + * @param Visitor $visitor The information for the currently recognized visitor. + * @param Action|null $action The current action information (if any). + * @return bool Return true to force a visit, false if otherwise. + * @api + */ + public function shouldForceNewVisit(Request $request, Visitor $visitor, Action $action = null) + { + return false; + } + /** * Get all visit dimensions that are defined by all activated plugins. * @return VisitDimension[] diff --git a/core/Tracker/Visit.php b/core/Tracker/Visit.php index 2530dc774b6..4e108450b5d 100644 --- a/core/Tracker/Visit.php +++ b/core/Tracker/Visit.php @@ -542,21 +542,14 @@ private function triggerHookOnDimensions($dimensions, $hook, $visitor, $action, return $valuesToUpdate; } - private function triggerPredicateHookOnDimensions($dimensions, $hook, Visitor $visitor, Action $action = null, $returnFirstTrue = true) + private function triggerPredicateHookOnDimensions($dimensions, $hook, Visitor $visitor, Action $action = null) { - $result = $returnFirstTrue ? false : array(); foreach ($dimensions as $dimension) { - $value = $dimension->$hook($this->request, $visitor, $action); - - if ($returnFirstTrue) { - if ($value) { - return true; - } - } else { - $result[] = $value; + if ($dimension->$hook($this->request, $visitor, $action)) { + return true; } } - return $result; + return false; } protected function getAllVisitDimensions() diff --git a/plugins/Referrers/Columns/Campaign.php b/plugins/Referrers/Columns/Campaign.php index df648fbd34d..5074578b161 100644 --- a/plugins/Referrers/Columns/Campaign.php +++ b/plugins/Referrers/Columns/Campaign.php @@ -27,7 +27,7 @@ class Campaign extends Base public function __construct() { - $this->createNewVisitWhenCampaignChanges = TrackerConfig::getConfigValue('tracker_create_new_visit_when_campaign_changes') == 1; + $this->createNewVisitWhenCampaignChanges = TrackerConfig::getConfigValue('create_new_visit_when_campaign_changes') == 1; } public function getName() diff --git a/plugins/Referrers/Columns/Website.php b/plugins/Referrers/Columns/Website.php index a806b8e8987..3bd5618b1d1 100644 --- a/plugins/Referrers/Columns/Website.php +++ b/plugins/Referrers/Columns/Website.php @@ -27,7 +27,7 @@ class Website extends Base public function __construct() { - $this->createNewVisitWhenWebsiteReferrerChanges = TrackerConfig::getConfigValue('tracker_create_new_visit_when_website_referrer_changes') == 1; + $this->createNewVisitWhenWebsiteReferrerChanges = TrackerConfig::getConfigValue('create_new_visit_when_website_referrer_changes') == 1; } public function getName() diff --git a/tests/PHPUnit/Fixtures/SomeVisitsCustomVariablesCampaignsNotHeuristics.php b/tests/PHPUnit/Fixtures/SomeVisitsCustomVariablesCampaignsNotHeuristics.php index b4adc3dbd17..678f1d50e64 100644 --- a/tests/PHPUnit/Fixtures/SomeVisitsCustomVariablesCampaignsNotHeuristics.php +++ b/tests/PHPUnit/Fixtures/SomeVisitsCustomVariablesCampaignsNotHeuristics.php @@ -37,7 +37,7 @@ public function tearDown() private function setPiwikEnviornmentOverrides() { $configOverride = $this->getTestEnvironment()->configOverride; - $configOverride['Tracker']['tracker_create_new_visit_when_website_referrer_changes'] = 1; + $configOverride['Tracker']['create_new_visit_when_website_referrer_changes'] = 1; $this->getTestEnvironment()->configOverride = $configOverride; $this->getTestEnvironment()->save(); } From 2d440603fe2613b8ae18ed740d3992b9a799a1a4 Mon Sep 17 00:00:00 2001 From: diosmosis Date: Fri, 12 Dec 2014 15:48:22 -0800 Subject: [PATCH 5/7] Tweak new option comments. --- config/global.ini.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/config/global.ini.php b/config/global.ini.php index 65e42247b8a..2b63eda9a65 100644 --- a/config/global.ini.php +++ b/config/global.ini.php @@ -586,12 +586,12 @@ ; Includes by default the GA style campaign keyword parameter utm_term campaign_keyword_var_name = "pk_kwd,pk_keyword,piwik_kwd,utm_term" -; if set to 1, actions that contain different campaign information from the visitor's in progress visit will +; if set to 1, actions that contain different campaign information from the visitor's ongoing visit will ; be treated as the start of a new visit. This will include situations when campaign information was absent before, ; but is present now. create_new_visit_when_campaign_changes = 1 -; if set to 1, actions that contain different website referrer information from the visitor's in progress visit +; if set to 1, actions that contain different website referrer information from the visitor's ongoingg visit ; will be treatedas the start of a new visit. This will include situations when website referrer information was ; absent before, but is present now. create_new_visit_when_website_referrer_changes = 0 From 568b65db641fb373986a83d53fe556276f3c2c33 Mon Sep 17 00:00:00 2001 From: diosmosis Date: Fri, 12 Dec 2014 15:49:50 -0800 Subject: [PATCH 6/7] Fix typo in last commit. --- config/global.ini.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config/global.ini.php b/config/global.ini.php index 2b63eda9a65..4ce72bd8135 100644 --- a/config/global.ini.php +++ b/config/global.ini.php @@ -591,7 +591,7 @@ ; but is present now. create_new_visit_when_campaign_changes = 1 -; if set to 1, actions that contain different website referrer information from the visitor's ongoingg visit +; if set to 1, actions that contain different website referrer information from the visitor's ongoing visit ; will be treatedas the start of a new visit. This will include situations when website referrer information was ; absent before, but is present now. create_new_visit_when_website_referrer_changes = 0 From 3a0571006d584be19979db511cfcb89768cd693c Mon Sep 17 00:00:00 2001 From: diosmosis Date: Fri, 12 Dec 2014 15:53:22 -0800 Subject: [PATCH 7/7] Fixing more typos. --- plugins/Referrers/Columns/Campaign.php | 2 +- plugins/Referrers/Columns/Website.php | 2 +- .../SomeVisitsCustomVariablesCampaignsNotHeuristics.php | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/plugins/Referrers/Columns/Campaign.php b/plugins/Referrers/Columns/Campaign.php index 5074578b161..ff2d5c24012 100644 --- a/plugins/Referrers/Columns/Campaign.php +++ b/plugins/Referrers/Columns/Campaign.php @@ -18,7 +18,7 @@ class Campaign extends Base { /** - * Obtained from the `[Tracker] tracker_create_new_visit_when_campaign_changes` INI config option. + * Obtained from the `[Tracker] create_new_visit_when_campaign_changes` INI config option. * If true, will create new visits when campaign name changes. * * @var bool diff --git a/plugins/Referrers/Columns/Website.php b/plugins/Referrers/Columns/Website.php index 3bd5618b1d1..42d05068d60 100644 --- a/plugins/Referrers/Columns/Website.php +++ b/plugins/Referrers/Columns/Website.php @@ -18,7 +18,7 @@ class Website extends Base { /** - * Set using the `[Tracker] tracker_create_new_visit_when_website_referrer_changes` INI config option. + * Set using the `[Tracker] create_new_visit_when_website_referrer_changes` INI config option. * If true, will force new visits if the referrer website changes. * * @var bool diff --git a/tests/PHPUnit/Fixtures/SomeVisitsCustomVariablesCampaignsNotHeuristics.php b/tests/PHPUnit/Fixtures/SomeVisitsCustomVariablesCampaignsNotHeuristics.php index 678f1d50e64..5c3dcc553ee 100644 --- a/tests/PHPUnit/Fixtures/SomeVisitsCustomVariablesCampaignsNotHeuristics.php +++ b/tests/PHPUnit/Fixtures/SomeVisitsCustomVariablesCampaignsNotHeuristics.php @@ -25,7 +25,7 @@ class SomeVisitsCustomVariablesCampaignsNotHeuristics extends Fixture public function setUp() { - $this->setPiwikEnviornmentOverrides(); + $this->setPiwikEnvironmentOverrides(); $this->setUpWebsitesAndGoals(); $this->trackVisits(); } @@ -34,7 +34,7 @@ public function tearDown() { } - private function setPiwikEnviornmentOverrides() + private function setPiwikEnvironmentOverrides() { $configOverride = $this->getTestEnvironment()->configOverride; $configOverride['Tracker']['create_new_visit_when_website_referrer_changes'] = 1;