Show sparkline evolution figures for visits, goals and ecommerce overviews #19057

bx80 · 2022-04-06T07:27:25Z

Description:

This PR adds evolution percentages and up/down icons to the end of the sparklines shown on the overview screens for visits, goals and ecommerce.

Review

…view

bx80 · 2022-04-06T07:29:29Z

Work in progress - visits and goals overviews are done, ecommerce renders sparklines a little differently and is not implemented yet.

bx80 · 2022-04-11T04:52:31Z

The Ecommerce overview spark lines have been implemented using a custom template rather than a visualization, probably to allow special handling for the abandoned cart goal metrics. There isn't enough time allocated for this issue to rework the overview to use a visualization, so instead I've chosen a pragmatic approach and rendered the evolution values in the custom spark line template. This isn't ideal but didn't take much time and will provide evolution metrics in one of the more useful places until things can be reworked at some future time.

In order for the override method Ecommerce.getMetricsForGoal() to gain access to the data row used in the parent Goals.getMetricsForGoal() I've added an optional parameter to the parent method to allow the preloaded data row to be passed by the child method. I thought this preferable to replicating the entire parent method, but open to better ideas! 🙂

github-actions · 2022-04-19T02:10:22Z

This issue is in "needs review" but there has been no activity for 7 days. ping @matomo-org/core-reviewers

sgiehl

Had a rough look through the changes. The approach looks fine, but we might have a similar issue like we had a plenty of times before. We always request the data formatted from the API and afterwards try to compare the data. That often fails for languages that use other formattings than english. I guess at some point we need to decide to put a lot more effort into that topic and start changing that behavior to request unformatted data and format it when it is shown. Not sure if we should do that now. But I'm pretty sure that for some languages this new evolutions might show incorrect values....

plugins/Goals/Reports/Get.php

plugins/VisitsSummary/Reports/Get.php

sgiehl · 2022-04-26T15:25:08Z

plugins/VisitsSummary/Reports/Get.php

+                    if ($columnName == 'avg_time_on_site') {
+                        // TODO Unable to calculate this evolution because the visit summary archive stores the time pre-formatted.
+                        return;
+                    }


I guess to get this and other metrics to work we would need to request the data with format_metrics=0 and format the metrics manually before they are displayed. This might need a bit more effort but would also fix some other issues that might currently occur when formatted values are compared.

I've committed a change which makes sparkline API calls with the format_metrics=0 parameter, this seems to be working well and the compute_evolution() callback now received unformatted data allowing time, percent and money metrics to be safely calculated.

The main sparkline visualization CoreVisualizations\Visualizations\Sparklines has been updated to format the sparkline values using the appropriate Metric class retrieved from the report.

The goals and visits summary overview reports have been updated to format the sparkline evolution tooltips by finding the Metric object for the column and using it's format method.

Ecommerce doesn't use a report or visualization and as noted before it needs some rework. For now I've done a quick fix to manually format the metrics by name.

The bandwidth plugin was pushing values into the datatable pre-formatted, so I've committed a change there to provide unformatted values instead and also added a new Metric class for formatting the overall bandwidth metric. It seems to be working well on the visit summary overview.

plugins/CoreVisualizations/Visualizations/Sparklines.php

plugins/VisitsSummary/Reports/Get.php

…back method extra parameter documentation

…into m-10716-evolution-on-overviews

…g added to the sparkline visualization and overview reports

sgiehl

Already goes in the right direction, but there are still a couple of issues with the latest changes.

core/ViewDataTable/Request.php

plugins/CoreVisualizations/Visualizations/Sparklines.php

plugins/Ecommerce/Controller.php

plugins/CoreVisualizations/Visualizations/Sparklines/Config.php

plugins/Ecommerce/templates/getSparklines.twig

plugins/Ecommerce/Controller.php

…sualization loadDataTableFromAPI methods

…le and Visualization classes

plugins/MediaAnalytics/Visualizations/MediaEvolution.php

plugins/Ecommerce/Controller.php

plugins/Ecommerce/templates/getSparklines.twig

plugins/CoreVisualizations/Visualizations/Sparklines.php

bx80 · 2022-05-18T05:00:51Z

@sgiehl Thanks for fixing the bandwidth metrics, that's looking really good 😃
I don't think I'm going to have the time to rework the ecommerce sparklines for this release.
I've tidied up the remaining tests.

Not sure if you want to do a quick final review or shall I go ahead and merge?

sgiehl · 2022-05-18T09:06:45Z

@bx80 I have merge in the latest changed from 4.x-dev to see if any tests are still failing. But looking through the changed UI tests it seems like the numbers for the sparklines aren't formatted anymore when comparing data. Haven't looked in detail yet. I will wait for the tests to finish, maybe they only need to be updated 🤔

sgiehl · 2022-05-18T20:26:33Z

@bx80 Seems you need to have another look at that one. The number formatting seems to fail when comparing data. I was able to reproduce that locally.

bx80 · 2022-05-19T01:58:30Z

@sgiehl I'm not able to see where the number formatting is failing, the failed UI tests all seem to be caused by the action data tables not grouping and when I run this test locally they group correctly. I've also checked all the obvious places in the UI both in English and Deutsch (in case it's locale specific) but the sparkline tooltips number comparisons seem to be formatted correctly. I'm clearly missing something, where should I be looking? 👀

sgiehl · 2022-05-19T05:44:23Z

Are the average spent time and bounce rate metric formatted for you?

bx80 · 2022-05-19T22:20:44Z

Are the average spent time and bounce rate metric formatted for you?

Yes, they seem ok.

sgiehl · 2022-05-20T05:38:15Z

As mentioned before it seems only broken in comparison mode. So when comparing dates or segments.

bx80 · 2022-05-21T20:10:11Z

@sgiehl Sorry, I was thinking comparing via the tooltips not via comparison mode. 🤦‍♂️ I see the issue now and I've commit a fix to also the format metrics when the sparklines are in comparison mode.

sgiehl

Found some more small issues that don't work properly. Will try to push some fixes for it, so we can go on and merge this issue soon

sgiehl · 2022-05-23T12:57:59Z

plugins/Goals/Reports/Get.php

+                $lastPeriod = PeriodFactory::build(Piwik::getPeriod(), $lastPeriodDate);
+                $lastPrettyDate = ($currentPeriod instanceof Month ? $lastPeriod->getLocalizedLongString() : $lastPeriod->getPrettyString());
+
+                $view->config->compute_evolution = function ($columns, $metrics) use ($currentPrettyDate, $lastPrettyDate, $previousDataRow) {


There currently seems to be a problem with overall revenue. There is no evolution calculated for it. Guess the problem is, that the column is formatted using a filter above. And so it isn't numeric here anymore...

sgiehl · 2022-05-23T12:59:32Z

plugins/CoreVisualizations/Visualizations/Sparklines.php

@@ -202,9 +230,21 @@ private function fetchConfiguredSparklines()

                        list($compareValues, $compareDescriptions, $evolutions) = $this->getValuesAndDescriptions($compareRow, $columnToUse, '_change', '_trend');

+                        $metricsObjs = $report->getProcessedMetricsById();


Guess using Report::getProcessedMetricsForTable would be more accurate.

sgiehl · 2022-05-23T13:02:12Z

plugins/CoreVisualizations/Visualizations/Sparklines.php

+                            foreach ($metricsObjs as $metricObj) {
+                                if ((is_array($column) && in_array($metricObj->getName(), ($column)) || $metricObj->getName() == $column)) {
+                                    $valueFormatted = $metricObj->format($value, $formatter);
+                                    break;
+                                }
+                            }


Finding the correct metric for formatting can be done outside the nested foreach loops, no need to do that over and over again. Also this is actually not always accurate. Their can be multiple columns given, which might not necessarily have the same format. e.g. it could be a percent and money value. Currently both metrics would be formatted with the first matching one. Guess we need to format each separately...

sgiehl · 2022-05-23T13:02:24Z

plugins/CoreVisualizations/Visualizations/Sparklines.php

+                    foreach ($metricsObjs as $metricObj) {
+                        if ((is_array($column) && in_array($metricObj->getName(), ($column)) || $metricObj->getName() == $column)) {
+                            $valueFormatted = $metricObj->format($value, $formatter);
+                            break;
+                        }
+                    }


Same as above

sgiehl · 2022-05-23T15:11:30Z

@bx80 with the fixes I've pushed, I think this should be good to merge now. Feel free to have another quick look at the changes and merge it if they look fine for you.
Regarding the ecommerce sparkline refactoring. Found this issue #15349, which we should handle that in.

Btw. before merging this one, you need to merge all related submodule PRs and update the submodules in this PR back to their 4.x-dev branch again. Otherwise we might get submodule reference issues....

Add evolution to sparklines on the visits overview and the goals over…

0c79404

…view

bx80 added the not-in-changelog For issues or pull requests that should not be included in our release changelog on matomo.org. label Apr 6, 2022

bx80 added this to the 4.10.0 milestone Apr 6, 2022

bx80 self-assigned this Apr 6, 2022

bx80 added 2 commits April 11, 2022 16:31

Add evolution to sparklines on the ecommerce overview

5507713

Merge branch '4.x-dev' into m-10716-evolution-on-overviews

4bd891a

bx80 added the Needs Review PRs that need a code review label Apr 11, 2022

bx80 added 2 commits April 11, 2022 17:41

Updated UI test screenshots

bde9a10

Update submodule

e44d000

github-actions bot added the Stale The label used by the Close Stale Issues action label Apr 19, 2022

Merge branch '4.x-dev' into m-10716-evolution-on-overviews

552d162

sgiehl requested changes Apr 26, 2022

View reviewed changes

sgiehl removed the Needs Review PRs that need a code review label Apr 27, 2022

bx80 added 3 commits April 28, 2022 15:37

Fix for tooltip date formatting, improved translation lookup and call…

0de6055

…back method extra parameter documentation

Merge remote-tracking branch 'origin/m-10716-evolution-on-overviews' …

e2de896

…into m-10716-evolution-on-overviews

Sparklines API calls modified to return unformatted values, formattin…

a230941

…g added to the sparkline visualization and overview reports

bx80 added Needs Review PRs that need a code review and removed Stale The label used by the Close Stale Issues action labels Apr 28, 2022

bx80 mentioned this pull request Apr 28, 2022

Evolution values on overviews matomo-org/plugin-Bandwidth#46

Merged

11 tasks

sgiehl added 2 commits April 28, 2022 10:38

Merge branch '4.x-dev' into m-10716-evolution-on-overviews

f0086b8

update submodule

7e88f70

sgiehl requested changes Apr 28, 2022

View reviewed changes

sgiehl removed the Needs Review PRs that need a code review label Apr 28, 2022

bx80 added 3 commits April 29, 2022 11:06

Updated inherited method signature

3ec71d1

Code improvements, added forcedParams parameter to ViewDataTable / Vi…

4259261

…sualization loadDataTableFromAPI methods

Revert change to loadDataTableFromAPI method signature in ViewDataTab…

ff494e0

…le and Visualization classes

sgiehl requested changes Apr 29, 2022

View reviewed changes

bx80 added 5 commits May 16, 2022 13:52

UI test screenshot updates

0fc8060

Update submodule

00474ef

Test fix

b753ceb

Test fixes

19b917d

Update system tests

a1b133d

Merge branch '4.x-dev' into m-10716-evolution-on-overviews

013f4e3

Rerun tests

bd704f3

Fix for unformatted metrics when comparing

f92b110

Update UI test screenshots for sparkline comparisons

1f8eedd

bx80 added the Needs Review PRs that need a code review label May 21, 2022

sgiehl mentioned this pull request May 23, 2022

Comparison sparklines in ecommerce report does not show multiple metrics #15349

Open

sgiehl reviewed May 23, 2022

View reviewed changes

apply some fixes

f36e398

sgiehl force-pushed the m-10716-evolution-on-overviews branch from cc57e97 to f36e398 Compare May 23, 2022 13:55

Update submodule

40eab2d

sgiehl merged commit fdbcb56 into 4.x-dev May 24, 2022

sgiehl deleted the m-10716-evolution-on-overviews branch May 24, 2022 07:51

sgiehl mentioned this pull request May 25, 2022

Goals conversion rate percentage not formatted properly in goals page #19263

Closed

bx80 mentioned this pull request May 29, 2022

Fix for undefined array index error when rendering sparklines #19275

Closed

sgiehl mentioned this pull request Jul 21, 2022

Graph Unit on "Visitors => Overview" matomo-org/plugin-Bandwidth#49

Closed

sgiehl mentioned this pull request Nov 2, 2022

update ecommerce grid #19931

Closed

11 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Show sparkline evolution figures for visits, goals and ecommerce overviews #19057

Show sparkline evolution figures for visits, goals and ecommerce overviews #19057

bx80 commented Apr 6, 2022

bx80 commented Apr 6, 2022

bx80 commented Apr 11, 2022

github-actions bot commented Apr 19, 2022

sgiehl left a comment

sgiehl Apr 26, 2022

bx80 Apr 28, 2022

sgiehl left a comment

bx80 commented May 18, 2022

sgiehl commented May 18, 2022

sgiehl commented May 18, 2022

bx80 commented May 19, 2022

sgiehl commented May 19, 2022

bx80 commented May 19, 2022

sgiehl commented May 20, 2022

bx80 commented May 21, 2022

sgiehl left a comment

sgiehl May 23, 2022

sgiehl May 23, 2022

sgiehl May 23, 2022

sgiehl May 23, 2022

sgiehl commented May 23, 2022 •

edited

		@@ -202,9 +230,21 @@ private function fetchConfiguredSparklines()

		list($compareValues, $compareDescriptions, $evolutions) = $this->getValuesAndDescriptions($compareRow, $columnToUse, '_change', '_trend');

		$metricsObjs = $report->getProcessedMetricsById();

Show sparkline evolution figures for visits, goals and ecommerce overviews #19057

Show sparkline evolution figures for visits, goals and ecommerce overviews #19057

Conversation

bx80 commented Apr 6, 2022

Description:

Review

bx80 commented Apr 6, 2022

bx80 commented Apr 11, 2022

github-actions bot commented Apr 19, 2022

sgiehl left a comment

Choose a reason for hiding this comment

sgiehl Apr 26, 2022

Choose a reason for hiding this comment

bx80 Apr 28, 2022

Choose a reason for hiding this comment

sgiehl left a comment

Choose a reason for hiding this comment

bx80 commented May 18, 2022

sgiehl commented May 18, 2022

sgiehl commented May 18, 2022

bx80 commented May 19, 2022

sgiehl commented May 19, 2022

bx80 commented May 19, 2022

sgiehl commented May 20, 2022

bx80 commented May 21, 2022

sgiehl left a comment

Choose a reason for hiding this comment

sgiehl May 23, 2022

Choose a reason for hiding this comment

sgiehl May 23, 2022

Choose a reason for hiding this comment

sgiehl May 23, 2022

Choose a reason for hiding this comment

sgiehl May 23, 2022

Choose a reason for hiding this comment

sgiehl commented May 23, 2022 • edited

sgiehl commented May 23, 2022 •

edited