From 81c329fbb8cb141335fd59c596dc5cde3bfb402a Mon Sep 17 00:00:00 2001
From: Gerhard Schlager <mail@gerhard-schlager.at>
Date: Sat, 11 May 2019 01:52:27 +0200
Subject: [PATCH] FIX: Customizing missing pluralized translations didn't work
---
.../admin/site_texts_controller.rb | 76 ++++++++---
app/serializers/site_text_serializer.rb | 8 +-
.../admin/site_texts_controller_spec.rb | 119 ++++++++++++++++++
3 files changed, 184 insertions(+), 19 deletions(-)
diff --git a/app/controllers/admin/site_texts_controller.rb b/app/controllers/admin/site_texts_controller.rb
index 65e60629f71..c1be047c3e3 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 0b7af1e2c4f..06987330b05 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 6f8a0d46bf7..234129a0e65 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
+
+ site_text = json['site_text']
+ expect(site_text).to be_present
+
+ expect(site_text['id']).to eq(id)
+ expect(site_text['value']).to eq(value)
+ end
+ end
+ end
+
+ context 'English' do
+ let(:locale) { :en }
+ let(:expected_translations) { { one: '%{count} colour', other: '%{count} colours' } }
+
+ include_examples 'has 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 'has correct plural keys'
+ end
+
+ context 'language with different plural keys and partial translation' do
+ before do
+ I18n.backend.store_translations(:ru, colour: { few: '%{count} цвета' })
+ end
+
+ let(:locale) { :ru }
+ let(:expected_translations) { { one: '%{count} colour', few: '%{count} цвета', other: '%{count} colours' } }
+
+ include_examples 'has correct plural keys'
+ end
+ end
end
describe '#update & #revert' do