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*
|
@coderabbitai review |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
WalkthroughThis change refactors I18n configuration by consolidating fallback and pluralization setup from multiple environment files into a centralized initializer with a custom FallbackLocaleList class that defines locale fallbacks as [locale, SiteSetting.default_locale.to_sym, :en] and adds lazy-loading via ensure_loaded! methods. Changes
Sequence DiagramsequenceDiagram
participant Controller as ApplicationController
participant I18n as I18n
participant FallbackList as FallbackLocaleList
participant Backend as I18n.backend
participant Accelerator as translate_accelerator
Controller->>I18n: set I18n.locale
Controller->>I18n: fallbacks.ensure_loaded!
I18n->>FallbackList: ensure_loaded!()
FallbackList->>FallbackList: Get current locale<br/>fallback sequence
FallbackList->>Accelerator: ensure_loaded!(each locale<br/>in sequence)
Accelerator->>Accelerator: Check `@loaded_locales`
Accelerator->>Backend: load_locale(if needed)
Accelerator-->>FallbackList: locale initialized
FallbackList-->>I18n: ready for fallback
I18n-->>Controller: fallback chain ready
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
config/initializers/i18n.rb (1)
4-5: Consider using class reopening instead ofsend(:include).While
send(:include, ...)works correctly, RuboCop flags it as non-idiomatic. If you prefer to address the style warning, you could reopen the class:-require 'i18n/backend/pluralization' -I18n::Backend::Simple.send(:include, I18n::Backend::Pluralization) +require 'i18n/backend/pluralization' +class I18n::Backend::Simple + include I18n::Backend::Pluralization +endThis is purely a style preference and the current code functions correctly.
As per static analysis hints.
📜 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
🪛 Brakeman (7.1.1)
app/controllers/application_controller.rb
[medium] protect_from_forgery should be configured with with: :exception
Type: Cross-Site Request Forgery
Confidence: Medium
More info: https://brakemanscanner.org/docs/warning_types/cross-site_request_forgery/
(Cross-Site Request Forgery)
🪛 RuboCop (1.81.7)
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)
158-159: LGTM! Fallback locales are now preloaded on each request.This change ensures that all fallback locales for the current user's locale are loaded before the request proceeds. The
ensure_loaded!method checks if locales are already loaded, so the performance impact after the first request should be minimal.lib/freedom_patches/translate_accelerator.rb (1)
62-65: LGTM! Efficient locale loading with minimal lock contention.The
ensure_loaded!method provides a public interface for on-demand locale loading. The check on line 64 avoids acquiring the mutex inload_localewhen the locale is already loaded, which is a sensible optimization for the common case.config/initializers/i18n.rb (1)
12-23: Well-designed fallback mechanism with clear extension path.The
FallbackLocaleListclass correctly implements the I18n fallbacks interface by inheriting fromHashand overriding the[]method. The fallback order[locale, default_locale, :en]withuniq.compactproperly handles edge cases where the user locale might match the site default or English.The TODO comment on lines 15-16 identifies a sensible future enhancement for language-specific fallbacks (e.g., Chinese variants).
2 participants
Test 9
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.
Replicated from ai-code-review-evaluation/discourse-coderabbit#9