Merge pull request #3917 from rockandror/improve-remote-translations

Improve remote translations

app/controllers/concerns/remotely_translatable.rb

@@ -2,7 +2,7 @@ module RemotelyTranslatable
   private
 
     def detect_remote_translations(*args)
-      return [] unless Setting["feature.remote_translations"].present?
+      return [] unless Setting["feature.remote_translations"].present? && api_key_has_been_set_in_secrets?
 
       resources_groups(*args).flatten.select { |resource| translation_empty?(resource) }.map do |resource|
         remote_translation_for(resource)
@@ -16,7 +16,7 @@ module RemotelyTranslatable
     end
 
     def translation_empty?(resource)
-      resource.translations.where(locale: I18n.locale).empty?
+      resource.class.translates? && resource.translations.where(locale: I18n.locale).empty?
     end
 
     def resources_groups(*args)
@@ -24,4 +24,8 @@ module RemotelyTranslatable
 
       args.compact - [feeds] + feeds.map(&:items)
     end
+
+    def api_key_has_been_set_in_secrets?
+      Rails.application.secrets.microsoft_api_key.present?
+    end
 end

app/models/remote_translation.rb

@@ -4,7 +4,8 @@ class RemoteTranslation < ApplicationRecord
   validates :remote_translatable_id, presence: true
   validates :remote_translatable_type, presence: true
   validates :locale, presence: true
-
+  validates :locale, inclusion: { in: RemoteTranslations::Microsoft::AvailableLocales.available_locales }
+  validate :already_translated_resource
   after_create :enqueue_remote_translation
 
   def enqueue_remote_translation
@@ -17,4 +18,10 @@ class RemoteTranslation < ApplicationRecord
           locale: remote_translation["locale"],
           error_message: nil).any?
   end
+
+  def already_translated_resource
+    if remote_translatable&.translations&.where(locale: locale).present?
+      errors.add(:locale, :already_translated)
+    end
+  end
 end

config/locales/en/activerecord.yml

@@ -548,6 +548,10 @@ en:
           attributes:
             valuation:
               cannot_comment_valuation: "You cannot comment a valuation"
+        remote_translation:
+          attributes:
+            locale:
+              already_translated: Already translated resource
       messages:
         translations_too_short: Is mandatory to provide one translation at least
         record_invalid: "Validation failed: %{errors}"

config/locales/es/activerecord.yml

@@ -550,6 +550,10 @@ es:
           attributes:
             valuation:
               cannot_comment_valuation: "No puedes comentar una evaluación"
+        remote_translation:
+          attributes:
+            locale:
+              already_translated: Recurso ya traducido
       messages:
         translations_too_short: El obligatorio proporcionar una traducción como mínimo
         record_invalid: "Error de validación: %{errors}"

spec/controllers/concerns/remotely_translatable_spec.rb

@@ -4,6 +4,7 @@ include RemotelyTranslatable
 describe RemotelyTranslatable do
   before do
     Setting["feature.remote_translations"] = true
+    allow(Rails.application.secrets).to receive(:microsoft_api_key).and_return("123")
   end
 
   describe "#detect_remote_translations" do
@@ -48,11 +49,29 @@ describe RemotelyTranslatable do
       end
     end
 
+    it "When api key has not been set in secrets should not detect remote_translations" do
+      allow(Rails.application.secrets).to receive(:microsoft_api_key).and_return(nil)
+      proposal = create(:proposal)
+      comment = create(:comment, commentable: proposal)
+
+      I18n.with_locale(:es) do
+        expect(detect_remote_translations([proposal, comment])).to eq []
+      end
+    end
+
     it "When defined in current locale should not detect remote_translations" do
       proposal = create(:proposal)
       comment = create(:comment, commentable: proposal)
 
       expect(detect_remote_translations([proposal, comment])).to eq []
     end
+
+    it "When resource class is not translatable should not detect remote_translations" do
+      legislation_proposal = create(:legislation_proposal)
+
+      I18n.with_locale(:es) do
+        expect(detect_remote_translations([legislation_proposal])).to eq []
+      end
+    end
   end
 end

spec/features/remote_translations_spec.rb

@@ -5,8 +5,9 @@ describe "Remote Translations" do
     Setting["feature.remote_translations"] = true
     create(:proposal)
     available_locales_response = %w[de en es fr pt zh-Hans]
-    expect(RemoteTranslations::Microsoft::AvailableLocales).to receive(:available_locales).
+    expect(RemoteTranslations::Microsoft::AvailableLocales).to receive(:available_locales).at_most(2).times.
                                                             and_return(available_locales_response)
+    allow(Rails.application.secrets).to receive(:microsoft_api_key).and_return("123")
   end
 
   describe "Display remote translation button when locale is included in microsoft translate client" do
@@ -31,20 +32,20 @@ describe "Remote Translations" do
       end
     end
 
-    context "with locale that has :en fallback" do
+    context "with locale that has :es fallback" do
       before do
         allow(I18n.fallbacks).to receive(:[]).and_return([:es])
         Globalize.set_fallbacks_to_all_available_locales
       end
 
-      scenario "with locale that has :es fallback" do
+      scenario "should display text in Spanish" do
         visit root_path(locale: :fr)
 
         expect(page).to have_css ".remote-translations-button"
         expect(page).to have_content "El contenido de esta página no está disponible en tu idioma"
       end
 
-      scenario "with locale that has :es fallback and need parse key" do
+      scenario "should display text in Spanish after parse key" do
         visit root_path(locale: :"pt-BR")
 
         expect(page).to have_css ".remote-translations-button"

spec/models/remote_translation_spec.rb

@@ -27,6 +27,16 @@ describe RemoteTranslation do
     expect(remote_translation).not_to be_valid
   end
 
+  it "is not valid without an available_locales" do
+    remote_translation.locale = "unavailable_locale"
+    expect(remote_translation).not_to be_valid
+  end
+
+  it "is not valid when exists a translation for locale" do
+    remote_translation.locale = :en
+    expect(remote_translation).not_to be_valid
+  end
+
   describe "#enqueue_remote_translation", :delay_jobs do
     it "after create enqueue Delayed Job" do
       expect { remote_translation.save }.to change { Delayed::Job.count }.by(1)

spec/shared/features/remotely_translatable.rb

@@ -10,8 +10,9 @@ shared_examples "remotely_translatable" do |factory_name, path_name, path_argume
   before do
     Setting["feature.remote_translations"] = true
     available_locales_response = %w[de en es fr pt zh-Hans]
-    expect(RemoteTranslations::Microsoft::AvailableLocales).to receive(:available_locales).at_most(3).times.
+    expect(RemoteTranslations::Microsoft::AvailableLocales).to receive(:available_locales).at_most(4).times.
                                                             and_return(available_locales_response)
+    allow(Rails.application.secrets).to receive(:microsoft_api_key).and_return("123")
   end
 
   context "Button to request remote translation" do