@tsteur opened this Pull Request on April 27th 2020 Member

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.

@diosmosis commented on April 28th 2020 Member

Code looks good, some system tests are failing though.

@tsteur commented on April 28th 2020 Member

@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

  • track pageview with request time 2020-05-05 05:05:05
  • track pageview with request time 2020-05-05 05:06:05
  • track pageview with request time 2020-05-05 05:04:05

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

@tsteur commented on April 28th 2020 Member

@diosmosis fyi tests pass now

This Pull Request was closed on April 28th 2020
Powered by GitHub Issue Mirror