Defect #42815: Limit available locales to those defined by Redmine itself no longer works - Redmine

Actions

Copy link

Defect #42815

closed

Limit available locales to those defined by Redmine itself no longer works

Added by Marius BĂLTEANU 24 days ago. Updated 15 days ago.

Status:

Closed

Priority:

Normal

Assignee:

Marius BĂLTEANU

Category:

I18n

Target version:

5.1.9

Start date:

Due date:

% Done:

Estimated time:

Resolution:

Fixed

Affected version:

Description

The change made in #38529 no longer works, Redmine loads available languages also from plugins or gems in the current trunk.

The issue can be reproduce by adding the following gem:

--- a/Gemfile
+++ b/Gemfile
@@ -19,6 +19,7 @@ gem 'rack', '>= 3.1.3'
 gem "stimulus-rails", "~> 1.3" 
 gem "importmap-rails", "~> 2.0" 
 gem 'commonmarker', '~> 2.3.0'
+gem "doorkeeper-i18n", "~> 5.2"

and then run the test

.F

Failure:
Redmine::I18nTest#test_locales_validness [test/unit/lib/redmine/i18n_test.rb:208]:
Expected: 50
  Actual: 54

I'm not sure which change caused this issue and I didn't test yet on 5.1-stable branch.

Files

Download all files

0001-Extract-available_locales-from-Implementation-module.patch (1.71 KB) 0001-Extract-available_locales-from-Implementation-module.patch		Marius BĂLTEANU, 2025-06-02 21:47
0002-Drop-Redmine-I18n-Backend.new-custom-backend.patch (2.55 KB) 0002-Drop-Redmine-I18n-Backend.new-custom-backend.patch		Marius BĂLTEANU, 2025-06-02 21:55

Related issues

Actions

Copy link

Updated by Marius BĂLTEANU 24 days ago

File 0001-Extract-available_locales-from-Implementation-module.patch 0001-Extract-available_locales-from-Implementation-module.patch added

The attach patch that extracts available_locales from the module Implementation seems to fix the issue; all tests pass.

Please ignore the change in the Gemfile; it is only for testing purposes.

Actions

Copy link

Updated by Marius BĂLTEANU 24 days ago

File 0002-Drop-Redmine-I18n-Backend.new-custom-backend.patch 0002-Drop-Redmine-I18n-Backend.new-custom-backend.patch added

Looking at the code, I'm not sure that we need the custom backend anymore, the available locales can be loaded only from the locales files provided by Redmine directly in config/application.rb.

All tests pass with the attached patch that completely removes the custom backed.

Actions

Copy link

Updated by Holger Just 23 days ago

I very much like the idea of the second patch!

However, I'm a bt conflicted if we should monkey-patch the "core" I18n::Backend::Simple class and forcibly include other modules there. It may be cleaner if we would retain our custom backend, even if it's just an "empty" subclass with some includes. The i18n gem recommends to patch the I18n::Backend::Simple in its documentation a lot though, so they might be okay with it and commit to keeping mostly stable internal interfaces...

In any case though, if you do want to keep your approach of using I18n::Backend::Simple directly, is it deliberate that you did not also include the ::I18n::Backend::Fallbacks module into I18n::Backend::Simple in your second patch?

Actions

Copy link

Updated by Marius BĂLTEANU 22 days ago

Holger Just wrote in #note-3:

I very much like the idea of the second patch!

However, I'm a bt conflicted if we should monkey-patch the "core" I18n::Backend::Simple class and forcibly include other modules there. It may be cleaner if we would retain our custom backend, even if it's just an "empty" subclass with some includes. The i18n gem recommends to patch the I18n::Backend::Simple in its documentation a lot though, so they might be okay with it and commit to keeping mostly stable internal interfaces...

That was my initial plan too, but then I saw the documentation.

In any case though, if you do want to keep your approach of using I18n::Backend::Simple directly, is it deliberate that you did not also include the ::I18n::Backend::Fallbacks module into I18n::Backend::Simple in your second patch?

I chose to simply not include it because from my understanding, the config option (config.i18n.fallbacks = true) from source:trunk/config/application.rb#L60 is doing the same thing, it fallsback to :en if a translation is missing. Also, the test test_fallback from source:trunk/test/unit/lib/redmine/i18n_test.rb is passing.

Actions

Copy link

Updated by Holger Just 15 days ago

Marius BĂLTEANU wrote in #note-4:

Holger Just wrote in #note-3:

However, I'm a bt conflicted if we should monkey-patch the "core" I18n::Backend::Simple class and forcibly include other modules there. It may be cleaner if we would retain our custom backend, even if it's just an "empty" subclass with some includes. The i18n gem recommends to patch the I18n::Backend::Simple in its documentation a lot though, so they might be okay with it and commit to keeping mostly stable internal interfaces...

That was my initial plan too, but then I saw the documentation.

Turns out, that is more or less also what Rails does. In that case, just including the module seems to be the accepted and intended way to enable this.

In any case though, if you do want to keep your approach of using I18n::Backend::Simple directly, is it deliberate that you did not also include the ::I18n::Backend::Fallbacks module into I18n::Backend::Simple in your second patch?

I chose to simply not include it because from my understanding, the config option (config.i18n.fallbacks = true) from source:trunk/config/application.rb#L60 is doing the same thing, it fallsback to :en if a translation is missing. Also, the test test_fallback from source:trunk/test/unit/lib/redmine/i18n_test.rb is passing.

I see. I was not aware how much Rails actually offers here via configuration. Still seems a little selective, but oh well... As such, I agree that enabling config.i18n.fallbacks = true is the way to go here.

Actions

Copy link