Make sure associated has_many/habtm objects get saved even when :validate => false is used. [#486 state:resolved]

Signed-off-by: Pratik Naik <pratiknaik@gmail.com>

2 files changed, 30 insertions, 8 deletions

diff --git a/activerecord/lib/active_record/associations.rb b/activerecord/lib/active_record/associations.rb
index db99b7165d..4b7154043f 100755
--- a/activerecord/lib/active_record/associations.rb
+++ b/activerecord/lib/active_record/associations.rb
@@ -711,7 +711,8 @@ module ActiveRecord
 
         configure_dependency_for_has_many(reflection)
 
-        add_multiple_associated_save_callbacks(reflection.name) unless options[:validate] == false
+        add_multiple_associated_validation_callbacks(reflection.name) unless options[:validate] == false
+        add_multiple_associated_save_callbacks(reflection.name)
         add_association_callbacks(reflection.name, reflection.options)
 
         if options[:through]
@@ -801,7 +802,7 @@ module ActiveRecord
           end
           after_save method_name
 
-          add_single_associated_save_callbacks(reflection.name) if options[:validate] == true
+          add_single_associated_validation_callbacks(reflection.name) if options[:validate] == true
           association_accessor_methods(reflection, HasOneAssociation)
           association_constructor_method(:build,  reflection, HasOneAssociation)
           association_constructor_method(:create, reflection, HasOneAssociation)
@@ -940,7 +941,7 @@ module ActiveRecord
           )
         end
 
-        add_single_associated_save_callbacks(reflection.name) if options[:validate] == true
+        add_single_associated_validation_callbacks(reflection.name) if options[:validate] == true
 
         configure_dependency_for_belongs_to(reflection)
       end
@@ -1043,7 +1044,8 @@ module ActiveRecord
       def has_and_belongs_to_many(association_id, options = {}, &extension)
         reflection = create_has_and_belongs_to_many_reflection(association_id, options, &extension)
 
-        add_multiple_associated_save_callbacks(reflection.name) unless options[:validate] == false
+        add_multiple_associated_validation_callbacks(reflection.name) unless options[:validate] == false
+        add_multiple_associated_save_callbacks(reflection.name)
         collection_accessor_methods(reflection, HasAndBelongsToManyAssociation)
 
         # Don't use a before_destroy callback since users' before_destroy
@@ -1163,7 +1165,7 @@ module ActiveRecord
           end
         end
         
-        def add_single_associated_save_callbacks(association_name)
+        def add_single_associated_validation_callbacks(association_name)
           method_name = "validate_associated_records_for_#{association_name}".to_sym
           define_method(method_name) do
             association = instance_variable_get("@#{association_name}")
@@ -1175,7 +1177,7 @@ module ActiveRecord
           validate method_name
         end
         
-        def add_multiple_associated_save_callbacks(association_name)
+        def add_multiple_associated_validation_callbacks(association_name)
           method_name = "validate_associated_records_for_#{association_name}".to_sym
           ivar = "@#{association_name}"
 
@@ -1196,6 +1198,10 @@ module ActiveRecord
           end
 
           validate method_name
+        end
+
+        def add_multiple_associated_save_callbacks(association_name)
+          ivar = "@#{association_name}"
 
           method_name = "before_save_associated_records_for_#{association_name}".to_sym
           define_method(method_name) do
@@ -1217,7 +1223,6 @@ module ActiveRecord
             else
               []
             end
-
             records_to_save.each { |record| association.send(:insert_record, record) } unless records_to_save.blank?
 
             # reconstruct the SQL queries now that we know the owner's id
diff --git a/activerecord/test/cases/associations/has_many_associations_test.rb b/activerecord/test/cases/associations/has_many_associations_test.rb
index b638143c5a..1e21614f45 100644
--- a/activerecord/test/cases/associations/has_many_associations_test.rb
+++ b/activerecord/test/cases/associations/has_many_associations_test.rb
@@ -345,7 +345,7 @@ class HasManyAssociationsTest < ActiveRecord::TestCase
   def test_invalid_adding_with_validate_false
     firm = Firm.find(:first)
     client = Client.new
-    firm.unvalidated_clients_of_firm << Client.new
+    firm.unvalidated_clients_of_firm << client
 
     assert firm.valid?
     assert !client.valid?
@@ -353,6 +353,23 @@ class HasManyAssociationsTest < ActiveRecord::TestCase
     assert client.new_record?
   end
 
+  def test_valid_adding_with_validate_false
+    no_of_clients = Client.count
+
+    firm = Firm.find(:first)
+    client = Client.new("name" => "Apple")
+
+    assert firm.valid?
+    assert client.valid?
+    assert client.new_record?
+
+    firm.unvalidated_clients_of_firm << client
+
+    assert firm.save
+    assert !client.new_record?
+    assert_equal no_of_clients+1, Client.count
+  end
+
   def test_build
     company = companies(:first_firm)
     new_client = assert_no_queries { company.clients_of_firm.build("name" => "Another Client") }