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.
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.
We're now handling these values on the frontend, so can keep this simple.
fixes up:
Add numericality validation for *
Add translation for Active Record error message
The browser is now responsible for dealing with the decimal separator, for all numeric preferences (as input[type=number]; see Spree::Admin::BaseHelper::preference_field_tag). Calculators are the only place that numeric preferences are used.
It will enforce that only one comma or dot (depending on user's locale) is entered, thus avoiding any ambiguity, and mis-interpretation (eg 100,001 could be interpreted as more than 100 thousand or 100 with a decimal place).
They're all numbers, so we can reliably validate in the browser, removing the need for additional validation logic.
They will still be validated server-side, and in the unlikely even that there is an error, the generic 'Calculator is invalid' message will appear, with the relevant fields highlighted red. I think that's fine.
But we still have the duplicate problem.
Wait a minute, it copies them in the same format that I am copying them. So.. I don't need to copy them at all!
Now I see that we just needed the right format in the translation file.
This is much simpler. Multiple messages get combined onto one line though, which is not perfect.
And.. it still seems to duplicate the errors. Weird, it's fine on my frontend though.
