FEATURE: Localization fallbacks (server-side)#2
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*
There was a problem hiding this comment.
Pull request overview
This PR introduces server-side localization fallbacks for improved internationalization support. The system now intelligently cascades through the requested language, site default, and English when translations are missing, with optimized on-demand locale loading.
Changes:
- Implemented custom fallback logic that resolves translations through user locale → site default → English
- Consolidated i18n initialization from deprecated environment configs into a centralized initializer
- Added on-demand locale loading to ensure fallback locales are loaded when needed
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| lib/freedom_patches/translate_accelerator.rb | Added ensure_loaded! helper method for on-demand locale loading |
| config/initializers/pluralization.rb | Removed deprecated pluralization initializer (moved to i18n.rb) |
| config/initializers/i18n.rb | New centralized i18n configuration with custom fallback implementation |
| config/environments/profile.rb | Removed Rails default fallback configuration |
| config/environments/production.rb | Removed Rails default fallback configuration |
| config/cloud/cloud66/files/production.rb | Removed Rails default fallback configuration |
| app/controllers/application_controller.rb | Added fallback locale loading trigger in set_locale |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| def ensure_loaded!(locale) | ||
| @loaded_locales ||= [] | ||
| load_locale locale unless @loaded_locales.include?(locale) | ||
| end |
There was a problem hiding this comment.
The @loaded_locales instance variable is initialized here but also initialized in the load_locale method (line 59). This duplicated initialization logic could lead to inconsistency. Consider initializing @loaded_locales once in an initialize method or at the class level to ensure consistency across methods.
| # user locale, site locale, english | ||
| # TODO - this can be extended to be per-language for a better user experience | ||
| # (e.g. fallback zh_TW to zh_CN / vice versa) | ||
| [locale, SiteSetting.default_locale.to_sym, :en].uniq.compact |
There was a problem hiding this comment.
If SiteSetting.default_locale returns nil, calling .to_sym on it will raise a NoMethodError. The fallback chain should handle the case where SiteSetting.default_locale might be nil before calling .to_sym. Consider adding a nil check or using &.to_sym safe navigation.
| [locale, SiteSetting.default_locale.to_sym, :en].uniq.compact | |
| [locale, SiteSetting.default_locale&.to_sym, :en].uniq.compact |
| I18n.backend.class.send(:include, I18n::Backend::Fallbacks) | ||
|
|
||
| # Configure custom fallback order | ||
| class FallbackLocaleList < Hash |
There was a problem hiding this comment.
The class name FallbackLocaleList suggests it's a list, but it inherits from Hash. This is misleading since the class behaves more like a locale resolver than a collection. Consider renaming to LocaleFallbackResolver or FallbackLocaleResolver to better reflect its purpose.
Test 9
Summary by CodeRabbit
Release Notes
Refactor
Chore
✏️ Tip: You can customize this high-level summary in your review settings.
Replicated from ai-code-review-evaluation/discourse-coderabbit#9