diff options
author | Jon Leighton <j@jonathanleighton.com> | 2011-01-03 23:48:53 +0000 |
---|---|---|
committer | Aaron Patterson <aaron.patterson@gmail.com> | 2011-01-03 16:24:32 -0800 |
commit | 40afcade0dc1450e765a91fc15a6ac6d442c9826 (patch) | |
tree | 4d205b197a761fcd8b74854ebf4cc0139277a280 | |
parent | 2120da7f733ba33183a42e71256db9652c5f5fcc (diff) | |
download | rails-40afcade0dc1450e765a91fc15a6ac6d442c9826.tar.gz rails-40afcade0dc1450e765a91fc15a6ac6d442c9826.tar.bz2 rails-40afcade0dc1450e765a91fc15a6ac6d442c9826.zip |
Remove undocumented feature from has_one where you could pass false as the second parameter to build_assoc or create_assoc, and the existing associated object would be untouched (the foreign key would not be nullified, and it would not be deleted). If you want behaviour similar to this you can do the following things:
* Use :dependent => :nullify (or don't specify :dependent) if you want to prevent the existing associated object from being deleted
* Use has_many if you actually want multiple associated objects
* Explicitly set the foreign key if, for some reason, you really need to have multiple objects associated with the same has_one. E.g.
previous = obj.assoc
obj.create_assoc
previous.update_attributes(:obj_id => obj.id)
4 files changed, 18 insertions, 64 deletions
diff --git a/activerecord/lib/active_record/associations.rb b/activerecord/lib/active_record/associations.rb index e7d3e45da2..bbf96f52ed 100644 --- a/activerecord/lib/active_record/associations.rb +++ b/activerecord/lib/active_record/associations.rb @@ -1556,20 +1556,15 @@ module ActiveRecord def association_constructor_method(constructor, reflection, association_proxy_class) redefine_method("#{constructor}_#{reflection.name}") do |*params| - attributees = params.first unless params.empty? - replace_existing = params[1].nil? ? true : params[1] - association = association_instance_get(reflection.name) + attributes = params.first unless params.empty? + association = association_instance_get(reflection.name) unless association association = association_proxy_class.new(self, reflection) association_instance_set(reflection.name, association) end - if association_proxy_class == HasOneAssociation - association.send(constructor, attributees, replace_existing) - else - association.send(constructor, attributees) - end + association.send(constructor, attributes) end end diff --git a/activerecord/lib/active_record/associations/has_one_association.rb b/activerecord/lib/active_record/associations/has_one_association.rb index 5716bef524..f1e01197b5 100644 --- a/activerecord/lib/active_record/associations/has_one_association.rb +++ b/activerecord/lib/active_record/associations/has_one_association.rb @@ -4,22 +4,22 @@ module ActiveRecord class HasOneAssociation < AssociationProxy #:nodoc: include HasAssociation - def create(attrs = {}, replace_existing = true) - new_record(replace_existing) do |reflection| + def create(attrs = {}) + new_record do |reflection| attrs = merge_with_conditions(attrs) reflection.create_association(attrs) end end - def create!(attrs = {}, replace_existing = true) - new_record(replace_existing) do |reflection| + def create!(attrs = {}) + new_record do |reflection| attrs = merge_with_conditions(attrs) reflection.create_association!(attrs) end end - def build(attrs = {}, replace_existing = true) - new_record(replace_existing) do |reflection| + def build(attrs = {}) + new_record do |reflection| attrs = merge_with_conditions(attrs) reflection.build_association(attrs) end @@ -82,21 +82,9 @@ module ActiveRecord construct_owner_attributes end - def new_record(replace_existing) - # Make sure we load the target first, if we plan on replacing the existing - # instance. Otherwise, if the target has not previously been loaded - # elsewhere, the instance we create will get orphaned. - load_target if replace_existing + def new_record record = scoped.scoping { yield @reflection } - - if replace_existing - replace(record, true) - else - record[@reflection.foreign_key] = @owner.id if @owner.persisted? - self.target = record - set_inverse_instance(record) - end - + replace(record, true) record end diff --git a/activerecord/test/cases/associations/has_one_associations_test.rb b/activerecord/test/cases/associations/has_one_associations_test.rb index 64449df8f5..fa36b527a2 100644 --- a/activerecord/test/cases/associations/has_one_associations_test.rb +++ b/activerecord/test/cases/associations/has_one_associations_test.rb @@ -116,35 +116,6 @@ class HasOneAssociationsTest < ActiveRecord::TestCase assert_equal company.account, account end - def test_assignment_without_replacement - apple = Firm.create("name" => "Apple") - citibank = Account.create("credit_limit" => 10) - apple.account = citibank - assert_equal apple.id, citibank.firm_id - - hsbc = apple.build_account({ :credit_limit => 20}, false) - assert_equal apple.id, hsbc.firm_id - hsbc.save - assert_equal apple.id, citibank.firm_id - - nykredit = apple.create_account({ :credit_limit => 30}, false) - assert_equal apple.id, nykredit.firm_id - assert_equal apple.id, citibank.firm_id - assert_equal apple.id, hsbc.firm_id - end - - def test_assignment_without_replacement_on_create - apple = Firm.create("name" => "Apple") - citibank = Account.create("credit_limit" => 10) - apple.account = citibank - assert_equal apple.id, citibank.firm_id - - hsbc = apple.create_account({:credit_limit => 10}, false) - assert_equal apple.id, hsbc.firm_id - hsbc.save - assert_equal apple.id, citibank.firm_id - end - def test_dependence num_accounts = Account.count diff --git a/activerecord/test/cases/associations/inverse_associations_test.rb b/activerecord/test/cases/associations/inverse_associations_test.rb index 0491b2d1fa..a0f94a22e3 100644 --- a/activerecord/test/cases/associations/inverse_associations_test.rb +++ b/activerecord/test/cases/associations/inverse_associations_test.rb @@ -146,9 +146,9 @@ class InverseHasOneTests < ActiveRecord::TestCase assert_equal m.name, f.man.name, "Name of man should be the same after changes to newly-created-child-owned instance" end - def test_parent_instance_should_be_shared_with_newly_built_child_when_we_dont_replace_existing + def test_parent_instance_should_be_shared_with_newly_built_child m = Man.find(:first) - f = m.build_face({:description => 'haunted'}, false) + f = m.build_face(:description => 'haunted') assert_not_nil f.man assert_equal m.name, f.man.name, "Name of man should be the same before changes to parent instance" m.name = 'Bongo' @@ -157,9 +157,9 @@ class InverseHasOneTests < ActiveRecord::TestCase assert_equal m.name, f.man.name, "Name of man should be the same after changes to just-built-child-owned instance" end - def test_parent_instance_should_be_shared_with_newly_created_child_when_we_dont_replace_existing + def test_parent_instance_should_be_shared_with_newly_created_child m = Man.find(:first) - f = m.create_face({:description => 'haunted'}, false) + f = m.create_face(:description => 'haunted') assert_not_nil f.man assert_equal m.name, f.man.name, "Name of man should be the same before changes to parent instance" m.name = 'Bongo' @@ -168,9 +168,9 @@ class InverseHasOneTests < ActiveRecord::TestCase assert_equal m.name, f.man.name, "Name of man should be the same after changes to newly-created-child-owned instance" end - def test_parent_instance_should_be_shared_with_newly_created_child_via_bang_method_when_we_dont_replace_existing + def test_parent_instance_should_be_shared_with_newly_created_child_via_bang_method m = Man.find(:first) - f = m.face.create!({:description => 'haunted'}, false) + f = m.face.create!(:description => 'haunted') assert_not_nil f.man assert_equal m.name, f.man.name, "Name of man should be the same before changes to parent instance" m.name = 'Bongo' @@ -203,7 +203,7 @@ class InverseHasOneTests < ActiveRecord::TestCase assert_equal m.name, f.man.name, "Name of man should be the same after changes to replaced-child-owned instance" end - def test_parent_instance_should_be_shared_with_replaced_via_method_child_when_we_dont_replace_existing + def test_parent_instance_should_be_shared_with_replaced_via_method_child m = Man.find(:first) f = Face.new(:description => 'haunted') m.face.replace(f, false) |