vendure icon indicating copy to clipboard operation
vendure copied to clipboard

Tax calculations incorrectly rounding

Open exceeded opened this issue 1 year ago • 6 comments

So, this has only just been brought to my attention but I'm finding that nearly all of the VAT calculations within Vendure are rounding incorrectly.

My products are all set up as "Standard Tax", which is set to 20% in Vendure.

Example order

Product is £38.23 (including VAT) and £31.86 (excluding VAT).

Customer has bought 10 of the product and the price shows as £318.60 excluding VAT and £382.30 including VAT.

If you times £318.60 by 1.2 you get £382.32, not £382.30 as shown in Vendure.

Once I've applied a discount of 6.2% the results are even more strange.

Vendure is showing the correct discount amount but then comes out short on the final result.

Please see the following picture and calculation in Excel. Screenshot 2024-02-02 085759 Screenshot 2024-02-02 091032

I'd expect the final amounts to be correct. I can provide more examples. My feeling is that because the price we enter is the VAT inclusive price, the excluded price has more decimals and then Vendure rounds incorrectly using the extra decimal.

  • Vendure Version - 2.1.6
  • Nodejs Version - 21.6.1
  • Database (mysql/postgres etc): Mysql (MariaDB 11.2.2)

exceeded avatar Feb 02 '24 09:02 exceeded

I believe the root of this issue is that the tax calculation strategy is being applied to the proratedUnitPrice, which has already been rounded. This can occur with or without a discount.

One approach could be to not round the prorated unit price, or to create a new property that might be something like rawProratedUnitPrice that is passed to the strategy.

I haven't dug into the discounting math, but a similar early rounding of the unit discounted price may be occurring there.

pevey avatar Feb 02 '24 13:02 pevey

Thanks for the detailed report.

@pevey is correct that the fundamental issue relates to the order of rounding. By default, Vendure will round monetary values first, and then multiply by the quantity.

The solution for this is that in v2 we introduced the MoneyStrategy which allows you to modify this behaviour.

So for instance if you define a MoneyStrategy like this:

import { VendureConfig, DefaultMoneyStrategy } from '@vendure/core';

class MyMoneyStrategy extends DefaultMoneyStrategy {
    round(value: number, quantity = 1): number {
        return Math.round(value * quantity);
    }
}

export const devConfig: VendureConfig = {
    //...
    entityOptions: {
        moneyStrategy: new MyMoneyStrategy(),
    },
};

With this custom strategy, the tax calculation will look like this: image

This should probably be the default but I didn't want to make such a breaking change in v2, but we can certainly make it more visible in the docs & guides.

michaelbromley avatar Feb 05 '24 09:02 michaelbromley

But then surely the version implemented currently is just wrong?

The values we got for VAT are all off, and incorrect.

I have implemented that money strategy and will let you know whether that is a fix for new orders.

exceeded avatar Feb 05 '24 12:02 exceeded

This may be US-only, but taxes are required to be calculated and rounded based on the total amount, not individual lines. Are there places where this is not the case? If not, I think it makes sense to make the change, even if it is a breaking one. As is, for most people, the default does not work as expected.

pevey avatar Feb 05 '24 14:02 pevey

@exceeded Did that solve your issue ? Currently I am still having 1 cents off issues even with a custom money strategy

OlivierPineau-qc avatar Mar 22 '24 19:03 OlivierPineau-qc

The default rounding behaviour will be fixed in v3.1: https://github.com/vendure-ecommerce/vendure/pull/3023

@OlivierPineau-qc can you provide details of the conditions that give and off-by-1 result even with the custom money strategy?

michaelbromley avatar Aug 21 '24 12:08 michaelbromley