From e13f49d91951a82d9989c2a0f0c4981832f2664f Mon Sep 17 00:00:00 2001
From: Jean-Philippe Lang <jp_lang@yahoo.fr>
Date: Thu, 24 Jan 2008 18:15:38 +0000
Subject: [PATCH] Prevent unexpected nil in custom value validation.

git-svn-id: http://redmine.rubyforge.org/svn/trunk@1101 e93f8b46-1217-0410-a6f0-8f06a7374b81
---
 app/models/custom_value.rb     | 31 ++++++++------
 test/unit/custom_value_test.rb | 74 +++++++++++++++++++++++++++++++++-
 2 files changed, 91 insertions(+), 14 deletions(-)

diff --git a/app/models/custom_value.rb b/app/models/custom_value.rb
index 94b797bcc..98ce6b168 100644
--- a/app/models/custom_value.rb
+++ b/app/models/custom_value.rb
@@ -27,19 +27,24 @@ class CustomValue < ActiveRecord::Base
   
 protected
   def validate
-    errors.add(:value, :activerecord_error_blank) and return if custom_field.is_required? and value.blank?    
-    errors.add(:value, :activerecord_error_invalid) unless custom_field.regexp.blank? or value =~ Regexp.new(custom_field.regexp)
-    errors.add(:value, :activerecord_error_too_short) if custom_field.min_length > 0 and value.length < custom_field.min_length and value.length > 0
-    errors.add(:value, :activerecord_error_too_long) if custom_field.max_length > 0 and value.length > custom_field.max_length
-    case custom_field.field_format
-    when 'int'
-      errors.add(:value, :activerecord_error_not_a_number) unless value.blank? || value =~ /^[+-]?\d+$/	
-    when 'float'
-      begin; !value.blank? && Kernel.Float(value); rescue; errors.add(:value, :activerecord_error_invalid) end
-    when 'date'
-      errors.add(:value, :activerecord_error_not_a_date) unless value =~ /^\d{4}-\d{2}-\d{2}$/ or value.blank?
-    when 'list'
-      errors.add(:value, :activerecord_error_inclusion) unless custom_field.possible_values.include?(value) or value.blank?
+    if value.blank?
+      errors.add(:value, :activerecord_error_blank) if custom_field.is_required? and value.blank?    
+    else
+      errors.add(:value, :activerecord_error_invalid) unless custom_field.regexp.blank? or value =~ Regexp.new(custom_field.regexp)
+      errors.add(:value, :activerecord_error_too_short) if custom_field.min_length > 0 and value.length < custom_field.min_length
+      errors.add(:value, :activerecord_error_too_long) if custom_field.max_length > 0 and value.length > custom_field.max_length
+    
+      # Format specific validations
+      case custom_field.field_format
+      when 'int'
+        errors.add(:value, :activerecord_error_not_a_number) unless value =~ /^[+-]?\d+$/	
+      when 'float'
+        begin; Kernel.Float(value); rescue; errors.add(:value, :activerecord_error_invalid) end
+      when 'date'
+        errors.add(:value, :activerecord_error_not_a_date) unless value =~ /^\d{4}-\d{2}-\d{2}$/
+      when 'list'
+        errors.add(:value, :activerecord_error_inclusion) unless custom_field.possible_values.include?(value)
+      end
     end
   end
 end
diff --git a/test/unit/custom_value_test.rb b/test/unit/custom_value_test.rb
index 4d488bae2..11578ae6b 100644
--- a/test/unit/custom_value_test.rb
+++ b/test/unit/custom_value_test.rb
@@ -20,7 +20,79 @@ require File.dirname(__FILE__) + '/../test_helper'
 class CustomValueTest < Test::Unit::TestCase
   fixtures :custom_fields
 
-  def test_float_field
+  def test_string_field_validation_with_blank_value
+    f = CustomField.new(:field_format => 'string')
+    v = CustomValue.new(:custom_field => f)
+    
+    v.value = nil
+    assert v.valid?
+    v.value = ''
+    assert v.valid?
+
+    f.is_required = true
+    v.value = nil
+    assert !v.valid?
+    v.value = ''
+    assert !v.valid?
+  end
+  
+  def test_string_field_validation_with_min_and_max_lengths
+    f = CustomField.new(:field_format => 'string', :min_length => 2, :max_length => 5)
+    v = CustomValue.new(:custom_field => f, :value => '')
+    assert v.valid?
+    v.value = 'a'
+    assert !v.valid?
+    v.value = 'a' * 2
+    assert v.valid?
+    v.value = 'a' * 6
+    assert !v.valid?
+  end
+  
+  def test_string_field_validation_with_regexp
+    f = CustomField.new(:field_format => 'string', :regexp => '^[A-Z0-9]*$')
+    v = CustomValue.new(:custom_field => f, :value => '')
+    assert v.valid?
+    v.value = 'abc'
+    assert !v.valid?
+    v.value = 'ABC'
+    assert v.valid?
+  end
+
+  def test_date_field_validation
+    f = CustomField.new(:field_format => 'date')
+    v = CustomValue.new(:custom_field => f, :value => '')
+    assert v.valid?
+    v.value = 'abc'
+    assert !v.valid?
+    v.value = '1975-07-14'
+    assert v.valid?
+  end
+
+  def test_list_field_validation
+    f = CustomField.new(:field_format => 'list', :possible_values => ['value1', 'value2'])
+    v = CustomValue.new(:custom_field => f, :value => '')
+    assert v.valid?
+    v.value = 'abc'
+    assert !v.valid?
+    v.value = 'value2'
+    assert v.valid?
+  end
+
+  def test_int_field_validation
+    f = CustomField.new(:field_format => 'int')
+    v = CustomValue.new(:custom_field => f, :value => '')
+    assert v.valid?
+    v.value = 'abc'
+    assert !v.valid?
+    v.value = '123'
+    assert v.valid?
+    v.value = '+123'
+    assert v.valid?
+    v.value = '-123'
+    assert v.valid?
+  end
+  
+  def test_float_field_validation
     v = CustomValue.new(:customized => User.find(:first), :custom_field => UserCustomField.find_by_name('Money'))
     v.value = '11.2'
     assert v.save