On every log_visit update it will try to update the idvisitor with the same value. In theory even if there was no other value to update then it will do
updateVisit($idVisit, array('idvisitor' => '...')) which be unneeded. As the value in most queries won't change this PR likely doesn't have an impact but be still good to have as it would avoid the need for checking that field on every log_visit update and if no other log_visit field is needed to be updated, then it avoids the query altogether.
Code looks good, some system tests are failing though.
@diosmosis just fixed the tests. They should pass now. I looked into it and the changed values are expected as it fixes an issue where eg when
And then it would falsely overwrite the last_visit_action_time
2020-05-05 05:06:05 with the value
2020-05-05 05:04:05. Causing eg the session to expire earlier (hence it has now one less visit in some reports because it could link a request to an existing visit) and also the visit time decreases as it more correctly calculates it.
Eg it first tracks an event where it adds 5 minutes: https://github.com/matomo-org/matomo/blob/3.13.5/tests/PHPUnit/Fixtures/ThreeVisitsWithCustomEvents.php#L134-L137
Then it tracks events where it adds only one minute or two minutes: https://github.com/matomo-org/matomo/blob/3.13.5/tests/PHPUnit/Fixtures/ThreeVisitsWithCustomEvents.php#L96-L106
And therefore it sets a lower
visit_last_action_time and therefore before it was calculating wrong time on site in that case