Skip to content

Commit

Permalink
Do not create new visit if no referrer info in current action, but re…
Browse files Browse the repository at this point in the history
…ferrer info present in last action. Also add integration + system tests for new behavior.
  • Loading branch information
diosmosis committed Dec 12, 2014
1 parent 7c4ffab commit a6cb3c8
Show file tree
Hide file tree
Showing 9 changed files with 275 additions and 26 deletions.
2 changes: 1 addition & 1 deletion core/Tracker/Visit.php
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down
7 changes: 3 additions & 4 deletions plugins/Referrers/Columns/Base.php
Original file line number Diff line number Diff line change
Expand Up @@ -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)) {
Expand All @@ -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;
}
}
4 changes: 2 additions & 2 deletions plugins/Referrers/Columns/Campaign.php
Original file line number Diff line number Diff line change
Expand Up @@ -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.");

Expand Down
4 changes: 2 additions & 2 deletions plugins/Referrers/Columns/Website.php
Original file line number Diff line number Diff line change
Expand Up @@ -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.");

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ class SomeVisitsCustomVariablesCampaignsNotHeuristics extends Fixture

public function setUp()
{
$this->setPiwikEnviornmentOverrides();
$this->setUpWebsitesAndGoals();
$this->trackVisits();
}
Expand All @@ -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)) {
Expand Down Expand Up @@ -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());
Expand Down Expand Up @@ -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
Expand Down
85 changes: 85 additions & 0 deletions tests/PHPUnit/Integration/Tracker/VisitTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand All @@ -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>
Expand Down Expand Up @@ -145,6 +145,46 @@
</row>
</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>
Expand Down
Loading

0 comments on commit a6cb3c8

Please sign in to comment.