From 39f0698405f8d52984f614d2aecafc514da9a3c5 Mon Sep 17 00:00:00 2001
From: Francesco Rodriguez <lrodriguezsanc@gmail.com>
Date: Mon, 28 May 2012 19:56:54 -0500
Subject: Add support for CollectionAssociation#delete by Fixnum or String

I found the next issue between CollectionAssociation `delete`
and `destroy`.

    class Person < ActiveRecord::Base
      has_many :pets
    end

    person.pets.destroy(1)
    # => OK, returns the destroyed object

    person.pets.destroy("2")
    # => OK, returns the destroyed object

    person.pets.delete(1)
    # => ActiveRecord::AssociationTypeMismatch

    person.pets.delete("2")
    # => ActiveRecord::AssociationTypeMismatch

Adding support for deleting with a fixnum or string like
`destroy` method.
---
 activerecord/CHANGELOG.md                          | 13 +++++++++++
 .../associations/collection_association.rb         |  1 +
 .../active_record/associations/collection_proxy.rb | 26 ++++++++++++++++++++++
 .../associations/has_many_associations_test.rb     | 20 ++++++++++++++---
 4 files changed, 57 insertions(+), 3 deletions(-)

(limited to 'activerecord')

diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md
index 8f1f315e42..1a8e233a51 100644
--- a/activerecord/CHANGELOG.md
+++ b/activerecord/CHANGELOG.md
@@ -1,5 +1,18 @@
 ## Rails 4.0.0 (unreleased) ##
 
+*   Added support to `CollectionAssociation#delete` for passing `fixnum`
+    or `string` values as record ids. This finds the records responding
+    to the `id` and executes delete on them.
+
+        class Person < ActiveRecord::Base
+          has_many :pets
+        end
+
+        person.pets.delete("1") # => [#<Pet id: 1>]
+        person.pets.delete(2, 3) # => [#<Pet id: 2>, #<Pet id: 3>]
+
+    *Francesco Rodriguez*
+
 *   Deprecated most of the 'dynamic finder' methods. All dynamic methods
     except for `find_by_...` and `find_by_...!` are deprecated. Here's
     how you can rewrite the code:
diff --git a/activerecord/lib/active_record/associations/collection_association.rb b/activerecord/lib/active_record/associations/collection_association.rb
index 4ec176e641..e94fe35170 100644
--- a/activerecord/lib/active_record/associations/collection_association.rb
+++ b/activerecord/lib/active_record/associations/collection_association.rb
@@ -230,6 +230,7 @@ module ActiveRecord
             delete_records(:all, dependent)
           end
         else
+          records = find(records) if records.any? { |record| record.kind_of?(Fixnum) || record.kind_of?(String) }
           delete_or_destroy(records, dependent)
         end
       end
diff --git a/activerecord/lib/active_record/associations/collection_proxy.rb b/activerecord/lib/active_record/associations/collection_proxy.rb
index 294aa63f75..2176fc4e40 100644
--- a/activerecord/lib/active_record/associations/collection_proxy.rb
+++ b/activerecord/lib/active_record/associations/collection_proxy.rb
@@ -469,6 +469,8 @@ module ActiveRecord
       #
       # :call-seq:
       #   delete(*records)
+      #   delete(*fixnum_ids)
+      #   delete(*string_ids)
       #
       # Deletes the +records+ supplied and removes them from the collection. For
       # +has_many+ associations, the deletion is done according to the strategy
@@ -560,6 +562,30 @@ module ActiveRecord
       #
       #   Pet.find(1)
       #   # => ActiveRecord::RecordNotFound: Couldn't find Pet with id=1
+      #
+      # You can pass +Fixnum+ or +String+ values, it finds the records
+      # responding to the +id+ and executes delete on them.
+      #
+      #   class Person < ActiveRecord::Base
+      #     has_many :pets
+      #   end
+      #
+      #   person.pets.size # => 3
+      #   person.pets
+      #   # => [
+      #   #       #<Pet id: 1, name: "Fancy-Fancy", person_id: 1>,
+      #   #       #<Pet id: 2, name: "Spook", person_id: 1>,
+      #   #       #<Pet id: 3, name: "Choo-Choo", person_id: 1>
+      #   #    ]
+      #
+      #   person.pets.delete("1")
+      #   # => [#<Pet id: 1, name: "Fancy-Fancy", person_id: 1>]
+      #
+      #   person.pets.delete(2, 3)
+      #   # => [
+      #   #       #<Pet id: 2, name: "Spook", person_id: 1>,
+      #   #       #<Pet id: 3, name: "Choo-Choo", person_id: 1>
+      #   #    ]
 
       ##
       # :method: destroy
diff --git a/activerecord/test/cases/associations/has_many_associations_test.rb b/activerecord/test/cases/associations/has_many_associations_test.rb
index 8b384c2513..0d8f311117 100644
--- a/activerecord/test/cases/associations/has_many_associations_test.rb
+++ b/activerecord/test/cases/associations/has_many_associations_test.rb
@@ -936,10 +936,24 @@ class HasManyAssociationsTest < ActiveRecord::TestCase
     assert_equal 2, summit.client_of
   end
 
-  def test_deleting_type_mismatch
+  def test_deleting_by_fixnum_id
     david = Developer.find(1)
-    david.projects.reload
-    assert_raise(ActiveRecord::AssociationTypeMismatch) { david.projects.delete(1) }
+
+    assert_difference 'david.projects.count', -1 do
+      assert_equal 1, david.projects.delete(1).size
+    end
+
+    assert_equal 1, david.projects.size
+  end
+
+  def test_deleting_by_string_id
+    david = Developer.find(1)
+
+    assert_difference 'david.projects.count', -1 do
+      assert_equal 1, david.projects.delete('1').size
+    end
+
+    assert_equal 1, david.projects.size
   end
 
   def test_deleting_self_type_mismatch
-- 
cgit v1.2.3