Improve included (VAT-style) taxes

[jhawthorn]

spree/spree#6295

[mamhoff]

FYI: While the initial need for the PR was MOSS, the PR also fixes a number of idiosyncrasies of the old VAT code (like the strange idea of "refunding" taxes when outside the default VAT zone). The big catch is that with the PR, product and line item prices actually change depending on the user's tax_address. There's another PR to allow more variables to influence prices (notably: Is my customer a tax exempt for some reason?): spree/spree#6662

... and one that fixes rounding issues with VAT spree/spree#6645

If you have any questions, do feel free to reach out.

[peterberkenbosch]

👍 Have some issues with tax included prices and whole order promotions as well. The promotion discount has to be taken into account when calculating tax as well. With the tax on line_items, and the promotions on the order this is currently very hard to do correctly.

In this sample there is a 10% discount on orders > $100. The tax amount should be calculated after promotions, making the tax amount $24.98 and not $27.75

God I hate taxes :D

[mamhoff]

We decided to just not have whole-order promotions. Your case is still sort of manageable, but if you have differently VAT-taxed items in your cart, the taxation of whole-order promotions just becomes a huge mess. You start having questions like how much of the promotion would apply to reduced VAT goods, and how much of the promotion would apply to the rest?

Oh, and mixed carts are just plain impossible with the VAT calculated on the order.

As for your case: Couldn't you tax adjust the order by the included VAT of your promotion?

[peterberkenbosch]

@mamhoff Yeah, I solved it in this instance with re-adjusting the tax amount. Also with non-vat orders, calculating the promotion on the pre-tax-amount on the order. The case you describe with the different taxes per product are nearly impossible indeed.

[mamhoff]

So... it turns out this one might be interesting, too: spree/spree#6852. Here I fix the ReturnItem so that its amount is a price including VAT but excluding promotions, which is what the thing was originally intended for.

[mamhoff]

I'm back from holidays, and will hopefully get some time to spend on this one. I'd like to get feedback on a number of ideas I've been having, hopefully making the entire taxing thing quite a bit more elegant:

1. Sales taxation works well - it should work just as it did before.
2. Prices should always be right. The tax system should adjust them as soon as they are added to the cart OR as soon as the customer changes the relevant tax_address. For this, there should be a TaxAdjustedPriceCalculator. For example: When shipping from a VAT-country to a non-VAT-country, that thing is responsible for removing the included tax from the price entered in the backend. If another VAT rate applies, it adds that other one.
3. The Spree::TaxRate class should do no updating (and especially no update_attributesing) of line items.
4. There should be a Spree::TaxAdjuster class that creates the correct adjustments. I'm almost in favor of one Adjuster class per total column (Spree::TaxAdjuster::LineItemAdditionalTax, Spree::TaxAdjuster::LineItemIncludedTax, Spree::TaxAdjuster::OrderIncludedTax etc). All of these would be pretty small, and called in a configurable sequence.
5. There should be no multiplication or implicit rounding in the database (adding is fine).
6. As much as possible of the tax rate selection should happen in the database, and as little as possible in ruby.

Especially the second one of these points will drastically simplify tax rate selection and allow a pretty large, beneficial refactoring of a number of fun spots.

Oh, very important: the semantic meaning of included taxes is that they are already included in the price. There should be no included tax adjustment that affects a line item total or order total (lamentably, that is the case right now). As a general rule:

total = price * quantity - promotions
pre_tax_total = round(total - included tax)

As far as I'm concerned, the pre_tax_amount column is not necessary unless it's there for performance reasons.

If anyone has input on anything here, do weigh in. Especially naming is paramount, and I might not have come up with the best names for everything. I'll be collaborating on this with @peterberkenbosch 😎

[jhawthorn]

We had some minor improvements into 1.2, and will target the rest for 1.3

[mamhoff]

Closing since 1.3 has been out for a while.