Fixed that non-empty blank strings as custom field values are not properly validated (#16169).

git-svn-id: https://svn.redmine.org/redmine/trunk@12938 e93f8b46-1217-0410-a6f0-8f06a7374b81

app/models/custom_field.rb

@@ -241,9 +241,7 @@ class CustomField < ActiveRecord::Base
         errs << ::I18n.t('activerecord.errors.messages.blank')
       end
     end
-    if custom_value.value.present?
-      errs += format.validate_custom_value(custom_value)
-    end
+    errs += format.validate_custom_value(custom_value)
     errs
   end
 

lib/redmine/field_format.rb

@@ -131,7 +131,8 @@ module Redmine
       # Returns the validation error messages for custom_value
       # Should return an empty array if custom_value is valid
       def validate_custom_value(custom_value)
-        errors = Array.wrap(custom_value.value).reject(&:blank?).map do |value|
+        values = Array.wrap(custom_value.value).reject {|value| value.to_s == ''}
+        errors = values.map do |value|
           validate_single_value(custom_value.custom_field, value, custom_value.customized)
         end
         errors.flatten.uniq
@@ -252,16 +253,15 @@ module Redmine
     class Unbounded < Base
       def validate_single_value(custom_field, value, customized=nil)
         errs = super
-        if value.present?
-          unless custom_field.regexp.blank? or value =~ Regexp.new(custom_field.regexp)
-            errs << ::I18n.t('activerecord.errors.messages.invalid')
-          end
-          if custom_field.min_length && value.length < custom_field.min_length
-            errs << ::I18n.t('activerecord.errors.messages.too_short', :count => custom_field.min_length)
-          end
-          if custom_field.max_length && custom_field.max_length > 0 && value.length > custom_field.max_length
-            errs << ::I18n.t('activerecord.errors.messages.too_long', :count => custom_field.max_length)
-          end
+        value = value.to_s
+        unless custom_field.regexp.blank? or value =~ Regexp.new(custom_field.regexp)
+          errs << ::I18n.t('activerecord.errors.messages.invalid')
+        end
+        if custom_field.min_length && value.length < custom_field.min_length
+          errs << ::I18n.t('activerecord.errors.messages.too_short', :count => custom_field.min_length)
+        end
+        if custom_field.max_length && custom_field.max_length > 0 && value.length > custom_field.max_length
+          errs << ::I18n.t('activerecord.errors.messages.too_long', :count => custom_field.max_length)
         end
         errs
       end
@@ -528,8 +528,9 @@ module Redmine
       end
 
       def validate_custom_value(custom_value)
-        invalid_values = Array.wrap(custom_value.value) - Array.wrap(custom_value.value_was) - custom_value.custom_field.possible_values
-        if invalid_values.select(&:present?).any?
+        values = Array.wrap(custom_value.value).reject {|value| value.to_s == ''}
+        invalid_values = values - Array.wrap(custom_value.value_was) - custom_value.custom_field.possible_values
+        if invalid_values.any?
           [::I18n.t('activerecord.errors.messages.inclusion')]
         else
           []

test/unit/custom_field_test.rb

@@ -146,6 +146,7 @@ class CustomFieldTest < ActiveSupport::TestCase
 
     assert f.valid_field_value?(nil)
     assert f.valid_field_value?('')
+    assert !f.valid_field_value?(' ')
     assert f.valid_field_value?('a' * 2)
     assert !f.valid_field_value?('a')
     assert !f.valid_field_value?('a' * 6)
@@ -156,6 +157,7 @@ class CustomFieldTest < ActiveSupport::TestCase
 
     assert f.valid_field_value?(nil)
     assert f.valid_field_value?('')
+    assert !f.valid_field_value?(' ')
     assert f.valid_field_value?('ABC')
     assert !f.valid_field_value?('abc')
   end
@@ -165,6 +167,7 @@ class CustomFieldTest < ActiveSupport::TestCase
 
     assert f.valid_field_value?(nil)
     assert f.valid_field_value?('')
+    assert !f.valid_field_value?(' ')
     assert f.valid_field_value?('1975-07-14')
     assert !f.valid_field_value?('1975-07-33')
     assert !f.valid_field_value?('abc')
@@ -175,6 +178,7 @@ class CustomFieldTest < ActiveSupport::TestCase
 
     assert f.valid_field_value?(nil)
     assert f.valid_field_value?('')
+    assert !f.valid_field_value?(' ')
     assert f.valid_field_value?('value2')
     assert !f.valid_field_value?('abc')
   end
@@ -184,6 +188,7 @@ class CustomFieldTest < ActiveSupport::TestCase
 
     assert f.valid_field_value?(nil)
     assert f.valid_field_value?('')
+    assert !f.valid_field_value?(' ')
     assert f.valid_field_value?('123')
     assert f.valid_field_value?('+123')
     assert f.valid_field_value?('-123')
@@ -195,6 +200,7 @@ class CustomFieldTest < ActiveSupport::TestCase
 
     assert f.valid_field_value?(nil)
     assert f.valid_field_value?('')
+    assert !f.valid_field_value?(' ')
     assert f.valid_field_value?('11.2')
     assert f.valid_field_value?('-6.250')
     assert f.valid_field_value?('5')
@@ -206,9 +212,11 @@ class CustomFieldTest < ActiveSupport::TestCase
 
     assert f.valid_field_value?(nil)
     assert f.valid_field_value?('')
+    assert !f.valid_field_value?(' ')
     assert f.valid_field_value?([])
     assert f.valid_field_value?([nil])
     assert f.valid_field_value?([''])
+    assert !f.valid_field_value?([' '])
 
     assert f.valid_field_value?('value2')
     assert !f.valid_field_value?('abc')