Umbraco 13.3.0; commerce 13.1.4; tax calculation is off. Most likely due to rounding

[SanderLos-Speak]

Describe the bug A clear and concise description of what the bug is. The tax calculation (and totals) calculation give incorrect values. This happens when you set rounding to unit (maybe orderline too) or there are discounts in play.

Steps To Reproduce Setup and replication steps are found in Umbraco support ticket with Ticket ID

71946

Expected behavior correct rounding of the tax

Umbraco Commerce version: Umbraco commerce 13.1.4

[mattbrailsford]

Hey @SanderLos-Speak

Thank you for reporting this. When investigating the support ticket it ultimately boiled down to two things:

1) Discounts are incorrectly rounding at the point of application. The should instead round depending on the stores rounding settings. 2) Regardless of rounding settings, Umbraco Commerce keeps a maximum of 4 decimal places for any price value. In your ticket, you were using quantities of 1000 which meant this was of a magnitude large enough that the 4 decimal place rounding also caused a rounding issue.

I have locally fixed the discount rounding issue which I believe is the bulk of your issue, but I could do with knowing, was the setting of quantities to 1000 purely for test purposes and to highlighting the calculation issue? Or, are you legitimately expecting orders to have orderline quantities exceeding 1000?

[SanderLos-Speak]

we are really expecting large numbers in the orders. We also saw rounding issues when adding for instance 21 products (different prices).

[mattbrailsford]

Ok, then I'll need to look at that more closely as this would require us updating the database tables to accept larger precision numbers. What kind of quantity numbers are you expecting?

Regarding the rounding issues with smaller quantities, is this with or without discount applied? (if it's with discount applied, this is covered by the discounts fix I mentioned above)

[SanderLos-Speak]

We are expecting quantities for a wide range of numbers. We expect the cart/commerce logic to handle anything between 1 and int.max (realistic for our current project up to 100.000).

Regarding the rounding issues with smaller quantities, is this with or without discount applied? (if it's with discount applied, this is covered by the discounts fix I mentioned above) -> this most probably is with discount applied

[mattbrailsford]

Ok, I'm going to check with our accounting department as I want to see if there is some kind of allowed limit to the number of decimal places a software system should support as it would seem odd to say it must support decimal places up to the int.max value.

[mattbrailsford]

I've just now pushed a 13.1.5--preview.4 build to our nightly server, if you want to give that a try, it should fix your issues.

This build updates discounts to only round based on the store rounding settings, and also increases the DB column precision of monetary values from 4 decimal places to 8 which should allow quantities of up to 10.000.000 before it becomes a problem again.

See here for details on how to use the Umbraco nightly feed https://docs.umbraco.com/umbraco-cms/fundamentals/setup/install/installing-nightly-builds

Also, whilst I've checked this myself, do take a DB backup before upgrading as this does perform database changes.

[SanderLos-Speak]

Hi Matt, Thanks for looking at the issue. We will try the new version and see if that takes care of the problem.

[SanderLos-Speak]

Hi Matt, We did some extensive testing and when rounding is set to total with excl tax pricing, the calculations seem to go ok. However, when we give a client 5% discount, we want the unit price rounded to that price and the rest of the calculations go from there.

An example:

we have a product (store set to prices including tax) with a product with price of 94,29. When I add 10 of these, the price is 942,90. If rounding is set to unit price the calculation also rounds the tax per unit. 94,29/121 * 21 = 16,36438016 = 16.36. For 10 units this will then be (in the order) 163,60.

However, when you take the total, we (and Mollie for instance) expect 942,90 / 121 * 21 = 163,6438 = 163,64 which give a rounding error. For this usecase, you could say, set rounding to orderline/total which then gives the correct value.

In our store there are products with a 5% discount on the unit price. the new price will be 94,29 * 95% = 89,5755. If you add 10 items and rounding is set to total, the price would be 895,76 and the tax calculation would be correct. However, we want to round this discount on unit price.

when we round this on unit price, there is a correct calculation of 89,58 per unit (the price the customer also sees on the frontend. However, due to unit price rounding, the tax is also calculated incorrect in the total picture. 89,58 /121 * 21 = 15,54694 which is rounded down to 15,54 in the order.

With example where prices are tax excluded, we see exactly the same rounding issues on unit prices.

The way we see how tax calculations should work is round on the exc/incl tax price and never round the tax price till the end as otherwise you will lose cents/euros in the tax totals.

[mattbrailsford]

Hmm, I don't think it make sense to not round the tax prices as really everything should be rounded at the same time (unless you can't point me to some actual literature that suggest this is the legal requirement for tax calculations)

Re Mollie, you are correct that you must use order line rounding for this and this is clearly highlighted in the documentation for it here https://docs.umbraco.com/umbraco-commerce-payment-providers/mollie/configuring-umbraco

I think this is ultimately getting into more of a feature request than a bug as it really sounds like you want to choose how and when a discount should be rounded (going against the stores rounding setting) so this would likely need changes to the discount reward rule to allow overriding this.

I think your options at the moment would be:

1) Write your own custom reward provider that matches the inbuilt one, but also has an option to override the rounding setting https://docs.umbraco.com/umbraco-commerce/key-concepts/discount-rules-and-rewards

2) Maybe override the default DiscountsPriceAdjuster and round the adjustments it returns, then replace the default implementation

builder.WithUmbracoCommerceBuilder().WithPriceAdjusters()
    .Replace<DiscountsPriceAdjuster, MyDiscountPriceAdjuster>();

[mattbrailsford]

Good morning.

So whilst I still believe our rounding strategy is correct, I've had a look at what we could do to allow you tweak this and so I've managed to implement a rounding service that can be swapped out if you want to handle the rounding differently.

You can find a preview build of it on our nightly feed 13.1.5--rounding-service.preview.10 If you want to test it out and see if it suites your needs that would be great.

You'll need to implement an IRoundingService and replace the default one in a composer calling builder.Services.AddUnique<IRoundingService, MyRoundingService>();

This is the contents of the default rounding service

public class DefaultRoundingService(ICurrencyService currencyService, IStoreService storeService) : IRoundingService
{
    protected readonly ICurrencyService CurrencyService = currencyService;
    protected readonly IStoreService StoreService = storeService;

    public virtual Price RoundPrice(Price price, RoundingPriceType priceType)
    {
        CurrencyReadOnly currency = CurrencyService.GetCurrency(price.CurrencyId);
        StoreReadOnly store = StoreService.GetStore(currency.StoreId);
        var cultureInfo = CultureInfo.GetCultureInfo(currency.CultureName);

        bool shouldRound;

        switch (priceType)
        {
            case RoundingPriceType.UnitBase:
            case RoundingPriceType.UnitBaseAdjustment:
            case RoundingPriceType.Unit:
            case RoundingPriceType.UnitAdjustment:
                shouldRound = store.OrderRoundingMethod == OrderRoundingMethod.Unit;
                break;
            case RoundingPriceType.LineTotalAdjustment:
            case RoundingPriceType.LineTotal:
                shouldRound = store.OrderRoundingMethod <= OrderRoundingMethod.Line;
                break;
            default:
                shouldRound = store.OrderRoundingMethod <= OrderRoundingMethod.Total;
                break;
        }

        if (!shouldRound)
        {
            return price;
        }

        return RoundPrice(price, cultureInfo, store.PricesIncludeTax);
    }

    public virtual Price RoundPrice(Price price, CultureInfo cultureInfo, bool pricesIncludeTax)
    {
        if (pricesIncludeTax)
        {
            var value = Math.Round(price.WithTax, cultureInfo.NumberFormat.CurrencyDecimalDigits, MidpointRounding.AwayFromZero);
            var tax = Math.Round(price.Tax, cultureInfo.NumberFormat.CurrencyDecimalDigits, MidpointRounding.AwayFromZero);
            return new Price(value - tax, tax, price.CurrencyId);
        }
        else
        {
            var value = Math.Round(price.WithoutTax, cultureInfo.NumberFormat.CurrencyDecimalDigits, MidpointRounding.AwayFromZero);
            var tax = Math.Round(price.Tax, cultureInfo.NumberFormat.CurrencyDecimalDigits, MidpointRounding.AwayFromZero);
            return new Price(value, tax, price.CurrencyId);
        }
    }

    public virtual Amount RoundAmount(Amount amount, RoundingAmountType amountType)
    {
        CurrencyReadOnly currency = CurrencyService.GetCurrency(amount.CurrencyId);
        var cultureInfo = CultureInfo.GetCultureInfo(currency.CultureName);
        return RoundAmount(amount, cultureInfo);
    }

    public virtual Amount RoundAmount(Amount amount, CultureInfo cultureInfo)
    {
        var value = Math.Round(amount.Value, cultureInfo.NumberFormat.CurrencyDecimalDigits, MidpointRounding.AwayFromZero);
        return new Amount(value, amount.CurrencyId);
    }
}

You should be able to inherit from this class and then either overide the RoundPrice(Price price, RoundingPriceType priceType) to control exactly when rounding takes place, or the Price RoundPrice(Price price, CultureInfo cultureInfo, bool pricesIncludeTax) if you still want to support the same store setting to control rounding at the Unit, Line or Total but just want to change how the prices are rounded if any rounding should take place.

This is currently in a feature branch as it's a fairly large change so I could do with your feedback before it is merged in.

[SanderLos-Speak]

Hi Matt,

Than you for your feedback and the links to the documentation (we read over the line rounding part of Mollie). Also thanks for looking into the roundingservice. We will look into this to see if this solves our issue.

We did see something odd while testing with these settings. When we set the store to be prices tax included and we add our product 1000 times, in the backend something odd happens. (price rounding is set to order line) (with 5% discount)

8,52 / 1,21 = 7,04132231.... 7,04 95% = 6,689256198.... 1000 6,689256198... = 6689,256198... => round to 2 digits => 6689,26

So it seems the subtotal is correct.

[SanderLos-Speak]

To make the above more clear. Subtotal seems correct, but the net Total is off by a cent. As the rounding is on orderline, I would expect net total = 6689,26 and the tax 21% * 6689,26 = 1404,7446 -> 1404,74

[mattbrailsford]

What version are you running these tests against? One of the nightlies I've produced? Or the current released version?

[SanderLos-Speak]

This is on version 13.1.5--preview.4.g6e2804a

[mattbrailsford]

What is your store "prices include tax setting"?

[SanderLos-Speak]

The store setting for prices include tax is true (prices are tax included) for this example

[mattbrailsford]

Ok so I think I've found the issue, I just need to check my workings.

I think the problem is we end up rounding the total adjustment, and then we used that when working out the order line total value, so the math became

rounded order line total without discount - rounded order line discount amount = orderline total

But this looks like it could end in rounding issue.

I can fix this by just bypassing this calculation and instead use

round(unit price with discount applied * order line quantity) = orderline total

The only issue with this, we still round the total adjustment so if you attempted to go through every object and add them together, the rounded adjustment wouldn't necessarily add up.

Maybe this is a better trade off however than the subtotal being slightly wrong 🤔

[mattbrailsford]

If you want to try the 13.1.5--rounding-service.preview.11 nightly build, this should include my fixes

[mattbrailsford]

Fixed in 13.1.5 released today. Thanks again for reporting.