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

Ecommerce Orders Conversion Rate Rounding Error #15637

Closed
beamylake opened this issue Feb 28, 2020 · 11 comments · Fixed by #15668
Closed

Ecommerce Orders Conversion Rate Rounding Error #15637

beamylake opened this issue Feb 28, 2020 · 11 comments · Fixed by #15668
Assignees
Labels
Regression Indicates a feature used to work in a certain way but it no longer does even though it should.
Milestone

Comments

@beamylake
Copy link

Hello,
we've encountered an rounding error (again) when displaying the ecommerce overview in german

Here's the english version, where everything is looking as expected:

eng

And here is the german version, where the Ecommerce Orders Conversion Rate gets rounded down to 0%:

deu

It feels like this problem might be a "leftover" of #15200

We are using Matomo 3.13.3 and the rest of our setup is still as described in #15240

Please let me know if you need any further information and thanks in advance for your efforts!

@tsteur tsteur added this to the 3.13.4 milestone Feb 29, 2020
@tsteur tsteur added the Regression Indicates a feature used to work in a certain way but it no longer does even though it should. label Feb 29, 2020
@tsteur
Copy link
Member

tsteur commented Feb 29, 2020

Thanks @beamylake we'll have a look to see if we can reproduce this.

@pebosi
Copy link
Contributor

pebosi commented Mar 5, 2020

I have the same problem on my current install:
in NumberFormater.php the method formatNumberWithPattern calls "replaceSymbols"
The php strtr function has a strange behaviour here. When i return the value before the call i get a correct result.
Can't reproduce on local machine, so maybe a specific php / server conf problem.

@sgiehl
Copy link
Member

sgiehl commented Mar 5, 2020

The php strtr function has a strange behaviour here. When i return the value before the call i get a correct result.

I would more likely assume the number formatting is done twice maybe. And after the number is converted to a german format it gets truncated when it's formatted again.

@pebosi could you try switching the language to english and check if the same problem exists?

@pebosi
Copy link
Contributor

pebosi commented Mar 5, 2020

Would be nice to have the whole NumberFormat stuff based on Intl Component / Extension, with upgrade to Twig 3 #15573

@pebosi
Copy link
Contributor

pebosi commented Mar 5, 2020

See the screenshots
conversion-rate-en
conversion-rate-german

@sgiehl sgiehl self-assigned this Mar 5, 2020
@pebosi
Copy link
Contributor

pebosi commented Mar 5, 2020

And now when returning the value above this line
$value = $this->replaceSymbols($value);

Eng:
conversion-rate-en-2

Deu:
conversion-rate-german-2

@pebosi
Copy link
Contributor

pebosi commented Mar 5, 2020

Matomo version: 3.13.3
MySQL version: 10.0.38-MariaDB-0ubuntu0.16.04.1
PHP version: 7.3.15-3+ubuntu16.04.1+deb.sury.org+1

@pebosi
Copy link
Contributor

pebosi commented Mar 5, 2020

The second screenshots show the correct value (maybe its better to have 2 digits like in the graph tooltip). But the dot is not replaced with a comma in german...

@sgiehl
Copy link
Member

sgiehl commented Mar 5, 2020

you try applying that linked PR. that should fix the issue

@sgiehl sgiehl linked a pull request Mar 5, 2020 that will close this issue
@pebosi
Copy link
Contributor

pebosi commented Mar 5, 2020

Works!

@beamylake
Copy link
Author

We can confirm, it's working for us too!
Thnx!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Regression Indicates a feature used to work in a certain way but it no longer does even though it should.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants