Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow overriding of entry resolving entry resolving separate from defaults #622

Conversation

movermeyer
Copy link
Contributor

What are you trying to accomplish?

Fixes #617. Alternative to #620.

#591 was incorrect, since I didn't check/realize that resolve is called in three different cases:

  1. When the value found in the translation file is a Symbol (Code)
  2. When a value is not found, then values from defaults are resolved. (Code)
    • When resolving defaults, you want to do it only at the level of the current locale, before falling back.
    • i.e., Go through all defaults before falling back through fallbacks locales, keeping the behaviour as it has always been.
  3. When the value found in the translation file is something else, especially a Proc (Code)
    • This is the same case as 1., where you are resolving a entry, but I mention it separately since it is being called from a different place in the code.

Prior to #591, there was no distinction between the cases. #591 should have introduced that distinction.

What approach did you choose and why?

I added a resolve_entry method which can be overridden to handle the case of resolving an entry separately from resolving defaults. By default resolve_entry is an alias of resolve.

I then changed the calls to resolve to use resolve_entry in cases where they are resolving entries.

What should reviewers focus on?

🤷

The impact of these changes

Fixes #617.

Testing

I added the regression test from #620 to show that it works with the existing Rails calls.

@movermeyer movermeyer force-pushed the movermeyer/resolve_defaults_with_current_locale branch from a5376cd to 19f190d Compare February 13, 2022 18:28
@radar
Copy link
Collaborator

radar commented Feb 14, 2022

LGTM, thank you @movermeyer for investigating this on a Sunday :)

@radar
Copy link
Collaborator

radar commented Feb 14, 2022

The tests confirm it, but just to double check as well, I've used the repro app from #617 along with this new branch -- I can (double extra) confirm this branch fixes the issue.

@radar radar merged commit 789d12e into ruby-i18n:master Feb 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG] Fallbacks do not work for Rails STI models
2 participants