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

4 files changed, 31 insertions, 10 deletions

diff --git a/activerecord/CHANGELOG b/activerecord/CHANGELOG
index d72a50022a..0c46b9cf11 100644
--- a/activerecord/CHANGELOG
+++ b/activerecord/CHANGELOG
@@ -1,5 +1,7 @@
 *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]
diff --git a/activerecord/lib/active_record/associations.rb b/activerecord/lib/active_record/associations.rb
index f9f5ec98a6..5bdc6247ff 100755
--- a/activerecord/lib/active_record/associations.rb
+++ b/activerecord/lib/active_record/associations.rb
@@ -153,7 +153,8 @@ module ActiveRecord
       # * <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 @@ module ActiveRecord
 
         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]
diff --git a/activerecord/lib/active_record/associations/has_many_association.rb b/activerecord/lib/active_record/associations/has_many_association.rb
index 0f79e4ccef..01374bd55a 100644
--- a/activerecord/lib/active_record/associations/has_many_association.rb
+++ b/activerecord/lib/active_record/associations/has_many_association.rb
@@ -113,11 +113,15 @@ module ActiveRecord
         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?
diff --git a/activerecord/test/associations_test.rb b/activerecord/test/associations_test.rb
index b35aadf4ff..c4f1022076 100755
--- a/activerecord/test/associations_test.rb
+++ b/activerecord/test/associations_test.rb
@@ -481,9 +481,21 @@ class HasManyAssociationsTest < Test::Unit::TestCase
   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 @@ class BelongsToAssociationsTest < Test::Unit::TestCase
 
   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 @@ class BelongsToAssociationsTest < Test::Unit::TestCase
     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