Prestashop: [1.7.7] Order totals not rounded to 0 decimals

[Hlavtox]

Describe the bug
In 1.7.6, I have currency->precision set to 0 and Store settings > General > Decimal places set to 0 in PS config. The order gets rounded fine.

In 1.7.7 - and after removal of decimal places from PS config, you set currency->precision to 0 and the order does not get rounded at all!

In @khouloudbelguith 's video, the order total displayed is 735, but in database it's not rounded and it's 734,57. That's wrong, 734,57 will be sent to payment gateway instead of 735.
https://drive.google.com/file/d/1BrY4FXVVpkx4N9A3TzICuedxSRMXHeh3/view

Why is it bad
The customer pays 734.57 instead of 735.00 as it should be and it immediately creates differences between the payment and the invoice from the financial software. More manual labour, additional rouding and complications on invoices, VAT calculation differences etc. etc.

To Reproduce
PRESTASHOP VERSION 1.7.6

1. Go to Store settings > General > Number of decimal places and set it to 0.
2. Go to database administration, open ps_currency table, set 'precision' to 0.
3. Order something.
4. Totals are rounded.

PRESTASHOP VERSION 1.7.7

1. Go to International > Localization > Currency. Open up your currency. Set decimal places ('precision' in DB) to 0.
2. Order something.
3. Totals are not rounded.

Additional information
PrestaShop version: 1.7.6 -> 1.7.7

[khouloudbelguith]

Hi @Hlavtox,

Thanks for your report.
I have the same issue with PS1.7.6.3 and 1.7.7.x branch.

Steps to reproduce the issue: The EUR currency precision = 0

1. Go to BO => Catalog => discounts => create a new cart rule 15%
2. Go to the FO => Add a product to the cart for example demo_6
   2.1 Price displayed Tax_included= €35
3. Apply the voucher =15%
   3.1 Total (tax incl.) = €30
   3.2 Discount = €5
4. Complete the order (same results in the BO => Orders details page)
   
5. Check the Database Select * from ps_orders where id_order=6

total_discounts = total_discounts_tax_incl = 5.220000=> not rounded
total_discounts_tax_excl = 4.350000 => not rounded
total_paid = total_paid_tax_incl = 29.580000 => not rounded
total_paid_tax_excl = 24.650000 => not rounded
total_products_wt = 34.800000 => not rounded.

PS: with PS1.7.5.2, we can not set the currency precision manually in the database to 0 because there is no precision in the ps_currency table.

So, I cannot follow the steps to reproduce the issue with PS1.7.5.

I’ll add this to the debug roadmap so that it’s fixed. If you have already fixed it on your end or if you think you can do it, please do send us a pull request!

Ping @eternoendless @matks @colinegin @marionf @jolelievre @MatShir what do you think?
This issue should be fixed in the PS1.7.6 or PS1.7.7?

Thanks!

[Hlavtox]

@khouloudbelguith I was talking about 1.7.6, the precision was added in this version. Try it there, please.

[khouloudbelguith]

@Hlavtox, yes!
I tried with PS1.7.6.3 and 1.7.7.x branch => the same behavior.

Thanks!

[Hlavtox]

@khouloudbelguith Do you have set 0 decimal places in 1.7.6.3, in prestashop config?

[khouloudbelguith]

@Hlavtox, Yes!
https://drive.google.com/file/d/1KiDGRC7lLkqNFgrJF4ucEVc9jAghBpDe/view

Thanks!

[matks]

It's a bit hard to understand but you have done so many modifications with precision that indeed the system might behave in weird ways 😅 .

⚠️ "Precision" is a generic name in prestashop that actually have multiple meanings.

• there is the precision used to round prices for display
• there is the precision used to round prices when doing computations
• there might be more I am not aware of 😅

The 2nd precision needs to be higher than the 1st.

[eternoendless]

Computed final prices should be rounded to the most significant number of decimal places according to the shop's default currency. This would also mean that changing this number in the currency settings would require to recalculate prices if this number decreases.

For example:

1. A price is calculated at 35.99 €
2. Euro is modified to remove cents
3. Price must be recalculated at either 36 € or 35 € according to the shop's rounding mode.

[eternoendless]

ping @PrestaShop/prestashop-product-team

[Hlavtox]

@khouloudbelguith Please, read me carefully 😄
@matks No mods in this area bro 😄 Just tested on vanilla 1.7.6 and it's rounding as it should.

PRESTASHOP VERSION 1.7.6

1. Go to Store settings > General > Number of decimal places and set it to 0.
2. Go to database administration, open ps_currency table, set 'precision' to 0.
3. Order something.
4. Totals are rounded.

PRESTASHOP VERSION 1.7.7

1. Go to International > Localization > Currency. Open up your currency. Set decimal places ('precision' in DB) to 0.
2. Order something.
   3. Totals are not rounded.

Screenshots from 1.7.6 version

[khouloudbelguith]

@Hlavtox, we just talked about this feature in this comment https://github.com/PrestaShop/PrestaShop/issues/17596#issuecomment-587080159 :smile:

Thanks!

[Hlavtox]

@khouloudbelguith Yeah but on the video you don't show it. 😄 And I swear I just tried it and the order is rounding in 1.7.6! You must have Store settings > General > Number of decimal places still set to 2.

[jolelievre]

I think we kept some hard coded value in a few places during the migration
There are some part of the legacy code:

https://github.com/PrestaShop/PrestaShop/blob/15b01d14104cb6f8001a639063308a25e6289533/classes/order/OrderDetail.php#L661

And in the migrated code:
https://github.com/PrestaShop/PrestaShop/blob/15b01d14104cb6f8001a639063308a25e6289533/src/Adapter/Order/CommandHandler/AddCartRuleToOrderHandler.php#L163

That stil use 2 as a hard coded precision, whereas they should use either:

Tools::ps_round((float) $amount_paid, Context::getContext()->getComputingPrecision())

or even better (when possible):

$computingPrecision = new ComputingPrecision();
$this->tools->round($priceTaxIncluded, $computingPrecision->getPrecision($currency->precision))

I guess we need to make a thorough research of all the places where rounding is applied, this has already been partially done in a previous PR https://github.com/PrestaShop/PrestaShop/pull/15084 But some new code didn't integrate it correctly, and some might have been forgotten

That raises the question about how we can ensure that a new PR won't reintegrate a falsy behaviour? Should we have automated checks about this?

[Hlavtox]

@jolelievre @eternoendless @matks Found another issue.

Situation

• You have product with product->price 164,876015. With tax 199,49997815‬.
• Correct price, and the price displayed in FO is 199 Kč.
• You order it, it shows 199 Kč everywhere EXCEPT ps_emailalerts, which shows 200 Kč.

Cause

1. Tools::displayPrice($unit_price, $currency, false). You need the price >>
2. $products = $order->getProducts(). You have product array. You need to set the prices >>
3. For each price - setProductPrices($row). Let's see how this function assigns the price.
4. $row['product_price_wt'] = Tools::ps_round($row['unit_price_tax_incl'], 2) << 2 decimals hardcoded.

Instead of rounding 199,49997815‬ to 200.
It rounds 199,49997815‬ to 199,50 in setProductPrices and then it rounds 199,50 to 200 in displayPrice.

---

I don't know if it's connected, but I give this product as a gift and in every order where this gift is present, my total_discounts are off by 1 CZK. Total paid is correct though.

[Hlavtox]

@jolelievre @eternoendless @matks @khouloudbelguith

BTW: Why is this issue minor and for example https://github.com/PrestaShop/PrestaShop/issues/17747 is major?

I think it's more important to calculate money correctly and have correct invoices than to say "DON'T PUT NEGATIVE DISCOUNTS' to employees 😒.

[khouloudbelguith]

@Hlavtox, you can follow this link to introduce the bug severity classification.

Thanks!

[Hlavtox]

@khouloudbelguith Thank you so much! I think you have the severity wrong.

Major
The bug affects major functionality or major data and there is a workaround, but it is not obvious or can be difficult to put in practice.

A major issue affects a large percentage of users (> 30%) and matches at least one of the following:
    It impacts law compliance
Examples:
    Impacts the price the customer pays

[khouloudbelguith]

Ping @marionf what do you think?

PS: the price is well displayed in the FO and it is well displayed in the BO.
FO:

BO: Migrated order page

BO: Legacy page

NOK int database

Thanks!

[marsaldev]

@Hlavtox I think that we appreciate your help, but this is not the right way to talk... Manners first, even more if is a woman.
Please, revise your tone and be more calm...
At this side we're not robot, but human like you... People can wrong, so be quite.

[Hlavtox]

@marsaldev How would you feel when every second issue you create is marked as duplicate with some nonsense that has nothing to do with it?

How would you feel when you need to text everything 10 times because somebody does not read?

How would you feel when simple stuff is marked as Major, important stuff regarding money is marked Minor and on your question, you get sent some stupid link you have read 5 times already?

[ttoine]

@Hlavtox, please follow the Code of Conduct.
https://github.com/PrestaShop/PrestaShop/blob/develop/CODE_OF_CONDUCT.md

Being agressive and insultant will not help. Please keep also in mind that we are exchanging in English here as a common language, while not all people are native English speakers. So there can be understanding problems, vocabulary problems, and different point of views about the expected behavior of the software or even technical limits for historical reasons (good or bad) that might not be easy to solve.

So, please be respectful and tolerant. If you feel that nobody understand your point, it's ok to tell it. Then tell in a new comment what is not clear for everybody in this discussion. The target is to solve the problem, and this it is possible to do it together.

[Hlavtox]

@colinegin @eternoendless @matks bump

Guys, I think that this should be marked as a regression and fixed for 1.7.7. You are fixing some other trouble with tax and rouding (GOOOOD!!!!), so maybe this can get included? 🙏

"Nobody cares" if there are bug in order administration, we can always tell customers, that we don't change orders, but in Front office, it has to be 100% right.

[colinegin]

ping @matthieu-rolland @jolelievre, since we fixed issues with hard coded values, do you think this might have fixed @Hlavtox's issue ?

[matthieu-rolland]

@colinegin ,

The computing precision has a minimal value of 2, that is to say, even if the display precision of the currency is set to 0, the computing precision will be 2. Maybe this is the cause of this issue ?

src/Core/Localization/CLDR/ComputingPrecision.php

const MINIMAL_VALUE = 2;

public function getPrecision(int $displayPrecision)
{
   // the MULTIPLIER attribute is set to 1 for now, so that it matches display precision
   $computingPrecision = $displayPrecision * self::MULTIPLIER;

   return ($computingPrecision < self::MINIMAL_VALUE) ? self::MINIMAL_VALUE : $computingPrecision;
}

@Hlavtox , could you edit this file, and set const MINIMAL_VALUE = 0 instead of 2, and see if it fixes your issue? That would confirm this is the cause of your issue, then we'll have to fix this

PS: this might not be the actual fix, it's just to confirm this is what is causing you this bug, then we'll know what to fix

[Hlavtox]

@matthieu-rolland Hi, yes, changing it to 0 made it round properly.

[Hlavtox]

@matthieu-rolland @eternoendless Bump, we need to fix this guys. 🙏 Would changing the minimum precision have any side effects? The number saved in the database and sent to payment gateway must be the same as the displayed price.

[matthieu-rolland]

thank you for your help @Hlavtox, @colinegin I take this issue, I'll see what can be done about this and come back with a proper solution if possible. We can probably set the minimum computing rounding value to zero for this version.

Should we put this issue in the 1.7.7 kanban?

[eternoendless]

Setting the minimum computing rounding to 0 will effectively prevent you from selling articles at a price less than 1. Eg. if a the currency § does not have cents, but you sell nails for §0.25 each, if you add 4 nails to your cart you might end up paying §4 instead of §1

[matthieu-rolland]

I would say it's the merchant's responsibility not to set this kind of prices when he wants to set his precision to 0.

[Hlavtox]

@eternoendless @matthieu-rolland

Exactly, if you want to sell screws for 0.1 CZK, you will set your precision to 2.

IMHO, the calculating precision should match the displayed one.

[matthieu-rolland]

@Hlavtox in the future, maybe 1.7.8, we would like the computing precision to be higher than the display precision, so that calculations are more precise, otherwise, you can have a small cents offset, that become more and more important on high quantity orders.

But this is not fixed in 1.7.7, for now we want the computing precision to match the display precision. The ZERO precision is indeed a very special case though.

[Hlavtox]

@matthieu-rolland @eternoendless How is Czech koruna a special currency? Another examples are Hungarian Forint, Japanese Yen and many other currencies, that don't use decimals in online commerce.

And, it's not my special request, to use zero decimals, see the most known czech stores.

[matthieu-rolland]

@Hlavtox yes I meant a special case codewise, in the way we handle this (for example the multiplier in the ComputingPrecision class won't have any effect if the precision is zero and we don't set a minimum). But this matter won't be dealt with in 1.7.7 anyway.

For now, we'll set the minimum computing precision to ZERO, as seen with @eternoendless et @marionf. 👍

[Hlavtox]

@matthieu-rolland OK!!! And won't it affect other users then? If you set minimal precision to 2, and somebody sets their currency precision to 2, will their calculations be the same?

[matthieu-rolland]

@Hlavtox for now (1.7.7), the display precision and computing precision will always match. The main edge case is for those who set prices having a higher precision than their currency precision. (for example you have a zero currency precision and price like 0.25)

[khouloudbelguith]

Hi @matthieu-rolland,

Can I change this issue from "To be tested" to "In progress"?

Thanks!

[matthieu-rolland]

yes @khouloudbelguith , (seems like it's done !) thanks

[matks]

Fixed by https://github.com/PrestaShop/PrestaShop/pull/20137