Merge pull request #1320 from netfighter/bugfix/5652_custom_fields_with_empty_name

app/models/custom_field.rb

@@ -55,9 +55,10 @@ class CustomField < ActiveRecord::Base
 
   alias_method_chain :translations_attributes=, :globalized
 
-  validates_presence_of :name, :field_format
+  validates_presence_of :field_format
 
   validate :uniqueness_of_name_with_scope
+
   def uniqueness_of_name_with_scope
     taken_names = CustomField.where(:type => type)
     taken_names = taken_names.where('id != ?', id) if id
@@ -66,12 +67,14 @@ class CustomField < ActiveRecord::Base
     errors.add(:name, :taken) if name.in?(taken_names)
   end
 
-  validates_length_of :name, :maximum => 30
   validates_inclusion_of :field_format, :in => Redmine::CustomFieldFormat.available_formats
 
   validate :validate_presence_of_possible_values
+
   validate :validate_default_value_in_translations
 
+  validate :validate_name
+
   def initialize(attributes = nil, options = {})
     super
     self.possible_values ||= []
@@ -106,6 +109,26 @@ class CustomField < ActiveRecord::Base
     self.is_required = required_field
   end
 
+  # check presence of name and check the length of name value
+  def validate_name
+    if self.translations.empty?
+      errors.add(:name, :blank) if self.name.nil?
+    else
+      fallback_name = self.translations.find{|el| el.name != nil}
+      self.translations.each do | translation |
+        if translation.name.nil? && fallback_name.nil?
+          errors.add(:name, :blank)
+        else
+          if ( translation.name.nil? && fallback_name.name.length > 30 ) || ( !translation.name.nil? && translation.name.length > 30 )
+            errors.add(:name, I18n.t('activerecord.errors.messages.wrong_length', :count => 30))
+          end
+
+          translation.name = fallback_name.name if translation.name.nil?
+        end
+      end
+    end
+  end
+
   def possible_values_options(obj=nil)
     case field_format
     when 'user', 'version'

features/custom_fields/edit_bool.feature

@@ -57,7 +57,7 @@ Feature: Editing a bool custom field
     And I fill in "custom_field_translations_attributes_0_name" with "Long name which forces an error"
     And I press "Save"
     Then the "custom_field_translations_attributes_0_name" field should contain "Long name which forces an error"
-    And I should see "Name is too long" within "#errorExplanation"
+    And I should see "Name is the wrong length" within "#errorExplanation"
 
   Scenario: Entering an already taken name displays an error
     Given the following issue custom fields are defined:

features/custom_fields/edit_text.feature

@@ -48,7 +48,7 @@ Feature: Editing text custom fields
     Then there should be the following localizations:
       | locale  | default_value   | name              |
       | en      | default         | My Custom Field   |
-      | de      | Standard        | nil               |
+      | de      | Standard        | My Custom Field   |
 
   @javascript
   Scenario: Changing a localization which is not present for any other attribute to a locale existing in another attribute deletes the localization completely
@@ -77,7 +77,7 @@ Feature: Editing text custom fields
     And I follow "My Custom Field"
     Then there should be the following localizations:
       | locale  | name            | default_value  |
-      | en      | nil             | default        |
+      | en      | My Custom Field | default        |
       | de      | My Custom Field | nil            |
 
 

spec/controllers/custom_fields_controller_spec.rb

@@ -46,21 +46,23 @@ describe CustomFieldsController do
     describe "WITH all ok params" do
       let(:de_name) { "Ticket Feld" }
       let(:en_name) { "Issue Field" }
-      let(:params) { { "custom_field" => { "translations_attributes" => { "0" => { "name" => de_name, "locale" => "de" }, "1" => { "name" => en_name, "locale" => "en" } } } } }
+      let(:params) { { "custom_field" => { "translations_attributes" => { "0" => { "name" => de_name, "locale" => "de" },
+                                                                          "1" => { "name" => en_name, "locale" => "en" } } } } }
 
       before do
         put :update, params
       end
 
       it { expect(response).to be_redirect }
-      it { expect(custom_field.name(:de)).to eq(de_name) }
+      it {expect(custom_field.name(:de)).to eq(de_name)}
       it { expect(custom_field.name(:en)).to eq(en_name) }
     end
 
     describe "WITH one empty name params" do
       let(:en_name) { "Issue Field" }
       let(:de_name) { "" }
-      let(:params) { { "custom_field" => { "translations_attributes" => { "0" => { "name" => de_name, "locale" => "de" }, "1" => { "name" => en_name, "locale" => "en" } } } } }
+      let(:params) { { "custom_field" => { "translations_attributes" => { "0" => { "name" => de_name, "locale" => "de" },
+                                                                          "1" => { "name" => en_name, "locale" => "en" } } } } }
 
       before do
         put :update, params
@@ -70,6 +72,7 @@ describe CustomFieldsController do
       it { expect(custom_field.name(:de)).to eq(en_name) }
       it { expect(custom_field.name(:en)).to eq(en_name) }
     end
+
   end
 
   describe "POST new" do
@@ -77,34 +80,71 @@ describe CustomFieldsController do
       Setting.available_languages = ["de", "en"]
     end
 
+    describe "WITH empty name param" do
+      let(:en_name) { "" }
+      let(:de_name) { "" }
+      let(:params) { { "type" => "WorkPackageCustomField",
+                       "custom_field" => { "translations_attributes" => { "0" => { "name" => de_name, "locale" => "de" },
+                                                                          "1" => { "name" => en_name, "locale" => "en" } } } } }
+      before do
+        post :create, params
+      end
+
+      it { expect(response).to render_template 'new' }
+      it { expect(assigns(:custom_field).errors.messages[:name].first).to eq "can't be blank" }
+      it { expect(assigns(:custom_field).translations).to be_empty }
+    end
+
     describe "WITH all ok params" do
       let(:de_name) { "Ticket Feld" }
       let(:en_name) { "Issue Field" }
       let(:params) { { "type" => "WorkPackageCustomField",
-                       "custom_field" => { "translations_attributes" => { "0" => { "name" => de_name, "locale" => "de" }, "1" => { "name" => en_name, "locale" => "en" } } } } }
+                       "custom_field" => { "translations_attributes" => { "0" => { "name" => de_name, "locale" => "de" },
+                                                                          "1" => { "name" => en_name, "locale" => "en" } } } } }
 
       before do
         post :create, params
       end
 
       it { expect(response).to be_success }
-      it { expect(assigns(:custom_field).name(:de)).to eq(de_name) }
-      it { expect(assigns(:custom_field).name(:en)).to eq(en_name) }
+      it { expect(assigns(:custom_field).translations.find{|elem| elem.locale == :de}[:name]).to eq(de_name) }
+      it { expect(assigns(:custom_field).translations.find{|elem| elem.locale == :en}[:name]).to eq(en_name) }
     end
 
     describe "WITH one empty name params" do
       let(:en_name) { "Issue Field" }
       let(:de_name) { "" }
       let(:params) { { "type" => "WorkPackageCustomField",
-                       "custom_field" => { "translations_attributes" => { "0" => { "name" => de_name, "locale" => "de" }, "1" => { "name" => en_name, "locale" => "en" } } } } }
+                       "custom_field" => { "translations_attributes" => { "0" => { "name" => de_name, "locale" => "de" },
+                                                                          "1" => { "name" => en_name, "locale" => "en" } } } } }
 
       before do
         post :create, params
       end
 
       it { expect(response).to be_success }
-      it { expect(assigns(:custom_field).name(:de)).to eq(en_name) }
-      it { expect(assigns(:custom_field).name(:en)).to eq(en_name) }
+      it { expect(assigns(:custom_field).translations.find{|elem| elem.locale == :de}).to be_nil }
+      it { expect(assigns(:custom_field).translations.find{|elem| elem.locale == :en}[:name]).to eq(en_name) }
+    end
+
+    describe "WITH different language and default_value params" do
+      let(:en_name) { "Issue Field" }
+      let(:en_default) { "EN Default Value" }
+      let(:de_name) { "" }
+      let(:de_default) { "DE Default Value" }
+      let(:params) { { "type" => "WorkPackageCustomField",
+                       "custom_field" => { "translations_attributes" =>
+                                           { "0" => { "name" => de_name, "locale" => "de", "default_value" => de_default },
+                                             "1" => { "name" => en_name, "locale" => "en", "default_value" => '' }}}}}
+      before do
+        post :create, params
+      end
+
+      it { expect(response).to be_success }
+      it { expect(assigns(:custom_field).translations.find{|elem| elem.locale == :de}[:name]).to eq(en_name) }
+      it { expect(assigns(:custom_field).translations.find{|elem| elem.locale == :en}[:name]).to eq(en_name) }
+      it { expect(assigns(:custom_field).translations.find{|elem| elem.locale == :en}[:default_value]).to be_nil }
+      it { expect(assigns(:custom_field).translations.find{|elem| elem.locale == :de}[:default_value]).to eq(de_default) }
     end
   end
 end