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

Incorrect rounding of decimal places in final price #695

Open
N0ps32 opened this Issue Feb 7, 2019 · 4 comments

Comments

Projects
None yet
3 participants
@N0ps32
Copy link

N0ps32 commented Feb 7, 2019

Description

Commerce incorrectly adds 1-2c to our order total even though it shouldn't.

Steps to reproduce

  1. Create products with the following prices:
    Luftpolsterfolie (4.917 EUR)
    Klebeband (1.667 EUR)
    BOX (3.75 EUR)
    Reisekoffer (8.25 EUR)
    Business-Trolley (5.75 EUR)
    Golftasche (12.417 EUR)

  2. Add a 20%, not included in price VAT tax rate.

  3. Add products to cart:
    3x BOX
    1x Reisekoffer
    1x Business-Trolley
    1x Golftasche
    1x Klebeband
    1x Luftpolsterfolie

  4. Your cart.totalPrice will now be € 53.11 even though it should be € 53.10.
    Because ((3.75 * 3) + 8.25 + 12.417 + 5.75 + (1.667 + 4.917)) * 1.2 = 53.1012

Screenshots:
https://i.imgur.com/kW5OSKG.png
https://i.imgur.com/DNPB0zX.png

This is a very big issue for us. We are essentially overcharging clients 2c that we would need to refund leading to much more complications.

Additional info

  • Craft version: Craft Pro 3.1.8
  • PHP version: 7.2.13
  • Database driver & version: MySQL 5.5.5
  • Plugins & versions: Craft Commerce | 2.0.4, Stripe for Craft Commerce 1.0.10
@ccchapman

This comment has been minimized.

Copy link
Contributor

ccchapman commented Feb 7, 2019

Your adjustments and line item totals seem to be correct in the screenshots. That makes me wonder if Currency::round() is causing this issue inside getTotalPrice().

return Currency::round($this->getItemSubTotal() + $this->getAdjustmentsTotal());

Recommend testing to see what the number is before and after it gets passed through that method.

Edit: It is the price for each line item being rounded causing this issue.

@N0ps32

This comment has been minimized.

Copy link
Author

N0ps32 commented Feb 7, 2019

With the help of @ccchapman I was able to track down the bug.
The problem is Line 407 in the LineItem model:

$this->salePrice = CurrencyHelper::round($this->saleAmount + $this->price);

salePrice is saved as a rounded value in the database before the tax adjuster gets a chance to produce a correct value. Rounding should be the last step not the first one.

If you change the line from
$this->salePrice = CurrencyHelper::round($this->saleAmount + $this->price);
to
$this->salePrice = $this->saleAmount + $this->price;
everything works correctly.

price field in database: 12.4170
salePrice field in database: 12.4200

@ccchapman

This comment has been minimized.

Copy link
Contributor

ccchapman commented Feb 7, 2019

FWIW, the currency helper is rounding to two places for Euro per ISO 4217.

This issue aside, should purchasables even be allowed to have a price with decimals that differ from the standard they'll be rounded to? This seems misleading to show a currency with three decimal points in the CP when it will be rounded for line items.

@N0ps32

This comment has been minimized.

Copy link
Author

N0ps32 commented Feb 7, 2019

Yes you absolutely should be able to have prices with higher accuracy. We designed our prices in a way that they are mostly round numbers after tax has been applied. We'd still like to keep them without tax in the cms.

I understand that they need to be rounded once you send them to a payment gateway, but why round them before adjusters can be applied to them?

Or at least there should be an option to disable premature rounding if it's not a desired behavior

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.
You signed in with another tab or window. Reload to refresh your session. You signed out in another tab or window. Reload to refresh your session.