FIX: An overridden text of a non-existent plural key resulted in error

FIX: An overridden text of a non-existent plural key resulted in error

When there is an overridden text in the database that belongs to a pluralized key which doesn’t exist in English anymore, the Customize Texts admin page was unusable. This stops those keys from ever being returned by a search.

diff --git a/app/controllers/admin/site_texts_controller.rb b/app/controllers/admin/site_texts_controller.rb
index a5f9d8e..14743c3 100644
--- a/app/controllers/admin/site_texts_controller.rb
+++ b/app/controllers/admin/site_texts_controller.rb
@@ -112,7 +112,7 @@ class Admin::SiteTextsController < Admin::AdminController
       value = override&.first
     end
 
-    value ||= I18n.t(key)
+    value ||= I18n.t(key, default: '')
     { id: key, value: value }
   end
 
@@ -149,6 +149,8 @@ class Admin::SiteTextsController < Admin::AdminController
     results = []
 
     translations.each do |key, value|
+      next unless I18n.exists?(key, :en)
+
       if value&.is_a?(Hash)
         value = fix_plural_keys(key, value)
         value.each do |plural_key, plural_value|
@@ -167,7 +169,7 @@ class Admin::SiteTextsController < Admin::AdminController
     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)
+    fallback_value = I18n.t(key, locale: :en, default: {})
     plural_keys.map do |k|
       [k, value[k] || fallback_value[k] || fallback_value[:other]]
     end.to_h
diff --git a/spec/requests/admin/site_texts_controller_spec.rb b/spec/requests/admin/site_texts_controller_spec.rb
index a2edd37..fcc3be2 100644
--- a/spec/requests/admin/site_texts_controller_spec.rb
+++ b/spec/requests/admin/site_texts_controller_spec.rb
@@ -91,6 +91,20 @@ RSpec.describe Admin::SiteTextsController do
         end
       end
 
+      it 'does not return overrides for keys that do not exist in English' do
+        SiteSetting.default_locale = :ru
+        TranslationOverride.create!(locale: :ru, translation_key: 'missing_plural_key.one', value: 'ONE')
+        TranslationOverride.create!(locale: :ru, translation_key: 'another_missing_key', value: 'foo')
+
+        get "/admin/customize/site_texts.json", params: { q: 'missing_plural_key' }
+        expect(response.status).to eq(200)
+        expect(JSON.parse(response.body)['site_texts']).to be_empty
+
+        get "/admin/customize/site_texts.json", params: { q: 'another_missing_key' }
+        expect(response.status).to eq(200)
+        expect(JSON.parse(response.body)['site_texts']).to be_empty
+      end
+
       context 'plural keys' do
         before do
           I18n.backend.store_translations(:en, colour: { one: '%{count} colour', other: '%{count} colours' })

GitHub sha: d516e492

2 Likes

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

Thanks @gschlager :+1:

Very nice commit message :ok_hand:

1 Like