fix: Rounding Issue in Cart/Orders #5091
Conversation
nnnnat
force-pushed
the
fix-nnnnat-total-rounding
branch
from
b1f3cc3
to
80bfa92
Compare
… against a price value is rounded correctly. Signed-off-by: Nat Hamilton <info@nathamilton.com>
… against a price value rounds correctly. Signed-off-by: Nat Hamilton <info@nathamilton.com>
| @@ -133,7 +134,7 @@ export default async function addCartItems(context, currentItems, inputItems, op | |||
| shopId: catalogProduct.shopId, | |||
| // Subtotal will be kept updated by event handler watching for catalog changes. | |||
| subtotal: { | |||
| amount: variantPriceInfo.price * quantity, | |||
| amount: +toFixed(variantPriceInfo.price * quantity, 3), | |||
There was a problem hiding this comment.
Am I reading this right that we're fixing to 3 decimal places? If so, why 3?
There was a problem hiding this comment.
Based of @aldeed's previous work on related issues, some currency codes go up to 3 decimals, so we need to accommodate those.
Check the E column of the Active Codes table: https://en.wikipedia.org/wiki/ISO_4217. Those listed with 4 are special inter-country currencies, and don't apply.
There was a problem hiding this comment.
@nnnnat If this needs to be merged ASAP, I'm fine with it, but I strongly suggest a follow-up PR to add a unit test for each updated util function, which shows an example of problematic math and proves these fixes work (fails when you changes are reverted, passes after). This is a type of bug we definitely want to have tests for so that it doesn't recur when code is refactored.
|
I want to also point out that while this should mitigate 99.999% of cases, it doesn't really FIX the issue of JS math errors. I've previously documented some of the gotchas here: #4692 @spencern It may be a good time to focus some effort on figuring out a 100% foolproof solution to storing and calculating with floats before this crops up again. |
There was a problem hiding this comment.
6 products @ 69.99 each works. See @aldeed's above comment about a long term solution, and testing, but for a right now fix: 👍
Impact: critica
Type: bugfix
Issue
When using Reaction with 3rd party pricing engines Reaction will sometimes incorrectly round the carts total/subtotal. For example adding 6 product with a price of $2.99 would result in a cart/order total of $17.93999999999994.
Solution
In this PR I've wrapped any cart/order "money math" with accounting-js
toFixedfunction to force the correct rounding.Breaking changes
N/A
Testing
price * quantitycalculations are also working.