Added that model.items.delete(child) will delete the child, not just set the...

Added that model.items.delete(child) will delete the child, not just set the foreign key to nil, if the child is dependent on the model #978 [bitsweat]

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

activerecord/CHANGELOG

 *SVN*
 
+* Added that model.items.delete(child) will delete the child, not just set the foreign key to nil, if the child is dependent on the model #978 [bitsweat]
+
 * Fixed auto-stamping of dates (created_on/updated_on) for PostgreSQL #985 [dave@cherryville.org]
 
 * Fixed Base.silence/benchmark to only log if a logger has been configured #986 [skaes@web.de]

activerecord/lib/active_record/associations.rb

@@ -153,7 +153,8 @@ module ClassMethods
       # * <tt>collection(force_reload = false)</tt> - returns an array of all the associated objects.
       #   An empty array is returned if none are found.
       # * <tt>collection<<(object, ...)</tt> - adds one or more objects to the collection by setting their foreign keys to the collection's primary key.
-      # * <tt>collection.delete(object, ...)</tt> - removes one or more objects from the collection by setting their foreign keys to NULL.  This does not destroy the objects.
+      # * <tt>collection.delete(object, ...)</tt> - removes one or more objects from the collection by setting their foreign keys to NULL.  
+      #   This will also destroy the objects if they're declared as belongs_to and dependent on this model.
       # * <tt>collection.clear</tt> - removes every object from the collection. This does not destroy the objects.
       # * <tt>collection.empty?</tt> - returns true if there are no associated objects.
       # * <tt>collection.size</tt> - returns the number of associated objects.
@@ -219,6 +220,8 @@ def has_many(association_id, options = {})
 
         if options[:dependent] and options[:exclusively_dependent]
           raise ArgumentError, ':dependent and :exclusively_dependent are mutually exclusive options.  You may specify one or the other.' # ' ruby-mode
+        # See HasManyAssociation#delete_records.  Dependent associations
+        # delete children, otherwise foreign key is set to NULL.
         elsif options[:dependent]
           module_eval "before_destroy '#{association_name}.each { |o| o.destroy }'"
         elsif options[:exclusively_dependent]

activerecord/lib/active_record/associations/has_many_association.rb

@@ -113,11 +113,15 @@ def insert_record(record)
         end
 
         def delete_records(records)
-          ids = quoted_record_ids(records)
-          @association_class.update_all(
-            "#{@association_class_primary_key_name} = NULL", 
-            "#{@association_class_primary_key_name} = #{@owner.quoted_id} AND #{@association_class.primary_key} IN (#{ids})"
-          )
+          if @options[:dependent]
+            records.each { |r| r.destroy }
+          else
+            ids = quoted_record_ids(records)
+            @association_class.update_all(
+              "#{@association_class_primary_key_name} = NULL", 
+              "#{@association_class_primary_key_name} = #{@owner.quoted_id} AND #{@association_class.primary_key} IN (#{ids})"
+            )
+          end
         end
 
         def target_obsolete?

activerecord/test/associations_test.rb

@@ -481,9 +481,21 @@ def test_destroy_all
   end
 
   def test_dependence
-    assert_equal 2, Client.find_all.length
+    assert_equal 2, Client.find_all.size
     Firm.find_first.destroy
-    assert_equal 0, Client.find_all.length
+    assert Client.find_all.empty?
+  end
+
+  def test_destroy_dependent_when_deleted_from_association
+    firm = Firm.find_first
+    assert_equal 2, firm.clients.size
+
+    client = firm.clients.first
+    firm.clients.delete(client)
+
+    assert_raise(ActiveRecord::RecordNotFound) { Client.find(client.id) }
+    assert_raise(ActiveRecord::RecordNotFound) { firm.clients.find(client.id) }
+    assert_equal 1, firm.clients.size
   end
 
   def test_three_levels_of_dependence
@@ -615,7 +627,7 @@ def test_assignment_before_either_saved
 
   def test_new_record_with_foreign_key_but_no_object
     c = Client.new("firm_id" => 1)
-    assert_equal @first_firm, c.firm_with_basic_id
+    assert_equal Firm.find_first, c.firm_with_basic_id
   end
 
   def test_forgetting_the_load_when_foreign_key_enters_late
@@ -623,7 +635,7 @@ def test_forgetting_the_load_when_foreign_key_enters_late
     assert_nil c.firm_with_basic_id
 
     c.firm_id = 1
-    assert_equal @first_firm, c.firm_with_basic_id
+    assert_equal Firm.find_first, c.firm_with_basic_id
   end
 
   def test_field_name_same_as_foreign_key