diff --git a/app/assets/javascripts/globalize.js.coffee b/app/assets/javascripts/globalize.js.coffee index 374d2b299..81e336ad6 100644 --- a/app/assets/javascripts/globalize.js.coffee +++ b/app/assets/javascripts/globalize.js.coffee @@ -34,10 +34,13 @@ App.Globalize = App.Globalize.disable_locale(locale) enable_locale: (locale) -> - $("#enabled_translations_" + locale).val(1) + App.Globalize.destroy_locale_field(locale).val(false) disable_locale: (locale) -> - $("#enabled_translations_" + locale).val(0) + App.Globalize.destroy_locale_field(locale).val(true) + + destroy_locale_field: (locale) -> + $(".destroy_locale[data-locale=" + locale + "]") refresh_visible_translations: -> locale = $('.js-globalize-locale-link.is-active').data("locale") diff --git a/app/controllers/admin/banners_controller.rb b/app/controllers/admin/banners_controller.rb index b17c17d1b..05656b6d7 100644 --- a/app/controllers/admin/banners_controller.rb +++ b/app/controllers/admin/banners_controller.rb @@ -38,10 +38,9 @@ class Admin::BannersController < Admin::BaseController private def banner_params - attributes = [:title, :description, :target_url, - :post_started_at, :post_ended_at, + attributes = [:target_url, :post_started_at, :post_ended_at, :background_color, :font_color, - *translation_params(Banner), + translation_params(Banner), web_section_ids: []] params.require(:banner).permit(*attributes) end diff --git a/app/controllers/concerns/translatable.rb b/app/controllers/concerns/translatable.rb index 0acc874d8..272a97e99 100644 --- a/app/controllers/concerns/translatable.rb +++ b/app/controllers/concerns/translatable.rb @@ -1,29 +1,13 @@ module Translatable extend ActiveSupport::Concern - included do - before_action :delete_translations, only: [:update] - end - private def translation_params(resource_model) - return [] unless params[:enabled_translations] - - resource_model.translated_attribute_names.product(enabled_translations).map do |attr_name, loc| - resource_model.localized_attr_name_for(attr_name, loc) - end - end - - def delete_translations - locales = resource.translated_locales - .select { |l| params.dig(:enabled_translations, l) == "0" } - - locales.each do |l| - Globalize.with_locale(l) do - resource.translation.destroy - end - end + { + translations_attributes: [:id, :_destroy, :locale] + + resource_model.translated_attribute_names + } end def enabled_translations diff --git a/app/helpers/translatable_form_helper.rb b/app/helpers/translatable_form_helper.rb index 1e468c4ce..e96ecfa28 100644 --- a/app/helpers/translatable_form_helper.rb +++ b/app/helpers/translatable_form_helper.rb @@ -1,13 +1,6 @@ module TranslatableFormHelper - def translatable_form_for(record_or_record_path, options = {}) - object = record_or_record_path.is_a?(Array) ? record_or_record_path.last : record_or_record_path - - form_for(record_or_record_path, options.merge(builder: TranslatableFormBuilder)) do |f| - - object.globalize_locales.each do |locale| - concat translation_enabled_tag(locale, enable_locale?(object, locale)) - end - + def translatable_form_for(record, options = {}) + form_for(record, options.merge(builder: TranslatableFormBuilder)) do |f| yield(f) end end @@ -39,29 +32,30 @@ module TranslatableFormHelper translatable_field(:cktext_area, method, options) end + def translatable_fields(&block) + @object.globalize_locales.map do |locale| + Globalize.with_locale(locale) do + fields_for(:translations, @object.translations.where(locale: locale).first_or_initialize) do |translations_form| + @template.concat translations_form.hidden_field( + :_destroy, + value: !@template.enable_locale?(@object, locale), + class: "destroy_locale", + data: { locale: locale }) + + @template.concat translations_form.hidden_field(:locale, value: locale) + + yield translations_form, locale + end + end + end.join.html_safe + end + private def translatable_field(field_type, method, options = {}) - @template.capture do - @object.globalize_locales.each do |locale| - Globalize.with_locale(locale) do - localized_attr_name = @object.localized_attr_name_for(method, locale) - - label_without_locale = @object.class.human_attribute_name(method) - final_options = @template.merge_translatable_field_options(options, locale) - .reverse_merge(label: label_without_locale) - - if field_type == :cktext_area - @template.concat content_tag :div, send(field_type, localized_attr_name, final_options), - class: "js-globalize-attribute", - style: @template.display_translation?(locale), - data: { locale: locale } - else - @template.concat send(field_type, localized_attr_name, final_options) - end - end - end - end + locale = options.delete(:locale) + final_options = @template.merge_translatable_field_options(options, locale) + send(field_type, method, final_options) end end end diff --git a/app/models/banner.rb b/app/models/banner.rb index 37824d01f..10399211b 100644 --- a/app/models/banner.rb +++ b/app/models/banner.rb @@ -6,10 +6,13 @@ class Banner < ActiveRecord::Base translates :title, touch: true translates :description, touch: true globalize_accessors + accepts_nested_attributes_for :translations, allow_destroy: true + + translation_class.instance_eval do + validates :title, presence: true, length: { minimum: 2 } + validates :description, presence: true + end - validates :title, presence: true, - length: { minimum: 2 } - validates :description, presence: true validates :target_url, presence: true validates :post_started_at, presence: true validates :post_ended_at, presence: true diff --git a/app/views/admin/banners/_form.html.erb b/app/views/admin/banners/_form.html.erb index 28e8a7192..9d6d971a7 100644 --- a/app/views/admin/banners/_form.html.erb +++ b/app/views/admin/banners/_form.html.erb @@ -28,13 +28,26 @@
-
- <%= f.translatable_text_field :title, - placeholder: t("admin.banners.banner.title"), - data: {js_banner_title: "js_banner_title"}, - label: t("admin.banners.banner.title") %> -
+ <%= f.translatable_fields do |translations_form, locale| %> +
+ <%= translations_form.translatable_text_field :title, + locale: locale, + placeholder: t("admin.banners.banner.title"), + data: {js_banner_title: "js_banner_title"}, + label: t("admin.banners.banner.title") %> +
+
+ <%= translations_form.translatable_text_field :description, + locale: locale, + placeholder: t("admin.banners.banner.description"), + data: {js_banner_description: "js_banner_description"}, + label: t("admin.banners.banner.description") %> +
+ <% end %> +
+ +
<%= f.label :target_url, t("admin.banners.banner.target_url") %> <%= f.text_field :target_url, @@ -43,15 +56,6 @@
-
-
- <%= f.translatable_text_field :description, - placeholder: t("admin.banners.banner.description"), - data: {js_banner_description: "js_banner_description"}, - label: t("admin.banners.banner.description") %> -
-
-
<%= f.label :sections, t("admin.banners.banner.sections_label") %> diff --git a/spec/features/admin/banners_spec.rb b/spec/features/admin/banners_spec.rb index 6eaaea486..7a0e7d76e 100644 --- a/spec/features/admin/banners_spec.rb +++ b/spec/features/admin/banners_spec.rb @@ -89,8 +89,8 @@ feature 'Admin banners magement' do click_link "Create banner" - fill_in 'banner_title_en', with: 'Such banner' - fill_in 'banner_description_en', with: 'many text wow link' + fill_in 'Title', with: 'Such banner' + fill_in 'Description', with: 'many text wow link' fill_in 'banner_target_url', with: 'https://www.url.com' last_week = Time.current - 7.days next_week = Time.current + 7.days @@ -115,7 +115,7 @@ feature 'Admin banners magement' do fill_in 'banner_background_color', with: '#850000' fill_in 'banner_font_color', with: '#ffb2b2' - fill_in 'banner_title_en', with: 'Fun with flags' + fill_in 'Title', with: 'Fun with flags' # This last step simulates the blur event on the page. The color pickers and the text_fields # has onChange events that update each one when the other changes, but this is only fired when @@ -146,8 +146,8 @@ feature 'Admin banners magement' do click_link "Edit banner" - fill_in 'banner_title_en', with: 'Modified title' - fill_in 'banner_description_en', with: 'Edited text' + fill_in 'Title', with: 'Modified title' + fill_in 'Description', with: 'Edited text' page.find("body").click diff --git a/spec/shared/features/translatable.rb b/spec/shared/features/translatable.rb index e3abf5f84..d5907cbba 100644 --- a/spec/shared/features/translatable.rb +++ b/spec/shared/features/translatable.rb @@ -16,6 +16,16 @@ shared_examples "translatable" do |factory_name, path_name, fields| end.to_h end + let(:optional_fields) do + fields.select do |field| + translatable.translations.last.dup.tap { |duplicate| duplicate.send(:"#{field}=", "") }.valid? + end + end + + let(:required_fields) do + fields - optional_fields + end + let(:translatable) { create(factory_name, attributes) } let(:path) { send(path_name, *resource_hierarchy_for(translatable)) } before { login_as(create(:administrator).user) } @@ -70,6 +80,24 @@ shared_examples "translatable" do |factory_name, path_name, fields| expect(page).to have_field(field_for(field, :es), with: updated_text) end + scenario "Update a translation with invalid data", :js do + skip("can't have invalid translations") if required_fields.empty? + + field = required_fields.sample + + visit path + click_link "Español" + + expect(page).to have_field(field_for(field, :es), with: text_for(field, :es)) + + fill_in field_for(field, :es), with: "" + click_button update_button_text + + expect(page).to have_css "#error_explanation" + + # TODO: check the field is now blank. + end + scenario "Remove a translation", :js do visit path @@ -85,13 +113,9 @@ shared_examples "translatable" do |factory_name, path_name, fields| end scenario 'Change value of a translated field to blank', :js do - possible_blanks = fields.select do |field| - translatable.dup.tap { |duplicate| duplicate.send(:"#{field}=", '') }.valid? - end + skip("can't have translatable blank fields") if optional_fields.empty? - skip("can't have translatable blank fields") if possible_blanks.empty? - - field = possible_blanks.sample + field = optional_fields.sample visit path expect(page).to have_field(field_for(field, :en), with: text_for(field, :en)) @@ -105,10 +129,12 @@ shared_examples "translatable" do |factory_name, path_name, fields| scenario "Add a translation for a locale with non-underscored name", :js do visit path - field = fields.sample select "Português brasileiro", from: "translation_locale" - fill_in field_for(field, :pt_br), with: text_for(field, :"pt-BR") + + fields.each do |field| + fill_in field_for(field, :"pt-BR"), with: text_for(field, :"pt-BR") + end click_button update_button_text @@ -116,7 +142,8 @@ shared_examples "translatable" do |factory_name, path_name, fields| select('Português brasileiro', from: 'locale-switcher') - expect(page).to have_field(field_for(field, :pt_br), with: text_for(field, :"pt-BR")) + field = fields.sample + expect(page).to have_field(field_for(field, :"pt-BR"), with: text_for(field, :"pt-BR")) end end @@ -176,7 +203,7 @@ def field_for(field, locale) if translatable_class.name == "I18nContent" "contents_content_#{translatable.key}values_#{field}_#{locale}" else - "#{translatable_class.model_name.singular}_#{field}_#{locale}" + find("[data-locale='#{locale}'][id$='#{field}']")[:id] end end