From 0a0261900c40a31fd87d0d8c765fdd8f1bc62ad0 Mon Sep 17 00:00:00 2001
From: voodoorai2000 <voodoorai2000@gmail.com>
Date: Thu, 4 Oct 2018 14:25:17 +0200
Subject: [PATCH] Display language name or language key

There where two issues with the current implementation:

- There was a possible duplication between looking up the language name in key "locale" and in key "i18n.language.name"

- The "default" option was not being picked up, as the fallback always returned the default locale's translation, "English"

With this implementation there is only a single place to put the language name: i18n.language.name. I think this place is easier to find and understand for Crowdin translators than a "locale" key hidden in general.yml

If the translation is not found we display the language key, instead of English, which makes more sense to me too :relieved:

Solution based on recent comments[1] on a related I18n issue

[1] https://github.com/svenfuchs/i18n/issues/365#issuecomment-419263847
---
 app/helpers/locales_helper.rb       |  5 +----
 spec/features/localization_spec.rb  | 26 +++++++++++++++++++++++
 spec/helpers/locales_helper_spec.rb | 32 +++++++++++++++++++++++++++++
 3 files changed, 59 insertions(+), 4 deletions(-)
 create mode 100644 spec/helpers/locales_helper_spec.rb

diff --git a/app/helpers/locales_helper.rb b/app/helpers/locales_helper.rb
index 78ab25189..3ae6bb5ef 100644
--- a/app/helpers/locales_helper.rb
+++ b/app/helpers/locales_helper.rb
@@ -1,10 +1,7 @@
 module LocalesHelper
 
   def name_for_locale(locale)
-    default = I18n.t("locale", locale: locale)
-    I18n.backend.translate(locale, "i18n.language.name", default: default)
-  rescue
-    nil
+    I18n.t("i18n.language.name", locale: locale, fallback: false, default: locale.to_s)
   end
 
 end
diff --git a/spec/features/localization_spec.rb b/spec/features/localization_spec.rb
index eb345cb7c..255847991 100644
--- a/spec/features/localization_spec.rb
+++ b/spec/features/localization_spec.rb
@@ -48,4 +48,30 @@ feature 'Localization' do
     expect(page).not_to have_content('Language')
     expect(page).not_to have_css('div.locale')
   end
+
+  context "Missing language names" do
+
+    let!(:default_enforce) { I18n.enforce_available_locales }
+    let!(:default_locales) { I18n.available_locales }
+
+    before do
+      I18n.enforce_available_locales = false
+      I18n.available_locales << :wl
+      I18n.locale = :wl
+    end
+
+    after do
+      I18n.enforce_available_locales = default_enforce
+      I18n.available_locales = default_locales
+    end
+
+    scenario 'Available locales without language translation display locale key' do
+      visit '/'
+
+      within('.locale-form .js-location-changer') do
+        expect(page).to have_content 'wl'
+      end
+    end
+
+  end
 end
diff --git a/spec/helpers/locales_helper_spec.rb b/spec/helpers/locales_helper_spec.rb
new file mode 100644
index 000000000..2eb10be05
--- /dev/null
+++ b/spec/helpers/locales_helper_spec.rb
@@ -0,0 +1,32 @@
+require 'rails_helper'
+
+describe LocalesHelper do
+
+  context "Language names" do
+
+    let!(:default_enforce) { I18n.enforce_available_locales }
+
+    before do
+      I18n.enforce_available_locales = false
+    end
+
+    after do
+      I18n.backend.reload!
+      I18n.enforce_available_locales = default_enforce
+    end
+
+    it "returns the language name in i18n.language.name translation" do
+      keys = { language: {
+                 name: "World Language" }}
+
+      I18n.backend.store_translations(:wl, { i18n: keys })
+
+      expect(name_for_locale(:wl)).to eq("World Language")
+    end
+
+    it "retuns the locale key if i18n.language.name translation is not found" do
+      expect(name_for_locale(:wl)).to eq("wl")
+    end
+
+  end
+end
\ No newline at end of file