Fixed validates_{confirmation,acceptance}_of to only happen when the virtual attributes are not nil #348 [dpiddy@gmail.com]

git-svn-id: http://svn-commit.rubyonrails.org/rails/trunk@241 5ecf4fe2-1ee6-0310-87b1-e25e094e27de

3 files changed, 26 insertions, 8 deletions

diff --git a/activerecord/CHANGELOG b/activerecord/CHANGELOG
index c642369bec..7e26b3c10a 100644
--- a/activerecord/CHANGELOG
+++ b/activerecord/CHANGELOG
@@ -1,5 +1,7 @@
 *SVN*
 
+* Fixed validates_{confirmation,acceptance}_of to only happen when the virtual attributes are not nil #348 [dpiddy@gmail.com]
+
 * Added a require_association hook on const_missing that makes it possible to use any model class without requiring it first. This makes STI look like:
 
     before:
diff --git a/activerecord/lib/active_record/validations.rb b/activerecord/lib/active_record/validations.rb
index 21ecf5f1c0..1f23909676 100755
--- a/activerecord/lib/active_record/validations.rb
+++ b/activerecord/lib/active_record/validations.rb
@@ -74,7 +74,8 @@ module ActiveRecord
       #     <%= password_field "person", "password_confirmation" %>
       #
       # The person has to already have a password attribute (a column in the people table), but the password_confirmation is virtual.
-      # It exists only as an in-memory variable for validating the password. This check is performed on save by default.
+      # It exists only as an in-memory variable for validating the password. This check is performed only if password_confirmation
+      # is not nil and by default on save.
       #
       # Configuration options:
       # * <tt>message</tt> - A custom error message (default is: "doesn't match confirmation")
@@ -85,7 +86,7 @@ module ActiveRecord
 
         for attr_name in attr_names
           attr_accessor "#{attr_name}_confirmation"
-          class_eval(%(#{validation_method(configuration[:on])} %{errors.add('#{attr_name}', "#{configuration[:message]}") unless #{attr_name} == #{attr_name}_confirmation}))
+          class_eval(%(#{validation_method(configuration[:on])} %{errors.add('#{attr_name}', "#{configuration[:message]}") unless #{attr_name}_confirmation.nil? or #{attr_name} == #{attr_name}_confirmation}))
         end
       end
 
@@ -96,7 +97,8 @@ module ActiveRecord
       #     validates_acceptance_of :eula, :message => "must be abided"
       #   end
       #
-      # The terms_of_service attribute is entirely virtual. No database column is needed. This check is performed on save by default.
+      # The terms_of_service attribute is entirely virtual. No database column is needed. This check is performed only if
+      # terms_of_service is not nil and by default on save.
       #
       # Configuration options:
       # * <tt>message</tt> - A custom error message (default is: "can't be empty")
@@ -109,7 +111,7 @@ module ActiveRecord
 
         for attr_name in attr_names
           attr_accessor(attr_name)
-          class_eval(%(#{validation_method(configuration[:on])} %{errors.add('#{attr_name}', '#{configuration[:message]}') unless #{attr_name} == "1"}))
+          class_eval(%(#{validation_method(configuration[:on])} %{errors.add('#{attr_name}', '#{configuration[:message]}') unless #{attr_name}.nil? or #{attr_name} == "1"}))
         end
       end
 
diff --git a/activerecord/test/validations_test.rb b/activerecord/test/validations_test.rb
index edd5356e23..867be0376d 100755
--- a/activerecord/test/validations_test.rb
+++ b/activerecord/test/validations_test.rb
@@ -125,20 +125,34 @@ class ValidationsTest < Test::Unit::TestCase
     assert developer.save
   end
 
+  def test_title_confirmation_no_confirm
+    Topic.validates_confirmation_of(:title)
+    
+    t = Topic.create("title" => "We should not be confirmed")
+    assert t.save
+  end
+  
   def test_title_confirmation
     Topic.validates_confirmation_of(:title)
 
-    t = Topic.create("title" => "We should be confirmed")
+    t = Topic.create("title" => "We should be confirmed","title_confirmation" => "")
     assert !t.save
 
     t.title_confirmation = "We should be confirmed"
     assert t.save
   end
 
+  def test_terms_of_service_agreement_no_acceptance
+    Topic.validates_acceptance_of(:terms_of_service, :on => :create)
+
+    t = Topic.create("title" => "We should not be confirmed")
+    assert t.save
+  end
+
   def test_terms_of_service_agreement
     Topic.validates_acceptance_of(:terms_of_service, :on => :create)
 
-    t = Topic.create("title" => "We should be confirmed")
+    t = Topic.create("title" => "We should be confirmed","terms_of_service" => "")
     assert !t.save
     assert_equal "must be accepted", t.errors.on(:terms_of_service)
 
@@ -150,7 +164,7 @@ class ValidationsTest < Test::Unit::TestCase
   def test_eula
     Topic.validates_acceptance_of(:eula, :message => "must be abided", :on => :create)
 
-    t = Topic.create("title" => "We should be confirmed")
+    t = Topic.create("title" => "We should be confirmed","eula" => "")
     assert !t.save
     assert_equal "must be abided", t.errors.on(:eula)
 
@@ -354,4 +368,4 @@ class ValidationsTest < Test::Unit::TestCase
     assert_equal 100, d.salary
     assert_equal "100,000", d.salary_before_type_cast
   end
-end
\ No newline at end of file
+end