From 4cd5158974b3247c7ea69c276bf332b89a223e91 Mon Sep 17 00:00:00 2001 From: Gerhard Schlager Date: Thu, 16 Dec 2021 16:54:45 +0100 Subject: [PATCH] FIX: "Customize Text" showed compiled MessageFormat string for overridden `_MF` translations --- .../admin/site_texts_controller.rb | 5 --- lib/freedom_patches/translate_accelerator.rb | 4 +- lib/js_locale_helper.rb | 21 ++++++---- .../admin/site_texts_controller_spec.rb | 38 +++++++++++++++++++ 4 files changed, 54 insertions(+), 14 deletions(-) diff --git a/app/controllers/admin/site_texts_controller.rb b/app/controllers/admin/site_texts_controller.rb index 313b578a83f..55e3611f1dd 100644 --- a/app/controllers/admin/site_texts_controller.rb +++ b/app/controllers/admin/site_texts_controller.rb @@ -157,11 +157,6 @@ class Admin::SiteTextsController < Admin::AdminController end def record_for(key:, value: nil, locale:) - if key.ends_with?("_MF") - override = TranslationOverride.where(translation_key: key, locale: locale).pluck(:value) - value = override&.first - end - value ||= I18n.with_locale(locale) { I18n.t(key) } { id: key, value: value, locale: locale } end diff --git a/lib/freedom_patches/translate_accelerator.rb b/lib/freedom_patches/translate_accelerator.rb index 744b5f843c0..d776670064b 100644 --- a/lib/freedom_patches/translate_accelerator.rb +++ b/lib/freedom_patches/translate_accelerator.rb @@ -169,14 +169,14 @@ module I18n if !by_site.has_key?(locale) # Load overrides - translations_overrides = TranslationOverride.where(locale: locale).pluck(:translation_key, :value, :compiled_js) + translations_overrides = TranslationOverride.where(locale: locale).pluck(:translation_key, :value) if translations_overrides.empty? by_site[locale] = {} else translations_overrides.each do |tuple| by_locale = by_site[locale] ||= {} - by_locale[tuple[0]] = tuple[2] || tuple[1] + by_locale[tuple[0]] = tuple[1] end end diff --git a/lib/js_locale_helper.rb b/lib/js_locale_helper.rb index 9ab61c941d0..e12fab50150 100644 --- a/lib/js_locale_helper.rb +++ b/lib/js_locale_helper.rb @@ -161,18 +161,25 @@ module JsLocaleHelper end def self.output_client_overrides(locale) - translations = (I18n.overrides_by_locale(locale) || {}).select { |k, _| k[/^(admin_js|js)\./] } - return "" if translations.blank? + overrides = TranslationOverride + .where(locale: locale) + .where("translation_key LIKE 'js.%' OR translation_key LIKE 'admin_js.%'") + .pluck(:translation_key, :value, :compiled_js) - message_formats = {} + return "" if overrides.blank? - translations.delete_if do |key, value| - if key.to_s.end_with?("_MF") - message_formats[key] = value + message_formats = [] + translations = {} + + overrides.each do |key, value, compiled_js| + if key.end_with?("_MF") + message_formats << "#{key.inspect}: #{compiled_js}" + else + translations[key] = value end end - message_formats = message_formats.map { |k, v| "#{k.inspect}: #{v}" }.join(", ") + message_formats = message_formats.join(", ") <<~JS I18n._mfOverrides = {#{message_formats}}; diff --git a/spec/requests/admin/site_texts_controller_spec.rb b/spec/requests/admin/site_texts_controller_spec.rb index 63d9f5349f2..90a0bd546f1 100644 --- a/spec/requests/admin/site_texts_controller_spec.rb +++ b/spec/requests/admin/site_texts_controller_spec.rb @@ -138,6 +138,21 @@ RSpec.describe Admin::SiteTextsController do expect(response.parsed_body['site_texts']).to be_empty end + it "returns site text from fallback locale if current locale doesn't have a translation" do + TranslationOverride.upsert!(:en, 'js.summary.description_time_MF', 'description_time_MF override') + TranslationOverride.upsert!(:en, 'education.new-topic', 'education.new-topic override') + + get "/admin/customize/site_texts.json", params: { q: 'js.summary.description_time_MF', locale: 'en_GB' } + expect(response.status).to eq(200) + value = response.parsed_body['site_texts'].find { |text| text['id'] == 'js.summary.description_time_MF' }['value'] + expect(value).to eq('description_time_MF override') + + get "/admin/customize/site_texts.json", params: { q: 'education.new-topic', locale: 'en_GB' } + expect(response.status).to eq(200) + value = response.parsed_body['site_texts'].find { |text| text['id'] == 'education.new-topic' }['value'] + expect(value).to eq('education.new-topic override') + end + context 'plural keys' do before do I18n.backend.store_translations(:en, colour: { one: '%{count} colour', other: '%{count} colours' }) @@ -272,6 +287,29 @@ RSpec.describe Admin::SiteTextsController do expect(response.status).to eq(400) end + it "returns site text from fallback locale if current locale doesn't have a translation" do + TranslationOverride.upsert!(:en, 'js.summary.description_time_MF', 'description_time_MF override') + TranslationOverride.upsert!(:en, 'education.new-topic', 'education.new-topic override') + + get "/admin/customize/site_texts/js.summary.description_time_MF.json", params: { locale: 'en_GB' } + expect(response.status).to eq(200) + + json = response.parsed_body + site_text = json['site_text'] + + expect(site_text['id']).to eq('js.summary.description_time_MF') + expect(site_text['value']).to eq('description_time_MF override') + + get "/admin/customize/site_texts/education.new-topic.json", params: { locale: 'en_GB' } + expect(response.status).to eq(200) + + json = response.parsed_body + site_text = json['site_text'] + + expect(site_text['id']).to eq('education.new-topic') + expect(site_text['value']).to eq('education.new-topic override') + end + context 'plural keys' do before do I18n.backend.store_translations(:en, colour: { one: '%{count} colour', other: '%{count} colours' })