FEATURE: Localization fallbacks (server-side) #19
Conversation
The FallbackLocaleList object tells I18n::Backend::Fallbacks what order the languages should be attempted in. Because of the translate_accelerator patch, the SiteSetting.default_locale is *not* guaranteed to be fully loaded after the server starts, so a call to ensure_loaded! is added after the locale is set for the current user. The declarations of config.i18n.fallbacks = true in the environment files were actually garbage, because the I18n.default_locale was SiteSetting.default_locale, so there was nothing to fall back to. *derp*
Code Review by Qodo (Alpha)Analysis CompletedPublished 2 inline comment(s) |
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
| SiteSetting.default_locale | ||
| end | ||
|
|
||
| I18n.fallbacks.ensure_loaded! |
There was a problem hiding this comment.
[remediation_recommended] [Fallback locales not loaded in background jobs]
• The I18n.fallbacks.ensure_loaded! method is only called in set_locale for HTTP requests via controllers
• Background jobs set I18n.locale = SiteSetting.default_locale directly without loading fallback locale files
• When a translation is missing and I18n attempts fallback to :en or site default locale, those locale files won't be loaded
• This causes fallback translations to silently fail, potentially returning MissingTranslation errors in async operations
Agent Prompt
Copy this prompt and use it to remediate the issue with your preferred AI generation tools
## Issue description
Background jobs set `I18n.locale` directly without ensuring fallback locale files are loaded. When translations fall back to other locales, those locale files haven't been loaded, causing fallback translations to fail.
## Issue Context
The new `I18n.fallbacks.ensure_loaded!` method loads all locales in the fallback chain, but it's only called in `set_locale` for HTTP requests. Background jobs in `app/jobs/base.rb` set the locale directly.
## Fix Focus Areas
- app/jobs/base.rb[149-153] - Add `I18n.fallbacks.ensure_loaded!` after setting `I18n.locale`
| def ensure_loaded!(locale) | ||
| @loaded_locales ||= [] | ||
| load_locale locale unless @loaded_locales.include?(locale) | ||
| end |
There was a problem hiding this comment.
[informational] [Thread safety pattern inconsistency]
• The new ensure_loaded! method initializes @loaded_locales with ||= [] outside any mutex protection
• The reload! method sets @loaded_locales = [] without synchronization
• There's a potential race condition where @loaded_locales could be modified between the nil check and subsequent operations
• While the existing translate method has the same pattern, duplicating it perpetuates the inconsistency
Agent Prompt
Copy this prompt and use it to remediate the issue with your preferred AI generation tools
## Issue description
The `ensure_loaded!` method accesses `@loaded_locales` outside mutex protection while `reload!` can modify it concurrently.
## Issue Context
The existing `translate` method has the same pattern, so this is consistent with current code but not ideal.
## Fix Focus Areas
- lib/freedom_patches/translate_accelerator.rb[62-65] - Consider adding a comment explaining why this is safe, or wrap access in mutex
Benchmark PR from qodo-benchmark/discourse-qodo#7