From: Tom Hughes <tom@compton.nu>
Date: Thu, 4 Jun 2020 16:23:16 +0000 (+0100)
Subject: Improve locale selection algorithm
X-Git-Tag: live~2638
X-Git-Url: https://git.openstreetmap.org./rails.git/commitdiff_plain/bf22c69286f97f72625c79d64e7c1a5cb6e95710?ds=sidebyside

Improve locale selection algorithm

Don't include locales which only have rails translations in
the candidates, and ensure that user specified options take
priority over less specific variants of earlier choices.
---

diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml
index 307a23f4b..6a1338851 100644
--- a/.rubocop_todo.yml
+++ b/.rubocop_todo.yml
@@ -18,7 +18,7 @@ require:
 # Configuration parameters: AutoCorrect, AllowHeredoc, AllowURI, URISchemes, IgnoreCopDirectives, IgnoredPatterns.
 # URISchemes: http, https
 Layout/LineLength:
-  Max: 260
+  Max: 370
 
 # Offense count: 35
 # Configuration parameters: AllowSafeAssignment.
diff --git a/lib/locale.rb b/lib/locale.rb
index e10f06632..b0ad20fc7 100644
--- a/lib/locale.rb
+++ b/lib/locale.rb
@@ -15,7 +15,11 @@ class Locale < I18n::Locale::Tag::Rfc4646
     end
 
     def expand
-      map(&:candidates).flatten.uniq << Locale.default
+      List.new(reverse.each_with_object([]) do |locale, expanded|
+                 locale.candidates.uniq.reverse_each do |candidate|
+                   expanded << candidate if candidate == locale || !expanded.include?(candidate)
+                 end
+               end.reverse.uniq << Locale.default)
     end
   end
 
@@ -28,17 +32,23 @@ class Locale < I18n::Locale::Tag::Rfc4646
   end
 
   def self.available
-    @available ||= List.new(I18n.available_locales)
+    @available ||= List.new(I18n.available_locales).reject!(&:invalid?)
+  end
+
+  def invalid?
+    I18n.t("activerecord.models.acl", :locale => self, :fallback => false, :raise => true).nil?
+  rescue I18n::MissingTranslationData
+    true
   end
 
   def candidates
-    [self.class.new(language, script, region, variant),
-     self.class.new(language, script, region),
-     self.class.new(language, script, nil, variant),
-     self.class.new(language, script),
-     self.class.new(language, nil, region, variant),
-     self.class.new(language, nil, region),
-     self.class.new(language, nil, nil, variant),
-     self.class.new(language)]
+    List.new([self.class.new(language, script, region, variant),
+              self.class.new(language, script, region),
+              self.class.new(language, script, nil, variant),
+              self.class.new(language, script),
+              self.class.new(language, nil, region, variant),
+              self.class.new(language, nil, region),
+              self.class.new(language, nil, nil, variant),
+              self.class.new(language)])
   end
 end
diff --git a/test/integration/user_creation_test.rb b/test/integration/user_creation_test.rb
index b825990b8..9c934dc07 100644
--- a/test/integration/user_creation_test.rb
+++ b/test/integration/user_creation_test.rb
@@ -32,7 +32,7 @@ class UserCreationTest < ActionDispatch::IntegrationTest
   end
 
   def test_user_create_submit_duplicate_email
-    I18n.available_locales.each do |locale|
+    Locale.available.each do |locale|
       dup_email = create(:user).email
       display_name = "#{locale}_new_tester"
       assert_difference("User.count", 0) do
@@ -50,7 +50,7 @@ class UserCreationTest < ActionDispatch::IntegrationTest
       end
       assert_response :success
       assert_template "users/new"
-      assert_equal locale.to_s, response.headers["Content-Language"] unless locale == :root
+      assert_equal locale.to_s, response.headers["Content-Language"]
       assert_select "form > fieldset > div.standard-form-row > input.field_with_errors#user_email"
       assert_no_missing_translations
     end
diff --git a/test/lib/locale_test.rb b/test/lib/locale_test.rb
index 4d4571160..13df643b2 100644
--- a/test/lib/locale_test.rb
+++ b/test/lib/locale_test.rb
@@ -80,7 +80,7 @@ class LocaleTest < ActiveSupport::TestCase
   end
 
   def test_available
-    assert_equal I18n.available_locales.count, Locale.available.count
+    assert Locale.available.count <= I18n.available_locales.count
   end
 
   def test_preferred
@@ -91,9 +91,7 @@ class LocaleTest < ActiveSupport::TestCase
     assert_equal "de", Locale.available.preferred(Locale.list("zh-Hant", "de")).to_s
     assert_equal "zh-TW", Locale.available.preferred(Locale.list("zh-Hant-TW", "de")).to_s
     assert_equal "zh-TW", Locale.available.preferred(Locale.list("zh-TW", "de")).to_s
-    assert_equal "zh-HK", Locale.available.preferred(Locale.list("yue", "zh-HK", "de")).to_s
-    assert_equal "zh-yue", Locale.available.preferred(Locale.list("yue", "zh-yue", "zh-HK", "de")).to_s
-    assert_equal "zh-yue", Locale.available.preferred(Locale.list("yue", "zh-YUE", "zh-HK", "de")).to_s
+    assert_equal "zh-TW", Locale.available.preferred(Locale.list("zh-HK", "zh-hk", "zh-Hant", "zh-hant", "zh-TW", "zh-tw", "zh", "zh-yue", "yue", "yue-Hant", "yue-HK", "yue-Hans", "zh-classical", "lzh", "ja-Hani", "ko-Hani", "ko_hanja", "vi-Hani", "ja-hani", "ko-hani", "vi-hani", "en-HK", "en-hk", "en-SG", "en-sg", "en-GB", "en-gb", "en-US", "en-us", "en", "ja")).to_s
     assert_equal "en", Locale.available.preferred(Locale.list("yue")).to_s
   end
 end