Fix time on page calculation in visits log

[sgiehl]

Description:

This change would actually remove the time on page for all actions other than pageviews. For pageviews the time on page would be summed up till the next pageview.

This is an approach to fix #18270

Did the changes in a couple of minutes while checking for a possible reason. To finish this some manual testing and updating/adding tests would be needed.

@tsteur If this is something that should be worked on further, feel free to assign the PR to a milestone and I will resume the work then.

Review

• Functional review done
• Potential edge cases thought about (behavior of the code with strange input, with strange internal state or possible interactions with other Matomo subsystems)
• Usability review done (is anything maybe unclear or think about anything that would cause people to reach out to support)
• Security review done
• Wording review done
• Code review done
• Tests were added if useful/possible
• Reviewed for breaking changes
• Developer changelog updated if needed
• Documentation added if needed
• Existing documentation updated if needed

[tsteur]

@sgiehl how much more time would it take to finish it? I guess originally the idea was to eventually fix the root cause of #9198 which then would fix this as well. As this might take a while until we work on #9198 could maybe get this done now if it's real quick.

There's a bit of a problem that then eg people who aggregate the page view time manually will find it's different to the reported average time. But we could probably ignore this.

[sgiehl]

@tsteur I think the code in the PR should actually work. It would remove the "time on page" metric for all non page view actions in the visits log API. The time on page views should be summed up.
To ensure that really works as expected I would need to build a simple system test case that tracks multiple different actions and if that works also adjust the now failing tests. Guess that would be doable in a couple of hours.

[tsteur]

@sgiehl this already changes quite a lot of system tests so maybe no new system test would need to be written ? https://app.travis-ci.com/github/matomo-org/matomo/jobs/547727804

[sgiehl]

I'd rather like to write a very simple new test that tracks some different actions and validate if the resulting number are correct and then simply update the expected results of all other tests assuming they are correct.
Could also try to do that with the existing tests, but might be more effort to look through the fixtures and check which results would be expected, as we often don't turn time forward when tracking additional actions.

[sgiehl]

This PR should be ready for a first review. The time on page should now only be calculated for page views. For all other action types the timeSpent and timeSpentRef columns are removed. If a visit doesn't contain a page view, there will actually be no action that contains a timeSpent

Some System tests of submodule plugins are still failing, but those can be updated once this one would be ready to merge.

[sgiehl]

@tsteur shall we merge this one for 4.7 then?

[tsteur]

OK sounds good

[sgiehl]

@AltamashShaikh This most likely will break the tests for various plugins. The time spent in visit details may change in various tests. Those can be simply updated, if there are no other changes.

[AltamashShaikh]

Okay Thanks,
Will keep this in mind