All tax calculations when discounts are present are wrong


#1

I filed a bug earlier today when I discovered that invoice calculations on line items with discounts was being incorrectly calculated.

While I have already fixed that (https://github.com/InvoicePlane/InvoicePlane/pull/637), I have now discovered that the bug also applies to quotes (easily fixable), HOWEVER way that tax is calculated on invoices with discounts is also incorrect. Applying a discount to an invoice as a whole over-collects the amount of tax that should be collected, then under-collects the total amount due for the invoice including the discounts.

I guess no one has ever checked this (I don’t see a single unit test in here), but it is a massive bug. It means users aren’t getting all the money they should be getting AND they are over paying their local government too much taxes.

Also, the method of calculating money is wrong.

Upon further inspection of the code, money amounts are calculated using floats, which is not the right way to do this in PHP. Short version is: floating point is base 2, money is base 10. You can’t make calculations that way and be accurate to the penny. (See: http://www.lsi.upc.edu/~robert/teaching/master/material/p5-goldberg.pdf).

Since we now have IP and IP2, I am not sure which one this needs to be corrected in, but it’s a huge problem if you give any discounts to anyone. Internally, our work-around is to never use the invoice level discounts, and only use the line-item discounts.


#2

Hello,
thank you for your input. The calculation of items and totals was discussed several times and users already reported that the current calculation is not correct. However, this is not that easy to fix. We cannot simply change the calculation process because this will break all existing quotes / invoices.

Related topics:
Allow users to modify the tax and item calculation order, Calculate invoice discount before invoice tax

This will be fixed in this issue: https://development.invoiceplane.com/browse/IP2-5

I also closed your pull request as it sadly does not provide a suitable fix for this problem even if it’s fixing the core issue.


The overall calculation will be rewritten and updated in InvoicePlane 2. We know that using floats is not the best way to calculate things and we will optimize the process in the next version.

See https://development.invoiceplane.com/browse/IP2-5 and https://development.invoiceplane.com/browse/IP2-6