Use better i18n for generating tax adjustment labels#914
Use better i18n for generating tax adjustment labels#914gmacdougall merged 1 commit intomasterfrom unknown repository
Conversation
There was a problem hiding this comment.
Please remove this space. This should be in the translation, not here
There was a problem hiding this comment.
In that case I could possible also remove the show_rate_in_label column, as that's be handled by I18n entirely, no?
|
|
|
Shouldn't we get rid of the |
|
That would require quite a bit of work, also in Promotions. I think I want to see how the discussion about removing taxes from adjustments generally pans out. If you look at #904, that's how I'd like to see it done in the future. |
There was a problem hiding this comment.
Extra empty line detected at block body beginning.
|
I'm cool with this, I didn't realize previously that the "refund" label could only apply to VAT transactions. Thanks martin 👍 |
|
👍 Much improved. Thank you. |
The previous code for generating tax adjustment labels has several drawbacks: * The i18n keys used are not namespaced * The i18n in place did not allow the main app to specify the number format, which is bad for Europe, where we often write numbers different than the US (1,40 Euro, anyone?) * The code in place did not allow the format of the label to be anything different from what was in Spree::TaxRate#create_label. * The code on place relied on MRI interpreter quirks (namely that a variable in an untouched `if` branch would become `nil` rather than raising a `NoMethodError`. In the future, adjustments should IMO not actually store their translations in the database at all, but instead create them using their own data. But that's not something I want to tackle now. Additionally, I added lots of specs, becaue all of this was entirely untested.
|
I changed the architecture of this a little bit to give more freedom to In case you're wondering why there's a translation for a sales tax refund: it is algorithmically possible if someone switches out the tax calculator. And as with taxes, I've seen pigs fly, I thought I'll allow the possibility. |
|
👍 |
…-labels Use better i18n for generating tax adjustment labels
6 participants
The previous code for generating tax adjustment labels has several
drawbacks:
the number format, which is bad for Europe, where we often
write numbers different than the US (1,40 Euro, anyone?)
to be anything different from what was in Spree::TaxRate#create_label.
variable in an untouched
ifbranch would becomenilratherthan raising a
NoMethodError).In the future, adjustments should IMO not actually store their translations
in the database at all, but instead create them using their own data. But
that's not something I want to tackle now.