From 7010ee361ec71ed1a37962897cebc8684d90aa35 Mon Sep 17 00:00:00 2001 From: Michael Koziarski Date: Tue, 18 Sep 2007 10:26:56 +0000 Subject: Stop users from calling .create on a has_many / habtm association when the owner is a new_record? git-svn-id: http://svn-commit.rubyonrails.org/rails/trunk@7511 5ecf4fe2-1ee6-0310-87b1-e25e094e27de --- .../associations/association_collection.rb | 11 +++++++++- .../has_and_belongs_to_many_association.rb | 21 +++++++++++++++++-- activerecord/test/associations_test.rb | 24 ++++++++++++++++++++-- activerecord/test/validations_test.rb | 4 ++-- 4 files changed, 53 insertions(+), 7 deletions(-) diff --git a/activerecord/lib/active_record/associations/association_collection.rb b/activerecord/lib/active_record/associations/association_collection.rb index b0be44ff42..460f410cf0 100644 --- a/activerecord/lib/active_record/associations/association_collection.rb +++ b/activerecord/lib/active_record/associations/association_collection.rb @@ -85,6 +85,7 @@ module ActiveRecord end def create(attrs = {}) + ensure_owner_is_not_new record = @reflection.klass.send(:with_scope, :create => construct_scope[:create]) { @reflection.klass.create(attrs) } @target ||= [] unless loaded? @target << record @@ -92,6 +93,7 @@ module ActiveRecord end def create!(attrs = {}) + ensure_owner_is_not_new record = @reflection.klass.send(:with_scope, :create => construct_scope[:create]) { @reflection.klass.create!(attrs) } @target ||= [] unless loaded? @target << record @@ -206,7 +208,14 @@ module ActiveRecord def callbacks_for(callback_name) full_callback_name = "#{callback_name}_for_#{@reflection.name}" @owner.class.read_inheritable_attribute(full_callback_name.to_sym) || [] - end + end + + def ensure_owner_is_not_new + if @owner.new_record? + raise ActiveRecord::RecordNotSaved, "You cannot call create unless the parent is saved" + end + end + end end end diff --git a/activerecord/lib/active_record/associations/has_and_belongs_to_many_association.rb b/activerecord/lib/active_record/associations/has_and_belongs_to_many_association.rb index 917a7fa5e1..663655213a 100644 --- a/activerecord/lib/active_record/associations/has_and_belongs_to_many_association.rb +++ b/activerecord/lib/active_record/associations/has_and_belongs_to_many_association.rb @@ -15,6 +15,7 @@ module ActiveRecord def create(attributes = {}) # Can't use Base.create because the foreign key may be a protected attribute. + ensure_owner_is_not_new if attributes.is_a?(Array) attributes.collect { |attr| create(attr) } else @@ -23,6 +24,18 @@ module ActiveRecord record end end + + def create!(attributes = {}) + # Can't use Base.create! because the foreign key may be a protected attribute. + ensure_owner_is_not_new + if attributes.is_a?(Array) + attributes.collect { |attr| create(attr) } + else + record = build(attributes) + insert_record(record, true) unless @owner.new_record? + record + end + end def find_first load_target.first @@ -75,9 +88,13 @@ module ActiveRecord load_target.size end - def insert_record(record) + def insert_record(record, force=true) if record.new_record? - return false unless record.save + if force + record.save! + else + return false unless record.save + end end if @reflection.options[:insert_sql] diff --git a/activerecord/test/associations_test.rb b/activerecord/test/associations_test.rb index a1ea3f1c23..222cb5dbca 100755 --- a/activerecord/test/associations_test.rb +++ b/activerecord/test/associations_test.rb @@ -576,6 +576,26 @@ class HasManyAssociationsTest < Test::Unit::TestCase assert_equal 3, first_firm.plain_clients.length assert_equal 3, first_firm.plain_clients.size end + + def test_create_with_bang_on_has_many_when_parent_is_new_raises + assert_raises(ActiveRecord::RecordNotSaved) do + firm = Firm.new + firm.plain_clients.create! :name=>"Whoever" + end + end + + def test_regular_create_on_has_many_when_parent_is_new_raises + assert_raises(ActiveRecord::RecordNotSaved) do + firm = Firm.new + firm.plain_clients.create :name=>"Whoever" + end + end + + def test_create_with_bang_on_habtm_when_parent_is_new_raises + assert_raises(ActiveRecord::RecordNotSaved) do + Developer.new("name" => "Aredridel").projects.create! + end + end def test_adding_a_mismatch_class assert_raises(ActiveRecord::AssociationTypeMismatch) { companies(:first_firm).clients_of_firm << nil } @@ -1540,8 +1560,8 @@ class HasAndBelongsToManyAssociationsTest < Test::Unit::TestCase def test_create_by_new_record devel = Developer.new(:name => "Marcel", :salary => 75000) - proj1 = devel.projects.create(:name => "Make bed") - proj2 = devel.projects.create(:name => "Lie in it") + proj1 = devel.projects.build(:name => "Make bed") + proj2 = devel.projects.build(:name => "Lie in it") assert_equal devel.projects.last, proj2 assert proj2.new_record? devel.save diff --git a/activerecord/test/validations_test.rb b/activerecord/test/validations_test.rb index 8c7ef759e6..d0c54d1528 100755 --- a/activerecord/test/validations_test.rb +++ b/activerecord/test/validations_test.rb @@ -675,7 +675,7 @@ class ValidationsTest < Test::Unit::TestCase t = Topic.new('title' => 'noreplies', 'content' => 'whatever') assert !t.save assert t.errors.on(:replies) - t.replies.create('title' => 'areply', 'content' => 'whateveragain') + reply = t.replies.build('title' => 'areply', 'content' => 'whateveragain') assert t.valid? end @@ -868,7 +868,7 @@ class ValidationsTest < Test::Unit::TestCase t = Topic.new('title' => 'あいうえお', 'content' => 'かきくけこ') assert !t.save assert t.errors.on(:replies) - t.replies.create('title' => 'あいうえお', 'content' => 'かきくけこ') + t.replies.build('title' => 'あいうえお', 'content' => 'かきくけこ') assert t.valid? end end -- cgit v1.2.3