Fix bug with overwriting of defaults with recent Rails 6.1 compatibility fix (#183)

* Fix default overwriting bug

* More bugfixes for default value and humanization on Rails 6.1

Author: pnikrat

CHANGELOG.md

@@ -4,8 +4,8 @@ All notable changes to this project will be documented in this file.
 The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
 
 ## [Unreleased]
-### Added
-- Proper CHANGELOG [@pnikrat](https://github.com/pnikrat)
+### Fixed
+- When running Lit on Rails 6.1 defaults could sometimes be overwritten [@pnikrat](https://github.com/pnikrat)
 
 ## [WIP]
 - Efforts to use Vanilla JS and remove jQuery [WIP]
@@ -17,6 +17,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
 - When using Cloud translations, support V2 `google-cloud-translate` gem [@pnikrat](https://github.com/pnikrat)
 - Proper Rails 6.1 support. Fixes new Rails translate logic not saving defaults in Lit [@pnikrat](https://github.com/pnikrat)
 - Add screenshots to README [@mlitwiniuk](https://github.com/mlitwiniuk)
+- Proper CHANGELOG [@pnikrat](https://github.com/pnikrat)
 
 ### Changed
 - Lit now uses Ruby 2.7.4 [@pnikrat](https://github.com/pnikrat)

app/helpers/lit/frontend_helper.rb

@@ -23,6 +23,18 @@ def pluralized_key(key, count)
     def t(key, options = {})
       translate(key, options)
     end
+
+    def missing_translation(key, options)
+      # We try to humanize the key. Rails will do
+      # it anyway in below call to super, but then it will wrap it also in
+      # translation_missing span.
+      # Humanizing key should be last resort
+      if Lit::Services::HumanizeService.should_humanize?(key)
+        return Lit::Services::HumanizeService.humanize_and_cache(key, options)
+      end
+
+      super(key, options)
+    end
   end
   prepend Lit::FrontendHelper::TranslationKeyWrapper
 

lib/lit/i18n_backend.rb

@@ -1,4 +1,5 @@
 require 'i18n'
+require 'lit/services/humanize_service'
 
 module Lit
   class I18nBackend
@@ -28,7 +29,7 @@ def translate(locale, key, options = {})
         @untranslated_key = key if key.present? && options[:default].instance_of?(Object)
 
         if key.nil? && options[:lit_default_copy].present?
-          update_default_localization(locale, content, options)
+          update_default_localization(locale, options)
         end
       end
 
@@ -72,10 +73,12 @@ def on_rails_6_1_or_higher?
         ::Rails::VERSION::MAJOR >= 7
     end
 
-    def update_default_localization(locale, content, options)
+    def update_default_localization(locale, options)
       parts = I18n.normalize_keys(locale, @untranslated_key, options[:scope], options[:separator])
       key_with_locale = parts.join('.')
-      @cache.update_locale(key_with_locale, content, content.is_a?(Array))
+      content = options[:lit_default_copy]
+      # we do not force array on singular strings packed into Array
+      @cache.update_locale(key_with_locale, content, content.is_a?(Array) && content.length > 1)
     end
 
     def can_dup_default(options = {})
@@ -118,35 +121,35 @@ def lookup(locale, key, scope = [], options = {})
             if options[:lit_default_copy].is_a?(Array)
               default = options[:lit_default_copy].map do |key_or_value|
                 if key_or_value.is_a?(Symbol)
-                  I18n.normalize_keys(nil, key_or_value.to_s, options[:scope], options[:separator]).join('.').to_sym
+                  normalized = I18n.normalize_keys(
+                    nil, key_or_value.to_s, options[:scope], options[:separator]
+                  ).join('.')
+                  if on_rails_6_1_or_higher? && Lit::Services::HumanizeService.should_humanize?(key)
+                    Lit::Services::HumanizeService.humanize(normalized)
+                  else
+                    normalized.to_sym
+                  end
                 else
                   key_or_value
                 end
               end
-              default = default.first if default.is_a?(Array)
             else
               default = options[:lit_default_copy]
             end
             content = default
           end
           # if we have content now, let's store it in cache
           if content.present?
-            @cache[key_with_locale] = content
-            content = @cache[key_with_locale]
-          end
-          # content might be nil - default value passed to cache was in fact
-          # useless.
-          # if content is still nil, we may try to humanize it. Rails will do
-          # it anyway if we return nil, but then it will wrap it also in
-          # translation_missing span.
-          # Humanizing key should be last resort
-          if content.nil? && Lit.humanize_key && Lit.humanize_key_ignored.match(key).nil?
-            content = key.to_s.split('.').last.humanize
-            if content.present?
-              @cache[key_with_locale] = content
-              content = @cache[key_with_locale]
+            content = Array.wrap(content).compact.reject(&:empty?).reverse.find do |default_cand|
+              @cache[key_with_locale] = default_cand
+              @cache[key_with_locale]
             end
           end
+
+          if content.nil? && !on_rails_6_1_or_higher? && Lit::Services::HumanizeService.should_humanize?(key)
+            @cache[key_with_locale] = Lit::Services::HumanizeService.humanize(key)
+            content = @cache[key_with_locale]
+          end
         end
       end
       # return translated content

lib/lit/services/humanize_service.rb

@@ -0,0 +1,28 @@
+# frozen_string_literal: true
+
+module Lit
+  module Services
+    # Checks if should humanize based on config and blacklist.
+    # Performs humanize if required
+    # Caches the value of humanization
+    class HumanizeService
+      def self.should_humanize?(key)
+        Lit.humanize_key && Lit.humanize_key_ignored.match(key).nil?
+      end
+
+      def self.humanize(key)
+        key.to_s.split('.').last.humanize
+      end
+
+      def self.humanize_and_cache(key, options)
+        content = humanize(key)
+        parts = I18n.normalize_keys(
+          options[:locale] || I18n.locale, key, options[:scope], options[:separator]
+        )
+        key_with_locale = parts.join('.')
+        I18n.cache_store[key_with_locale] = content
+        I18n.cache_store[key_with_locale]
+      end
+    end
+  end
+end

test/dummy/app/views/projects/_form.html.erb

@@ -11,6 +11,8 @@
     </div>
   <% end %>
 
+  <div><%= t('some_random.key') %></div>
+  <div><%= t('another_random.key', default: 'Random') %></div>
   <div class="field">
     <%= f.label :name %><br />
     <%= f.text_field :name %>

test/unit/i18n_backend_test.rb

@@ -32,6 +32,8 @@ def teardown
   end
 
   test 'auto-humanizes key when Lit.humanize_key=true' do
+    # since Rails 6.1 pure i18n calls should not be humanized
+    skip if I18n.backend.send(:on_rails_6_1_or_higher?)
     Lit.humanize_key = true
     I18n.locale = :en
     test_key = 'this_will_get_humanized'
@@ -65,7 +67,8 @@ def teardown
     loc_key_count = -> { Lit::LocalizationKey.where(localization_key: [test_key, fallback_key]).count }
     assert_equal 0, loc_key_count.call
     assert_equal 'foobar', I18n.t(test_key, default: [fallback_key, 'foobar'])
-    assert_equal 2, loc_key_count.call
+    # We do not create localization key record for fallback keys if value is found
+    assert_equal 1, loc_key_count.call
 
     # on subsequent translation calls, they should not be fetched from DB
     assert_no_database_queries do

test/unit/lit_behaviour_test.rb

@@ -252,6 +252,18 @@ def teardown
     assert_equal hash_result[:sub_one], 'Left leaf'
   end
 
+  test 'it must not overwrite default with I18n content when calling same key twice ' \
+    'with different interpolations' do
+    assert_difference 'Lit::LocalizationKey.count', 1 do
+      I18n.t('interpolated_key', default: 'Candidate %{name}', name: 'Tester')
+    end
+    assert_no_difference 'Lit::LocalizationKey.count' do
+      result = I18n.t('interpolated_key', default: 'Candidate %{name}', name: 'Famous person')
+      assert_equal result, 'Candidate Famous person'
+    end
+    assert_equal find_localization_for('interpolated_key', :en).default_value, 'Candidate %{name}'
+  end
+
   private
 
   def find_localization_for(key, locale)