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

@@ -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

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

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

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')