diff options
author | Francesco Rodriguez <lrodriguezsanc@gmail.com> | 2012-05-28 19:56:54 -0500 |
---|---|---|
committer | Francesco Rodriguez <lrodriguezsanc@gmail.com> | 2012-05-28 19:57:43 -0500 |
commit | 39f0698405f8d52984f614d2aecafc514da9a3c5 (patch) | |
tree | 81dfb1c5627c125f534801232b2f66ac8b48240f | |
parent | 68f23bea6b3eec48f26e87ab9cdf3ff361820831 (diff) | |
download | rails-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.
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 |