Automatically update idvisitor column across log tables on change

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;
+    }
 }

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
      *

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();

plugins/CoreHome/Tracker/LogTable/Conversion.php

@@ -37,4 +37,9 @@ public function getPrimaryKey()
     {
         return array('idvisit', 'idgoal', 'buster');
     }
+
+    public function hasIdVisitorColumn(): bool
+    {
+        return true;
+    }
 }

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;
+    }
 }

plugins/CoreHome/Tracker/LogTable/LinkVisitAction.php

@@ -42,4 +42,9 @@ public function getPrimaryKey()
     {
         return array('idlink_va');
     }
+
+    public function hasIdVisitorColumn(): bool
+    {
+        return true;
+    }
 }

plugins/CoreHome/Tracker/LogTable/Visit.php

@@ -42,4 +42,9 @@ public function getPrimaryKey()
     {
         return array('idvisit');
     }
+
+    public function hasIdVisitorColumn(): bool
+    {
+        return true;
+    }
 }

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)

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
                     ],
                 ];

tests/PHPUnit/System/expected/test_ChangingVisitorIds__Actions.getPageUrls_day.xml

@@ -3,7 +3,7 @@
 	<row>
 		<label>/index.htm</label>
 		<nb_visits>2</nb_visits>
-		<nb_uniq_visitors>4</nb_uniq_visitors>
+		<nb_uniq_visitors>2</nb_uniq_visitors>
 		<nb_hits>4</nb_hits>
 		<sum_time_spent>90</sum_time_spent>
 		<sum_bandwidth>0</sum_bandwidth>
@@ -43,7 +43,7 @@
 	<row>
 		<label>/product.htm</label>
 		<nb_visits>1</nb_visits>
-		<nb_uniq_visitors>2</nb_uniq_visitors>
+		<nb_uniq_visitors>1</nb_uniq_visitors>
 		<nb_hits>2</nb_hits>
 		<sum_time_spent>26</sum_time_spent>
 		<sum_bandwidth>0</sum_bandwidth>

tests/PHPUnit/System/expected/test_ChangingVisitorIds__Actions.getPageUrls_month.xml

@@ -29,7 +29,7 @@
 				<nb_conversions_entry>4</nb_conversions_entry>
 			</row>
 		</goals>
-		<sum_daily_nb_uniq_visitors>12</sum_daily_nb_uniq_visitors>
+		<sum_daily_nb_uniq_visitors>8</sum_daily_nb_uniq_visitors>
 		<sum_daily_entry_nb_uniq_visitors>6</sum_daily_entry_nb_uniq_visitors>
 		<sum_daily_exit_nb_uniq_visitors>6</sum_daily_exit_nb_uniq_visitors>
 		<avg_bandwidth>0</avg_bandwidth>
@@ -64,7 +64,7 @@
 				<nb_conversions_entry>2</nb_conversions_entry>
 			</row>
 		</goals>
-		<sum_daily_nb_uniq_visitors>6</sum_daily_nb_uniq_visitors>
+		<sum_daily_nb_uniq_visitors>5</sum_daily_nb_uniq_visitors>
 		<entry_nb_visits>2</entry_nb_visits>
 		<entry_nb_actions>4</entry_nb_actions>
 		<entry_sum_visit_length>66</entry_sum_visit_length>

tests/PHPUnit/System/expected/test_ChangingVisitorIds__Events.getCategory_day.xml

@@ -2,7 +2,7 @@
 <result>
 	<row>
 		<label>category</label>
-		<nb_uniq_visitors>2</nb_uniq_visitors>
+		<nb_uniq_visitors>1</nb_uniq_visitors>
 		<nb_visits>1</nb_visits>
 		<nb_events>2</nb_events>
 		<nb_events_with_value>0</nb_events_with_value>
@@ -14,7 +14,7 @@
 		<subtable>
 			<row>
 				<label>action</label>
-				<nb_uniq_visitors>2</nb_uniq_visitors>
+				<nb_uniq_visitors>1</nb_uniq_visitors>
 				<nb_visits>1</nb_visits>
 				<nb_events>2</nb_events>
 				<nb_events_with_value>0</nb_events_with_value>

tests/PHPUnit/System/expected/test_ChangingVisitorIds__Events.getCategory_month.xml

@@ -8,7 +8,7 @@
 		<sum_event_value>0</sum_event_value>
 		<min_event_value />
 		<max_event_value />
-		<sum_daily_nb_uniq_visitors>6</sum_daily_nb_uniq_visitors>
+		<sum_daily_nb_uniq_visitors>3</sum_daily_nb_uniq_visitors>
 		<avg_event_value>0</avg_event_value>
 		<segment>eventCategory==category</segment>
 		<subtable>
@@ -20,7 +20,7 @@
 				<sum_event_value>0</sum_event_value>
 				<min_event_value />
 				<max_event_value />
-				<sum_daily_nb_uniq_visitors>6</sum_daily_nb_uniq_visitors>
+				<sum_daily_nb_uniq_visitors>3</sum_daily_nb_uniq_visitors>
 				<avg_event_value>0</avg_event_value>
 			</row>
 		</subtable>

tests/PHPUnit/System/expected/test_ChangingVisitorIds__Goals.getItemsSku_day.xml

@@ -3,7 +3,7 @@
 	<row>
 		<label>custom sku</label>
 		<nb_visits>1</nb_visits>
-		<nb_uniq_visitors>2</nb_uniq_visitors>
+		<nb_uniq_visitors>1</nb_uniq_visitors>
 		<nb_actions>2</nb_actions>
 		<avg_price>500</avg_price>
 		<avg_quantity>0</avg_quantity>

tests/PHPUnit/System/expected/test_ChangingVisitorIds__Goals.getItemsSku_month.xml

@@ -4,7 +4,7 @@
 		<label>custom sku</label>
 		<nb_visits>5</nb_visits>
 		<nb_actions>6</nb_actions>
-		<sum_daily_nb_uniq_visitors>6</sum_daily_nb_uniq_visitors>
+		<sum_daily_nb_uniq_visitors>5</sum_daily_nb_uniq_visitors>
 		<avg_price>1500</avg_price>
 		<avg_quantity>0</avg_quantity>
 		<conversion_rate>0%</conversion_rate>