Localize the models #397
Localize the models #397
Conversation
ceicke
force-pushed
the
model_localization
branch
from
10b0e1b
to
ecabf71
Compare
There was a problem hiding this comment.
Looking good! Thanks for kicking this off. I read through the Rails Guide on i18n and skimmed a couple articles and just have some feedback on organizing the translation files. I think we should be able to almost always avoid having to type really long keys like model.conversation.grouping... That's my code smell that we're not taking advantage of one of the core rails patterns in some way (like the conversation.rb example which revealed that I had let some view text strings sneak into a model)
app/models/document.rb
Outdated
| @@ -70,7 +70,7 @@ def set_default_bytes | |||
| end | |||
|
|
|||
| def file_present | |||
| errors.add(:file, "must be attached") unless file.attached? | |||
| errors.add(:file, I18n.t('activerecord.errors.messages.must_be_attached')) unless file.attached? | |||
There was a problem hiding this comment.
I don't love the long keys we'll have to reference, lots of opportunity for typos and inadvertent breakage as we're managing multiple files over time. I'm doing some research and it looks like we can simply do: errors.add(:file) and if we organize our translation yml correctly it knows what key that refers to.
Item 5 on this article shows how:
https://medium.com/committed-engineers/rails-i18n-the-power-of-internationalisation-the-burden-of-processes-65f8d122e1a6
| I18n.t('models.conversation.groupings.this_month'), | ||
| I18n.t('models.conversation.groupings.last_month'), | ||
| I18n.t('models.conversation.groupings.older'), | ||
| ] |
There was a problem hiding this comment.
I think this is a good example of a view consideration sneaking it's way into the model. It'd be really nice if we could push this up to a partial somehow.
One slightly hacky way to do it would be to turn these keys into symbols rather than strings—or even better, we turn these into the strings that represent the i18n name! So this array would be: keys = [ ".today", ".yesterday" ...] and then the view which iterates over these keys (somewhere we must have .each do |header, _| we do display it with <%= t header %>
I got this hack by realizing that views support lazy lookups of keys
There was a problem hiding this comment.
Yeah, I hear you. I would put any changes of the logic into a separate PR. I cannot add issues, so maybe it's something you could do so that it doesn't get forgotten :-)
app/models/message.rb
Outdated
| end | ||
|
|
||
| def validate_assistant | ||
| errors.add(:assistant, 'is invalid') unless assistant.user == Current.user | ||
| errors.add(:assistant, I18n.t('errors.messages.invalid')) unless assistant.user == Current.user |
There was a problem hiding this comment.
We can also get these with simply errors.add(:assistant)
| @@ -1,31 +1,228 @@ | |||
| # Files in the config/locales directory are used for internationalization and | |||
There was a problem hiding this comment.
Let's break these long files up like this:
https://guides.rubyonrails.org/i18n.html#organization-of-locale-files
I think that will make it much easier for us to manage them over time, especially when we have multiple languages and we may eventually break up translation tasks (or get GPT to help us :)
Start with localizing the messages from the models.
ceicke
force-pushed
the
model_localization
branch
from
49b5b20
to
9085f19
Compare
Start with localizing the messages from the models.