Add support for strings in gems

CHANGELOG.md

@@ -1,3 +1,8 @@
+## Unreleased
+
+* Support for strings coming from gems ([@coorasse][])
+* Support for new strings (not yet translated) ([@coorasse][])
+
 ## 0.1.1
 
 * Review Stimulus controller ([@coorasse][])

README.md

@@ -188,8 +188,7 @@ See the image below as an example:
 * Support for interpolation
 * Support for count variants
 * Better inline editing tool
-* Performance of translations lookup
-* Support for translations and strings coming from other gems
+* Support for fallbacks: it should identify that a fallback string is being used on not try to override the value.
 
 ## License
 The gem is available as open source under the terms of the [MIT License](https://opensource.org/licenses/MIT).

app/assets/javascripts/moirai_translation_controller.js

@@ -23,7 +23,7 @@ export default class MoiraiTranslationController extends Controller {
       body: JSON.stringify({
         translation: {
           key: this.keyValue,
-          locale: this.localeValue
+          locale: this.localeValue,
           value: event.target.innerText
         }
       })

app/controllers/moirai/translation_files_controller.rb

@@ -28,7 +28,7 @@ def create_or_update
     end
 
     def open_pr
-      flash.notice = "I created an amazing PR"
+      flash.notice = "I created an amazing Pull Request"
       changes = Moirai::TranslationDumper.new.call
       Moirai::PullRequestCreator.new.create_pull_request(changes)
       redirect_back_or_to(root_path)
@@ -37,7 +37,7 @@ def open_pr
     private
 
     def handle_update(translation)
-      if translation_params[:value].blank? || translation_same_as_in_file?
+      if translation_params[:value].blank? || translation_same_as_current?
         translation.destroy
         flash.notice = "Translation #{translation.key} was successfully deleted."
         redirect_to_translation_file(translation.file_path)
@@ -54,27 +54,20 @@ def handle_update(translation)
     end
 
     def handle_create
-      file_path = KeyFinder.new.file_path_for(translation_params[:key], locale: translation_params[:locale])
-      if translation_same_as_in_file?
+      if translation_same_as_current?
         flash.alert = "Translation #{translation_params[:key]} already exists."
-        redirect_to_translation_file(file_path)
+        redirect_back_or_to moirai_translation_files_path, status: :unprocessable_entity
         return
       end
 
       translation = Translation.new(translation_params)
-      translation.locale = @file_handler.get_first_key(file_path) if file_path.present?
 
       if translation.save
         flash.notice = "Translation #{translation.key} was successfully created."
         success_response(translation)
       else
         flash.alert = translation.errors.full_messages.join(", ")
-        if file_path.present?
-          flash.alert = "Translation #{translation.key} already exists."
-          redirect_back_or_to moirai_translation_file_path(Digest::SHA256.hexdigest(file_path))
-        else
-          redirect_back_or_to moirai_translation_files_path, status: :unprocessable_entity
-        end
+        redirect_back_or_to moirai_translation_files_path, status: :unprocessable_entity
       end
     end
 
@@ -109,13 +102,17 @@ def load_file_handler
       @file_handler = Moirai::TranslationFileHandler.new
     end
 
-    def translation_same_as_in_file?
-      file_path = KeyFinder.new.file_path_for(translation_params[:key], locale: translation_params[:locale])
+    # TODO: to resolve the last point of the TODOs we could look at the current translation (without moirai)
+    # I quickly tried but I need to use the original backend instead of the moirai one
+    # The problem is that if we set a value that is the same as currently being used via fallback,
+    # it will create an entry in the database, and afterwards will try to add it in the PR, which we don't want.
+    def translation_same_as_current?
+      file_paths = KeyFinder.new.file_paths_for(translation_params[:key], locale: translation_params[:locale])
 
-      return false if file_path.blank?
-      return false unless File.exist?(file_path)
+      return false if file_paths.empty?
+      return false unless file_paths.all? { |file_path| File.exist?(file_path) }
 
-      translation_params[:value] == @file_handler.parse_file(file_path)[translation_params[:key]]
+      translation_params[:value] == @file_handler.parse_file(file_paths.first)[translation_params[:key]]
     end
   end
 end

app/controllers/moirai/translations_controller.rb

@@ -0,0 +1,7 @@
+module Moirai
+  class TranslationsController < ApplicationController
+    def index
+      @translations = Translation.order(created_at: :desc).pluck(:locale, :key, :value)
+    end
+  end
+end

app/models/moirai/key_finder.rb

@@ -12,16 +12,22 @@ def initialize
     end
 
     # TODO: remove locale default
-    def file_path_for(key, locale: I18n.locale)
+    # Returns all the file_paths where the key is found, including gems.
+    def file_paths_for(key, locale: I18n.locale)
+      return [] if key.blank?
+
       locale ||= I18n.locale
-      moirai_translations[locale.to_sym][key]
+      moirai_translations[locale.to_sym].select do |_filename, data|
+        data.dig(*key.split(".")).present?
+      end.map { |k, _| k }.sort { |file_path| file_path.start_with?(Rails.root.to_s) ? 0 : 1 }
     end
 
     def store_moirai_translations(filename, locale, data, options)
       moirai_translations[locale] ||= Concurrent::Hash.new
-      flatten_data = flatten_hash(filename, data)
-      flatten_data = I18n::Utils.deep_symbolize_keys(flatten_data) unless options.fetch(:skip_symbolize_keys, false)
-      I18n::Utils.deep_merge!(moirai_translations[locale], flatten_data)
+
+      locale = locale.to_sym
+      moirai_translations[locale] ||= Concurrent::Hash.new
+      moirai_translations[locale][filename] = data.with_indifferent_access
     end
 
     def moirai_translations(do_init: false)
@@ -45,27 +51,5 @@ def load_file(filename)
 
       data
     end
-
-    def flatten_hash(filename, hash, parent_key = "", result = {})
-      hash.each do |key, value|
-        new_key = parent_key.empty? ? key.to_s : "#{parent_key}.#{key}"
-        case value
-        when Hash
-          flatten_hash(filename, value, new_key, result)
-        when Array
-          value.each_with_index do |item, index|
-            array_key = "#{new_key}.#{index}"
-            if item.is_a?(Hash)
-              flatten_hash(filename, item, array_key, result)
-            else
-              result[array_key] = filename
-            end
-          end
-        else
-          result[new_key] = filename
-        end
-      end
-      result
-    end
   end
 end

app/models/moirai/translation.rb

@@ -4,14 +4,15 @@ module Moirai
   class Translation < Moirai::ApplicationRecord
     validates_presence_of :key, :locale, :value
 
+    # what if the key is present in multiple file_paths?
     def file_path
       @key_finder ||= KeyFinder.new
-      @key_finder.file_path_for(key, locale: locale)
+      @key_finder.file_paths_for(key, locale: locale).first
     end
 
     def self.by_file_path(file_path)
       key_finder = KeyFinder.new
-      all.select { |translation| key_finder.file_path_for(translation.key, locale: translation.locale) == file_path }
+      all.select { |translation| key_finder.file_paths_for(translation.key, locale: translation.locale).include?(file_path) }
     end
   end
 end

app/models/moirai/translation_dumper.rb

@@ -15,12 +15,25 @@ def call
       changes
     end
 
+    def best_file_path_for(key, locale)
+      file_paths = @key_finder.file_paths_for(key, locale: locale)
+      file_paths.filter! { |p| p.start_with?(Rails.root.to_s) }
+      if file_paths.any?
+        file_paths.first
+      elsif key.split(".").size > 1
+        parent_key = key.split(".")[0..-2].join(".")
+        best_file_path_for(parent_key, locale)
+      else
+        "./config/locales/#{locale}.yml"
+      end
+    end
+
     private
 
     def group_translations_by_file_path
       translations_grouped_by_file_path = {}
       Moirai::Translation.order(created_at: :asc).each do |translation|
-        file_path = @key_finder.file_path_for(translation.key, locale: translation.locale)
+        file_path = best_file_path_for(translation.key, translation.locale)
         absolute_file_path = File.expand_path(file_path, Rails.root)
 
         # skip file paths that don't belong to the project

app/views/moirai/translation_files/_form.html.erb

@@ -1,9 +1,9 @@
 <span 
   contenteditable 
-  data-action="blur->moirai-translation#submit click->moirai-translation#click" 
+  data-action="blur->moirai-translation#submit click->moirai-translation#click"
   style="border: 1px dashed #1d9f74; min-width: 30px; display: inline-block;" 
-  data-key="<%= key %>" 
-  data-locale="<%= locale %>"
+  data-moirai-translation-key-value="<%= key %>"
+  data-moirai-translation-locale-value="<%= locale %>"
   data-controller="moirai-translation">
   <%= value %>
 </span>

app/views/moirai/translation_files/index.html.erb

@@ -2,7 +2,7 @@
   <h1>Translation files</h1>
 
 <% if Moirai::PullRequestCreator.available? %>
-  <%= button_to "Create or update PR", moirai_open_pr_path %>
+  <%= button_to "Create or update Pull Request", moirai_open_pr_path %>
 <% else %>
   <p>PR creation is not available. Add the gem octokit to your gemfile to enable this feature</p>
 <% end %>

app/views/moirai/translations/index.html.erb

@@ -0,0 +1,7 @@
+<% @translations.each do |locale, key, value| %>
+  <div class="card">
+    <p><strong>Locale:</strong> <%=locale %></p>
+    <p><strong>Key:</strong> <%=key %></p>
+    <p><strong>Value:</strong> <%= value %></p>
+  </div>
+<% end %>

config/routes.rb

@@ -3,6 +3,7 @@
 Moirai::Engine.routes.draw do
   root to: "translation_files#index"
 
+  resources :translations, only: %i[index]
   resources :translation_files, only: %i[index show], as: "moirai_translation_files", param: :hashed_file_path
   post "/translation_files/open_pr", to: "translation_files#open_pr", as: "moirai_open_pr"
   post "/translation_files", to: "translation_files#create_or_update", as: "moirai_create_or_update_translation"

lib/moirai/engine.rb

@@ -9,9 +9,7 @@ class Engine < ::Rails::Engine
     end
 
     config.after_initialize do
-      moirai_backend = I18n::Backend::Moirai.new
-      moirai_backend.eager_load!
-      I18n.backend = I18n::Backend::Chain.new(moirai_backend, I18n.backend)
+      I18n.backend = I18n::Backend::Chain.new(I18n::Backend::Moirai.new, I18n.backend)
     end
 
     # TODO: how to do this without rewriting the entire method?
@@ -26,16 +24,11 @@ def translate(key, **options)
 
             if moirai_edit_enabled?
               @key_finder ||= Moirai::KeyFinder.new
-              file_path = @key_finder.file_path_for(scope_key_by_partial(key), locale: I18n.locale)
-
-              if file_path.present?
-                render(partial: "moirai/translation_files/form",
-                  locals: {key: scope_key_by_partial(key),
-                           locale: I18n.locale,
-                           value: value})
-              else
-                value
-              end
+
+              render(partial: "moirai/translation_files/form",
+                locals: {key: scope_key_by_partial(key),
+                         locale: I18n.locale,
+                         value: value})
             else
               value
             end

test/controllers/moirai/translation_files_controller_test.rb

@@ -48,8 +48,7 @@ class TranslationFilesControllerTest < ActionDispatch::IntegrationTest
 
     post translation_files_url, params: {translation: {key: "locales.german", locale: "en", value: "German"}}
 
-    assert_response :redirect
-    assert_redirected_to translation_file_url("config/locales/en.yml")
+    assert_response :unprocessable_entity
     assert_equal "Translation locales.german already exists.", flash[:alert]
     assert_equal translation_count_before, Moirai::Translation.count
   end

test/dummy/app/views/home/index.html.erb

@@ -10,3 +10,12 @@
 <button><%= t('buttons.very.much.nested.only_english') %></button>
 <button><%= t('buttons.very.much.nested.only_italian') %></button>
 
+<p>
+  'date.formats.short' => <%= t('date.formats.short') %>
+</p>
+<p>
+  'date.formats.default' => <%= t('date.formats.default') %>
+</p>
+<p>
+  'time.formats.default' => <%= t('time.formats.default') %>
+</p>

test/dummy/config/locales/another.de.yml

@@ -0,0 +1,4 @@
+de:
+  enumerations:
+    countries:
+      ch: Schweiz

test/dummy/config/locales/rails.en.yml

@@ -0,0 +1,4 @@
+en:
+  date:
+    formats:
+      short: "%Y"

test/models/moirai/key_finder_test.rb

@@ -4,12 +4,39 @@
 
 module Moirai
   class KeyFinderTest < ActiveSupport::TestCase
+    def setup
+      @key_finder = KeyFinder.new
+    end
     test "it finds the file path of a given key" do
-      key_finder = KeyFinder.new
-      assert_match(%r{config/locales/de.yml}, key_finder.file_path_for("locales.italian", locale: :de))
-      assert_match(%r{config/locales/de.yml}, key_finder.file_path_for("locales.italian", locale: "de"))
-      assert_match(%r{config/locales/it.yml}, key_finder.file_path_for("locales.italian", locale: :it))
-      assert_nil key_finder.file_path_for("missing.key")
+      assert_match %r{config/locales/de.yml}, @key_finder.file_paths_for("locales.italian", locale: :de).first
+      assert_match %r{config/locales/de.yml}, @key_finder.file_paths_for("locales.italian", locale: "de").first
+      assert_match %r{config/locales/it.yml}, @key_finder.file_paths_for("locales.italian", locale: :it).first
+      assert_match %r{config/locales/another.de.yml},
+        @key_finder.file_paths_for("enumerations.countries.ch", locale: :de).first
+    end
+
+    test "it finds parent keys" do
+      assert_match %r{config/locales/en.yml}, @key_finder.file_paths_for("buttons.very.much", locale: :en).first
+    end
+
+    test "it returns an empty array for an empty key" do
+      assert_empty @key_finder.file_paths_for("", locale: :en)
+    end
+
+    test "it finds keys in gems" do
+      assert_match %r{active_support/locale/en.yml}, @key_finder.file_paths_for("date.month_names", locale: :en).first
+    end
+
+    test "it returns local files before gem files" do
+      file_paths = @key_finder.file_paths_for("date.formats.short", locale: :en)
+      assert_match %r{config/locales/rails.en.yml}, file_paths.first
+      assert_match %r{active_support/locale/en.yml}, file_paths.second
+    end
+
+    test "it returns nil if there is not match" do
+      assert_empty @key_finder.file_paths_for("missing.key")
+      assert_empty @key_finder.file_paths_for("buttons.very.much.nested.only_german", locale: :en)
+      assert @key_finder.file_paths_for("buttons.very.much.nested", locale: :en)
     end
   end
 end