Correctly apply relevant tax rates #4327
Conversation
This is where I expect them to be
The check for relevant tax categories is now done in the TaxRate model
…to 1 method Rather than two methods which do the exact same thing, just with different object names
|
👍 |
|
Will this be merged to 2-0-stable or just 2-1-stable and upwards? |
|
@jdurand Just 2-2-stable and upwards, as that has the proper adjustments on the taxes. 2-1-stable and below have adjustments being applied to sometimes the order and sometimes the line item. |
|
Going to merge this into 2-2-stable and master tomorrow if nobody sees any problems with it. Will fix up 2-1-stable next week. |
| def display_price | ||
| price = display_base_price.to_s | ||
| if tax_rate | ||
| amount = "#{display_tax_amount} #{tax_rate.name}" |
There was a problem hiding this comment.
It would be great to have this translatable/localizable
|
Hi @radar We have been testing master and your relevant-taxes branch following your recommendation for European VAT support. Our objective is to try use Spree without modifications to solve the European VAT case. We worked the example of an Spanish company selling to Spain, France and Kenya. We have added screenshots and what we expected to be shown in the following document. https://docs.google.com/spreadsheet/ccc?key=0Ai2VOmMTsnohdEdiQW14ZDFSSmFpelNoNUtVZTIwMEE&usp=sharing Our results and comments are:
Hope this work helps and ask anything you need. I will like to see that Spree is European VAT Ready and well done with the adjustments refactoring . Thanks Genís |
|
I've added this PR to master and 2-2-stable today. The 2.2 release is imminent and I think it's worthwhile having this PR in as it addresses some (but admittedly not all) issues with taxes. We can take a look at those issues after 2.2 has been released as part of our work on 2.2.1. The thing I said I would do for 2-1-stable will also be done at a later stage. I will be leaving this PR open. |
|
Hi @cduv, I have done some (read: I started at a little after 9:30am and it's now almost 4pm) work on this today and I think I've solved it. Which probably means that I haven't at all and someone else will find some weird edge case but that's OK. I think. Without further ado, here's Spain (the default tax zone), France (the not-so-default tax zone) and Australia (no tax zones). All amounts according to my calculations are correct. Here are the commits: radar/spree@850c04e9cd61529b246c83517780c4968c6fb146 And finally: radar/spree@189f3e1ddf90ac7d39a4e47c7003e238ce205ab4 (which took forever to track down!) |
|
Hi @radar Great work!!! We are going to check it, but from your results looks that is working nice. I will confirm. Thanks |
…itive adjustment for the order's tax zone if the tax rate is included in the price This should fix the Spain + France + Kenya problems outlined here: #4327 (comment)
|
🎉 Thank you @cduv. That confirmation would be very much appreciated, but no rush on it. |
…itive adjustment for the order's tax zone if the tax rate is included in the price This should fix the Spain + France + Kenya problems outlined here: #4327 (comment)
|
Run into any issues @cduv or have you confirmed things are working as expected? |
|
Going to close out, but will reopen if needed. |
This is a medium size PR which addresses a couple of things:
1. Shipments were not being taxed correctly if the rate was an inclusive tax.
This is now fixed by shipping methods linking to tax categories, and then when shipping rates are generated the relevant tax rate (if any) is linked to the shipment. As a "side-effect" of this, shipping rates are able to display the amount of tax they will incur.
2. Fixes #4318
I've personally witnessed a case where tax rates from the same tax category were being applied to an item in the order. See my example in #4318. There is now a fix for this within this PR.
3. Altered the Spree::TaxRate specs to call class-level adjust, not instance-level
This means that the fix for point number 2 here is tested. A small difference, but an important one.
4. Pre-tax amounts are now being stored
Previously, if you wanted to gain access to the pre-tax amount for an item you would need to re-calculate that based off the tax for that item. Now it's stored and you don't need to do the calculation. This amount is then used to calculate the amount of tax that should be applied to an item. For instance, a $10 item with 10% inclusive tax should have a tax amount of $0.91, not $1.00. Sales tax of 10% would cause the tax rate to be an additional one of 10%, which would be $1.00 and NOT $0.91.
(Have I mentioned how much fun tax is yet?)
5. Fixed some silly double looping in checkout's sidebar
It was seriously dumb and I think it was my fault. It was causing tax rates to appear not once but twice. Fix was simple.
Build is green on CI.
/cc @cduv @jdurand