Merge pull request #2886 from javierm/backport-change-translatable-implementation

[Backport] Change Translatable implementation to accommodate new requirements

app/assets/javascripts/globalize.js.coffee

@@ -1,6 +1,7 @@
 App.Globalize =
 
   display_locale: (locale) ->
+    App.Globalize.enable_locale(locale)
     $(".js-globalize-locale-link").each ->
       if $(this).data("locale") == locale
         $(this).show()
@@ -27,7 +28,13 @@ App.Globalize =
     next = $(".js-globalize-locale-link:visible").first()
     App.Globalize.highlight_locale(next)
     App.Globalize.display_translations(next.data("locale"))
-    $("#delete_translations_" + locale).val(1)
+    App.Globalize.disable_locale(locale)
+
+  enable_locale: (locale) ->
+    $("#enabled_translations_" + locale).val(1)
+
+  disable_locale: (locale) ->
+    $("#enabled_translations_" + locale).val(0)
 
   initialize: ->
     $('.js-globalize-locale').on 'change', ->

app/controllers/admin/banners_controller.rb

@@ -41,8 +41,9 @@ class Admin::BannersController < Admin::BaseController
       attributes = [:title, :description, :target_url,
                     :post_started_at, :post_ended_at,
                     :background_color, :font_color,
+                    *translation_params(Banner),
                     web_section_ids: []]
-      params.require(:banner).permit(*attributes, *translation_params(params[:banner]))
+      params.require(:banner).permit(*attributes)
     end
 
     def banner_styles
@@ -65,8 +66,4 @@ class Admin::BannersController < Admin::BaseController
       @banner = Banner.find(params[:id]) unless @banner
       @banner
     end
-
-    def resource_model
-      Banner
-    end
 end

app/controllers/admin/budget_investment_milestones_controller.rb

@@ -47,9 +47,10 @@ class Admin::BudgetInvestmentMilestonesController < Admin::BaseController
     image_attributes = [:id, :title, :attachment, :cached_attachment, :user_id, :_destroy]
     documents_attributes = [:id, :title, :attachment, :cached_attachment, :user_id, :_destroy]
     attributes = [:title, :description, :publication_date, :budget_investment_id, :status_id,
+                  *translation_params(Budget::Investment::Milestone),
                   image_attributes: image_attributes, documents_attributes: documents_attributes]
 
-    params.require(:budget_investment_milestone).permit(*attributes, translation_params(params[:budget_investment_milestone]))
+    params.require(:budget_investment_milestone).permit(*attributes)
   end
 
   def load_budget_investment
@@ -64,10 +65,6 @@ class Admin::BudgetInvestmentMilestonesController < Admin::BaseController
     Budget::Investment::Milestone.find(params[:id])
   end
 
-  def resource_model
-    Budget::Investment::Milestone
-  end
-
   def resource
     get_milestone
   end

app/controllers/admin/site_customization/information_texts_controller.rb

@@ -9,7 +9,7 @@ class Admin::SiteCustomization::InformationTextsController < Admin::SiteCustomiz
 
   def update
     content_params.each do |content|
-      values = content[:values].slice(*translation_params(content[:values]))
+      values = content[:values].slice(*translation_params(I18nContent))
 
       unless values.empty?
         values.each do |key, value|
@@ -33,17 +33,8 @@ class Admin::SiteCustomization::InformationTextsController < Admin::SiteCustomiz
 
   private
 
-    def i18n_content_params
-      attributes = [:key, :value]
-      params.require(:information_texts).permit(*attributes, translation_params(params[:information_texts]))
-    end
-
-    def resource_model
-      I18nContent
-    end
-
     def resource
-      resource_model.find(content_params[:id])
+      I18nContent.find(content_params[:id])
     end
 
     def content_params
@@ -51,7 +42,8 @@ class Admin::SiteCustomization::InformationTextsController < Admin::SiteCustomiz
     end
 
     def delete_translations
-      languages_to_delete = params[:delete_translations].select { |k, v| params[:delete_translations][k] == '1' }.keys
+      languages_to_delete = params[:enabled_translations].select { |_, v| v == '0' }
+                                                         .keys
       languages_to_delete.each do |locale|
         I18nContentTranslation.destroy_all(locale: locale)
       end

app/controllers/concerns/translatable.rb

@@ -7,13 +7,16 @@ module Translatable
 
   private
 
-    def translation_params(params)
-      resource_model.globalize_attribute_names.select { |k, v| params.include?(k.to_sym) && params[k].present? }
+    def translation_params(resource_model)
+      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_model.globalize_locales.
-      select { |k, v| params[:delete_translations].include?(k.to_sym) && params[:delete_translations][k] == "1" }
+      locales = resource.translated_locales
+                        .select { |l| params.dig(:enabled_translations, l) == "0" }
+
       locales.each do |l|
         Globalize.with_locale(l) do
           resource.translation.destroy
@@ -21,4 +24,9 @@ module Translatable
       end
     end
 
+    def enabled_translations
+      params.fetch(:enabled_translations)
+            .select { |_, v| v == '1' }
+            .keys
+    end
 end

app/helpers/globalize_helper.rb

@@ -14,8 +14,24 @@ module GlobalizeHelper
     same_locale?(neutral_locale(I18n.locale), neutral_locale(locale)) ? "" : "display: none"
   end
 
+  def render_translations_to_delete(resource)
+    capture do
+      resource.globalize_locales.each do |locale|
+        concat translation_enabled_tag(locale, enable_locale?(resource, locale))
+      end
+    end
+  end
+
+  def translation_enabled_tag(locale, enabled)
+    hidden_field_tag("enabled_translations[#{locale}]", (enabled ? 1 : 0))
+  end
+
   def css_to_display_translation?(resource, locale)
-    resource.translated_locales.include?(neutral_locale(locale)) || locale == I18n.locale ? "" : "display: none"
+    enable_locale?(resource, locale) ? "" : "display: none"
+  end
+
+  def enable_locale?(resource, locale)
+    resource.translated_locales.include?(neutral_locale(locale)) || locale == I18n.locale
   end
 
   def highlight_current?(locale)

app/helpers/site_customization_helper.rb

@@ -1,5 +1,9 @@
 module SiteCustomizationHelper
+  def site_customization_enable_translation?(locale)
+    I18nContentTranslation.existing_languages.include?(neutral_locale(locale)) || locale == I18n.locale
+  end
+
   def site_customization_display_translation?(locale)
-    I18nContentTranslation.existing_languages.include?(neutral_locale(locale)) || locale == I18n.locale ? "" : "display: none"
+    site_customization_enable_translation?(locale) ? "" : "display: none"
   end
 end

app/views/admin/banners/_form.html.erb

@@ -4,9 +4,7 @@
 
   <%= render 'errors' %>
 
-  <% @banner.globalize_locales.each do |locale| %>
-    <%= hidden_field_tag "delete_translations[#{locale}]", 0 %>
-  <% end %>
+  <%= render_translations_to_delete(@banner) %>
 
   <div class="row">
     <% date_started_at = @banner.post_started_at.present? ? I18n.localize(@banner.post_started_at) : "" %>

app/views/admin/budget_investment_milestones/_form.html.erb

@@ -2,6 +2,8 @@
 
 <%= form_for [:admin, @investment.budget, @investment, @milestone] do |f| %>
 
+  <%= render_translations_to_delete(@milestone) %>
+
   <%= f.hidden_field :title, value: l(Time.current, format: :datetime),
                              maxlength: Budget::Investment::Milestone.title_max_length %>
 
@@ -16,7 +18,6 @@
 
   <%= f.label :description, t("admin.milestones.new.description") %>
   <% @milestone.globalize_locales.each do |locale| %>
-    <%= hidden_field_tag "delete_translations[#{locale}]", 0 %>
     <% globalize(locale) do %>
       <%= f.text_area "description_#{locale}", rows: 5,
                       class: "js-globalize-attribute",

app/views/admin/site_customization/information_texts/_form.html.erb

@@ -2,7 +2,7 @@
 
 <%= form_tag admin_site_customization_information_texts_path do %>
   <% I18n.available_locales.each do |l| %>
-    <%= hidden_field_tag "delete_translations[#{l}]", 0 %>
+    <%= translation_enabled_tag l, site_customization_enable_translation?(l) %>
   <% end %>
   <% contents.each do |group| %>
     <% group.each do |content| %>

spec/features/translations_spec.rb

@@ -71,6 +71,18 @@ feature "Translations" do
       expect(page).not_to have_link "Español"
     end
 
+    scenario 'Change value of a translated field to blank' do
+      visit @edit_milestone_url
+
+      fill_in 'budget_investment_milestone_description_en', with: ''
+
+      click_button "Update milestone"
+      expect(page).to have_content "Milestone updated successfully"
+
+      expect(page).to have_content "Milestone updated successfully"
+      expect(page).not_to have_content "Description in English"
+    end
+
     context "Globalize javascript interface" do
 
       scenario "Highlight current locale", :js do