From b35c8bda4b683d260f8052c1e0564fe349f58781 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Tue, 5 Oct 2021 19:05:05 +0200 Subject: [PATCH 1/3] Move form field partial to a component This way it's easier to test; changing it will also be easier. During my experiments I made a mistake which wasn't covered by the test suite. We're adding a test for this case. Note we're using `i18n_content` in the component instead of `content` because there's already a `content` method provided by ViewComponent. --- .../form_field_component.html.erb | 9 +++++++++ .../information_texts/form_field_component.rb | 19 +++++++++++++++++++ app/helpers/site_customization_helper.rb | 8 -------- .../information_texts/_form.html.erb | 2 +- .../information_texts/_form_field.html.erb | 9 --------- .../form_field_component_spec.rb | 14 ++++++++++++++ 6 files changed, 43 insertions(+), 18 deletions(-) create mode 100644 app/components/admin/site_customization/information_texts/form_field_component.html.erb create mode 100644 app/components/admin/site_customization/information_texts/form_field_component.rb delete mode 100644 app/views/admin/site_customization/information_texts/_form_field.html.erb create mode 100644 spec/components/admin/site_customization/information_texts/form_field_component_spec.rb diff --git a/app/components/admin/site_customization/information_texts/form_field_component.html.erb b/app/components/admin/site_customization/information_texts/form_field_component.html.erb new file mode 100644 index 000000000..95f9fedc3 --- /dev/null +++ b/app/components/admin/site_customization/information_texts/form_field_component.html.erb @@ -0,0 +1,9 @@ +<% globalize(locale) do %> + <%= hidden_field_tag "contents[content_#{i18n_content.key}][id]", i18n_content.key %> + <%= text_area_tag "contents[content_#{i18n_content.key}]values[value_#{locale}]", + text || t(i18n_content.key, locale: locale), + rows: 5, + class: "js-globalize-attribute", + style: site_customization_display_translation_style(locale), + data: { locale: locale } %> +<% end %> diff --git a/app/components/admin/site_customization/information_texts/form_field_component.rb b/app/components/admin/site_customization/information_texts/form_field_component.rb new file mode 100644 index 000000000..f292b1edf --- /dev/null +++ b/app/components/admin/site_customization/information_texts/form_field_component.rb @@ -0,0 +1,19 @@ +class Admin::SiteCustomization::InformationTexts::FormFieldComponent < ApplicationComponent + attr_reader :i18n_content, :locale + delegate :globalize, :site_customization_display_translation_style, to: :helpers + + def initialize(i18n_content, locale:) + @i18n_content = i18n_content + @locale = locale + end + + private + + def text + if i18n_content.present? + I18nContentTranslation.find_by(i18n_content_id: i18n_content.id, locale: locale)&.value + else + false + end + end +end diff --git a/app/helpers/site_customization_helper.rb b/app/helpers/site_customization_helper.rb index bd2824021..686b5747f 100644 --- a/app/helpers/site_customization_helper.rb +++ b/app/helpers/site_customization_helper.rb @@ -7,14 +7,6 @@ module SiteCustomizationHelper site_customization_enable_translation?(locale) ? "" : "display: none;" end - def translation_for_locale(content, locale) - if content.present? - I18nContentTranslation.find_by(i18n_content_id: content.id, locale: locale)&.value - else - false - end - end - def information_texts_tabs [:basic, :debates, :community, :proposals, :polls, :layouts, :mailers, :management, :welcome, :machine_learning] end diff --git a/app/views/admin/site_customization/information_texts/_form.html.erb b/app/views/admin/site_customization/information_texts/_form.html.erb index f31aa17f4..b7b4adf5b 100644 --- a/app/views/admin/site_customization/information_texts/_form.html.erb +++ b/app/views/admin/site_customization/information_texts/_form.html.erb @@ -8,7 +8,7 @@ <% group.each do |content| %> <%= content.key %> <% content.globalize_locales.each do |locale| %> - <%= render "form_field", content: content, locale: locale %> + <%= render Admin::SiteCustomization::InformationTexts::FormFieldComponent.new(content, locale: locale) %> <% end %> <% end %> <% end %> diff --git a/app/views/admin/site_customization/information_texts/_form_field.html.erb b/app/views/admin/site_customization/information_texts/_form_field.html.erb deleted file mode 100644 index 799a76b6e..000000000 --- a/app/views/admin/site_customization/information_texts/_form_field.html.erb +++ /dev/null @@ -1,9 +0,0 @@ -<% globalize(locale) do %> - <%= hidden_field_tag "contents[content_#{content.key}][id]", content.key %> - <%= text_area_tag "contents[content_#{content.key}]values[value_#{locale}]", - translation_for_locale(content, locale) || t(content.key, locale: locale), - rows: 5, - class: "js-globalize-attribute", - style: site_customization_display_translation_style(locale), - data: { locale: locale } %> -<% end %> diff --git a/spec/components/admin/site_customization/information_texts/form_field_component_spec.rb b/spec/components/admin/site_customization/information_texts/form_field_component_spec.rb new file mode 100644 index 000000000..40ad3cee6 --- /dev/null +++ b/spec/components/admin/site_customization/information_texts/form_field_component_spec.rb @@ -0,0 +1,14 @@ +require "rails_helper" + +describe Admin::SiteCustomization::InformationTexts::FormFieldComponent do + after { I18n.backend.reload! } + + it "uses the I18n translation when the record exists without a database translation" do + I18n.backend.store_translations(:en, { testing: "It works!" }) + content = I18nContent.create!(key: "testing") + + render_inline Admin::SiteCustomization::InformationTexts::FormFieldComponent.new(content, locale: :en) + + expect(page).to have_field with: "It works!" + end +end From c7230e668f3fde30bc8fb21e75194b8496ccb680 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Tue, 5 Oct 2021 20:05:46 +0200 Subject: [PATCH 2/3] Refactor method to get custom texts translations It was a bit confusing to return `false` in the method and then handle this case in the view. --- .../information_texts/form_field_component.html.erb | 2 +- .../information_texts/form_field_component.rb | 12 +++++++++--- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/app/components/admin/site_customization/information_texts/form_field_component.html.erb b/app/components/admin/site_customization/information_texts/form_field_component.html.erb index 95f9fedc3..dc0e18a3f 100644 --- a/app/components/admin/site_customization/information_texts/form_field_component.html.erb +++ b/app/components/admin/site_customization/information_texts/form_field_component.html.erb @@ -1,7 +1,7 @@ <% globalize(locale) do %> <%= hidden_field_tag "contents[content_#{i18n_content.key}][id]", i18n_content.key %> <%= text_area_tag "contents[content_#{i18n_content.key}]values[value_#{locale}]", - text || t(i18n_content.key, locale: locale), + text, rows: 5, class: "js-globalize-attribute", style: site_customization_display_translation_style(locale), diff --git a/app/components/admin/site_customization/information_texts/form_field_component.rb b/app/components/admin/site_customization/information_texts/form_field_component.rb index f292b1edf..8c2233504 100644 --- a/app/components/admin/site_customization/information_texts/form_field_component.rb +++ b/app/components/admin/site_customization/information_texts/form_field_component.rb @@ -10,10 +10,16 @@ class Admin::SiteCustomization::InformationTexts::FormFieldComponent < Applicati private def text + database_text || i18n_text + end + + def database_text if i18n_content.present? - I18nContentTranslation.find_by(i18n_content_id: i18n_content.id, locale: locale)&.value - else - false + i18n_content.translations.find_by(locale: locale)&.value end end + + def i18n_text + t(i18n_content.key, locale: locale) + end end From 7243ace903e4cedb7c0340d10317fe21515cc7ea Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Tue, 5 Oct 2021 23:37:20 +0200 Subject: [PATCH 3/3] Improve performance when editing custom texts We were executing SQL queries searching for translations even for records which were not persisted in the database and therefore had no translations. So we're now only looking for translations when the record is persisted. Note that, when the record isn't persisted, we're using `I18n.translate` instead of the `t` method. That's because the `t` method would also perform database queries since we overwrite the `t` method in the `i18n_translation` initializer. On tabs with many records, like the proposals tab, requests are now up to three times faster on production environments. Tests related to this part of the application were failing sometimes because requests took longer than `Capybara.default_max_wait_time`. After these changes, the custom information texts pages load about 30% faster when running the tests. --- .../information_texts/form_field_component.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/components/admin/site_customization/information_texts/form_field_component.rb b/app/components/admin/site_customization/information_texts/form_field_component.rb index 8c2233504..c5278a305 100644 --- a/app/components/admin/site_customization/information_texts/form_field_component.rb +++ b/app/components/admin/site_customization/information_texts/form_field_component.rb @@ -14,12 +14,12 @@ class Admin::SiteCustomization::InformationTexts::FormFieldComponent < Applicati end def database_text - if i18n_content.present? + if i18n_content.persisted? i18n_content.translations.find_by(locale: locale)&.value end end def i18n_text - t(i18n_content.key, locale: locale) + I18n.translate(i18n_content.key, locale: locale) end end