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

git-svn-id: http://svn.redmine.org/redmine/trunk@12938 e93f8b46-1217-0410-a6f0-8f06a7374b81
This commit is contained in:
Jean-Philippe Lang 2014-02-28 10:05:27 +00:00
parent 0cf7a27b50
commit 4c7a76785d
3 changed files with 23 additions and 16 deletions

View File

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

View File

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

View File

@ -146,6 +146,7 @@ class CustomFieldTest < ActiveSupport::TestCase
assert f.valid_field_value?(nil) 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?('a' * 2) assert f.valid_field_value?('a' * 2)
assert !f.valid_field_value?('a') assert !f.valid_field_value?('a')
assert !f.valid_field_value?('a' * 6) 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?(nil)
assert f.valid_field_value?('') assert f.valid_field_value?('')
assert !f.valid_field_value?(' ')
assert f.valid_field_value?('ABC') assert f.valid_field_value?('ABC')
assert !f.valid_field_value?('abc') assert !f.valid_field_value?('abc')
end end
@ -165,6 +167,7 @@ class CustomFieldTest < ActiveSupport::TestCase
assert f.valid_field_value?(nil) 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?('1975-07-14') assert f.valid_field_value?('1975-07-14')
assert !f.valid_field_value?('1975-07-33') assert !f.valid_field_value?('1975-07-33')
assert !f.valid_field_value?('abc') 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?(nil)
assert f.valid_field_value?('') assert f.valid_field_value?('')
assert !f.valid_field_value?(' ')
assert f.valid_field_value?('value2') assert f.valid_field_value?('value2')
assert !f.valid_field_value?('abc') assert !f.valid_field_value?('abc')
end end
@ -184,6 +188,7 @@ class CustomFieldTest < ActiveSupport::TestCase
assert f.valid_field_value?(nil) 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?('123') assert f.valid_field_value?('123')
assert f.valid_field_value?('+123') 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?(nil)
assert f.valid_field_value?('') assert f.valid_field_value?('')
assert !f.valid_field_value?(' ')
assert f.valid_field_value?('11.2') assert f.valid_field_value?('11.2')
assert f.valid_field_value?('-6.250') assert f.valid_field_value?('-6.250')
assert f.valid_field_value?('5') 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?(nil)
assert f.valid_field_value?('') assert f.valid_field_value?('')
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?([nil])
assert f.valid_field_value?(['']) assert f.valid_field_value?([''])
assert !f.valid_field_value?([' '])
assert f.valid_field_value?('value2') assert f.valid_field_value?('value2')
assert !f.valid_field_value?('abc') assert !f.valid_field_value?('abc')