-
Notifications
You must be signed in to change notification settings - Fork 0
FEATURE: Localization fallbacks (server-side) #9
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
base: localization-system-pre
Are you sure you want to change the base?
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*
WalkthroughThis change refactors I18n fallback and pluralization configuration. It removes direct fallback settings from environment files, consolidates pluralization and fallback logic into a new initializer, introduces a custom fallback locale list, and adds an Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant ApplicationController
participant I18n
participant FallbackLocaleList
User->>ApplicationController: Request with/without locale
ApplicationController->>I18n: Set locale
ApplicationController->>I18n: fallbacks.ensure_loaded!
I18n->>FallbackLocaleList: ensure_loaded!
FallbackLocaleList->>I18n: ensure_loaded!(locale)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~15 minutes Poem
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. ✨ Finishing Touches
🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
config/initializers/i18n.rb (1)
4-5: Use direct include instead of send(:include).The static analysis correctly identifies a style issue. Use direct
includefor better readability and to follow Ruby conventions.-require 'i18n/backend/pluralization' -I18n::Backend::Simple.send(:include, I18n::Backend::Pluralization) +require 'i18n/backend/pluralization' +I18n::Backend::Simple.include I18n::Backend::Pluralization
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
app/controllers/application_controller.rb(1 hunks)config/cloud/cloud66/files/production.rb(0 hunks)config/environments/production.rb(0 hunks)config/environments/profile.rb(0 hunks)config/initializers/i18n.rb(1 hunks)config/initializers/pluralization.rb(0 hunks)lib/freedom_patches/translate_accelerator.rb(1 hunks)
💤 Files with no reviewable changes (4)
- config/cloud/cloud66/files/production.rb
- config/environments/production.rb
- config/initializers/pluralization.rb
- config/environments/profile.rb
🧰 Additional context used
🧬 Code Graph Analysis (2)
app/controllers/application_controller.rb (2)
config/initializers/i18n.rb (1)
ensure_loaded!(20-22)lib/freedom_patches/translate_accelerator.rb (1)
ensure_loaded!(62-65)
lib/freedom_patches/translate_accelerator.rb (1)
config/initializers/i18n.rb (3)
ensure_loaded!(20-22)locale(12-23)locale(13-18)
🪛 RuboCop (1.76.1)
config/initializers/i18n.rb
[warning] 5-5: Use include I18n::Backend::Pluralization instead of send(:include, I18n::Backend::Pluralization).
(Lint/SendWithMixinArgument)
🔇 Additional comments (3)
app/controllers/application_controller.rb (1)
159-159: LGTM! Proper integration with custom fallback system.The addition of
I18n.fallbacks.ensure_loaded!correctly ensures that fallback locales are preloaded immediately after setting the primary locale, integrating well with the customFallbackLocaleListimplementation.lib/freedom_patches/translate_accelerator.rb (1)
62-65: LGTM! Well-implemented safe locale loading.The
ensure_loaded!method correctly prevents redundant locale loading by checking@loaded_localesbefore callingload_locale. The implementation is thread-safe and integrates well with the existing mutex-protectedload_localemethod.config/initializers/i18n.rb (1)
12-24: LGTM! Well-designed custom fallback implementation.The
FallbackLocaleListclass provides a sensible fallback order (user locale → site default → English) with proper deduplication. Theensure_loaded!method correctly integrates with the newI18n.ensure_loaded!method to preload all fallback locales.
Test 9
Summary by CodeRabbit
New Features
Chores