diff options
author | Thomas Steur <tsteur@users.noreply.github.com> | 2020-04-29 01:54:09 +0300 |
---|---|---|
committer | GitHub <noreply@github.com> | 2020-04-29 01:54:09 +0300 |
commit | 2466e9f4447e2491faea1201c838657e2385cd94 (patch) | |
tree | f49b08c30f6b42540b7ecc8bced4736af5eacbc3 /plugins | |
parent | f88b7fbcfac0aa4f0563bbbffa3c6a5965c41181 (diff) |
Update visit only when needed (#15869)
* update idvisitor only when needed
* better implementation
* fix tests
Diffstat (limited to 'plugins')
6 files changed, 148 insertions, 18 deletions
diff --git a/plugins/CoreHome/Columns/VisitLastActionTime.php b/plugins/CoreHome/Columns/VisitLastActionTime.php index ed341ec673..802f9a1755 100644 --- a/plugins/CoreHome/Columns/VisitLastActionTime.php +++ b/plugins/CoreHome/Columns/VisitLastActionTime.php @@ -14,6 +14,7 @@ use Piwik\Tracker\Action; use Piwik\Tracker\Request; use Piwik\Tracker\Visitor; use Piwik\Metrics\Formatter; +use Piwik\Tracker\VisitorRecognizer; require_once PIWIK_INCLUDE_PATH . '/plugins/VisitTime/functions.php'; @@ -67,7 +68,15 @@ class VisitLastActionTime extends VisitDimension if ($request->getParam('ping') == 1) { return false; } - + + $originalVisit = $visitor->getVisitorColumn(VisitorRecognizer::KEY_ORIGINAL_VISIT_ROW); + + if (!empty($originalVisit['visit_last_action_time']) + && Date::factory($originalVisit['visit_last_action_time'])->getTimestamp() > $request->getCurrentTimestamp()) { + // make sure to not set visit_last_action_time to an earlier time eg if tracking requests aren't sent in order + return $originalVisit['visit_last_action_time']; + } + return $this->onNewVisit($request, $visitor, $action); } }
\ No newline at end of file diff --git a/plugins/CoreHome/tests/Integration/Column/VisitLastActionTimeTest.php b/plugins/CoreHome/tests/Integration/Column/VisitLastActionTimeTest.php new file mode 100644 index 0000000000..d58e429960 --- /dev/null +++ b/plugins/CoreHome/tests/Integration/Column/VisitLastActionTimeTest.php @@ -0,0 +1,121 @@ +<?php +/** + * Piwik - free/libre analytics platform + * + * @link https://matomo.org + * @license http://www.gnu.org/licenses/gpl-3.0.html GPL v3 or later + */ + +namespace Piwik\Plugins\CoreHome\tests\Integration\Column; + +use Piwik\Cache; +use Piwik\Common; +use Piwik\DataAccess\ArchiveTableCreator; +use Piwik\Date; +use Piwik\Db; +use Piwik\Metrics; +use Piwik\Plugins\CoreHome\Columns\UserId; +use Piwik\Plugins\CoreHome\Columns\VisitLastActionTime; +use Piwik\Tests\Framework\Fixture; +use Piwik\Tests\Framework\Mock\FakeAccess; +use Piwik\Tests\Framework\TestCase\IntegrationTestCase; +use Piwik\DataTable; +use Piwik\Tracker\Request; +use Piwik\Tracker\Visit\VisitProperties; +use Piwik\Tracker\Visitor; +use Piwik\Tracker\VisitorRecognizer; + +/** + * @group CoreHome + * @group VisitLastActionTimeTest + * @group Plugins + * @group Column + */ +class VisitLastActionTimeTest extends IntegrationTestCase +{ + /** + * @var VisitLastActionTime + */ + private $lastAction; + + public function setUp() + { + parent::setUp(); + $this->lastAction = new VisitLastActionTime(); + } + + public function tearDown() + { + parent::tearDown(); + } + + + private function makeRequest($request) + { + $request['idsite'] = 1; + + return new Request($request); + } + + private function getVisitor() + { + $visit = new VisitProperties(); + $visit->setProperty('idvisit', '321'); + $visit->setProperty('idvisitor', Common::hex2bin('1234567890234567')); + $visitor = new Visitor($visit, $isKnown = false); + + return $visitor; + } + + public function test_onExistingVisit_whenPing() + { + $request = $this->makeRequest(array('ping' => 1)); + $visitor = $this->getVisitor(); + $this->assertFalse($this->lastAction->onExistingVisit($request, $visitor, $action = null)); + } + + public function test_onExistingVisit_whenNewVisitReturnsTimeFromRequest() + { + $now = time() - 5; // -5 so we make sure this time is used and not actually now + $request = $this->makeRequest(array('cdt' => $now)); + $this->assertEquals($now, $request->getCurrentTimestamp()); + + $visitor = $this->getVisitor(); + + $expected = Date::factory($now)->getDatetime(); + $this->assertSame($expected, $this->lastAction->onExistingVisit($request, $visitor, $action = null)); + } + + public function test_onExistingVisit_whenKnownVisitRequestTimeIsNewer() + { + $now = time() - 5; // -5 so we make sure this time is used and not actually now + $previousTime = $now - 10; // is older + $request = $this->makeRequest(array('cdt' => $now)); + $this->assertEquals($now, $request->getCurrentTimestamp()); + + $visitor = $this->getVisitor(); + $visitor->setVisitorColumn(VisitorRecognizer::KEY_ORIGINAL_VISIT_ROW, + array('visit_last_action_time' => Date::factory($previousTime)->getDatetime()) + ); + + $expected = Date::factory($now)->getDatetime(); + $this->assertSame($expected, $this->lastAction->onExistingVisit($request, $visitor, $action = null)); + } + + public function test_onExistingVisit_whenKnownVisitAndPreviousVisitTimeIsNewer() + { + $now = time() - 5; // -5 so we make sure this time is used and not actually now + $previousTime = $now + 10; // is newer + $request = $this->makeRequest(array('cdt' => $now)); + $this->assertEquals($now, $request->getCurrentTimestamp()); + + $visitor = $this->getVisitor(); + $visitor->setVisitorColumn(VisitorRecognizer::KEY_ORIGINAL_VISIT_ROW, + array('visit_last_action_time' => Date::factory($previousTime)->getDatetime()) + ); + + $expected = Date::factory($previousTime)->getDatetime(); + // should keep existing visit last action time + $this->assertSame($expected, $this->lastAction->onExistingVisit($request, $visitor, $action = null)); + } +} diff --git a/plugins/Goals/tests/System/expected/test_trackGoals_allowMultipleConversionsPerVisit__Goals.get_day.xml b/plugins/Goals/tests/System/expected/test_trackGoals_allowMultipleConversionsPerVisit__Goals.get_day.xml index 27200051aa..3a8d1b2d0c 100644 --- a/plugins/Goals/tests/System/expected/test_trackGoals_allowMultipleConversionsPerVisit__Goals.get_day.xml +++ b/plugins/Goals/tests/System/expected/test_trackGoals_allowMultipleConversionsPerVisit__Goals.get_day.xml @@ -1,14 +1,14 @@ <?xml version="1.0" encoding="utf-8" ?> <result> - <nb_conversions>8</nb_conversions> + <nb_conversions>7</nb_conversions> <nb_visits_converted>2</nb_visits_converted> <revenue>1332</revenue> <conversion_rate>100%</conversion_rate> - <nb_conversions_new_visit>6</nb_conversions_new_visit> + <nb_conversions_new_visit>4</nb_conversions_new_visit> <nb_visits_converted_new_visit>1</nb_visits_converted_new_visit> <revenue_new_visit>1332</revenue_new_visit> <conversion_rate_new_visit>100%</conversion_rate_new_visit> - <nb_conversions_returning_visit>2</nb_conversions_returning_visit> + <nb_conversions_returning_visit>3</nb_conversions_returning_visit> <nb_visits_converted_returning_visit>1</nb_visits_converted_returning_visit> <revenue_returning_visit>0</revenue_returning_visit> <conversion_rate_returning_visit>100%</conversion_rate_returning_visit> diff --git a/plugins/Goals/tests/System/expected/test_trackGoals_allowMultipleConversionsPerVisit__VisitTime.getVisitInformationPerServerTime_day.xml b/plugins/Goals/tests/System/expected/test_trackGoals_allowMultipleConversionsPerVisit__VisitTime.getVisitInformationPerServerTime_day.xml index 4a3f7b7062..c5be750480 100644 --- a/plugins/Goals/tests/System/expected/test_trackGoals_allowMultipleConversionsPerVisit__VisitTime.getVisitInformationPerServerTime_day.xml +++ b/plugins/Goals/tests/System/expected/test_trackGoals_allowMultipleConversionsPerVisit__VisitTime.getVisitInformationPerServerTime_day.xml @@ -6,9 +6,9 @@ <nb_visits>2</nb_visits> <nb_actions>5</nb_actions> <nb_users>0</nb_users> - <max_actions>3</max_actions> - <sum_visit_length>363</sum_visit_length> - <bounce_count>0</bounce_count> + <max_actions>4</max_actions> + <sum_visit_length>1121</sum_visit_length> + <bounce_count>1</bounce_count> <nb_visits_converted>2</nb_visits_converted> <goals> <row idgoal='1'> @@ -22,8 +22,8 @@ <revenue>666</revenue> </row> <row idgoal='3'> - <nb_conversions>2</nb_conversions> - <nb_visits_converted>2</nb_visits_converted> + <nb_conversions>1</nb_conversions> + <nb_visits_converted>1</nb_visits_converted> <revenue>0</revenue> </row> <row idgoal='4'> @@ -37,7 +37,7 @@ <revenue>0</revenue> </row> </goals> - <nb_conversions>8</nb_conversions> + <nb_conversions>7</nb_conversions> <revenue>1332</revenue> <segment>visitStartServerHour==0</segment> </row> diff --git a/plugins/Goals/tests/System/expected/test_trackGoals_allowMultipleConversionsPerVisit__VisitsSummary.get_day.xml b/plugins/Goals/tests/System/expected/test_trackGoals_allowMultipleConversionsPerVisit__VisitsSummary.get_day.xml index 7168091d51..bfbd730f69 100644 --- a/plugins/Goals/tests/System/expected/test_trackGoals_allowMultipleConversionsPerVisit__VisitsSummary.get_day.xml +++ b/plugins/Goals/tests/System/expected/test_trackGoals_allowMultipleConversionsPerVisit__VisitsSummary.get_day.xml @@ -5,10 +5,10 @@ <nb_visits>2</nb_visits> <nb_actions>5</nb_actions> <nb_visits_converted>2</nb_visits_converted> - <bounce_count>0</bounce_count> - <sum_visit_length>363</sum_visit_length> - <max_actions>3</max_actions> - <bounce_rate>0%</bounce_rate> + <bounce_count>1</bounce_count> + <sum_visit_length>1121</sum_visit_length> + <max_actions>4</max_actions> + <bounce_rate>50%</bounce_rate> <nb_actions_per_visit>2.5</nb_actions_per_visit> - <avg_time_on_site>182</avg_time_on_site> + <avg_time_on_site>561</avg_time_on_site> </result>
\ No newline at end of file diff --git a/plugins/Goals/tests/System/expected/test_trackGoals_allowMultipleConversionsPerVisit_withLogLinkVisitActionSegment__Goals.get_day.xml b/plugins/Goals/tests/System/expected/test_trackGoals_allowMultipleConversionsPerVisit_withLogLinkVisitActionSegment__Goals.get_day.xml index 27200051aa..3a8d1b2d0c 100644 --- a/plugins/Goals/tests/System/expected/test_trackGoals_allowMultipleConversionsPerVisit_withLogLinkVisitActionSegment__Goals.get_day.xml +++ b/plugins/Goals/tests/System/expected/test_trackGoals_allowMultipleConversionsPerVisit_withLogLinkVisitActionSegment__Goals.get_day.xml @@ -1,14 +1,14 @@ <?xml version="1.0" encoding="utf-8" ?> <result> - <nb_conversions>8</nb_conversions> + <nb_conversions>7</nb_conversions> <nb_visits_converted>2</nb_visits_converted> <revenue>1332</revenue> <conversion_rate>100%</conversion_rate> - <nb_conversions_new_visit>6</nb_conversions_new_visit> + <nb_conversions_new_visit>4</nb_conversions_new_visit> <nb_visits_converted_new_visit>1</nb_visits_converted_new_visit> <revenue_new_visit>1332</revenue_new_visit> <conversion_rate_new_visit>100%</conversion_rate_new_visit> - <nb_conversions_returning_visit>2</nb_conversions_returning_visit> + <nb_conversions_returning_visit>3</nb_conversions_returning_visit> <nb_visits_converted_returning_visit>1</nb_visits_converted_returning_visit> <revenue_returning_visit>0</revenue_returning_visit> <conversion_rate_returning_visit>100%</conversion_rate_returning_visit> |