Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse files

Fix associations to call :destroy or :delete based on the right :depe…

…ndent option

Signed-off-by: José Valim <jose.valim@gmail.com>
  • Loading branch information...
commit 47d252f9928568620844edce2161acd457c352c0 1 parent 8e9d923
@carlosantoniodasilva carlosantoniodasilva authored josevalim committed
View
6 activerecord/lib/active_record/associations.rb
@@ -1544,12 +1544,12 @@ def configure_dependency_for_has_one(reflection)
when :destroy, :delete
define_method(method_name) do
association = send(reflection.name)
- association.destroy unless association.nil?
+ association.send(name) if association
end
when :nullify
define_method(method_name) do
association = send(reflection.name)
- association.update_attribute(reflection.primary_key_name, nil) unless association.nil?
+ association.update_attribute(reflection.primary_key_name, nil) if association
end
else
raise ArgumentError, "The :dependent option expects either :destroy, :delete or :nullify (#{reflection.options[:dependent].inspect})"
@@ -1570,7 +1570,7 @@ def configure_dependency_for_belongs_to(reflection)
method_name = :"belongs_to_dependent_#{name}_for_#{reflection.name}"
define_method(method_name) do
association = send(reflection.name)
- association.destroy unless association.nil?
+ association.send(name) if association
end
after_destroy method_name
end
View
32 activerecord/test/cases/associations/belongs_to_associations_test.rb
@@ -18,7 +18,8 @@
class BelongsToAssociationsTest < ActiveRecord::TestCase
fixtures :accounts, :companies, :developers, :projects, :topics,
- :developers_projects, :computers, :authors, :posts, :tags, :taggings, :comments
+ :developers_projects, :computers, :authors, :author_addresses,
+ :posts, :tags, :taggings, :comments
def test_belongs_to
Client.find(3).firm.name
@@ -346,14 +347,14 @@ def test_cant_save_readonly_association
assert_raise(ActiveRecord::ReadOnlyRecord) { companies(:first_client).readonly_firm.save! }
assert companies(:first_client).readonly_firm.readonly?
end
-
+
def test_polymorphic_assignment_foreign_type_field_updating
# should update when assigning a saved record
sponsor = Sponsor.new
member = Member.create
sponsor.sponsorable = member
assert_equal "Member", sponsor.sponsorable_type
-
+
# should update when assigning a new record
sponsor = Sponsor.new
member = Member.new
@@ -374,15 +375,15 @@ def test_polymorphic_assignment_with_primary_key_foreign_type_field_updating
essay.writer = writer
assert_equal "Author", essay.writer_type
end
-
+
def test_polymorphic_assignment_updates_foreign_id_field_for_new_and_saved_records
sponsor = Sponsor.new
saved_member = Member.create
new_member = Member.new
-
+
sponsor.sponsorable = saved_member
assert_equal saved_member.id, sponsor.sponsorable_id
-
+
sponsor.sponsorable = new_member
assert_equal nil, sponsor.sponsorable_id
end
@@ -424,4 +425,23 @@ def test_save_of_record_with_loaded_belongs_to
Account.find(@account.id, :include => :firm).save!
end
end
+
+ def test_dependent_delete_and_destroy_with_belongs_to
+ author_address = author_addresses(:david_address)
+ author_address_extra = author_addresses(:david_address_extra)
+ assert_equal [], AuthorAddress.destroyed_author_address_ids
+
+ assert_difference "AuthorAddress.count", -2 do
+ authors(:david).destroy
+ end
+
+ assert_equal [], AuthorAddress.find_all_by_id([author_address.id, author_address_extra.id])
+ assert_equal [author_address.id], AuthorAddress.destroyed_author_address_ids
+ end
+
+ def test_invalid_belongs_to_dependent_option_raises_exception
+ assert_raise ArgumentError do
+ Author.belongs_to :special_author_address, :dependent => :nullify
+ end
+ end
end
View
20 activerecord/test/cases/associations/has_many_associations_test.rb
@@ -14,7 +14,7 @@
class HasManyAssociationsTest < ActiveRecord::TestCase
fixtures :accounts, :categories, :companies, :developers, :projects,
- :developers_projects, :topics, :authors, :comments, :author_addresses,
+ :developers_projects, :topics, :authors, :comments,
:people, :posts, :readers, :taggings
def setup
@@ -684,24 +684,6 @@ def test_creation_respects_hash_condition
assert_equal 'Microsoft', another_ms_client.name
end
- def test_dependent_delete_and_destroy_with_belongs_to
- author_address = author_addresses(:david_address)
- assert_equal [], AuthorAddress.destroyed_author_address_ids[authors(:david).id]
-
- assert_difference "AuthorAddress.count", -2 do
- authors(:david).destroy
- end
-
- assert_equal nil, AuthorAddress.find_by_id(authors(:david).author_address_id)
- assert_equal nil, AuthorAddress.find_by_id(authors(:david).author_address_extra_id)
- end
-
- def test_invalid_belongs_to_dependent_option_raises_exception
- assert_raise ArgumentError do
- Author.belongs_to :special_author_address, :dependent => :nullify
- end
- end
-
def test_clearing_without_initial_access
firm = companies(:first_firm)
View
4 activerecord/test/cases/associations/has_one_associations_test.rb
@@ -14,7 +14,7 @@ def test_has_one
assert_equal companies(:first_firm).account, Account.find(1)
assert_equal Account.find(1).credit_limit, companies(:first_firm).account.credit_limit
end
-
+
def test_has_one_cache_nils
firm = companies(:another_firm)
assert_queries(1) { assert_nil firm.account }
@@ -96,7 +96,7 @@ def test_nullification_on_association_change
assert_nil Account.find(old_account_id).firm_id
end
- def test_association_changecalls_delete
+ def test_association_change_calls_delete
companies(:first_firm).deletable_account = Account.new
assert_equal [], Account.destroyed_account_ids[companies(:first_firm).id]
end
View
4 activerecord/test/cases/calculations_test.rb
@@ -38,12 +38,12 @@ def test_should_get_maximum_of_field
end
def test_should_get_maximum_of_field_with_include
- assert_equal 50, Account.maximum(:credit_limit, :include => :firm, :conditions => "companies.name != 'Summit'")
+ assert_equal 55, Account.maximum(:credit_limit, :include => :firm, :conditions => "companies.name != 'Summit'")
end
def test_should_get_maximum_of_field_with_scoped_include
Account.send :with_scope, :find => { :include => :firm, :conditions => "companies.name != 'Summit'" } do
- assert_equal 50, Account.maximum(:credit_limit)
+ assert_equal 55, Account.maximum(:credit_limit)
end
end
View
9 activerecord/test/models/author.rb
@@ -35,7 +35,7 @@ def testing_proxy_target
has_many :ordered_uniq_comments, :through => :posts, :source => :comments, :uniq => true, :order => 'comments.id'
has_many :ordered_uniq_comments_desc, :through => :posts, :source => :comments, :uniq => true, :order => 'comments.id DESC'
has_many :readonly_comments, :through => :posts, :source => :comments, :readonly => true
-
+
has_many :special_posts
has_many :special_post_comments, :through => :special_posts, :source => :comments
@@ -130,14 +130,11 @@ class AuthorAddress < ActiveRecord::Base
has_one :author
def self.destroyed_author_address_ids
- @destroyed_author_address_ids ||= Hash.new { |h,k| h[k] = [] }
+ @destroyed_author_address_ids ||= []
end
before_destroy do |author_address|
- if author_address.author
- AuthorAddress.destroyed_author_address_ids[author_address.author.id] << author_address.id
- end
- true
+ AuthorAddress.destroyed_author_address_ids << author_address.id
end
end
View
4 activerecord/test/models/company.rb
@@ -82,7 +82,7 @@ class Firm < Company
has_one :account_using_primary_key, :primary_key => "firm_id", :class_name => "Account", :order => "id"
has_one :account_using_foreign_and_primary_keys, :foreign_key => "firm_name", :primary_key => "name", :class_name => "Account"
has_one :deletable_account, :foreign_key => "firm_id", :class_name => "Account", :dependent => :delete
-
+
has_one :account_limit_500_with_hash_conditions, :foreign_key => "firm_id", :class_name => "Account", :conditions => { :credit_limit => 500 }
has_one :unautosaved_account, :foreign_key => "firm_id", :class_name => 'Account', :autosave => false
@@ -155,7 +155,7 @@ class VerySpecialClient < SpecialClient
end
class Account < ActiveRecord::Base
- belongs_to :firm
+ belongs_to :firm, :class_name => 'Company'
belongs_to :unautosaved_firm, :foreign_key => "firm_id", :class_name => "Firm", :autosave => false
def self.destroyed_account_ids
Please sign in to comment.
Something went wrong with that request. Please try again.