From 7a34c6e3f0dc736832516b2b4128f4522eaab00a Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Fri, 9 Mar 2018 16:38:41 +1100 Subject: [PATCH 1/6] Introduce I18nConfig as single point of truth It will be used in application.rb and views. See https://github.com/openfoodfoundation/openfoodnetwork/issues/2113 --- lib/open_food_network/i18n_config.rb | 22 +++++ .../lib/open_food_network/i18n_config_spec.rb | 94 +++++++++++++++++++ 2 files changed, 116 insertions(+) create mode 100644 lib/open_food_network/i18n_config.rb create mode 100644 spec/lib/open_food_network/i18n_config_spec.rb diff --git a/lib/open_food_network/i18n_config.rb b/lib/open_food_network/i18n_config.rb new file mode 100644 index 0000000000..73ab508315 --- /dev/null +++ b/lib/open_food_network/i18n_config.rb @@ -0,0 +1,22 @@ +module OpenFoodNetwork + # Provides access to the language settings. + # Currently, language settings are read from the environment. + # See: config/application.yml + class I18nConfig + def self.selectable_locales + ENV["AVAILABLE_LOCALES"].andand.split(/[\s,]/).andand.map(&:strip) || [] + end + + def self.available_locales + (selectable_locales + [default_locale, 'en']).uniq + end + + def self.default_locale + ENV["LOCALE"] || ENV["I18N_LOCALE"] || source_locale + end + + def self.source_locale + "en" + end + end +end diff --git a/spec/lib/open_food_network/i18n_config_spec.rb b/spec/lib/open_food_network/i18n_config_spec.rb new file mode 100644 index 0000000000..ac056ab921 --- /dev/null +++ b/spec/lib/open_food_network/i18n_config_spec.rb @@ -0,0 +1,94 @@ +require 'spec_helper' +require 'open_food_network/i18n_config' + +module OpenFoodNetwork + describe I18nConfig do + context "in default test configuration" do + before do + allow(ENV).to receive(:[]).with("LOCALE").and_return("en") + allow(ENV).to receive(:[]).with("AVAILABLE_LOCALES").and_return("en,es") + end + + it "provides the source locale" do + expect(I18nConfig.source_locale).to eq "en" + end + + it "provides the default locale" do + expect(I18nConfig.default_locale).to eq "en" + end + + it "provides the default selectable locales" do + expect(I18nConfig.selectable_locales).to eq ["en", "es"] + end + + it "provides the default available locales" do + expect(I18nConfig.available_locales).to eq ["en", "es"] + end + end + + context "without configuration" do + before do + allow(ENV).to receive(:[]).with("LOCALE").and_return(nil) + allow(ENV).to receive(:[]).with("I18N_LOCALE").and_return(nil) + allow(ENV).to receive(:[]).with("AVAILABLE_LOCALES").and_return(nil) + end + + it "provides the source locale" do + expect(I18nConfig.source_locale).to eq "en" + end + + it "provides the default locale" do + expect(I18nConfig.default_locale).to eq "en" + end + + it "provides the default selectable locales" do + expect(I18nConfig.selectable_locales).to eq [] + end + + it "provides the default available locales" do + expect(I18nConfig.available_locales).to eq ["en"] + end + end + + context "with UK configuration" do + before do + allow(ENV).to receive(:[]).with("LOCALE").and_return("en_GB") + allow(ENV).to receive(:[]).with("I18N_LOCALE").and_return(nil) + allow(ENV).to receive(:[]).with("AVAILABLE_LOCALES").and_return("en_GB") + end + + it "provides the source locale" do + expect(I18nConfig.source_locale).to eq "en" + end + + it "provides the default locale" do + expect(I18nConfig.default_locale).to eq "en_GB" + end + + it "provides the default selectable locales" do + expect(I18nConfig.selectable_locales).to eq ["en_GB"] + end + + it "provides the default available locales" do + expect(I18nConfig.available_locales).to eq ["en_GB", "en"] + end + end + + context "with human syntax" do + before do + allow(ENV).to receive(:[]).with("LOCALE").and_return("es") + allow(ENV).to receive(:[]).with("AVAILABLE_LOCALES").and_return("es, fr") + end + + xit "provides the default selectable locales" do + # current: expect(I18nConfig.selectable_locales).to eq ["es", "", "fr"] + expect(I18nConfig.selectable_locales).to eq ["es", "fr"] + end + + xit "provides the default available locales" do + # current: expect(I18nConfig.available_locales).to eq ["es", "", "fr", "en"] + expect(I18nConfig.available_locales).to eq ["es", "fr", "en"] + end + end + end +end From 3743cf1bc3ee9105a6b3f00921c39ea044745617 Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Fri, 9 Mar 2018 16:54:00 +1100 Subject: [PATCH 2/6] Use I18nConfig for application defaults --- config/application.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/config/application.rb b/config/application.rb index 98bdb1e336..f93d224129 100644 --- a/config/application.rb +++ b/config/application.rb @@ -1,6 +1,7 @@ require_relative 'boot' require 'rails/all' +require_relative "../lib/open_food_network/i18n_config" if defined?(Bundler) # If you precompile assets before deploying to production, use this line @@ -104,9 +105,8 @@ module Openfoodnetwork # The default locale is :en and all translations from config/locales/*.rb,yml are auto loaded. # config.i18n.load_path += Dir[Rails.root.join('my', 'locales', '*.{rb,yml}').to_s] - config.i18n.default_locale = ENV["LOCALE"] || ENV["I18N_LOCALE"] || "en" - config.i18n.available_locales = ENV["AVAILABLE_LOCALES"].andand.split(/[\s,]/).andand.map(&:strip) || [] - config.i18n.available_locales = (config.i18n.available_locales + [config.i18n.default_locale, 'en']).uniq + config.i18n.default_locale = OpenFoodNetwork::I18nConfig.default_locale + config.i18n.available_locales = OpenFoodNetwork::I18nConfig.available_locales I18n.locale = config.i18n.locale = config.i18n.default_locale # Setting this to true causes a performance regression in Rails 3.2.17 From 8017921e894c13097379695dece296d1520b8b7e Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Fri, 9 Mar 2018 17:26:14 +1100 Subject: [PATCH 3/6] Use I18nConfig in views --- app/views/shared/menu/_large_menu.html.haml | 4 +- app/views/shared/menu/_mobile_menu.html.haml | 4 +- spec/features/consumer/multilingual_spec.rb | 43 ++++++++++++-------- 3 files changed, 29 insertions(+), 22 deletions(-) diff --git a/app/views/shared/menu/_large_menu.html.haml b/app/views/shared/menu/_large_menu.html.haml index 96347a5d80..96f4ad5691 100644 --- a/app/views/shared/menu/_large_menu.html.haml +++ b/app/views/shared/menu/_large_menu.html.haml @@ -60,12 +60,12 @@ = t 'powered_by' %a{href: '/'} = t 'title' - - if I18n.available_locales.count > 1 + - if OpenFoodNetwork::I18nConfig.selectable_locales.count > 1 %li.language-switcher.has-dropdown %a{href: '#'} %i.ofn-i_071-globe %ul.dropdown - - I18n.available_locales.each do |l| + - OpenFoodNetwork::I18nConfig.selectable_locales.each do |l| %li %a{href: "?locale=#{l.to_s}" }= t('language_name', locale: l) - if spree_current_user.nil? diff --git a/app/views/shared/menu/_mobile_menu.html.haml b/app/views/shared/menu/_mobile_menu.html.haml index 2766a1a8ee..2aa708bfb4 100644 --- a/app/views/shared/menu/_mobile_menu.html.haml +++ b/app/views/shared/menu/_mobile_menu.html.haml @@ -40,13 +40,13 @@ %span.nav-primary %i.ofn-i_036-producers = t 'label_producers' - - if I18n.available_locales.count > 1 + - if OpenFoodNetwork::I18nConfig.selectable_locales.count > 1 %li.language-switcher.li-menu %a %i.ofn-i_071-globe = t('language_name') %ul - - I18n.available_locales.each do |l| + - OpenFoodNetwork::I18nConfig.selectable_locales.each do |l| - if I18n.locale != l %li %a{href: "?locale=#{l.to_s}" }= t('language_name', locale: l) diff --git a/spec/features/consumer/multilingual_spec.rb b/spec/features/consumer/multilingual_spec.rb index d7bda79dbe..2844f479ca 100644 --- a/spec/features/consumer/multilingual_spec.rb +++ b/spec/features/consumer/multilingual_spec.rb @@ -71,11 +71,10 @@ feature 'Multilingual', js: true do describe "using the language switcher UI" do context "when there is only one language available" do - around do |example| - available_locales = I18n.available_locales - I18n.available_locales = ['en'] - example.run - I18n.available_locales = available_locales + before do + allow(ENV).to receive(:[]).and_call_original + allow(ENV).to receive(:[]).with("LOCALE").and_return("en") + allow(ENV).to receive(:[]).with("AVAILABLE_LOCALES").and_return("en") end it "hides the dropdown language menu" do @@ -84,21 +83,29 @@ feature 'Multilingual', js: true do end end - it "allows switching language via the main navigation" do - visit root_path - - expect(page).to have_content 'SHOPS' - - find('ul.right li.language-switcher').click - within'ul.right li.language-switcher ul.dropdown' do - expect(page).to have_link I18n.t('language_name', locale: :en), href: '?locale=en' - expect(page).to have_link I18n.t('language_name', locale: :es, default: 'Language Name'), href: '?locale=es' - - find('li a[href="?locale=es"]').click + context "when there are multiple languages available" do + before do + allow(ENV).to receive(:[]).and_call_original + allow(ENV).to receive(:[]).with("LOCALE").and_return("en") + allow(ENV).to receive(:[]).with("AVAILABLE_LOCALES").and_return("en,es") end - expect(page.driver.browser.cookies['locale'].value).to eq 'es' - expect(page).to have_content 'TIENDAS' + it "allows switching language via the main navigation" do + visit root_path + + expect(page).to have_content 'SHOPS' + + find('ul.right li.language-switcher').click + within'ul.right li.language-switcher ul.dropdown' do + expect(page).to have_link I18n.t('language_name', locale: :en), href: '?locale=en' + expect(page).to have_link I18n.t('language_name', locale: :es, default: 'Language Name'), href: '?locale=es' + + find('li a[href="?locale=es"]').click + end + + expect(page.driver.browser.cookies['locale'].value).to eq 'es' + expect(page).to have_content 'TIENDAS' + end end end end From db61a2bb17cacdc14ec626da7db3982a53cad5d2 Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Fri, 9 Mar 2018 19:41:49 +1100 Subject: [PATCH 4/6] Parse available language string more relaxed Splitting the string with any whitespace makes it more likely the code knows what the human meant. It also makes `strip` obsolete, since there can't be any whitespace within the splitted strings. --- lib/open_food_network/i18n_config.rb | 2 +- spec/lib/open_food_network/i18n_config_spec.rb | 12 +++++------- 2 files changed, 6 insertions(+), 8 deletions(-) diff --git a/lib/open_food_network/i18n_config.rb b/lib/open_food_network/i18n_config.rb index 73ab508315..8ef0b3e563 100644 --- a/lib/open_food_network/i18n_config.rb +++ b/lib/open_food_network/i18n_config.rb @@ -4,7 +4,7 @@ module OpenFoodNetwork # See: config/application.yml class I18nConfig def self.selectable_locales - ENV["AVAILABLE_LOCALES"].andand.split(/[\s,]/).andand.map(&:strip) || [] + ENV["AVAILABLE_LOCALES"].andand.split(/[\s,]+/) || [] end def self.available_locales diff --git a/spec/lib/open_food_network/i18n_config_spec.rb b/spec/lib/open_food_network/i18n_config_spec.rb index ac056ab921..306566b5e8 100644 --- a/spec/lib/open_food_network/i18n_config_spec.rb +++ b/spec/lib/open_food_network/i18n_config_spec.rb @@ -77,17 +77,15 @@ module OpenFoodNetwork context "with human syntax" do before do allow(ENV).to receive(:[]).with("LOCALE").and_return("es") - allow(ENV).to receive(:[]).with("AVAILABLE_LOCALES").and_return("es, fr") + allow(ENV).to receive(:[]).with("AVAILABLE_LOCALES").and_return("es, fr ,, ,de") end - xit "provides the default selectable locales" do - # current: expect(I18nConfig.selectable_locales).to eq ["es", "", "fr"] - expect(I18nConfig.selectable_locales).to eq ["es", "fr"] + it "provides the default selectable locales" do + expect(I18nConfig.selectable_locales).to eq ["es", "fr", "de"] end - xit "provides the default available locales" do - # current: expect(I18nConfig.available_locales).to eq ["es", "", "fr", "en"] - expect(I18nConfig.available_locales).to eq ["es", "fr", "en"] + it "provides the default available locales" do + expect(I18nConfig.available_locales).to eq ["es", "fr", "de", "en"] end end end From 64063f305bae068d5543affe90cf90415b0d3261 Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Tue, 13 Mar 2018 10:24:42 +1100 Subject: [PATCH 5/6] Describe methods of I18nConfig --- lib/open_food_network/i18n_config.rb | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/lib/open_food_network/i18n_config.rb b/lib/open_food_network/i18n_config.rb index 8ef0b3e563..4e93c9b967 100644 --- a/lib/open_food_network/i18n_config.rb +++ b/lib/open_food_network/i18n_config.rb @@ -3,18 +3,23 @@ module OpenFoodNetwork # Currently, language settings are read from the environment. # See: config/application.yml class I18nConfig + # Locales that can be selected by users. def self.selectable_locales ENV["AVAILABLE_LOCALES"].andand.split(/[\s,]+/) || [] end + # All locales that can be accessed by the application, including fallbacks. def self.available_locales (selectable_locales + [default_locale, 'en']).uniq end + # The default locale that is used when the user doesn't have a preference. def self.default_locale ENV["LOCALE"] || ENV["I18N_LOCALE"] || source_locale end + # This locale is changed with the code and should always be complete. + # All translations are done from this locale. def self.source_locale "en" end From 01dc51c84b25318e27e812160d49149cb29d1409 Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Tue, 13 Mar 2018 10:25:31 +1100 Subject: [PATCH 6/6] Consistent reference of the source locale --- lib/open_food_network/i18n_config.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/open_food_network/i18n_config.rb b/lib/open_food_network/i18n_config.rb index 4e93c9b967..3807aeae5d 100644 --- a/lib/open_food_network/i18n_config.rb +++ b/lib/open_food_network/i18n_config.rb @@ -10,7 +10,7 @@ module OpenFoodNetwork # All locales that can be accessed by the application, including fallbacks. def self.available_locales - (selectable_locales + [default_locale, 'en']).uniq + (selectable_locales + [default_locale, source_locale]).uniq end # The default locale that is used when the user doesn't have a preference.