Calculate taxes in OrderContents instead of in a LineItem callback #1463
Conversation
And mark Tax::ItemAdjuster as api-private (to discourage stores & extensions from calling it outside of the context of Tax::OrderAdjuster). This PR may result in a little more load because we're now calling Tax::OrderAdjuster at some times when previously we would've just called Tax::ItemAdjuster. Also, we're now _always_ calling Tax::OrderAdjuster after OrderContents#update_cart, instead of only when the location changes. Benefits: - It gets rid of an ActiveRecord callback - It seems safer to call this in more cases - It moves toward performing tax calculations at the order level instead of at the line level.
|
I'm 👍 on this. |
|
First let me say, the change looks great from a technical level. Always happy to see more red than green. 👍 Wanted to comment on this:
I'll say one challenge with this is refunds, at least with TaxCloud, because you don't get any line item breakdown of the tax calculation. If a customer makes a partial refund of an order, without line item records of how much tax was collected, it's impossible to create tax-accurate refunds. This topic hasn't yet been broached in #1252, so perhaps that's a better forum for it? |
@bbuchalter to be clear -- the idea is still for taxes to be calculated and recorded in Solidus at the line level. But we need the tax calculation to be invoked at the order level (i.e. calculate taxes on all the order's items at the same time). Does that address your concern at all? |
|
Yes, absolutely. Thank you. Will be very interested to see how that is done in a performant way! Thanks for all your work and making the work so visible. ❤️ |
This turns out not to be true. For TaxCloud the individual taxes are recorded in the |
And mark Tax::ItemAdjuster as api-private (to discourage stores &
extensions from calling it outside of the context of
Tax::OrderAdjuster).
This PR may result in a little more load because we're now calling
Tax::OrderAdjuster at some times when previously we would've just
called Tax::ItemAdjuster.
Also, we're now always calling Tax::OrderAdjuster after
OrderContents#update_cart, instead of only when the location changes.
Benefits:
instead of at the line level.
See also #1252