diff --git a/core/Tracker/LogTable.php b/core/Tracker/LogTable.php index 0348a1540c7..2c59513318c 100644 --- a/core/Tracker/LogTable.php +++ b/core/Tracker/LogTable.php @@ -120,4 +120,14 @@ public function getPrimaryKey() { return array(); } + + /** + * Defines if the log table holds an `idvisitor` column. + * If set to yes, existing records are automatically updated when the visitor id changes during a visit + * + */ + public function hasIdVisitorColumn(): bool + { + return false; + } } diff --git a/core/Tracker/Model.php b/core/Tracker/Model.php index 1d5562decdd..e2f96d5019e 100644 --- a/core/Tracker/Model.php +++ b/core/Tracker/Model.php @@ -464,6 +464,30 @@ public function updateAction($idLinkVa, $valuesToUpdate) return $wasInserted; } + public function updateIdVisitorInLogTable(string $logTable, string $idVisitor, array $conditions): bool + { + if (empty($idVisitor) || empty($conditions)) { + return false; + } + + $table = Common::prefixTable($logTable); + + $sqlQuery = "UPDATE `$table` SET `idvisitor` = ? WHERE "; + $sqlConditions = []; + $sqlBind = [$idVisitor]; + + foreach ($conditions as $name => $value) { + $sqlConditions[] = $name . " = ?"; + $sqlBind[] = $value; + } + + $sqlQuery .= implode(' AND ', $sqlConditions); + + $db = $this->getDb(); + $result = $db->query($sqlQuery, $sqlBind); + return $db->rowCount($result) != 0; + } + /** * Attempt to find an existing visit record in the database * diff --git a/core/Tracker/Visit.php b/core/Tracker/Visit.php index fbba48706ce..4b2b2ebe4b6 100644 --- a/core/Tracker/Visit.php +++ b/core/Tracker/Visit.php @@ -14,6 +14,7 @@ use Piwik\Container\StaticContainer; use Matomo\Network\IPUtils; use Piwik\Plugin\Dimension\VisitDimension; +use Piwik\Plugin\LogTablesProvider; use Piwik\Plugins\Actions\Tracker\ActionsRequestProcessor; use Piwik\Tracker; use Piwik\Tracker\Visit\VisitProperties; @@ -375,8 +376,11 @@ protected function updateExistingVisit($valuesToUpdate) $wasInserted = $this->getModel()->updateVisit($idSite, $idVisit, $valuesToUpdate); - // Debug output if (isset($valuesToUpdate['idvisitor'])) { + $this->updateIdVisitorAcrossLogTables($valuesToUpdate['idvisitor']); + Common::printDebug('Updating idvisitor across tables for idvisit = ' . $idVisit); + + //For debug output below $valuesToUpdate['idvisitor'] = bin2hex($valuesToUpdate['idvisitor']); } @@ -394,6 +398,26 @@ protected function updateExistingVisit($valuesToUpdate) } } + protected function updateIdVisitorAcrossLogTables(string $idVisitor): void + { + $allLogTables = StaticContainer::get(LogTablesProvider::class)->getAllLogTables(); + + foreach ($allLogTables as $logTable) { + if (!$logTable->hasIdVisitorColumn() || $logTable instanceof \Piwik\Plugins\CoreHome\Tracker\LogTable\Visit) { + // skip all log tables that do not contain the idvisitor column, and `log_visit`, as it is already handled + continue; + } + + $conditions = ['idvisitor' => $this->previousVisitProperties->getProperty('idvisitor')]; + + if ($logTable->getIdColumn() !== 'idvisitor' && $logTable->getColumnToJoinOnIdVisit()) { + $conditions[$logTable->getColumnToJoinOnIdVisit()] = $this->visitProperties->getProperty('idvisit'); + } + + $this->getModel()->updateIdVisitorInLogTable($logTable->getName(), $idVisitor, $conditions); + } + } + private function printVisitorInformation() { $debugVisitInfo = $this->visitProperties->getProperties(); diff --git a/plugins/CoreHome/Tracker/LogTable/Conversion.php b/plugins/CoreHome/Tracker/LogTable/Conversion.php index 27f7b0bbd98..5e5937560ce 100644 --- a/plugins/CoreHome/Tracker/LogTable/Conversion.php +++ b/plugins/CoreHome/Tracker/LogTable/Conversion.php @@ -37,4 +37,9 @@ public function getPrimaryKey() { return array('idvisit', 'idgoal', 'buster'); } + + public function hasIdVisitorColumn(): bool + { + return true; + } } diff --git a/plugins/CoreHome/Tracker/LogTable/ConversionItem.php b/plugins/CoreHome/Tracker/LogTable/ConversionItem.php index 499cc96afc9..2dc92104c10 100644 --- a/plugins/CoreHome/Tracker/LogTable/ConversionItem.php +++ b/plugins/CoreHome/Tracker/LogTable/ConversionItem.php @@ -32,4 +32,9 @@ public function getPrimaryKey() { return array('idvisit', 'idorder', 'idaction_sku'); } + + public function hasIdVisitorColumn(): bool + { + return true; + } } diff --git a/plugins/CoreHome/Tracker/LogTable/LinkVisitAction.php b/plugins/CoreHome/Tracker/LogTable/LinkVisitAction.php index a80e01fc42a..3ab25441917 100644 --- a/plugins/CoreHome/Tracker/LogTable/LinkVisitAction.php +++ b/plugins/CoreHome/Tracker/LogTable/LinkVisitAction.php @@ -42,4 +42,9 @@ public function getPrimaryKey() { return array('idlink_va'); } + + public function hasIdVisitorColumn(): bool + { + return true; + } } diff --git a/plugins/CoreHome/Tracker/LogTable/Visit.php b/plugins/CoreHome/Tracker/LogTable/Visit.php index 2ff1fa0d436..04daa8ed875 100644 --- a/plugins/CoreHome/Tracker/LogTable/Visit.php +++ b/plugins/CoreHome/Tracker/LogTable/Visit.php @@ -42,4 +42,9 @@ public function getPrimaryKey() { return array('idvisit'); } + + public function hasIdVisitorColumn(): bool + { + return true; + } } diff --git a/tests/PHPUnit/Integration/Tracker/UserIdVisitorIdTest.php b/tests/PHPUnit/Integration/Tracker/UserIdVisitorIdTest.php index 4b00c9eaa7a..2de18cc24b9 100644 --- a/tests/PHPUnit/Integration/Tracker/UserIdVisitorIdTest.php +++ b/tests/PHPUnit/Integration/Tracker/UserIdVisitorIdTest.php @@ -780,6 +780,10 @@ private function assertVisitorIdsCount($expected) { $visitorIdsCount = Db::fetchOne("SELECT COUNT(DISTINCT idvisitor) FROM " . Common::prefixTable('log_visit')); $this->assertEquals($expected, $visitorIdsCount); + + // check that idvisitor column was updated. + $visitorIdsCount = Db::fetchOne("SELECT COUNT(DISTINCT idvisitor) FROM " . Common::prefixTable('log_link_visit_action')); + $this->assertEquals($expected, $visitorIdsCount); } private function assertUserIdsCount($expected) diff --git a/tests/PHPUnit/System/TimezonesTest.php b/tests/PHPUnit/System/TimezonesTest.php index c528bb4b540..9ee7080a000 100644 --- a/tests/PHPUnit/System/TimezonesTest.php +++ b/tests/PHPUnit/System/TimezonesTest.php @@ -52,6 +52,9 @@ public function getApiForTesting() 'filter_limit' => 100, 'doNotFetchActions' => 1, ], + 'xmlFieldsToRemove' => [ + 'idVisit', + ], 'testSuffix' => '_yesterday' . $appendix, ], ]; @@ -65,6 +68,9 @@ public function getApiForTesting() 'filter_limit' => 100, 'doNotFetchActions' => 1, ], + 'xmlFieldsToRemove' => [ + 'idVisit', + ], 'testSuffix' => '_today' . $appendix, ], ]; @@ -78,6 +84,9 @@ public function getApiForTesting() 'filter_limit' => 100, 'doNotFetchActions' => 1, ], + 'xmlFieldsToRemove' => [ + 'idVisit', + ], 'testSuffix' => '_range' . $appendix, ], ]; @@ -92,6 +101,9 @@ public function getApiForTesting() 'doNotFetchActions' => 1, ], 'segment' => 'pageUrl=@example.org;pageUrl=@index', + 'xmlFieldsToRemove' => [ + 'idVisit', + ], 'testSuffix' => '_range' . $appendix, // using same suffix as results are the same ], ]; @@ -106,6 +118,9 @@ public function getApiForTesting() 'doNotFetchActions' => 1, ], 'segment' => 'pageUrl=@example.org;pageUrl!@index', + 'xmlFieldsToRemove' => [ + 'idVisit', + ], 'testSuffix' => '_range_nomatch' . $appendix, // this segment should match nothing ], ]; diff --git a/tests/PHPUnit/System/expected/test_ChangingVisitorIds__Actions.getPageUrls_day.xml b/tests/PHPUnit/System/expected/test_ChangingVisitorIds__Actions.getPageUrls_day.xml index 0dac0ab21dc..53e4829789f 100644 --- a/tests/PHPUnit/System/expected/test_ChangingVisitorIds__Actions.getPageUrls_day.xml +++ b/tests/PHPUnit/System/expected/test_ChangingVisitorIds__Actions.getPageUrls_day.xml @@ -3,7 +3,7 @@ 2 - 4 + 2 4 90 0 @@ -43,7 +43,7 @@ 1 - 2 + 1 2 26 0 diff --git a/tests/PHPUnit/System/expected/test_ChangingVisitorIds__Actions.getPageUrls_month.xml b/tests/PHPUnit/System/expected/test_ChangingVisitorIds__Actions.getPageUrls_month.xml index bff9b503b6b..44687baf5d1 100644 --- a/tests/PHPUnit/System/expected/test_ChangingVisitorIds__Actions.getPageUrls_month.xml +++ b/tests/PHPUnit/System/expected/test_ChangingVisitorIds__Actions.getPageUrls_month.xml @@ -29,7 +29,7 @@ 4 - 12 + 8 6 6 0 @@ -64,7 +64,7 @@ 2 - 6 + 5 2 4 66 diff --git a/tests/PHPUnit/System/expected/test_ChangingVisitorIds__Events.getCategory_day.xml b/tests/PHPUnit/System/expected/test_ChangingVisitorIds__Events.getCategory_day.xml index 4e536cd3190..e96d3af2fdc 100644 --- a/tests/PHPUnit/System/expected/test_ChangingVisitorIds__Events.getCategory_day.xml +++ b/tests/PHPUnit/System/expected/test_ChangingVisitorIds__Events.getCategory_day.xml @@ -2,7 +2,7 @@ - 2 + 1 1 2 0 @@ -14,7 +14,7 @@ - 2 + 1 1 2 0 diff --git a/tests/PHPUnit/System/expected/test_ChangingVisitorIds__Events.getCategory_month.xml b/tests/PHPUnit/System/expected/test_ChangingVisitorIds__Events.getCategory_month.xml index 4cdd755f1a6..3152e313b97 100644 --- a/tests/PHPUnit/System/expected/test_ChangingVisitorIds__Events.getCategory_month.xml +++ b/tests/PHPUnit/System/expected/test_ChangingVisitorIds__Events.getCategory_month.xml @@ -8,7 +8,7 @@ 0 - 6 + 3 0 eventCategory==category @@ -20,7 +20,7 @@ 0 - 6 + 3 0 diff --git a/tests/PHPUnit/System/expected/test_ChangingVisitorIds__Goals.getItemsSku_day.xml b/tests/PHPUnit/System/expected/test_ChangingVisitorIds__Goals.getItemsSku_day.xml index 2ac08f4061c..b6b4d9fd888 100644 --- a/tests/PHPUnit/System/expected/test_ChangingVisitorIds__Goals.getItemsSku_day.xml +++ b/tests/PHPUnit/System/expected/test_ChangingVisitorIds__Goals.getItemsSku_day.xml @@ -3,7 +3,7 @@ 1 - 2 + 1 2 500 0 diff --git a/tests/PHPUnit/System/expected/test_ChangingVisitorIds__Goals.getItemsSku_month.xml b/tests/PHPUnit/System/expected/test_ChangingVisitorIds__Goals.getItemsSku_month.xml index b41302d7865..faa29a7608b 100644 --- a/tests/PHPUnit/System/expected/test_ChangingVisitorIds__Goals.getItemsSku_month.xml +++ b/tests/PHPUnit/System/expected/test_ChangingVisitorIds__Goals.getItemsSku_month.xml @@ -4,7 +4,7 @@ 5 6 - 6 + 5 1500 0 0%