Nop 4.30 Critical Tax Rounding Error

Hi Team,

I believe I have found what amounts to a critical flaw in Nop 4.30. Cart Total is often incorrect. We run many instances of NopCommerce and haven't experienced the issue in any 3.XX version.

Tax can often be incorrect by 0.01


My use case is:
Prices Include Tax
Prices are shown including Tax
Tax is simple VAT at 20%

It seems that NopCommerce maintains subtotal using a Ex-Tax amount even when pricing is Inc-Tax. It then recalculates the Order Total from this and because of the 4 Decimal Place database table, the Order total is often wrong by 0.01.

Any suggestion on a workaround, fix or code-fix would be much appreciated. This incorrect total is pushed through to the payment gateway. We obviously cannot go live with this site with such a serious problem.

Thanks for any help!

Giles

Good Day NOP Team, Our cart https://store.gedusa.com is experiencing rounding errors, I did see multiple threads regarding this topic, is there a patch and/or hot fix out there to address this bug

ckoteles wrote:
It looks like an ongoing open bug in the git issues. Have you tried this workaround?
https://www.nopcommerce.com/en/boards/topic/52066/tax-rounding-to-the-third-decimal

Thanks for the suggestion.

I've found a workaround by changing this setting to false "shoppingcartsettings.roundpricesduringcalculation"

It's losing accuracy when it rounds for a few reasons (including what seems to be a really unforgivable bit of coding):
1) Everything is converted back to Net pricing to do calculations, then converted back. Causes a lot of problems with discounts.
2) The wrong type of rounding is being used
3) At one point a Float is being used for money. Floats and Doubles should never be used for financial calculations as they are fundamentally not accurate (this is really naughty)

Really, in my opinion, the whole system of calculating Gross (inc tax) pricing tax and discounts needs to be rewritten from scratch, so that it is fundamentally based on Gross prices if that's what is being stored as the product price. It's the only way to guarantee accuracy.

We've just released a store for a supermarket chain in the Channel Islands which is taking 10s of thousands of pounds a day and we had to do an enormous amount of work to get this sorted. The ERP system picked up on it initially (during testing thankfully).

Obviously I still love NopCommerce after 12 years or I wouldn't still be using it. So all of the above is meant as constructive critique!

Thanks for the info. We are going to reconsider the tax calculation (and tax providers) process. Follow this work item.

Is there a solution to this problem yet?

We have already gone live, not realising this was a thing and now we are trying desperately to unpick.

I have looked at the various threads and cannot find a solution. Here is a picture of a typical last stage of checkout.

https://1drv.ms/u/s!AhP5LO0kFiAC5hebkknevnzywflU?e=FKnxTp

Thanks in anticipation of your help.

Steve

Hugely grateful for the workaround from fireblade669.

Is this still the best solution 9 months later?

Steve

Hello,
it is weird such a functionnality does not work on a commerce platform... I would not comment the code but having classes with thousands of line of code is not optimized to modify it and ensure we do not break anything.
Anyway we did a modification that solved our problem.
The solution is an awful hack but i share it here if someone has the need. The real solution would be to calculate correctly the price. That is double check that TotalPrice = SubTotal + Tax. As the algorithm is very (too much) complex, in a function that has more than 100 lines, we did not want to break it. Therefore we just modified how the values are set in the model.
In ShoppingCartFactoryModel around line 1312 you have : return model;
Just above this line add the following:
try
            {

                decimal.TryParse(model.OrderTotal.TrimStart('€'), out decimal ttc);
                decimal.TryParse(model.SubTotal.TrimStart('€'), out decimal ht);
                decimal.TryParse(model.Tax.TrimStart('€'), out decimal tva);
                
                if(ttc != ht + tva)
                {
                    tva = ttc - ht;
                    model.Tax = _priceFormatter.FormatPrice(tva, true, false);
                }
            }
            catch(Exception ex)
            {
                //We don't want to throw. If there is an issue, the model will remain as it is.
            }

I would have prefered not to parse strings to do so, but as the calculation process is not straight forward, this little hack do the job.

As firblade said, the whole stuff should be rewritten but it is a lot of work !

Be aware that modifying the code will prevent any update unless we patch the new code again.

To the Nop Team, please respect SOLID principles, please do not write so much code in Controllers be rather do it in services or other buisiness classes, please do not pass so many out parameters in your functions as all these are real modification blocker.

Best regards.