From c1e9a70d59b644a21f254d7511b54ae3ee584880 Mon Sep 17 00:00:00 2001 From: Gerhard Schlager Date: Thu, 23 May 2019 21:23:31 +0200 Subject: [PATCH] FIX: Fallback locale was not available for extra translations Translations from fallback locales were not sent to the client for admin_js and wizard_js. --- app/assets/javascripts/locales/i18n.js | 35 ++++------ app/controllers/extra_locales_controller.rb | 17 +++-- lib/js_locale_helper.rb | 18 ++++-- .../requests/extra_locales_controller_spec.rb | 35 ++++++---- .../javascripts/helpers/component-test.js.es6 | 9 ++- test/javascripts/lib/i18n-test.js.es6 | 64 +++++++++++++++++-- test/javascripts/widgets/widget-test.js.es6 | 18 ++++-- vendor/assets/javascripts/i18n-patches.js | 5 -- 8 files changed, 135 insertions(+), 66 deletions(-) diff --git a/app/assets/javascripts/locales/i18n.js b/app/assets/javascripts/locales/i18n.js index 74991a7561e..58b824f5c8e 100644 --- a/app/assets/javascripts/locales/i18n.js +++ b/app/assets/javascripts/locales/i18n.js @@ -28,24 +28,6 @@ I18n.isValidNode = function(obj, node, undefined) { return obj[node] !== null && obj[node] !== undefined; }; -function checkExtras(origScope, sep, extras) { - if (!extras || extras.length === 0) { return; } - - for (var i = 0; i < extras.length; i++) { - var messages = extras[i]; - scope = origScope.split(sep); - - if (scope[0] === 'js') { scope.shift(); } - - while (messages && scope.length > 0) { - currentScope = scope.shift(); - messages = messages[currentScope]; - } - - if (messages !== undefined) { return messages; } - } -} - I18n.lookup = function(scope, options) { options = options || {}; @@ -64,17 +46,26 @@ I18n.lookup = function(scope, options) { scope = options.scope.toString() + this.SEPARATOR + scope; } - var origScope = "" + scope; + var originalScope = scope; + scope = scope.split(this.SEPARATOR); - scope = origScope.split(this.SEPARATOR); + if (scope.length > 0 && scope[0] !== "js") { + scope.unshift("js"); + } while (messages && scope.length > 0) { currentScope = scope.shift(); messages = messages[currentScope]; } - if (messages === undefined) { - messages = checkExtras(origScope, this.SEPARATOR, this.extras); + if (messages === undefined && this.extras && this.extras[locale]) { + messages = this.extras[locale]; + scope = originalScope.split(this.SEPARATOR); + + while (messages && scope.length > 0) { + currentScope = scope.shift(); + messages = messages[currentScope]; + } } if (messages === undefined) { diff --git a/app/controllers/extra_locales_controller.rb b/app/controllers/extra_locales_controller.rb index dfcc64719a2..1baadebc666 100644 --- a/app/controllers/extra_locales_controller.rb +++ b/app/controllers/extra_locales_controller.rb @@ -40,21 +40,20 @@ class ExtraLocalesController < ApplicationController translations = JsLocaleHelper.translations_for(locale_str) - for_key = {} - translations.values.each { |v| for_key.deep_merge!(v[bundle_str]) if v.has_key?(bundle_str) } + translations.keys.each do |l| + translations[l].keys.each do |k| + bundle_translations = translations[l].delete(k) + translations[l].deep_merge!(bundle_translations) if k == bundle_str + end + end js = "" - if for_key.present? - if plugin_for_key = JsLocaleHelper.plugin_translations(locale_str)[bundle_str] - for_key.deep_merge!(plugin_for_key) - end - + if translations.present? js = <<~JS.squish (function() { if (window.I18n) { - window.I18n.extras = window.I18n.extras || []; - window.I18n.extras.push(#{for_key.to_json}); + window.I18n.extras = #{translations.to_json}; } })(); JS diff --git a/lib/js_locale_helper.rb b/lib/js_locale_helper.rb index 9856766b9c0..40c48aa0518 100644 --- a/lib/js_locale_helper.rb +++ b/lib/js_locale_helper.rb @@ -104,28 +104,32 @@ module JsLocaleHelper end end + def self.clear_cache! + @loaded_translations = nil + @plugin_translations = nil + @loaded_merges = nil + end + def self.translations_for(locale_str) - if Rails.env.development? - @loaded_translations = nil - @plugin_translations = nil - @loaded_merges = nil - end + clear_cache! if Rails.env.development? locale_sym = locale_str.to_sym - I18n.with_locale(locale_sym) do + translations = I18n.with_locale(locale_sym) do if locale_sym == :en load_translations(locale_sym) else load_translations_merged(*I18n.fallbacks[locale_sym]) end end + + Marshal.load(Marshal.dump(translations)) end def self.output_locale(locale) locale_str = locale.to_s fallback_locale_str = LocaleSiteSetting.fallback_locale(locale_str)&.to_s - translations = Marshal.load(Marshal.dump(translations_for(locale_str))) + translations = translations_for(locale_str) message_formats = remove_message_formats!(translations, locale) mf_locale, mf_filename = find_message_format_locale([locale_str], fallback_to_english: true) diff --git a/spec/requests/extra_locales_controller_spec.rb b/spec/requests/extra_locales_controller_spec.rb index bd472272e44..9615bf364f0 100644 --- a/spec/requests/extra_locales_controller_spec.rb +++ b/spec/requests/extra_locales_controller_spec.rb @@ -26,23 +26,32 @@ describe ExtraLocalesController do expect(response.status).to eq(404) end - it "includes plugin translations" do - JsLocaleHelper.expects(:plugin_translations) - .with(any_of("en", "en_US")) - .returns("admin_js" => { - "admin" => { - "site_settings" => { - "categories" => { - "github_badges" => "Github Badges" + context "with plugin" do + before do + JsLocaleHelper.clear_cache! + JsLocaleHelper.expects(:plugin_translations) + .with(any_of("en", "en_US")) + .returns("admin_js" => { + "admin" => { + "site_settings" => { + "categories" => { + "github_badges" => "Github Badges" + } } } - } - }).at_least_once + }).at_least_once + end - get "/extra-locales/admin" + after do + JsLocaleHelper.clear_cache! + end - expect(response.status).to eq(200) - expect(response.body.include?("github_badges")).to eq(true) + it "includes plugin translations" do + get "/extra-locales/admin" + + expect(response.status).to eq(200) + expect(response.body.include?("github_badges")).to eq(true) + end end end end diff --git a/test/javascripts/helpers/component-test.js.es6 b/test/javascripts/helpers/component-test.js.es6 index 969f5aac74d..c65394643c0 100644 --- a/test/javascripts/helpers/component-test.js.es6 +++ b/test/javascripts/helpers/component-test.js.es6 @@ -54,8 +54,15 @@ export default function(name, opts) { andThen(() => { return this.render(opts.template); }); + andThen(() => { - opts.test.call(this, assert); + try { + opts.test.call(this, assert); + } finally { + if (opts.afterEach) { + opts.afterEach.call(opts); + } + } }); }); } diff --git a/test/javascripts/lib/i18n-test.js.es6 b/test/javascripts/lib/i18n-test.js.es6 index 0f142589015..b68c871d437 100644 --- a/test/javascripts/lib/i18n-test.js.es6 +++ b/test/javascripts/lib/i18n-test.js.es6 @@ -99,12 +99,68 @@ QUnit.test("translations", assert => { }); QUnit.test("extra translations", assert => { - I18n.extras = [{ admin: { title: "Discourse Admin" } }]; + I18n.locale = "pl_PL"; + I18n.extras = { + en: { + admin: { + dashboard: { + title: "Dashboard", + backup_count: { + one: "%{count} backup", + other: "%{count} backups" + } + }, + web_hooks: { + events: { + incoming: { + one: "There is a new event.", + other: "There are %{count} new events." + } + } + } + } + }, + pl_PL: { + admin: { + dashboard: { + title: "Raporty" + }, + web_hooks: { + events: { + incoming: { + one: "Istnieje nowe wydarzenie", + few: "Istnieją %{count} nowe wydarzenia.", + many: "Istnieje %{count} nowych wydarzeń.", + other: "Istnieje %{count} nowych wydarzeń." + } + } + } + } + } + }; + I18n.pluralizationRules.pl_PL = function(n) { + if (n === 1) return "one"; + if (n % 10 >= 2 && n % 10 <= 4) return "few"; + if (n % 10 === 0) return "many"; + return "other"; + }; assert.equal( - I18n.t("admin.title"), - "Discourse Admin", - "it check extra translations when they exists" + I18n.t("admin.dashboard.title"), + "Raporty", + "it uses extra translations when they exists" + ); + + assert.equal( + I18n.t("admin.web_hooks.events.incoming", { count: 2 }), + "Istnieją 2 nowe wydarzenia.", + "it uses pluralized extra translation when it exists" + ); + + assert.equal( + I18n.t("admin.dashboard.backup_count", { count: 2 }), + "2 backups", + "it falls back to English and uses extra translations when they exists" ); }); diff --git a/test/javascripts/widgets/widget-test.js.es6 b/test/javascripts/widgets/widget-test.js.es6 index 061e2d5735c..fcb35f96b9a 100644 --- a/test/javascripts/widgets/widget-test.js.es6 +++ b/test/javascripts/widgets/widget-test.js.es6 @@ -258,6 +258,8 @@ widgetTest("handlebars d-icon", { }); widgetTest("handlebars i18n", { + _translations: I18n.translations, + template: `{{mount-widget widget="hbs-i18n-test" args=args}}`, beforeEach() { @@ -268,15 +270,21 @@ widgetTest("handlebars i18n", { test ` }); - I18n.extras = [ - { - hbs_test0: "evil", - hbs_test1: "trout" + I18n.translations = { + en: { + js: { + hbs_test0: "evil", + hbs_test1: "trout" + } } - ]; + }; this.set("args", { key: "hbs_test1" }); }, + afterEach() { + I18n.translations = this._translations; + }, + test(assert) { // comin up assert.equal(find("span.string").text(), "evil"); diff --git a/vendor/assets/javascripts/i18n-patches.js b/vendor/assets/javascripts/i18n-patches.js index 90d8496c37b..ee4df93b160 100644 --- a/vendor/assets/javascripts/i18n-patches.js +++ b/vendor/assets/javascripts/i18n-patches.js @@ -1,10 +1,5 @@ (function() { if (typeof I18n !== "undefined") { - var oldI18nlookup = I18n.lookup; - I18n.lookup = function(scope, options) { - return oldI18nlookup.apply(this, ["js." + scope, options]); - }; - // Default format for storage units var oldI18ntoHumanSize = I18n.toHumanSize; I18n.toHumanSize = function(number, options) {