diff options
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) |