Skip to content
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 for missing formatting of conversion rate on goals reports #19295

Merged
merged 4 commits into from May 31, 2022

Conversation

bx80
Copy link
Contributor

@bx80 bx80 commented May 31, 2022

Description:

Fixes #19263

The overall conversion rate was not being formatted on goal reports for some datatable types due to a non-object processed metric set in Goals.Get

This can be seen on the Funnels plugin FunnelsReport_goalreport_without_funnel.png UI test (./console tests:run-ui FunnelsReport)

Review

@bx80 bx80 added not-in-changelog For issues or pull requests that should not be included in our release changelog on matomo.org. Needs Review PRs that need a code review labels May 31, 2022
@bx80 bx80 added this to the 4.11.0 milestone May 31, 2022
@bx80 bx80 self-assigned this May 31, 2022
@sgiehl sgiehl force-pushed the m-19263-funnels-conversion-formatting branch 2 times, most recently from 11326b2 to e7373ce Compare May 31, 2022 09:59
@sgiehl sgiehl force-pushed the m-19263-funnels-conversion-formatting branch from e7373ce to 6ba0fb4 Compare May 31, 2022 11:19
@sgiehl
Copy link
Member

sgiehl commented May 31, 2022

@bx80 I've updated the PR and applied some changes. Setting the metric class as processed metric actually broke the API.get report. I think the problem is, that the conversion rate actually isn't really a processed metric for the Goals.get report. It's the processed metric of Goals.getMetrics, which is used by Goals.get to gather the metrics. Anyway, having it as processed metric caused a double formatting, which failed. You can see that in the test failures.
I have now instead added the metric class as processed metric to the datatable metadata for sparklines only. This fixes the formatting issue for sparklines as well.
But at some point we need to clean up all that metric stuff, so it's clear what is happening where. It took me really long to narrow down the issue...

@sgiehl sgiehl merged commit 9a7dd7d into 4.x-dev May 31, 2022
@sgiehl sgiehl deleted the m-19263-funnels-conversion-formatting branch May 31, 2022 11:55
@bx80
Copy link
Contributor Author

bx80 commented Jun 1, 2022

Thanks @sgiehl 👍 Totally agree that metrics need to be tidied up at some point and the differences between a real processed metric and a metric class used for formatting documented, it gets confusing very quickly!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs Review PRs that need a code review not-in-changelog For issues or pull requests that should not be included in our release changelog on matomo.org.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Goals conversion rate percentage not formatted properly in goals page
2 participants