Skip to content

Instantly share code, notes, and snippets.

@edannenberg
Last active January 26, 2017 17:41
Show Gist options
  • Star 6 You must be signed in to star a gist
  • Fork 0 You must be signed in to fork a gist
  • Save edannenberg/8041401 to your computer and use it in GitHub Desktop.
Save edannenberg/8041401 to your computer and use it in GitHub Desktop.
Magento 1.8.x tax calculation when using prices including tax.

We upgraded to 1.8 a couple of weeks ago, today i had to investigate this little gem:

trolol

The issue was reproducable by adding 3 of the above items to the cart, checking the db confirmed that the 2nd quote item already had the wrong tax value.

Digging down the culprit turned out to be in Mage_Tax_Model_Sales_Total_Quote_Subtotal and Mage_Tax_Model_Sales_Total_Quote_Tax:

collect() will call for each quote item:

  • _processItem() which calls:
  • _totalBaseCalculation() which then uses a custom rounding function:
  • _deltaRound($price, $rate, $direction, $type = 'regular')

This method applies a delta to the input price before rounding it, starting with a default delta of 0.

$this->_roundingDeltas[$type][$rate] = $price - $this->_calculator->round($price)

Should another call to _deltaRound() use the same type and rate parameters the function will apply the delta from the previous call.

But lets see what actually happens:

1st item added to cart, price is 19.90 incl. tax:

  • tax calculated: 3.1773109243697
  • delta value: 0
  • rounded tax value: 3.18

2nd item added to cart, price is 19.90 incl. tax:

  • tax calculated: 3.1773109243697
  • delta value: -0.0026890756302529
  • tax before rounding: 3.1746218487395
  • rounded tax value: 3.17

3rd item added to cart, price is 19.90 incl. tax:

  • tax calculated: 3.1773109243697
  • delta value: 0.0046228487394946
  • tax before rounding: 3.1819337731092
  • rounded tax value: 3.18

I don't really get the point of this function, only thing i can think of is working around the precision problems of using floats. Maybe it is just used in the wrong way as i don't see why the rounding delta of a totally different item should matter for rounding another one. Tested it with couple other item combinations, the more items you add to the cart the higher the chance of getting a wrong tax value at some point.

Upgraded to 1.8.1, _deltaRounding() got some love but they only changed the default delta value to 0.000001 to work around the trollish behaviour of php's round() when handling x.5 values. The issue still persists.

As a quick fix you could modify _deltaRounding() in both classes to always use the default delta which fixes this specific issue, no idea what side effects this might cause though.


Edit: I think i get the point of this function now. Looks like it's for working around the fact that Magento saves all money values already rounded to 2 digits. Those value are then used for totals calculation which leads to errors.

By always using the default delta you end up with correct rounded values for the tax per position, but the total tax will be wrong:

3.18 * 3 = 9.54 total item tax in backend after fix

3.1773109243697 * 3 = 9.53193577311 or 9.53 rounded

Without the fix some of the rounded tax values per position can be wrong, but it will produce the correct final tax amount. IMHO all this mess could be avoided by not saving already rounded money values into the db, atleast not rounded to 2 digits.

In our case the connected erp will use the potential wrong values of row totals and barf all over it. Will probably just fix the api method to feed correct values to the erp.

@choukalos
Copy link

I assume you're doing calculate on total.
What ERP are you using (name, link to site)? How many digits of precision does your ERP accept?
What API method are you using for your ERP to get order data?

Thanks

@edannenberg
Copy link
Author

Yea, the process starts in $quote->collectTotals()

The ERP uses a precision of 5 internally, the problem is not the wrong tax, but the wrong base row totals that Magento calculates by using the wrong tax. For the order from above the salesOrderInfo() api method returns for the 2nd item:

<row_total xsi:type="xsd:string">16.7300</row_total>
<base_row_total xsi:type="xsd:string">16.7300</base_row_total>
<row_invoiced xsi:type="xsd:string">16.7300</row_invoiced>
<base_row_invoiced xsi:type="xsd:string">16.7300</base_row_invoiced>

16.72 would be correct. The ERP does it's own tax calculation by using one of the above row totals and the endresult will not match with the actual payment recieved as the price incl. tax will come out as 19.91.

@choukalos
Copy link

Okay; so ideally we'd pass 5 digits of precision for those amounts with out rounding and assume the ERP would do the rounding / rounding for user display. Would that be the ideal solution? Could that be a separate set of fields that we show for APIs/DB for integration activities?

Which ERP system is this? Can you provide a link to their web site?

@edannenberg
Copy link
Author

Thats what i use now as a workaround yea. I think ideally Magento should never calculate with values that got prev. rounded to 2 digits of precision. That is the root of the problem. What is the reason for saving to db rounded with only 2 digits of precision?

@tang-yu
Copy link

tang-yu commented Feb 26, 2014

Sometimes, we send the line item information to payment systems, e.g., PayPal, each line item need to be rounded to 2 digits of precision. In your use case, could you use row_total_incl_tax, which should be 19.90 consistently?

@edannenberg
Copy link
Author

Ideally you should round to 2 digits of precision before sending to paypal then.

Anyways, regarding your question, the problem is that i don't have access to the source code of the connector. So right now there are not many options besides overriding the magento api to deliver correct values.

Today another rounding error popped up. The shipping_amount via api is only displayed without tax and rounded to 2 digits of precision. The erp has to calculate the final shipping costs and again ends up with 1 cent over what it should be for certain values, resulting in underpayment errors.

@sasoriza
Copy link

After playing around with the tax settings, it seem changing Tax Calculation Method Based On
from Total to Unit Price fixed my rounding errors in 1.8.1.0. I remember this setting would cause other strange behavior in 1.7.1.0 but sofar this seems to be fixed in 1.8.1.0. Will report again if this really does fix it...

@sasoriza
Copy link

sasoriza commented Apr 8, 2014

Nope, not fixed at all!
Even carts containing only one simple product will be rejected.

26,90 including 19% tax -> always rejected due to the rounding issue (22,61 / 22,60 net value)

@m0rjc
Copy link

m0rjc commented Jan 22, 2016

Re: I don't really get the point of this function, only thing i can think of is working around the precision problems of using floats.

It distributes rounding error amongst the data. Consider an input continually made up of 4/3. You can round the normal way

{1.33, 1.33, 1.33, 1.33, 1.33, 1.33, 1.33, 1.33, 1.33}

But now you have a cumulate error. If you want the total to match the original total you need to place it somewhere. For example on the end:

{1.33, 1.33, 1.33, 1.33, 1.33, 1.33, 1.33, 1.33, 1.36}

This is often done in things like billing where you final or initial bill will be different.

Or you can distribute the error as you go

{1.33, 1.33, 1.34, 1.33, 1.33, 1.34, 1.33, 1.33, 1.34}

At any point in time your total error is kept small, but it's a bit of a pain if your monthly bill keeps changing.

@edannenberg
Copy link
Author

Or you could avoid the issue completely by saving with higher precision at the database level... Ironically the Magento db schema for prices is designed for 4 digits of precision.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment