From bc47d84a1eceefe570693cb86be3d7ead959272e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Fri, 6 Aug 2021 17:09:29 +0200 Subject: [PATCH 1/2] Extract method do update I18n contents This way we can test it properly, which will be helpful when fixing bugs. --- .../information_texts_controller.rb | 25 +---------------- app/models/i18n_content.rb | 27 +++++++++++++++++++ spec/models/i18n_content_spec.rb | 22 +++++++++++++++ 3 files changed, 50 insertions(+), 24 deletions(-) diff --git a/app/controllers/admin/site_customization/information_texts_controller.rb b/app/controllers/admin/site_customization/information_texts_controller.rb index 22f36ca15..4a8591b24 100644 --- a/app/controllers/admin/site_customization/information_texts_controller.rb +++ b/app/controllers/admin/site_customization/information_texts_controller.rb @@ -7,24 +7,7 @@ class Admin::SiteCustomization::InformationTextsController < Admin::SiteCustomiz end def update - content_params.each do |content| - values = content[:values].slice(*translation_params) - - unless values.empty? - values.each do |key, value| - locale = key.split("_").last - - if value == t(content[:id], locale: locale) || value.match(/translation missing/) - next - else - text = I18nContent.find_or_create_by!(key: content[:id]) - Globalize.with_locale(locale) do - text.update!(value: value) - end - end - end - end - end + I18nContent.update(content_params, enabled_translations) redirect_to admin_site_customization_information_texts_path, notice: t("flash.actions.update.translation") @@ -48,12 +31,6 @@ class Admin::SiteCustomization::InformationTextsController < Admin::SiteCustomiz end end - def translation_params - I18nContent.translated_attribute_names.product(enabled_translations).map do |attr_name, loc| - I18nContent.localized_attr_name_for(attr_name, loc) - end - end - def enabled_translations params.fetch(:enabled_translations, {}).select { |_, v| v == "1" }.keys end diff --git a/app/models/i18n_content.rb b/app/models/i18n_content.rb index f474aa700..6029ac306 100644 --- a/app/models/i18n_content.rb +++ b/app/models/i18n_content.rb @@ -118,4 +118,31 @@ class I18nContent < ApplicationRecord end.to_h end end + + def self.update(contents, enabled_translations = I18n.available_locales) + contents.each do |content| + values = content[:values].slice(*translation_params(enabled_translations)) + + unless values.empty? + values.each do |key, value| + locale = key.split("_").last + + if value == I18n.t(content[:id], locale: locale) || value.match(/translation missing/) + next + else + text = I18nContent.find_or_create_by!(key: content[:id]) + Globalize.with_locale(locale) do + text.update!(value: value) + end + end + end + end + end + end + + def self.translation_params(enabled_translations) + translated_attribute_names.product(enabled_translations).map do |attr_name, loc| + localized_attr_name_for(attr_name, loc) + end + end end diff --git a/spec/models/i18n_content_spec.rb b/spec/models/i18n_content_spec.rb index 99b115440..dd765c5bd 100644 --- a/spec/models/i18n_content_spec.rb +++ b/spec/models/i18n_content_spec.rb @@ -133,4 +133,26 @@ RSpec.describe I18nContent, type: :model do expect(I18nContent.translations_hash(:en)["great"]).to be nil end end + + describe ".update" do + it "stores new keys with a different translation" do + I18nContent.update([{ id: "shared.yes", values: { "value_en" => "Oh, yeah" }}]) + + expect(I18nContent.count).to eq 1 + expect(I18nContent.first.translations.count).to eq 1 + expect(I18nContent.first.value).to eq "Oh, yeah" + end + + it "does not store new keys with the default translation" do + I18nContent.update([{ id: "shared.yes", values: { "value_en" => "Yes" }}]) + + expect(I18nContent.all).to be_empty + end + + it "does not store new keys for disabled translations" do + I18nContent.update([{ id: "shared.yes", values: { "value_en" => "Oh, yeah" }}], [:es]) + + expect(I18nContent.all).to be_empty + end + end end From f0d0f1f62320061c6e5dee6d8426c1867ce70297 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Fri, 6 Aug 2021 17:52:45 +0200 Subject: [PATCH 2/2] Fix updating a translation to its original value Users were unable to reset a translation to its original value after updating it because we weren't storing anything in the database in that case. I've considered deleting the existing translation when this happens. I'm not sure about which approach is the better one, so I'm using the less destructive one. --- app/models/i18n_content.rb | 6 +++++- spec/models/i18n_content_spec.rb | 10 ++++++++++ 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/app/models/i18n_content.rb b/app/models/i18n_content.rb index 6029ac306..de4edc172 100644 --- a/app/models/i18n_content.rb +++ b/app/models/i18n_content.rb @@ -127,8 +127,12 @@ class I18nContent < ApplicationRecord values.each do |key, value| locale = key.split("_").last - if value == I18n.t(content[:id], locale: locale) || value.match(/translation missing/) + if value.match(/translation missing/) next + elsif value == I18n.t(content[:id], locale: locale) + Globalize.with_locale(locale) do + I18nContent.find_by(key: content[:id])&.update!(value: value) + end else text = I18nContent.find_or_create_by!(key: content[:id]) Globalize.with_locale(locale) do diff --git a/spec/models/i18n_content_spec.rb b/spec/models/i18n_content_spec.rb index dd765c5bd..5d2c085c1 100644 --- a/spec/models/i18n_content_spec.rb +++ b/spec/models/i18n_content_spec.rb @@ -149,6 +149,16 @@ RSpec.describe I18nContent, type: :model do expect(I18nContent.all).to be_empty end + it "updates existing keys with the default translation" do + I18nContent.create!(key: "shared.yes", value_en: "Oh, yeah") + + I18nContent.update([{ id: "shared.yes", values: { "value_en" => "Yes" }}]) + + expect(I18nContent.count).to eq 1 + expect(I18nContent.first.translations.count).to eq 1 + expect(I18nContent.first.value).to eq "Yes" + end + it "does not store new keys for disabled translations" do I18nContent.update([{ id: "shared.yes", values: { "value_en" => "Oh, yeah" }}], [:es])