convert sparking API to one request - fixes archive
Copy the logic from here : https://github.com/matomo-org/matomo/blob/e3218471244d08039b1b84f1c907f1be0e018768/plugins/Ecommerce/Controller.php#L37
@tsteur do you want to try that one, I think this is kind of working, reduce my CPU a little bit on my local on a one year period, still have failed some XML compare tests but should be fine, I will fix and tidy code more, once we confirm it reduce the load.
@peterhashair let's maybe keep the changes for the lock separate and only apply the UI changes here. Be good to check out my comment in chat to make use of Visualisations as much as possible.
@tsteur right, I was stuck on here, so go for the template option. Let me convert to Visualisations.
$view = ViewDataTableFactory::build(Sparklines::ID, $apiMethod, 'Goals.' . __METHOD__, $forceDefault = true);
@tsteur as we discussed. I used the goal API
getMetries method instead of
get it should remove the for loop improve the performance a little bit.
Awesome, can you check if it loads faster now?
@tsteur yes, about 3-5 seconds faster compared to load period for 6 months on my local.
@peterhashair @bx80 This actually kind of broke the goals overview page. If you would have had a look at the UI test failures you would have been able to see that. See https://builds-artifacts.matomo.org/matomo-org/matomo/m-18088-fixes-archive/52262/GoalsPages_overview.png
Please try to be a bit more accurate on that. It's not only part of the development to fix failing tests that are related, but also part of the review to check if tests are failing and if updates to any tests and expected UI files were actually valid.
I will now revert this PR. Feel free to recreate it, once it fully works.
@sgiehl right let me fix that it. But I got a little confused here, it seems like the preview Test hides the revenue doesn't matter on which conditions (I mean is that on purpose or a regression), the new approach has actually loaded the revenue when
$isEcommerceEnabled. I add can hide revenue sparkline like it was or update the screenshot. which approach do you want me to?
@peterhashair Actually it doesn't really matter if that would have been a regression or not. It was not part of the issue to change the content of the reports in any way. And it's actually not only the additional sparkline, also the headline misses
Goal and the other sparklines show differently. In general it would for sure be fine to fix regressions if there are any, but we should avoid doing that silently in PRs without mentioning it anywhere.