Properly compute shipping order line total

[olivierguerriat]

The total column of the order line didn't consider the tax for shipping lines.

I added a few assertions and updated CalculateTax to fill every amounts.

I also simplified ApplyShipping to avoid recomputing the same amounts and only fill the breakdown. Please check that it makes sense, I'm out of my comfort zone there.

[vercel]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
lunar-docs	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Aug 22, 2024 2:30pm

[alecritson]

Appreciate this has been hanging around a bit. I'm not sure the shippingSubTotal should be in the calculate tax action as we need it before we calculate tax , the sub total needs to be available to any actions that may come between and it's a bit out the scope of what the CalculateTax action is attempting to do.

[olivierguerriat]

Thanks for reviewing. Splitting concerns makes sense, I reverted ApplyShipping and adapted CalculateTax accordingly.

[glennjacobs]

Having reviewed the code these are my thoughts...

• We need documentation to reflect these changes
• If there is no shipping, should the shipping values be 0 rather than null?
• Maybe we should have a test to cover the no-shipping scenario?

[glennjacobs]

@alecritson I think this is probably needed, however, do we need to consider any updates to the Orders side of things?