Discource-C
/
discourse
mirror of https://github.com/discourse/discourse.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

FIX: Translation overrides from fallback locale didn't work on client 

Discourse sent only translation overrides for the current language to the client instead of sending overrides from fallback locales as well. This especially impacted en_GB -> en since most overrides would be done in English instead of English (UK).

This also adds lots of tests for previously untested code.

There's a small caveat: The client currently doesn't handle fallback locales for MessageFormat strings. That is why overrides for those strings always have a higher priority than regular translations. So, as an example, the lookup order for MessageFormat strings in German is:
1. override for de
2. override for en
3. value from de
4. value from en
This commit is contained in:
Gerhard Schlager 2021-12-16 21:53:50 +01:00 committed by Gerhard Schlager
parent 4cd5158974
commit 769388b8ba
6 changed files with 223 additions and 65 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

43
app/assets/javascripts/discourse/app/initializers/localization.js
View File

@ -24,35 +24,28 @@ export default {
}
// Merge any overrides into our object
const overrides = I18n._overrides || {};
Object.keys(overrides).forEach((k) => {
const v = overrides[k];
k = k.replace("admin_js", "js");
for (const [locale, overrides] of Object.entries(I18n._overrides || {})) {
for (const [key, value] of Object.entries(overrides)) {
const segs = key.replace(/^admin_js\./, "js.admin.").split(".");
let node = I18n.translations[locale] || {};
const segs = k.split(".");
let node = I18n.translations[I18n.locale] || {};
let i = 0;
for (; i < segs.length - 1; i++) {
if (!(segs[i] in node)) {
node[segs[i]] = {};
for (let i = 0; i < segs.length - 1; i++) {
if (!(segs[i] in node)) {
node[segs[i]] = {};
}
node = node[segs[i]];
}
if (typeof node === "object") {
node[segs[segs.length - 1]] = value;
}
node = node[segs[i]];
}
}
if (typeof node === "object") {
node[segs[segs.length - 1]] = v;
}
});
const mfOverrides = I18n._mfOverrides || {};
Object.keys(mfOverrides).forEach((k) => {
const v = mfOverrides[k];
k = k.replace(/^[a-z_]*js\./, "");
I18n._compiledMFs[k] = v;
});
for (let [key, value] of Object.entries(I18n._mfOverrides || {})) {
key = key.replace(/^[a-z_]*js\./, "");
I18n._compiledMFs[key] = value;
}
bootbox.addLocale(I18n.currentLocale(), {
OK: I18n.t("composer.modal_ok"),

140
app/assets/javascripts/discourse/tests/unit/localization-test.js
View File

@ -6,7 +6,10 @@ import { getApplication } from "@ember/test-helpers";
module("initializer:localization", {
_locale: I18n.locale,
_translations: I18n.translations,
_extras: I18n.extras,
_compiledMFs: I18n._compiledMFs,
_overrides: I18n._overrides,
_mfOverrides: I18n._mfOverrides,
beforeEach() {
I18n.locale = "fr";
@ -15,16 +18,43 @@ module("initializer:localization", {
fr: {
js: {
composer: {
reply: "Répondre",
both_languages1: "composer.both_languages1 (FR)",
both_languages2: "composer.both_languages2 (FR)",
},
},
},
en: {
js: {
topic: {
reply: {
help: "begin composing a reply to this topic",
},
composer: {
both_languages1: "composer.both_languages1 (EN)",
both_languages2: "composer.both_languages2 (EN)",
only_english1: "composer.only_english1 (EN)",
only_english2: "composer.only_english2 (EN)",
},
},
},
};
I18n._compiledMFs = {
"user.messages.some_key_MF": () => "user.messages.some_key_MF (FR)",
};
I18n.extras = {
fr: {
admin: {
api: {
both_languages1: "admin.api.both_languages1 (FR)",
both_languages2: "admin.api.both_languages2 (FR)",
},
},
},
en: {
admin: {
api: {
both_languages1: "admin.api.both_languages1 (EN)",
both_languages2: "admin.api.both_languages2 (EN)",
only_english1: "admin.api.only_english1 (EN)",
only_english2: "admin.api.only_english2 (EN)",
},
},
},
@ -34,35 +64,115 @@ module("initializer:localization", {
afterEach() {
I18n.locale = this._locale;
I18n.translations = this._translations;
I18n.extras = this._extras;
I18n._compiledMFs = this._compiledMFs;
I18n._overrides = this._overrides;
I18n._mfOverrides = this._mfOverrides;
},
});
test("translation overrides", function (assert) {
I18n._overrides = {
"js.composer.reply": "WAT",
"js.topic.reply.help": "foobar",
fr: {
"js.composer.both_languages1": "composer.both_languages1 (FR override)",
"js.composer.only_english2": "composer.only_english2 (FR override)",
},
en: {
"js.composer.both_languages2": "composer.both_languages2 (EN override)",
"js.composer.only_english1": "composer.only_english1 (EN override)",
},
};
LocalizationInitializer.initialize(getApplication());
assert.strictEqual(
I18n.t("composer.reply"),
"WAT",
I18n.t("composer.both_languages1"),
"composer.both_languages1 (FR override)",
"overrides existing translation in current locale"
);
assert.strictEqual(
I18n.t("topic.reply.help"),
"foobar",
"overrides translation in default locale"
I18n.t("composer.only_english1"),
"composer.only_english1 (EN override)",
"overrides translation in fallback locale"
);
assert.strictEqual(
I18n.t("composer.only_english2"),
"composer.only_english2 (FR override)",
"overrides translation that doesn't exist in current locale"
);
assert.strictEqual(
I18n.t("composer.both_languages2"),
"composer.both_languages2 (FR)",
"prefers translation in current locale over override in fallback locale"
);
});
test("translation overrides (admin_js)", function (assert) {
I18n._overrides = {
fr: {
"admin_js.api.both_languages1": "admin.api.both_languages1 (FR override)",
"admin_js.api.only_english2": "admin.api.only_english2 (FR override)",
},
en: {
"admin_js.api.both_languages2": "admin.api.both_languages2 (EN override)",
"admin_js.api.only_english1": "admin.api.only_english1 (EN override)",
},
};
LocalizationInitializer.initialize(getApplication());
assert.strictEqual(
I18n.t("admin.api.both_languages1"),
"admin.api.both_languages1 (FR override)",
"overrides existing translation in current locale"
);
assert.strictEqual(
I18n.t("admin.api.only_english1"),
"admin.api.only_english1 (EN override)",
"overrides translation in fallback locale"
);
assert.strictEqual(
I18n.t("admin.api.only_english2"),
"admin.api.only_english2 (FR override)",
"overrides translation that doesn't exist in current locale"
);
assert.strictEqual(
I18n.t("admin.api.both_languages2"),
"admin.api.both_languages2 (FR)",
"prefers translation in current locale over override in fallback locale"
);
});
test("translation overrides for MessageFormat strings", function (assert) {
I18n._mfOverrides = {
"js.user.messages.some_key_MF": () =>
"user.messages.some_key_MF (FR override)",
};
LocalizationInitializer.initialize(getApplication());
assert.strictEqual(
I18n.messageFormat("user.messages.some_key_MF", {}),
"user.messages.some_key_MF (FR override)",
"overrides existing MessageFormat string"
);
});
test("skip translation override if parent node is not an object", function (assert) {
I18n._overrides = {
"js.composer.reply": "WAT",
"js.composer.reply.help": "foobar",
fr: {
"js.composer.both_languages1.foo":
"composer.both_languages1.foo (FR override)",
},
};
LocalizationInitializer.initialize(getApplication());
assert.strictEqual(I18n.t("composer.reply.help"), "[fr.composer.reply.help]");
assert.strictEqual(
I18n.t("composer.both_languages1.foo"),
"[fr.composer.both_languages1.foo]"
);
});

2
app/controllers/admin/site_texts_controller.rb
View File

@ -183,7 +183,7 @@ class Admin::SiteTextsController < Admin::AdminController
def find_translations(query, overridden, locale)
translations = Hash.new { |hash, key| hash[key] = {} }
search_results = I18n.with_locale(locale) do
I18n.search(query, overridden: overridden)
I18n.search(query, only_overridden: overridden)
end
search_results.each do |key, value|

53
lib/js_locale_helper.rb
View File

@ -160,31 +160,44 @@ module JsLocaleHelper
result
end
def self.output_client_overrides(locale)
overrides = TranslationOverride
.where(locale: locale)
.where("translation_key LIKE 'js.%' OR translation_key LIKE 'admin_js.%'")
.pluck(:translation_key, :value, :compiled_js)
def self.output_client_overrides(main_locale)
all_overrides = {}
has_overrides = false
return "" if overrides.blank?
I18n.fallbacks[main_locale].each do |locale|
overrides = all_overrides[locale] = TranslationOverride
.where(locale: locale)
.where("translation_key LIKE 'js.%' OR translation_key LIKE 'admin_js.%'")
.pluck(:translation_key, :value, :compiled_js)
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
has_overrides ||= overrides.present?
end
message_formats = message_formats.join(", ")
return "" if !has_overrides
<<~JS
I18n._mfOverrides = {#{message_formats}};
I18n._overrides = #{translations.to_json};
JS
result = +"I18n._overrides = {};"
existing_keys = Set.new
message_formats = []
all_overrides.each do |locale, overrides|
translations = {}
overrides.each do |key, value, compiled_js|
next if existing_keys.include?(key)
existing_keys << key
if key.end_with?("_MF")
message_formats << "#{key.inspect}: #{compiled_js}"
else
translations[key] = value
end
end
result << "I18n._overrides['#{locale}'] = #{translations.to_json};" if translations.present?
end
result << "I18n._mfOverrides = {#{message_formats.join(", ")}};"
result
end
def self.output_extra_locales(bundle, locale)

13
spec/requests/admin/site_texts_controller_spec.rb
View File

@ -153,6 +153,19 @@ RSpec.describe Admin::SiteTextsController do
expect(value).to eq('education.new-topic override')
end
it "returns only overridden translations" do
TranslationOverride.upsert!(:en, 'education.new-topic', 'education.new-topic override')
get "/admin/customize/site_texts.json", params: { locale: 'en', overridden: true }
expect(response.status).to eq(200)
site_texts = response.parsed_body['site_texts']
expect(site_texts.size).to eq(1)
value = 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' })

37
spec/requests/extra_locales_controller_spec.rb
View File

@ -109,10 +109,39 @@ describe ExtraLocalesController do
ctx.eval("I18n = {};")
ctx.eval(response.body)
expect(ctx.eval('typeof I18n._mfOverrides["js.client_MF"]')).to eq("function")
expect(ctx.eval('I18n._overrides["js.some_key"]')).to eq("client-side translation")
expect(ctx.eval('I18n._overrides["js.client_MF"] === undefined')).to eq(true)
expect(ctx.eval('I18n._overrides["admin_js.another_key"]')).to eq("admin client js")
expect(ctx.eval("typeof I18n._mfOverrides['js.client_MF']")).to eq("function")
expect(ctx.eval("I18n._overrides['#{I18n.locale}']['js.some_key']")).to eq("client-side translation")
expect(ctx.eval("I18n._overrides['#{I18n.locale}']['js.client_MF'] === undefined")).to eq(true)
expect(ctx.eval("I18n._overrides['#{I18n.locale}']['admin_js.another_key']")).to eq("admin client js")
end
it "returns overrides from fallback locale" do
TranslationOverride.upsert!(:en, 'js.some_key', 'some key (en)')
TranslationOverride.upsert!(:fr, 'js.some_key', 'some key (fr)')
TranslationOverride.upsert!(:en, 'js.only_en', 'only English')
TranslationOverride.upsert!(:fr, 'js.only_fr', 'only French')
TranslationOverride.upsert!(:en, 'js.some_client_MF', '{NUM_RESULTS, plural, one {1 result} other {many} }')
TranslationOverride.upsert!(:fr, 'js.some_client_MF', '{NUM_RESULTS, plural, one {1 result} other {many} }')
TranslationOverride.upsert!(:en, 'js.only_en_MF', '{NUM_RESULTS, plural, one {1 result} other {many} }')
TranslationOverride.upsert!(:fr, 'js.only_fr_MF', '{NUM_RESULTS, plural, one {1 result} other {many} }')
SiteSetting.allow_user_locale = true
user = Fabricate(:user, locale: :fr)
sign_in(user)
get "/extra-locales/overrides"
expect(response.status).to eq(200)
ctx = MiniRacer::Context.new
ctx.eval("I18n = {};")
ctx.eval(response.body)
overrides = ctx.eval("I18n._overrides")
expect(overrides.keys).to contain_exactly("en", "fr")
expect(overrides["en"]).to eq({ 'js.only_en' => 'only English' })
expect(overrides["fr"]).to eq({ 'js.some_key' => 'some key (fr)', 'js.only_fr' => 'only French' })
expect(ctx.eval("Object.keys(I18n._mfOverrides)")).to contain_exactly("js.some_client_MF", "js.only_en_MF", "js.only_fr_MF")
end
end
end