New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix total time of visit is not the sum of times of page visits if heartbeat feature is used #9617
Conversation
…sits if heartbeat is used
@@ -407,6 +407,10 @@ public function setParam($name, $value) | |||
{ | |||
$this->params[$name] = $value; | |||
unset($this->paramsCache[$name]); | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this change is kinda unrelated but noticed it while fixing this issue
Ideally, we would fix this in Piwik 3.0 the proper way. For example we could have a |
LGTM. updated tests files, when build is green we can merge! |
Fix total time of visit is not the sum of times of page visits if heartbeat feature is used
We can fix the incorrect action time by not updating
visit_last_action_time
see problem explained in #9610 (comment) . A ping is not really an action so it should not updatevisit_last_action_time
.This changes behaviour of heart beat in a way that the "time spent on last action" can be max 30 minutes (or custom value specified in
visit_standard_length
config setting). Why? So far we updatedvisit_last_action_time
on each ping request and set it to now. This kept the visitors session active and theVisitorRecognizer
would find an existing visit even if the user did not click within 30 minutes but if there were ping request.Now the
visit_last_action_time
is no longer updated. This means after 30 minutes of sending ping requests theVisitorRecognizer
will no longer find an existing visit and therefore all further ping requests will be aborted by this line https://github.com/piwik/piwik/blob/2.16.0-b6/plugins/Heartbeat/Tracker/PingRequestProcessor.php#L33-L35fixes #9610