Proportional evolution comparison for incomplete periods #18099
Conversation
…rcent of the current period which is complete
bx80
added
not-in-changelog
There was a problem hiding this comment.
@bx80 nice, I did a very basic check locally and this seems to have worked 🎉
Could we also add some kind of system tests for this to ensure this works (and keeps working in the future)?
Not sure if easy to do. Be good to add a tooltip on the evolution percentage when the currently viewed report includes today to then say something like that this percentage is based on comparing the time that has been past in the current period.
Or if I understand the code directly, that's not just for periods that include today but eg would apply when viewing a month and the previous month has more or less days.
Otherwise people will look at the previous month, and calculate the percentage themselves and be confused why they come up with different percentage.
@mattab can you confirm we also want to adjust the percentage when eg one month has 30 days and the other 31 and then adjust the percentage accordingly? Or should this happen only when the currently viewed period includes today?
This "proportional-adjustement" logic should happen only when currently viewed period includes today. |
…nit test, added tooltip
|
I've added a tooltip to explain when the comparison is proportional. If the current period doesn't contain the current date/time then the proportional adjustment will not be applied (ratio = 1, do nothing) Also added a unit test which feeds some fixed date empty datatables to the ratio calculation code to check that the expected ratio is returned. |
|
Q: would make the tooltip maybe more descriptive if possible to remove any possible confusion in how we came up the with number, for example this below (or a better text):
(note: the last line is from the existing tooltip used in Acquisition > Overview report) |
|
@mattab yes it would be, and thought about it, but considering 80/20 it be likely good enough to just have something generic |
|
It was quick to add some more detail to the tool tip, so I went ahead and did it, hope that's okay! |
There was a problem hiding this comment.
@bx80 nice, that works already. Left few comments and seems there are some tests that need adjusting https://app.travis-ci.com/github/matomo-org/matomo/jobs/542051052 and https://app.travis-ci.com/github/matomo-org/matomo/jobs/542051049
…nges to allow row metadata to be preserved during datatable merges
There was a problem hiding this comment.
Nice. I see you got the ts_archived working 👍
There are some tests failing in https://app.travis-ci.com/github/matomo-org/matomo/builds/239598140
Can we maybe remove this metadata from each row in the final API output? This way we reduce chances of tests failing anymore randomly as ts_archived might be sometimes bit different and reduce a lot of the output in the API response.
We can still use the ts_archived in our other filters.
|
This issue is in "needs review" but there has been no activity for 7 days. ping @matomo-org/core-reviewers |
github-actions
bot
added
the
Stale
|
Awesome 👍 @sgiehl can you maybe have a last look? |
github-actions
bot
removed
the
Stale
…moved additional constructor param to end, null checks
plugins/MultiSites/angularjs/dashboard/dashboard-model.service.js
Outdated
Show resolved
Hide resolved
Co-authored-by: Stefan Giehl <stefan@matomo.org>
Co-authored-by: Stefan Giehl <stefan@matomo.org>
plugins/MultiSites/angularjs/dashboard/dashboard-model.service.js
Outdated
Show resolved
Hide resolved
plugins/MultiSites/angularjs/dashboard/dashboard-model.service.js
Outdated
Show resolved
Hide resolved
plugins/MultiSites/angularjs/dashboard/dashboard-model.service.js
Outdated
Show resolved
Hide resolved
Fix for row metadata removed too early Co-authored-by: Stefan Giehl <stefan@matomo.org>
Description:
Fixes #6784
This PR changes multi site evolution metrics to calculate proportionally to the percent of the current period which is complete.
For example, if there were 1,000 site visits last week, the current time is midnight on Monday and there have been 150 visits so far this week, then the All Websites visits evolution percent would have shown as a red -85% because 150 visits is a big drop from 1,000. (This Week Visits - Last Week Visits) / (Last Week Visits)
However this in an unfair comparison as the current week is incomplete. If the remaining days of the week continue to at 150 visits then the current week will be an improvement over the previous week.
Since only ~14% of the current week has elapsed a better comparison would be 14% of the previous week total visits vs the current visits in this week, this results in a green 5% evolution percent. (This Week Visits - (Last Week Visits * Percent of Period Complete) / (Last Week Visits * Percent of Period Complete)
Review