From: Tom Hughes <tom@compton.nu>
Date: Wed, 6 Jan 2016 18:43:25 +0000 (+0000)
Subject: Rework locale selection
X-Git-Tag: live~4486
X-Git-Url: https://git.openstreetmap.org./rails.git/commitdiff_plain/4028f4cdb91f616bc774dc9b9fa8c7183eb1f6dd?hp=5f4b7d3f916e132f860ab12c017aba638556abf8

Rework locale selection

Implement our own matching algorithm rather than trying to
patch the http_accept_language one and make sure everything is
using it in a consistent way.

Fixes #1125
---

diff --git a/app/controllers/amf_controller.rb b/app/controllers/amf_controller.rb
index f6ae7f231..f2b592b49 100644
--- a/app/controllers/amf_controller.rb
+++ b/app/controllers/amf_controller.rb
@@ -190,11 +190,13 @@ class AmfController < ApplicationController
     user = getuser(usertoken)
 
     if user && !user.languages.empty?
-      http_accept_language.user_preferred_languages = user.languages
+      langs = Locale.list(user.languages)
+    else
+      langs = Locale.list(http_accept_language.user_preferred_languages)
     end
 
-    lang = http_accept_language.compatible_language_from(getlocales)
-    (real_lang, localised) = getlocalized(lang)
+    lang = getlocales.preferred(langs)
+    (real_lang, localised) = getlocalized(lang.to_s)
 
     # Tell Potlatch what language it's using
     localised["__potlatch_locale"] = real_lang
@@ -874,7 +876,7 @@ class AmfController < ApplicationController
   end
 
   def getlocales
-    Dir.glob("#{Rails.root}/config/potlatch/locales/*").collect { |f| File.basename(f, ".yml") }
+    @locales ||= Locale.list(Dir.glob("#{Rails.root}/config/potlatch/locales/*").collect { |f| File.basename(f, ".yml") })
   end
 
   ##
diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb
index 2ee701e9c..9954b775b 100644
--- a/app/controllers/application_controller.rb
+++ b/app/controllers/application_controller.rb
@@ -288,47 +288,30 @@ class ApplicationController < ActionController::Base
     end
   end
 
-  def set_locale
-    response.header["Vary"] = "Accept-Language"
-
-    if @user && !@user.languages.empty?
-      http_accept_language.user_preferred_languages = @user.languages
-      response.header["Vary"] = "*"
-    end
+  def preferred_languages
+    @languages ||= if params[:locale]
+                     Locale.list(params[:locale])
+                   elsif @user
+                     @user.preferred_languages
+                   else
+                     Locale.list(http_accept_language.user_preferred_languages)
+                   end
+  end
 
-    I18n.locale = select_locale
+  helper_method :preferred_languages
 
+  def set_locale
     if @user && @user.languages.empty? && !http_accept_language.user_preferred_languages.empty?
       @user.languages = http_accept_language.user_preferred_languages
       @user.save
     end
 
-    response.headers["Content-Language"] = I18n.locale.to_s
-  end
-
-  def select_locale(locales = I18n.available_locales)
-    if params[:locale]
-      http_accept_language.user_preferred_languages = [params[:locale]]
-    end
-
-    if http_accept_language.compatible_language_from(locales).nil?
-      http_accept_language.user_preferred_languages = http_accept_language.user_preferred_languages.collect do |pl|
-        pls = [pl]
-
-        while pl.match(/^(.*)-[^-]+$/)
-          pls.push($1) if locales.include?($1) || locales.include?($1.to_sym)
-          pl = $1
-        end
-
-        pls
-      end.flatten
-    end
+    I18n.locale = Locale.available.preferred(preferred_languages)
 
-    http_accept_language.compatible_language_from(locales) || I18n.default_locale
+    response.headers["Vary"] = "Accept-Language"
+    response.headers["Content-Language"] = I18n.locale.to_s
   end
 
-  helper_method :select_locale
-
   def api_call_handle_error
     yield
   rescue ActiveRecord::RecordNotFound => ex
diff --git a/app/models/notifier.rb b/app/models/notifier.rb
index 018226566..a9bc0d101 100644
--- a/app/models/notifier.rb
+++ b/app/models/notifier.rb
@@ -175,14 +175,8 @@ class Notifier < ActionMailer::Base
   private
 
   def with_recipient_locale(recipient)
-    old_locale = I18n.locale
-
-    begin
-      I18n.locale = recipient.preferred_language_from(I18n.available_locales)
-
+    I18n.with_locale Locale.available.preferred(recipient.preferred_languages) do
       yield
-    ensure
-      I18n.locale = old_locale
     end
   end
 
diff --git a/app/models/user.rb b/app/models/user.rb
index 0053333e2..eae917af8 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -131,8 +131,8 @@ class User < ActiveRecord::Base
     languages.find { |l| Language.exists?(:code => l) }
   end
 
-  def preferred_language_from(array)
-    (languages & array.collect(&:to_s)).first
+  def preferred_languages
+    @locales ||= Locale.list(languages)
   end
 
   def nearby(radius = NEARBY_RADIUS, num = NEARBY_USERS)
diff --git a/app/views/site/_potlatch2.html.erb b/app/views/site/_potlatch2.html.erb
index 3aeafa9cf..1c203b2fb 100644
--- a/app/views/site/_potlatch2.html.erb
+++ b/app/views/site/_potlatch2.html.erb
@@ -10,7 +10,7 @@
 <script type="text/javascript">alert("<%= t 'site.edit.potlatch2_not_configured' %>")</script>
 <% end %>
 
-<% locale = select_locale(Potlatch2::LOCALES.keys).to_s %>
+<% locale = Locale.list(Potlatch2::LOCALES.keys).preferred(preferred_languages).to_s %>
 
 <script type="text/javascript" defer="defer">
   var changesaved=true;
diff --git a/app/views/site/id.html.erb b/app/views/site/id.html.erb
index 27a3e835b..7d186c5f6 100644
--- a/app/views/site/id.html.erb
+++ b/app/views/site/id.html.erb
@@ -16,7 +16,7 @@
       'Please upgrade your browser or use Potlatch 2 to edit the map.';
     document.getElementById('id-container').className = 'unsupported';
   } else {
-    <% locale = select_locale(ID::LOCALES).to_s %>
+    <% locale = ID::LOCALES.preferred(preferred_languages).to_s %>
 
     var id = iD()
       .presets(iD.data.presets)
diff --git a/config/initializers/http_accept_language.rb b/config/initializers/http_accept_language.rb
deleted file mode 100644
index 2e5c92df1..000000000
--- a/config/initializers/http_accept_language.rb
+++ /dev/null
@@ -1,14 +0,0 @@
-#
-# Monkey patch HttpAcceptLanguage pending integration of
-# https://github.com/iain/http_accept_language/pull/6
-#
-module HttpAcceptLanguage
-  class Parser
-    def compatible_language_from(available_languages)
-      user_preferred_languages.find do |x|
-        available_languages.find { |y| y.to_s == x.to_s } ||
-          available_languages.find { |y| y.to_s =~ /^#{Regexp.escape(x.to_s)}-/ }
-      end
-    end
-  end
-end
diff --git a/config/initializers/i18n.rb b/config/initializers/i18n.rb
index 91efa2557..a94618c7e 100644
--- a/config/initializers/i18n.rb
+++ b/config/initializers/i18n.rb
@@ -8,6 +8,16 @@ module I18n
         ex.entry[:other]
       end
     end
+
+    class Simple
+      def store_translations_with_normalisation(locale, data, options = {})
+        locale = I18n::Locale::Tag::Rfc4646.tag(locale).to_s
+
+        store_translations_without_normalisation(locale, data, options)
+      end
+
+      alias_method_chain :store_translations, :normalisation
+    end
   end
 
   module JS
diff --git a/lib/id.rb b/lib/id.rb
index 15f918e79..858bc53c3 100644
--- a/lib/id.rb
+++ b/lib/id.rb
@@ -1,3 +1,3 @@
 module ID
-  LOCALES = Rails.root.join("vendor/assets/iD/iD/locales").entries.map { |p| p.basename.to_s[/(.*).json/] && $1 }.compact
+  LOCALES = Locale.list(Rails.root.join("vendor/assets/iD/iD/locales").entries.map { |p| p.basename.to_s[/(.*).json/] && $1 }.compact)
 end
diff --git a/lib/locale.rb b/lib/locale.rb
new file mode 100644
index 000000000..bb09ae327
--- /dev/null
+++ b/lib/locale.rb
@@ -0,0 +1,44 @@
+class Locale < I18n::Locale::Tag::Rfc4646
+  class List < Array
+    attr_reader :locales
+
+    def initialize(tags)
+      super(tags.map { |tag| Locale.tag(tag) })
+    end
+
+    def candidates(preferred)
+      preferred.expand & self
+    end
+
+    def preferred(preferred)
+      candidates(preferred).first
+    end
+
+    def expand
+      map(&:candidates).flatten.uniq << Locale.default
+    end
+  end
+
+  def self.list(*tags)
+    List.new(tags.flatten)
+  end
+
+  def self.default
+    tag(I18n.default_locale)
+  end
+
+  def self.available
+    @available ||= List.new(I18n.available_locales)
+  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)]
+  end
+end
diff --git a/test/integration/user_creation_test.rb b/test/integration/user_creation_test.rb
index 1ff8f9a65..6c6a10447 100644
--- a/test/integration/user_creation_test.rb
+++ b/test/integration/user_creation_test.rb
@@ -40,7 +40,7 @@ class UserCreationTest < ActionDispatch::IntegrationTest
       end
       assert_response :success
       assert_template "user/new"
-      assert_equal response.headers["Content-Language"][0..1], locale.to_s[0..1] unless locale == :root
+      assert_equal locale.to_s, response.headers["Content-Language"] unless locale == :root
       assert_select "form > fieldset > div.form-row > input.field_with_errors#user_email"
       assert_no_missing_translations
     end
diff --git a/test/models/user_test.rb b/test/models/user_test.rb
index b3e6eae8f..4a8d9a894 100644
--- a/test/models/user_test.rb
+++ b/test/models/user_test.rb
@@ -199,7 +199,7 @@ class UserTest < ActiveSupport::TestCase
     assert_equal %w(de fr en), user.languages
     user.languages = %w(fr de sl)
     assert_equal "de", user.preferred_language
-    assert_equal "de", user.preferred_language_from(%w(en sl de es))
+    assert_equal %w(fr de sl), user.preferred_languages.map(&:to_s)
     user = users(:public_user)
     assert_equal %w(en de), user.languages
   end