FIX: Customizing missing pluralized translations didn't work

FIX: Customizing missing pluralized translations didn’t work

diff --git a/app/controllers/admin/site_texts_controller.rb b/app/controllers/admin/site_texts_controller.rb
index 65e6062..c1be047 100644
--- a/app/controllers/admin/site_texts_controller.rb
+++ b/app/controllers/admin/site_texts_controller.rb
@@ -27,13 +27,9 @@ class Admin::SiteTextsController < Admin::AdminController
       extras[:recommended] = true
       results = self.class.preferred_keys.map { |k| record_for(k) }
     else
-      results = []
-      translations = I18n.search(query, overridden: overridden)
-      translations.each do |k, v|
-        results << record_for(k, v)
-      end
+      results = find_translations(query, overridden)
 
-      unless translations.empty?
+      if results.any?
         extras[:regex] = I18n::Backend::DiscourseI18n.create_search_regexp(query, as_string: true)
       end
 
@@ -110,20 +106,70 @@ class Admin::SiteTextsController < Admin::AdminController
 
   protected
 
-  def record_for(k, value = nil)
-    if k.ends_with?("_MF")
-      ovr = TranslationOverride.where(translation_key: k, locale: I18n.locale).pluck(:value)
-      value = ovr[0] if ovr.present?
+  def record_for(key, value = nil)
+    if key.ends_with?("_MF")
+      override = TranslationOverride.where(translation_key: key, locale: I18n.locale).pluck(:value)
+      value = override&.first
     end
 
-    value ||= I18n.t(k)
-    { id: k, value: value }
+    value ||= I18n.t(key)
+    { id: key, value: value }
   end
 
+  PLURALIZED_REGEX = /(.*)\.(zero|one|two|few|many|other)$/
+
   def find_site_text
-    raise Discourse::NotFound unless I18n.exists?(params[:id])
-    raise Discourse::InvalidAccess.new(nil, nil, custom_message: 'email_template_cant_be_modified') if self.class.restricted_keys.include?(params[:id])
-    record_for(params[:id])
+    if self.class.restricted_keys.include?(params[:id])
+      raise Discourse::InvalidAccess.new(nil, nil, custom_message: 'email_template_cant_be_modified')
+    end
+
+    if I18n.exists?(params[:id]) || TranslationOverride.exists?(locale: I18n.locale, translation_key: params[:id])
+      return record_for(params[:id])
+    end
+
+    if PLURALIZED_REGEX.match(params[:id])
+      value = fix_plural_keys($1, {}).fetch($2.to_sym)
+      return record_for(params[:id], value) if value
+    end
+
+    raise Discourse::NotFound
   end
 
+  def find_translations(query, overridden)
+    translations = Hash.new { |hash, key| hash[key] = {} }
+
+    I18n.search(query, overridden: overridden).each do |key, value|
+      if PLURALIZED_REGEX.match(key)
+        translations[$1][$2] = value
+      else
+        translations[key] = value
+      end
+    end
+
+    results = []
+
+    translations.each do |key, value|
+      if value&.is_a?(Hash)
+        value = fix_plural_keys(key, value)
+        value.each do |plural_key, plural_value|
+          results << record_for("#{key}.#{plural_key}", plural_value)
+        end
+      else
+        results << record_for(key, value)
+      end
+    end
+
+    results
+  end
+
+  def fix_plural_keys(key, value)
+    value = value.with_indifferent_access
+    plural_keys = I18n.t('i18n.plural.keys')
+    return value if value.keys.size == plural_keys.size && plural_keys.all? { |k| value.key?(k) }
+
+    fallback_value = I18n.t(key, locale: :en)
+    plural_keys.map do |k|
+      [k, value[k] || fallback_value[k] || fallback_value[:other]]
+    end.to_h
+  end
 end
diff --git a/app/serializers/site_text_serializer.rb b/app/serializers/site_text_serializer.rb
index 0b7af1e..0698733 100644
--- a/app/serializers/site_text_serializer.rb
+++ b/app/serializers/site_text_serializer.rb
@@ -12,10 +12,10 @@ class SiteTextSerializer < ApplicationSerializer
   end
 
   def overridden?
-    current_val = value
-
-    I18n.overrides_disabled do
-      return I18n.t(object[:id]) != current_val
+    if I18n.exists?(object[:id])
+      I18n.overrides_disabled { I18n.t(object[:id]) != object[:value] }
+    else
+      TranslationOverride.exists?(locale: I18n.locale, translation_key: object[:id])
     end
   end
 
diff --git a/spec/requests/admin/site_texts_controller_spec.rb b/spec/requests/admin/site_texts_controller_spec.rb
index 6f8a0d4..234129a 100644
--- a/spec/requests/admin/site_texts_controller_spec.rb
+++ b/spec/requests/admin/site_texts_controller_spec.rb
@@ -91,6 +91,72 @@ RSpec.describe Admin::SiteTextsController do
         end
       end
 
+      context 'plural keys' do
+        before do
+          I18n.backend.store_translations(:en, colour: { one: '%{count} colour', other: '%{count} colours' })
+        end
+
+        shared_examples 'finds correct plural keys' do
+          it 'finds the correct plural keys for the locale' do
+            SiteSetting.default_locale = locale
+
+            get '/admin/customize/site_texts.json', params: { q: 'colour' }
+            expect(response.status).to eq(200)
+
+            json = ::JSON.parse(response.body, symbolize_names: true)
+            expect(json).to be_present
+
+            site_texts = json[:site_texts]
+            expect(site_texts).to be_present
+
+            expected_search_result = expected_translations.map do |key, value|
+              overridden = defined?(expected_overridden) ? expected_overridden[key] || false : false
+              { id: "colour.#{key}", value: value, can_revert: overridden, overridden: overridden }
+            end
+
+            expect(site_texts).to match_array(expected_search_result)
+          end
+        end
+
+        context 'English' do
+          let(:locale) { :en }
+          let(:expected_translations) { { one: '%{count} colour', other: '%{count} colours' } }
+
+          include_examples 'finds correct plural keys'
+        end
+
+        context 'language with different plural keys and missing translations' do
+          let(:locale) { :ru }
+          let(:expected_translations) { { one: '%{count} colour', few: '%{count} colours', other: '%{count} colours' } }
+
+          include_examples 'finds correct plural keys'
+        end
+
+        context 'language with different plural keys and partial translation' do
+          before do
+            I18n.backend.store_translations(:ru, colour: { few: '%{count} цвета', many: '%{count} цветов' })
+          end
+
+          let(:locale) { :ru }
+          let(:expected_translations) { { one: '%{count} colour', few: '%{count} цвета', other: '%{count} colours' } }
+
+          include_examples 'finds correct plural keys'
+        end
+
+        context 'with overridden translation not in original translation' do
+          before do
+            I18n.backend.store_translations(:ru, colour: { few: '%{count} цвета', many: '%{count} цветов' })
+            TranslationOverride.create!(locale: :ru, translation_key: 'colour.one', value: 'ONE')
+            TranslationOverride.create!(locale: :ru, translation_key: 'colour.few', value: 'FEW')
+          end
+
+          let(:locale) { :ru }
+          let(:expected_translations) { { one: 'ONE', few: 'FEW', other: '%{count} colours' } }
+          let(:expected_overridden) { { one: true, few: true } }
+
+          include_examples 'finds correct plural keys'
+        end
+      end
     end
 
     describe '#show' do
@@ -110,6 +176,59 @@ RSpec.describe Admin::SiteTextsController do
         get "/admin/customize/site_texts/made_up_no_key_exists.json"
         expect(response.status).to eq(404)
       end
+
+      context 'plural keys' do
+        before do
+          I18n.backend.store_translations(:en, colour: { one: '%{count} colour', other: '%{count} colours' })
+        end
+
+        shared_examples 'has correct plural keys' do
+          it 'returns the correct plural keys for the locale' do
+            SiteSetting.default_locale = locale
+
+            expected_translations.each do |key, value|
+              id = "colour.#{key}"
+
+              get "/admin/customize/site_texts/#{id}.json"
+              expect(response.status).to eq(200)
+
+              json = ::JSON.parse(response.body)
+              expect(json).to be_present
+

[... diff too long, it was truncated ...]

GitHub sha: 81c329fb

1 Like

This commit has been mentioned on Discourse Meta. There might be relevant details there: