It turns out the "tax_rate" association isn't used and wasn't working.
Same for the "voucher" one, which I added to be consistent with existing
code.
Both of these weren't caught by the specs because you can't test associations
with a custome relation with 'shouda-matchers' see: https://github.com/thoughtbot/shoulda-matchers/issues/981
Voucher adjustment have complicated calculation that prevent us from
using Spree::Calculator, and we need to override CalculatedAdjustments
method, so no point using it.
We still keep the same methods to stay as consitent as possible in
regards to adjustments
Add missing, "inverse_of" and "dependent" options. Use :nullify as
for "dependent" because we would want to keep the adjustments on order
even if the voucher is deleted.
At the time when we can apply a voucher to an order (payment step) we
don't have any data on possible taxes and payment fees. These are
calculated when we move to the summary step. So voucher are applied in
two step:
- create an "open" voucher adjustment on the order when a customer enters
a voucher code.
- when we move to the summary step, recalulate the amount and
possible tax for the voucher adjustment, once taxes and payment fees
have been included in the order. And then close the original and update
order to reflect the changes
The checkout payment step will also show a warning.
The amount used is stored in the adjustment created, so we will be able
to track how much has been used down the line.
An ExchangeVariant is a simple representation of a join table between Exchange and Variant. Previously this code was triggering an additional INSERT query for every variant added to the newly cloned exchange. Some exchanges have ~3000 variants! The code now creates them in bulk in a single INSERT statement. When cloning large order cycles this can improve performance by ~1000% or so.
I changed the text to focus on the resolution: the user needs to choose
a tax category or a different calculator.
I associated the error to the model to prevent the attribute name from
being included in the error message. Alternatively, we could have
changed the name of the attribute to match the UI. But this error
affects the combination of two attributes, none of them is invalid on
its own.
I'm using Rails' default lazy lookup for error messages which results in
shorter code and a standard structure.
I also added a simple spec.
