Merge pull request #3599 from rockandror/block-updates-without-translations

Block translatable resource updates with no translations

app/helpers/globalize_helper.rb

@@ -1,7 +1,7 @@
 module GlobalizeHelper
 
   def options_for_select_language(resource)
-    options_for_select(available_locales(resource), first_available_locale(resource))
+    options_for_select(available_locales(resource), selected_locale(resource))
   end
 
   def available_locales(resource)
@@ -13,20 +13,22 @@ module GlobalizeHelper
   def enabled_locale?(resource, locale)
     return site_customization_enable_translation?(locale) if resource.blank?
 
-    if resource.translations.empty?
-      locale == I18n.locale
-    else
+    if resource.locales_not_marked_for_destruction.any?
       resource.locales_not_marked_for_destruction.include?(locale)
+    elsif resource.locales_persisted_and_marked_for_destruction.any?
+      locale == first_marked_for_destruction_translation(resource)
+    else
+      locale == I18n.locale
     end
   end
 
-  def first_available_locale(resource)
+  def selected_locale(resource)
     return first_i18n_content_translation_locale if resource.blank?
 
-    if translations_for_locale?(resource, I18n.locale)
-      I18n.locale
-    elsif resource.translations.any?
-      resource.translations.first.locale
+    if resource.locales_not_marked_for_destruction.any?
+      first_translation(resource)
+    elsif resource.locales_persisted_and_marked_for_destruction.any?
+      first_marked_for_destruction_translation(resource)
     else
       I18n.locale
     end
@@ -41,18 +43,44 @@ module GlobalizeHelper
     end
   end
 
-  def translations_for_locale?(resource, locale)
-    resource.present? && resource.translations.any? &&
-      resource.locales_not_marked_for_destruction.include?(locale)
+  def first_translation(resource)
+    if resource.locales_not_marked_for_destruction.include? I18n.locale
+      I18n.locale
+    else
+      resource.locales_not_marked_for_destruction.first
+    end
+  end
+
+  def first_marked_for_destruction_translation(resource)
+    if resource.locales_persisted_and_marked_for_destruction.include? I18n.locale
+      I18n.locale
+    else
+      resource.locales_persisted_and_marked_for_destruction.first
+    end
+  end
+
+  def translations_for_locale?(resource)
+    resource.locales_not_marked_for_destruction.any?
   end
 
   def selected_languages_description(resource)
     t("shared.translations.languages_in_use_html", count: active_languages_count(resource))
   end
 
+  def select_language_error(resource)
+    return if resource.blank?
+
+    current_translation = resource.translation_for(selected_locale(resource))
+    if current_translation.errors.added? :base, :translations_too_short
+      content_tag :div, class: "small error" do
+        current_translation.errors[:base].join(", ")
+      end
+    end
+  end
+
   def active_languages_count(resource)
     if resource.blank?
-      languages_count
+      no_resource_languages_count
     elsif resource.locales_not_marked_for_destruction.size > 0
       resource.locales_not_marked_for_destruction.size
     else
@@ -60,7 +88,7 @@ module GlobalizeHelper
     end
   end
 
-  def languages_count
+  def no_resource_languages_count
     count = I18nContentTranslation.existing_languages.count
     count > 0 ? count : 1
   end
@@ -70,11 +98,14 @@ module GlobalizeHelper
   end
 
   def display_translation?(resource, locale)
-    if !resource || resource.translations.empty? ||
-       resource.locales_not_marked_for_destruction.include?(I18n.locale)
-      locale == I18n.locale
+    return locale == I18n.locale if resource.blank?
+
+    if resource.locales_not_marked_for_destruction.any?
+      locale == first_translation(resource)
+    elsif resource.locales_persisted_and_marked_for_destruction.any?
+      locale == first_marked_for_destruction_translation(resource)
     else
-      locale == resource.translations.first.locale
+      locale == I18n.locale
     end
   end
 
@@ -83,7 +114,7 @@ module GlobalizeHelper
   end
 
   def display_destroy_locale_link?(resource, locale)
-    first_available_locale(resource) == locale
+    selected_locale(resource) == locale
   end
 
   def options_for_add_language

app/helpers/translatable_form_helper.rb

@@ -38,6 +38,7 @@ module TranslatableFormHelper
           @template.content_tag :div, translations_options(translations_form.object, locale) do
             @template.concat translations_form.hidden_field(
               :_destroy,
+              value: !@template.enabled_locale?(translations_form.object.globalized_model, locale),
               data: { locale: locale }
             )
 
@@ -64,9 +65,7 @@ module TranslatableFormHelper
 
       def new_translation_for(locale)
         @object.translations.new(locale: locale).tap do |translation|
-          unless locale == I18n.locale && no_other_translations?(translation)
-            translation.mark_for_destruction
-          end
+          translation.mark_for_destruction
         end
       end
 

app/models/concerns/globalizable.rb

@@ -1,18 +1,34 @@
 module Globalizable
+  MIN_TRANSLATIONS = 1
   extend ActiveSupport::Concern
 
   included do
     globalize_accessors
     accepts_nested_attributes_for :translations, allow_destroy: true
 
-    def locales_not_marked_for_destruction
-      translations.reject(&:_destroy).map(&:locale)
-    end
+    validate :check_translations_number, on: :update, if: :translations_required?
+    after_validation :copy_error_to_current_translation, on: :update
 
     def description
       self.read_attribute(:description).try :html_safe
     end
 
+    def locales_not_marked_for_destruction
+      translations.reject(&:marked_for_destruction?).map(&:locale)
+    end
+
+    def locales_marked_for_destruction
+      I18n.available_locales - locales_not_marked_for_destruction
+    end
+
+    def locales_persisted_and_marked_for_destruction
+      translations.select{|t| t.persisted? && t.marked_for_destruction? }.map(&:locale)
+    end
+
+    def translations_required?
+      translated_attribute_names.any?{|attr| required_attribute?(attr)}
+    end
+
     if self.paranoid? && translation_class.attribute_names.include?("hidden_at")
       translation_class.send :acts_as_paranoid, column: :hidden_at
     end
@@ -21,6 +37,38 @@ module Globalizable
 
     private
 
+      def required_attribute?(attribute)
+        presence_validators = [ActiveModel::Validations::PresenceValidator,
+          ActiveRecord::Validations::PresenceValidator]
+
+        attribute_validators(attribute).any?{|validator| presence_validators.include? validator }
+      end
+
+      def attribute_validators(attribute)
+        self.class.validators_on(attribute).map(&:class)
+      end
+
+      def check_translations_number
+        errors.add(:base, :translations_too_short) unless traslations_count_valid?
+      end
+
+      def traslations_count_valid?
+        translations.reject(&:marked_for_destruction?).count >= MIN_TRANSLATIONS
+      end
+
+      def copy_error_to_current_translation
+        return unless errors.added?(:base, :translations_too_short)
+
+        if locales_persisted_and_marked_for_destruction.include?(I18n.locale)
+          locale = I18n.locale
+        else
+          locale = locales_persisted_and_marked_for_destruction.first
+        end
+
+        translation = translation_for(locale)
+        translation.errors.add(:base, :translations_too_short)
+      end
+
       def searchable_globalized_values
         values = {}
         translations.each do |translation|

app/views/shared/_common_globalize_locales.html.erb

@@ -10,6 +10,7 @@
                    options_for_select_language(resource),
                    prompt: t("shared.translations.select_language_prompt"),
                    class: "js-select-language" %>
+    <%= select_language_error(resource) %>
     <div class="margin-bottom">
       <% if manage_languages %>
         <% I18n.available_locales.each do |locale| %>

config/locales/en/activerecord.yml

@@ -448,6 +448,7 @@ en:
             valuation:
               cannot_comment_valuation: "You cannot comment a valuation"
       messages:
+        translations_too_short: Is mandatory to provide one translation at least
         record_invalid: "Validation failed: %{errors}"
         another_poll_active: There is another poll active for the given period
         restrict_dependent_destroy:

config/locales/es/activerecord.yml

@@ -450,6 +450,7 @@ es:
             valuation:
               cannot_comment_valuation: "No puedes comentar una evaluación"
       messages:
+        translations_too_short: El obligatorio proporcionar una traducción como mínimo
         record_invalid: "Error de validación: %{errors}"
         another_poll_active: Hay otra encuesta activa para este periodo.
         restrict_dependent_destroy:

spec/shared/features/edit_translatable.rb

@@ -236,6 +236,19 @@ shared_examples "edit_translatable" do |factory_name, path_name, input_fields, t
       expect_not_to_have_language "Español"
     end
 
+    scenario "Remove all translations should show an error message", :js do
+      skip("can't have invalid translations") if required_fields.empty?
+
+      visit path
+
+      click_link "Remove language"
+      click_link "Remove language"
+
+      click_button update_button_text
+
+      expect(page).to have_content "Is mandatory to provide one translation at least"
+    end
+
     scenario "Remove a translation with invalid data", :js do
       skip("can't have invalid translations") if required_fields.empty?