Merge remote-tracking branch 'origin/3.x-dev' into 4.x-dev

2 files changed, 131 insertions, 1 deletions

diff --git a/plugins/CoreHome/Columns/VisitLastActionTime.php b/plugins/CoreHome/Columns/VisitLastActionTime.php
index bc3ba0f5b1..6dadc3a0ad 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..2d82d93689
--- /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(): void
+    {
+        parent::setUp();
+        $this->lastAction = new VisitLastActionTime();
+    }
+
+    public function tearDown(): void
+    {
+        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));
+    }
+}