Skip to content

Conversation

@hussam789
Copy link

@hussam789 hussam789 commented Oct 30, 2025

User description

PR #9


PR Type

Enhancement


Description

  • Implement server-side localization fallbacks with custom fallback order

  • Add ensure_loaded! method to guarantee locale availability

  • Remove redundant config.i18n.fallbacks declarations from environment files

  • Consolidate I18n configuration into dedicated initializer


Diagram Walkthrough

flowchart LR
  A["set_locale in ApplicationController"] -->|calls| B["I18n.fallbacks.ensure_loaded!"]
  B -->|loads| C["FallbackLocaleList"]
  C -->|defines order| D["user locale → site locale → en"]
  E["config/initializers/i18n.rb"] -->|configures| C
  F["Removed redundant config from environments"] -->|cleanup| G["Simplified configuration"]
Loading

File Walkthrough

Relevant files
Enhancement
application_controller.rb
Ensure fallback locales loaded on locale set                         

app/controllers/application_controller.rb

  • Added call to I18n.fallbacks.ensure_loaded! in set_locale method
  • Ensures all fallback locales are loaded after locale is set for
    current user
+2/-0     
i18n.rb
Create I18n configuration with fallback support                   

config/initializers/i18n.rb

  • New initializer file that configures I18n with fallbacks support
  • Includes pluralization and fallbacks modules from I18n backend
  • Defines FallbackLocaleList class with custom fallback order: user
    locale → site locale → English
  • Implements ensure_loaded! method to preload all fallback locales
+24/-0   
translate_accelerator.rb
Add ensure_loaded method for locale loading                           

lib/freedom_patches/translate_accelerator.rb

  • Added ensure_loaded! method to load a specific locale if not already
    loaded
  • Method checks @loaded_locales array and calls load_locale if needed
+5/-0     
Configuration changes
pluralization.rb
Remove pluralization configuration                                             

config/initializers/pluralization.rb

  • Removed pluralization configuration (moved to i18n.rb initializer)
  • File is now empty and can be deleted
+0/-2     
production.rb
Remove redundant fallbacks configuration                                 

config/environments/production.rb

  • Removed redundant config.i18n.fallbacks = true declaration
  • Removed associated comment about locale fallbacks
+0/-4     
profile.rb
Remove redundant fallbacks configuration                                 

config/environments/profile.rb

  • Removed redundant config.i18n.fallbacks = true declaration
  • Removed associated comment about locale fallbacks
+0/-4     
production.rb
Remove redundant fallbacks configuration                                 

config/cloud/cloud66/files/production.rb

  • Removed redundant config.i18n.fallbacks = true declaration
  • Removed associated comment about locale fallbacks
+0/-5     

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*
@qodo-code-review
Copy link

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
Unvalidated fallback locale

Description: Custom fallback order uses SiteSetting.default_locale without validation, which could
allow an unexpected or attacker-influenced locale symbol to be used for fallbacks if
SiteSetting is misconfigured, potentially loading unintended translation files.
i18n.rb [12-21]

Referred Code
class FallbackLocaleList < Hash
  def [](locale)
    # 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
  end

  def ensure_loaded!
    self[I18n.locale].each { |l| I18n.ensure_loaded! l }
Ticket Compliance
🎫 No ticket provided
  • Create ticket/issue
Codebase Duplication Compliance
Codebase context is not defined

Follow the guide to enable codebase context checks.

Custom Compliance
🟢
Generic: Meaningful Naming and Self-Documenting Code

Objective: Ensure all identifiers clearly express their purpose and intent, making code
self-documenting

Status: Passed

Generic: Secure Error Handling

Objective: To prevent the leakage of sensitive system information through error messages while
providing sufficient detail for internal debugging.

Status: Passed

Generic: Secure Logging Practices

Objective: To ensure logs are useful for debugging and auditing without exposing sensitive
information like PII, PHI, or cardholder data.

Status: Passed

Generic: Security-First Input Validation and Data Handling

Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent
vulnerabilities

Status: Passed

Generic: Comprehensive Audit Trails

Objective: To create a detailed and reliable record of critical system actions for security analysis
and compliance.

Status:
Missing auditing: The added localization fallback loading action is not audited or logged, which may hinder
traceability of locale-related behavior changes.

Referred Code
def set_locale
  I18n.locale = if current_user
                  current_user.effective_locale
                else
                  SiteSetting.default_locale
                end

  I18n.fallbacks.ensure_loaded!
end
Generic: Robust Error Handling and Edge Case Management

Objective: Ensure comprehensive error handling that provides meaningful context and graceful
degradation

Status:
Missing error handling: The new ensure_loaded! method and fallback sequence perform I/O-dependent loading without
rescuing or logging failures, leaving edge cases like missing locale files unhandled.

Referred Code
def ensure_loaded!(locale)
  @loaded_locales ||= []
  load_locale locale unless @loaded_locales.include?(locale)
end
Compliance status legend 🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

@qodo-code-review
Copy link

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
Possible issue
Avoid database access during initialization

To prevent crashes during initialization tasks like rake assets:precompile,
check if the application is initialized before accessing
SiteSetting.default_locale, and use I18n.default_locale as a fallback.

config/initializers/i18n.rb [12-23]

 class FallbackLocaleList < Hash
   def [](locale)
     # 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
+    default_locale = Rails.application.initialized? ? SiteSetting.default_locale.to_sym : I18n.default_locale
+    [locale, default_locale, :en].uniq.compact
   end
 
   def ensure_loaded!
     self[I18n.locale].each { |l| I18n.ensure_loaded! l }
   end
 end
  • Apply / Chat
Suggestion importance[1-10]: 9

__

Why: The suggestion correctly identifies a critical issue where database access via SiteSetting during initialization can cause boot failures, and it provides a robust solution.

High
Prevent race condition in locale loading

Remove the redundant and non-thread-safe unless @loaded_locales.include?(locale)
check from ensure_loaded! and unconditionally call load_locale, which already
contains a thread-safe check.

lib/freedom_patches/translate_accelerator.rb [62-65]

 def ensure_loaded!(locale)
   @loaded_locales ||= []
-  load_locale locale unless @loaded_locales.include?(locale)
+  load_locale locale
 end
  • Apply / Chat
Suggestion importance[1-10]: 6

__

Why: The suggestion correctly identifies a redundant, non-thread-safe check, and removing it improves code correctness and simplifies the logic by relying on the already-synchronized load_locale method.

Low
  • More

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants