From e19a7a7c8d65bb428708ab09ec4fc9af2ed6ca9a Mon Sep 17 00:00:00 2001 From: Gerhard Schlager Date: Thu, 16 Dec 2021 16:44:21 +0100 Subject: [PATCH] FIX: translation precedence was different on client and server As an example, the lookup order for German was: 1. override for de 2. override for en 3. value from de 4. value from en After this change the lookup order is the same as on the client: 1. override for de 2. value from de 3. override for en 4. value from en see /t/16381 --- lib/freedom_patches/translate_accelerator.rb | 64 ++++++++------- lib/i18n/backend/discourse_i18n.rb | 8 +- .../translate_accelerator_spec.rb | 82 +++++++++++++++++-- spec/lib/i18n/discourse_i18n_spec.rb | 5 -- 4 files changed, 114 insertions(+), 45 deletions(-) diff --git a/lib/freedom_patches/translate_accelerator.rb b/lib/freedom_patches/translate_accelerator.rb index 7bf8988d422..744b5f843c0 100644 --- a/lib/freedom_patches/translate_accelerator.rb +++ b/lib/freedom_patches/translate_accelerator.rb @@ -72,21 +72,33 @@ module I18n execute_reload if @requires_reload locale = (opts[:locale] || config.locale).to_sym - load_locale(locale) unless @loaded_locales.include?(locale) - opts ||= {} - - target = opts[:backend] || backend - results = opts[:overridden] ? {} : target.search(locale, query) + results = {} regexp = I18n::Backend::DiscourseI18n.create_search_regexp(query) - (overrides_by_locale(locale) || {}).each do |k, v| - results.delete(k) - results[k] = v if (k =~ regexp || v =~ regexp) + + if opts[:only_overridden] + add_if_matches(overrides_by_locale(locale), results, regexp) + else + target = opts[:backend] || backend + + I18n.fallbacks[locale].reverse_each do |fallback| + add_if_matches(target.search(fallback, query), results, regexp) + add_if_matches(overrides_by_locale(fallback), results, regexp) + end end + results end + def add_if_matches(translations, results, regexp) + translations.each do |key, value| + if key =~ regexp || value =~ regexp + results[key] = value + end + end + end + def ensure_loaded!(locale) locale = locale.to_sym @loaded_locales ||= [] @@ -144,7 +156,7 @@ module I18n end def overrides_by_locale(locale) - return unless @overrides_enabled + return {} unless @overrides_enabled return {} if GlobalSetting.skip_db? locale = locale.to_sym @@ -191,30 +203,26 @@ module I18n if @overrides_enabled overrides = {} - - # for now lets do all the expensive work for keys with count - # no choice really - has_override = !!options[:count] + has_count = options.has_key?(:count) I18n.fallbacks[locale].each do |l| - override = overrides[l] = overrides_by_locale(l) - has_override ||= override.key?(key) + overrides_for_locale = overrides_by_locale(l) + overrides[l] = overrides_for_locale if has_count || overrides_for_locale.key?(key) end - if has_override && overrides.present? - if options.present? - options[:overrides] = overrides + if overrides.present? + no_options = options.empty? || (options.size == 1 && options.has_key?(:locale)) - # I18n likes to use throw... - catch(:exception) do - return backend.translate(locale, key, options) - end - else - overrides.each do |_k, v| - if result = v[key] - return result - end - end + # Shortcut if the current locale has an override and there are no options. + if no_options && (override = overrides.dig(locale, key)) + return override + end + + options[:overrides] = overrides + + # I18n likes to use throw... + catch(:exception) do + return backend.translate(locale, key, options) end end end diff --git a/lib/i18n/backend/discourse_i18n.rb b/lib/i18n/backend/discourse_i18n.rb index 9962206f637..1511e67e961 100644 --- a/lib/i18n/backend/discourse_i18n.rb +++ b/lib/i18n/backend/discourse_i18n.rb @@ -55,14 +55,8 @@ module I18n end def search(locale, query) - results = {} regexp = self.class.create_search_regexp(query) - - I18n.fallbacks[locale].each do |fallback| - find_results(regexp, results, translations[fallback]) - end - - results + find_results(regexp, {}, translations[locale]) end protected diff --git a/spec/components/freedom_patches/translate_accelerator_spec.rb b/spec/components/freedom_patches/translate_accelerator_spec.rb index 103973256ff..53346b92feb 100644 --- a/spec/components/freedom_patches/translate_accelerator_spec.rb +++ b/spec/components/freedom_patches/translate_accelerator_spec.rb @@ -51,14 +51,14 @@ describe "translate accelerator" do it "overrides for both string and symbol keys" do key = 'user.email.not_allowed' - text_overriden = 'foobar' + text_overridden = 'foobar' expect(I18n.t(key)).to be_present - override_translation('en', key, text_overriden) + override_translation('en', key, text_overridden) - expect(I18n.t(key)).to eq(text_overriden) - expect(I18n.t(key.to_sym)).to eq(text_overriden) + expect(I18n.t(key)).to eq(text_overridden) + expect(I18n.t(key.to_sym)).to eq(text_overridden) end describe ".overrides_by_locale" do @@ -197,7 +197,7 @@ describe "translate accelerator" do expect(I18n.t('keys.magic', count: 2)).to eq('no magic keys') end - it "returns the overriden text when falling back" do + it "returns the overridden text when falling back" do override_translation('en', 'got', 'summer') expect(I18n.t('got')).to eq('summer') expect(I18n.with_locale(:zh_TW) { I18n.t('got') }).to eq('summer') @@ -224,4 +224,76 @@ describe "translate accelerator" do expect(Fish.model_name.human).to eq('Fish') end end + + context "translation precedence" do + def translation_should_equal(key, expected_value) + I18n.locale = :en + expect(I18n.t(key, locale: :de)).to eq(expected_value) + expect(I18n.search(key, locale: :de)[key]).to eq(expected_value) + + I18n.locale = :de + expect(I18n.t(key)).to eq(expected_value) + expect(I18n.search(key)[key]).to eq(expected_value) + end + + context "with existing translations in current locale and fallback locale" do + context "with overrides in both locales" do + it "should return the override from the current locale" do + override_translation("de", "foo", "Override of foo in :de") + override_translation("en", "foo", "Override of foo in :en") + translation_should_equal("foo", "Override of foo in :de") + end + end + + context "with override only in current locale" do + it "should return the override from the current locale" do + override_translation("de", "foo", "Override of foo in :de") + translation_should_equal("foo", "Override of foo in :de") + end + end + + context "with override only in fallback locale" do + it "should return the translation from the current locale" do + override_translation("en", "foo", "Override of foo in :en") + translation_should_equal("foo", "Foo in :de") + end + end + + context "with no overrides" do + it "should return the translation from the current locale" do + translation_should_equal("foo", "Foo in :de") + end + end + end + + context "with existing translation in fallback locale" do + context "with overrides in both locales" do + it "should return the override from the current locale" do + override_translation("de", "fish", "Override of fish in :de") + override_translation("en", "fish", "Override of fish in :en") + translation_should_equal("fish", "Override of fish in :de") + end + end + + context "with override only in current locale" do + it "should return the override from the current locale" do + override_translation("de", "fish", "Override of fish in :de") + translation_should_equal("fish", "Override of fish in :de") + end + end + + context "with override only in fallback locale" do + it "should return the translation from the current locale" do + override_translation("en", "fish", "Override of fish in :en") + translation_should_equal("fish", "Override of fish in :en") + end + end + + context "with no overrides" do + it "should return the translation from the fallback locale" do + translation_should_equal("fish", "original fish") + end + end + end + end end diff --git a/spec/lib/i18n/discourse_i18n_spec.rb b/spec/lib/i18n/discourse_i18n_spec.rb index ee2258db139..759467675dc 100644 --- a/spec/lib/i18n/discourse_i18n_spec.rb +++ b/spec/lib/i18n/discourse_i18n_spec.rb @@ -44,11 +44,6 @@ describe I18n::Backend::DiscourseI18n do end describe 'fallbacks' do - it 'uses fallback locales for searching' do - expect(backend.search(:de, 'bar')).to eq('bar' => 'Bar in :de') - expect(backend.search(:de, 'foo')).to eq('foo' => 'Foo in :en') - end - it 'uses fallback locales for translating' do expect(backend.translate(:de, 'bar')).to eq('Bar in :de') expect(backend.translate(:de, 'foo')).to eq('Foo in :en')