From 086e38c96956074e3291dd835e47220e56493dcf Mon Sep 17 00:00:00 2001 From: taitus Date: Fri, 27 Dec 2019 18:40:53 +0100 Subject: [PATCH 1/4] Improve display remote translation button - Do not display remote translations button when API key is not configured --- app/controllers/concerns/remotely_translatable.rb | 6 +++++- .../concerns/remotely_translatable_spec.rb | 11 +++++++++++ spec/features/remote_translations_spec.rb | 1 + spec/shared/features/remotely_translatable.rb | 1 + 4 files changed, 18 insertions(+), 1 deletion(-) diff --git a/app/controllers/concerns/remotely_translatable.rb b/app/controllers/concerns/remotely_translatable.rb index b21f6c39b..7f47f0c8f 100644 --- a/app/controllers/concerns/remotely_translatable.rb +++ b/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) @@ -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 diff --git a/spec/controllers/concerns/remotely_translatable_spec.rb b/spec/controllers/concerns/remotely_translatable_spec.rb index 149ce0c4f..8320e778c 100644 --- a/spec/controllers/concerns/remotely_translatable_spec.rb +++ b/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,6 +49,16 @@ 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) diff --git a/spec/features/remote_translations_spec.rb b/spec/features/remote_translations_spec.rb index 980920243..c144a406a 100644 --- a/spec/features/remote_translations_spec.rb +++ b/spec/features/remote_translations_spec.rb @@ -7,6 +7,7 @@ describe "Remote Translations" do available_locales_response = %w[de en es fr pt zh-Hans] expect(RemoteTranslations::Microsoft::AvailableLocales).to receive(:available_locales). 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 diff --git a/spec/shared/features/remotely_translatable.rb b/spec/shared/features/remotely_translatable.rb index 38c491e12..d7acff59f 100644 --- a/spec/shared/features/remotely_translatable.rb +++ b/spec/shared/features/remotely_translatable.rb @@ -12,6 +12,7 @@ shared_examples "remotely_translatable" do |factory_name, path_name, path_argume available_locales_response = %w[de en es fr pt zh-Hans] expect(RemoteTranslations::Microsoft::AvailableLocales).to receive(:available_locales).at_most(3).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 From d853366d38d8451ea01984c9f0927ea1097e60e6 Mon Sep 17 00:00:00 2001 From: taitus Date: Thu, 23 Jan 2020 15:55:24 +0100 Subject: [PATCH 2/4] Add RemoteTranslation validations - Validate that locale is a valid locale for RemoteTranslation Client. - RemoteTranslation can only be created for resources that do not have the requested language translated --- app/models/remote_translation.rb | 9 ++++++++- config/locales/en/activerecord.yml | 4 ++++ config/locales/es/activerecord.yml | 4 ++++ spec/features/remote_translations_spec.rb | 2 +- spec/models/remote_translation_spec.rb | 10 ++++++++++ spec/shared/features/remotely_translatable.rb | 2 +- 6 files changed, 28 insertions(+), 3 deletions(-) diff --git a/app/models/remote_translation.rb b/app/models/remote_translation.rb index e9ef75a1c..85fc77bdd 100644 --- a/app/models/remote_translation.rb +++ b/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 diff --git a/config/locales/en/activerecord.yml b/config/locales/en/activerecord.yml index aae364b39..d30a8785b 100644 --- a/config/locales/en/activerecord.yml +++ b/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}" diff --git a/config/locales/es/activerecord.yml b/config/locales/es/activerecord.yml index 5a3ed55cf..ceef5bb12 100644 --- a/config/locales/es/activerecord.yml +++ b/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}" diff --git a/spec/features/remote_translations_spec.rb b/spec/features/remote_translations_spec.rb index c144a406a..7f4e1bb92 100644 --- a/spec/features/remote_translations_spec.rb +++ b/spec/features/remote_translations_spec.rb @@ -5,7 +5,7 @@ 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 diff --git a/spec/models/remote_translation_spec.rb b/spec/models/remote_translation_spec.rb index 0c6723a0e..399952939 100644 --- a/spec/models/remote_translation_spec.rb +++ b/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) diff --git a/spec/shared/features/remotely_translatable.rb b/spec/shared/features/remotely_translatable.rb index d7acff59f..478c9a049 100644 --- a/spec/shared/features/remotely_translatable.rb +++ b/spec/shared/features/remotely_translatable.rb @@ -10,7 +10,7 @@ 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 From 2f500a6b56af58d1c6c6bf33d4459f785e6ccb09 Mon Sep 17 00:00:00 2001 From: taitus Date: Thu, 23 Jan 2020 16:39:44 +0100 Subject: [PATCH 3/4] Fix detect_remote_translations for Legislation::Proposal Legislation::Proposal is not Globalize model but use CommentableActions and try detect remote translations. Add new condition to discard Non Globalize models. This fix is necessary since the following commit was included: c1f3a4ad. --- app/controllers/concerns/remotely_translatable.rb | 2 +- spec/controllers/concerns/remotely_translatable_spec.rb | 8 ++++++++ 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/app/controllers/concerns/remotely_translatable.rb b/app/controllers/concerns/remotely_translatable.rb index 7f47f0c8f..6bbf791d1 100644 --- a/app/controllers/concerns/remotely_translatable.rb +++ b/app/controllers/concerns/remotely_translatable.rb @@ -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) diff --git a/spec/controllers/concerns/remotely_translatable_spec.rb b/spec/controllers/concerns/remotely_translatable_spec.rb index 8320e778c..d9cd7e73e 100644 --- a/spec/controllers/concerns/remotely_translatable_spec.rb +++ b/spec/controllers/concerns/remotely_translatable_spec.rb @@ -65,5 +65,13 @@ describe RemotelyTranslatable do 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 From c5c771f011e60ea14c48f190b051034b9ddc6849 Mon Sep 17 00:00:00 2001 From: taitus Date: Wed, 26 Feb 2020 12:14:05 +0100 Subject: [PATCH 4/4] Improve specs texts --- spec/features/remote_translations_spec.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/spec/features/remote_translations_spec.rb b/spec/features/remote_translations_spec.rb index 7f4e1bb92..58759584c 100644 --- a/spec/features/remote_translations_spec.rb +++ b/spec/features/remote_translations_spec.rb @@ -32,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"