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

FIX: Using the `default_locale` in locale fallbacks caused problems 

Locale files get precompiled after deployment and they contained translations from the `default_locale`. That's especially bad in multisites, because the initial `default_locale` is `en_US`. Sites where the `default_locale` isn't `en_US` could see missing translations. The same thing could happen when users are allowed to chose a different locale.

This change simplifies the logic by not using the `default_locale` in the locale chain. It always falls back to `en` in case of missing translations.
This commit is contained in:
Gerhard Schlager 2020-05-06 22:57:14 +02:00
parent 15a938e861
commit ec2f3169ff
6 changed files with 72 additions and 64 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

16
lib/i18n/backend/fallback_locale_list.rb
View File

@ -6,19 +6,15 @@ module I18n
class FallbackLocaleList < Hash
def [](locale)
locale = locale.to_sym
return [locale] if locale == :en
locale_list = [locale]
return locale_list if locale == :en
fallback_locale = LocaleSiteSetting.fallback_locale(locale)
site_locale = SiteSetting.default_locale.to_sym
locale_list =
if locale == site_locale || site_locale == :en || fallback_locale == :en
[locale, fallback_locale, :en]
else
site_fallback_locale = LocaleSiteSetting.fallback_locale(site_locale)
[locale, fallback_locale, site_locale, site_fallback_locale, :en]
while (fallback_locale = LocaleSiteSetting.fallback_locale(locale))
locale_list << fallback_locale
locale = fallback_locale
end
locale_list << :en
locale_list.uniq.compact
end
end

12
spec/components/js_locale_helper_spec.rb
View File

@ -130,7 +130,7 @@ describe JsLocaleHelper do
end
end
it 'performs fallbacks to english if a translation is not available' do
it 'performs fallbacks to English if a translation is not available' do
JsLocaleHelper.set_translations('en', "en" => {
"js" => {
"only_english" => "1-en",
@ -161,12 +161,12 @@ describe JsLocaleHelper do
expected = {
"none" => "[uk.js.none]",
"only_english" => "1-en",
"only_site" => "2-ru",
"english_and_site" => "3-ru",
"only_site" => "[uk.js.only_site]",
"english_and_site" => "3-en",
"only_user" => "4-uk",
"english_and_user" => "5-uk",
"site_and_user" => "6-uk",
"all_three" => "7-uk",
"all_three" => "7-uk"
}
SiteSetting.default_locale = 'ru'
@ -178,9 +178,9 @@ describe JsLocaleHelper do
ctx.eval(JsLocaleHelper.output_locale(I18n.locale))
ctx.eval('I18n.defaultLocale = "ru";')
expect(ctx.eval('I18n.translations.en.js').keys).to contain_exactly("only_english")
expect(ctx.eval('I18n.translations.ru.js').keys).to contain_exactly("only_site", "english_and_site")
expect(ctx.eval('I18n.translations').keys).to contain_exactly("uk", "en")
expect(ctx.eval('I18n.translations.uk.js').keys).to contain_exactly("all_three", "english_and_user", "only_user", "site_and_user")
expect(ctx.eval('I18n.translations.en.js').keys).to contain_exactly("only_english", "english_and_site")
expected.each do |key, expect|
expect(ctx.eval("I18n.t(#{"js.#{key}".inspect})")).to eq(expect)

9
spec/lib/i18n/discourse_i18n_spec.rb
View File

@ -13,7 +13,6 @@ describe I18n::Backend::DiscourseI18n do
backend.store_translations(:en, foo: 'Foo in :en', bar: 'Bar in :en', wat: 'Hello %{count}')
backend.store_translations(:en, items: { one: 'one item', other: '%{count} items' })
backend.store_translations(:de, bar: 'Bar in :de')
backend.store_translations(:ru, baz: 'Baz in :ru')
backend.store_translations(:en, link: '[text](url)')
end
@ -54,13 +53,6 @@ describe I18n::Backend::DiscourseI18n do
expect(backend.translate(:de, 'bar')).to eq('Bar in :de')
expect(backend.translate(:de, 'foo')).to eq('Foo in :en')
end
it 'uses default_locale as fallback when key exists' do
SiteSetting.default_locale = 'ru'
expect(backend.translate(:de, 'bar')).to eq('Bar in :de')
expect(backend.translate(:de, 'baz')).to eq('Baz in :ru')
expect(backend.translate(:de, 'foo')).to eq('Foo in :en')
end
end
describe '#exists?' do
@ -75,7 +67,6 @@ describe I18n::Backend::DiscourseI18n do
it 'returns true when an existing key and an existing locale is given' do
expect(backend.exists?(:en, :foo)).to eq(true)
expect(backend.exists?(:de, :bar)).to eq(true)
expect(backend.exists?(:ru, :baz)).to eq(true)
end
it 'returns false when a non-existing key and an existing locale is given' do

21
spec/lib/i18n/fallback_locale_list_spec.rb
View File

@ -16,7 +16,7 @@ describe I18n::Backend::FallbackLocaleList do
it "works when default_locale is English (United States)" do
SiteSetting.default_locale = :en_US
expect(list[:ru]).to eq([:ru, :en_US, :en])
expect(list[:ru]).to eq([:ru, :en])
expect(list[:en_US]).to eq([:en_US, :en])
expect(list[:en]).to eq([:en])
end
@ -24,7 +24,7 @@ describe I18n::Backend::FallbackLocaleList do
it "works when default_locale is not English" do
SiteSetting.default_locale = :de
expect(list[:ru]).to eq([:ru, :de, :en])
expect(list[:ru]).to eq([:ru, :en])
expect(list[:de]).to eq([:de, :en])
expect(list[:en]).to eq([:en])
expect(list[:en_US]).to eq([:en_US, :en])
@ -34,6 +34,7 @@ describe I18n::Backend::FallbackLocaleList do
before do
DiscoursePluginRegistry.register_locale("es_MX", fallbackLocale: "es")
DiscoursePluginRegistry.register_locale("de_AT", fallbackLocale: "de")
DiscoursePluginRegistry.register_locale("de_AT-formal", fallbackLocale: "de_AT")
end
after do
@ -44,15 +45,27 @@ describe I18n::Backend::FallbackLocaleList do
SiteSetting.default_locale = :en
expect(list[:de_AT]).to eq([:de_AT, :de, :en])
expect(list[:"de_AT-formal"]).to eq([:"de_AT-formal", :de_AT, :de, :en])
expect(list[:de]).to eq([:de, :en])
expect(list[:en]).to eq([:en])
end
it "works when default_locale is English (United States)" do
SiteSetting.default_locale = :en_US
expect(list[:de_AT]).to eq([:de_AT, :de, :en])
expect(list[:"de_AT-formal"]).to eq([:"de_AT-formal", :de_AT, :de, :en])
expect(list[:de]).to eq([:de, :en])
expect(list[:en]).to eq([:en])
expect(list[:en_US]).to eq([:en_US, :en])
end
it "works when default_locale is not English" do
SiteSetting.default_locale = :de
expect(list[:es_MX]).to eq([:es_MX, :es, :de, :en])
expect(list[:es]).to eq([:es, :de, :en])
expect(list[:es_MX]).to eq([:es_MX, :es, :en])
expect(list[:"de_AT-formal"]).to eq([:"de_AT-formal", :de_AT, :de, :en])
expect(list[:es]).to eq([:es, :en])
expect(list[:en]).to eq([:en])
end
end

20
spec/models/theme_field_spec.rb
View File

@ -8,6 +8,10 @@ describe ThemeField do
ThemeField.destroy_all
end
before do
I18n.locale = :en
end
describe "scope: find_by_theme_ids" do
it "returns result in the specified order" do
theme = Fabricate(:theme)
@ -301,8 +305,8 @@ HTML
let!(:theme3) { Fabricate(:theme) }
let!(:en1) {
ThemeField.create!(theme: theme, target_id: Theme.targets[:translations], name: "en_US",
value: { en_US: { somestring1: "helloworld", group: { key1: "enval1" } } }
ThemeField.create!(theme: theme, target_id: Theme.targets[:translations], name: "en",
value: { en: { somestring1: "helloworld", group: { key1: "enval1" } } }
.deep_stringify_keys.to_yaml
)
}
@ -313,21 +317,21 @@ HTML
)
}
let!(:fr2) { ThemeField.create!(theme: theme2, target_id: Theme.targets[:translations], name: "fr", value: "") }
let!(:en2) { ThemeField.create!(theme: theme2, target_id: Theme.targets[:translations], name: "en_US", value: "") }
let!(:en2) { ThemeField.create!(theme: theme2, target_id: Theme.targets[:translations], name: "en", value: "") }
let!(:ca3) { ThemeField.create!(theme: theme3, target_id: Theme.targets[:translations], name: "ca", value: "") }
let!(:en3) { ThemeField.create!(theme: theme3, target_id: Theme.targets[:translations], name: "en_US", value: "") }
let!(:en3) { ThemeField.create!(theme: theme3, target_id: Theme.targets[:translations], name: "en", value: "") }
describe "scopes" do
it "filter_locale_fields returns results in the correct order" do
expect(ThemeField.find_by_theme_ids([theme3.id, theme.id, theme2.id])
.filter_locale_fields(
["en_US", "fr"]
["en", "fr"]
)).to eq([en3, en1, fr1, en2, fr2])
end
it "find_first_locale_fields returns only the first locale for each theme" do
expect(ThemeField.find_first_locale_fields(
[theme3.id, theme.id, theme2.id], ["ca", "en_US", "fr"]
[theme3.id, theme.id, theme2.id], ["ca", "en", "fr"]
)).to eq([ca3, en1, en2])
end
end
@ -358,7 +362,7 @@ HTML
it "loads correctly" do
expect(fr1.translation_data).to eq(
fr: { somestring1: "bonjourworld", group: { key2: "frval2" } },
en_US: { somestring1: "helloworld", group: { key1: "enval1" } }
en: { somestring1: "helloworld", group: { key1: "enval1" } }
)
end
@ -380,7 +384,7 @@ HTML
theme.reload
expect(fr1.translation_data).to eq(
fr: { somestring1: "bonjourworld", group: { key2: "frval2" } },
en_US: { somestring1: "helloworld", group: { key1: "overriddentest1" } }
en: { somestring1: "helloworld", group: { key1: "overriddentest1" } }
)
end
end

10
spec/models/theme_spec.rb
View File

@ -7,6 +7,10 @@ describe Theme do
Theme.clear_cache!
end
before do
I18n.locale = :en
end
fab! :user do
Fabricate(:user)
end
@ -657,8 +661,8 @@ HTML
end
it "can create a hash of overridden values" do
en_translation = ThemeField.create!(theme_id: theme.id, name: "en_US", type_id: ThemeField.types[:yaml], target_id: Theme.targets[:translations], value: <<~YAML)
en_US:
en_translation = ThemeField.create!(theme_id: theme.id, name: "en", type_id: ThemeField.types[:yaml], target_id: Theme.targets[:translations], value: <<~YAML)
en:
group_of_translations:
translation1: en test1
YAML
@ -668,7 +672,7 @@ HTML
theme.update_translation("group_of_translations.translation1", "overriddentest2")
theme.reload
expect(theme.translation_override_hash).to eq(
"en_US" => {
"en" => {
"group_of_translations" => {
"translation1" => "overriddentest1"
}