Skip to content

Conversation

@everettbu
Copy link
Contributor

Test 9

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*
Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Greptile Summary

This PR implements a comprehensive server-side localization fallback system for Discourse. The changes replace Rails' default i18n fallback mechanism with a custom three-tier fallback hierarchy: user locale → site default locale → English.

The implementation centers around a new config/initializers/i18n.rb file that creates a custom FallbackLocaleList class extending Hash. This class overrides the [] method to return the fallback chain and includes an ensure_loaded! method that preloads all necessary locales. The system integrates with Discourse's existing translation acceleration infrastructure by adding an I18n.ensure_loaded! method in lib/freedom_patches/translate_accelerator.rb.

The fallback system is activated through ApplicationController#set_locale, where I18n.fallbacks.ensure_loaded! is called on every request to ensure all fallback locales are preloaded. This prevents on-demand loading during translation calls, improving performance and avoiding potential race conditions.

As part of this centralization, the previous standalone config/initializers/pluralization.rb file has been consolidated into the main i18n initializer, and Rails' default config.i18n.fallbacks = true settings have been removed from all environment files (production, profile, and cloud66 production) to prevent conflicts with the custom implementation.

Confidence score: 3/5

• This PR introduces significant changes to core i18n functionality that could impact translation behavior across the entire application
• Thread safety concerns exist in the I18n.ensure_loaded! implementation where @loaded_locales ||= [] could cause race conditions in multi-threaded environments
• The consolidation from pluralization.rb needs verification to ensure pluralization functionality wasn't accidentally dropped during the merge into i18n.rb

7 files reviewed, 1 comment

Edit Code Review Bot Settings | Greptile

end

def ensure_loaded!(locale)
@loaded_locales ||= []
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

logic: Race condition: @loaded_locales ||= [] is not thread-safe. If two threads call this simultaneously before @loaded_locales is initialized, both could initialize it, potentially causing inconsistencies.

@everettbu
Copy link
Contributor Author

@greptileai

greptile-apps[bot]

This comment was marked as duplicate.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants