aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorFrancesco Rodriguez <lrodriguezsanc@gmail.com>2012-05-28 19:56:54 -0500
committerFrancesco Rodriguez <lrodriguezsanc@gmail.com>2012-05-28 19:57:43 -0500
commit39f0698405f8d52984f614d2aecafc514da9a3c5 (patch)
tree81dfb1c5627c125f534801232b2f66ac8b48240f
parent68f23bea6b3eec48f26e87ab9cdf3ff361820831 (diff)
downloadrails-39f0698405f8d52984f614d2aecafc514da9a3c5.tar.gz
rails-39f0698405f8d52984f614d2aecafc514da9a3c5.tar.bz2
rails-39f0698405f8d52984f614d2aecafc514da9a3c5.zip
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.
-rw-r--r--activerecord/CHANGELOG.md13
-rw-r--r--activerecord/lib/active_record/associations/collection_association.rb1
-rw-r--r--activerecord/lib/active_record/associations/collection_proxy.rb26
-rw-r--r--activerecord/test/cases/associations/has_many_associations_test.rb20
4 files changed, 57 insertions, 3 deletions
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