Skip to content
Browse files

Stop users from calling .create on a has_many / habtm association whe…

…n the owner is a new_record?

git-svn-id: http://svn-commit.rubyonrails.org/rails/trunk@7511 5ecf4fe2-1ee6-0310-87b1-e25e094e27de
  • Loading branch information...
1 parent 4a1388a commit 7010ee361ec71ed1a37962897cebc8684d90aa35 @NZKoz NZKoz committed
View
11 activerecord/lib/active_record/associations/association_collection.rb
@@ -85,6 +85,7 @@ def destroy_all
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 @@ def create(attrs = {})
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 @@ def callback(method, record)
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
View
21 activerecord/lib/active_record/associations/has_and_belongs_to_many_association.rb
@@ -15,6 +15,7 @@ def build(attributes = {})
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 @@ def create(attributes = {})
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 @@ def count_records
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]
View
24 activerecord/test/associations_test.rb
@@ -576,6 +576,26 @@ def test_adding_using_create
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 @@ def test_create
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
View
4 activerecord/test/validations_test.rb
@@ -675,7 +675,7 @@ def test_validates_size_of_association
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 @@ def test_validates_size_of_association_utf8
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

0 comments on commit 7010ee3

Please sign in to comment.
Something went wrong with that request. Please try again.