Permalink
Browse files

Merge pull request #3329 from armstrjare/autosave_collection_new_reco…

…rd_bug

Autosave association doesn't save all records on a new record for a collection association if there are records marked for destruction
  • Loading branch information...
josevalim committed Mar 18, 2012
2 parents 0c18faf + f1903d8 commit d802a6c275fa79d25173cda1c06764e6b0e87902
@@ -328,13 +328,14 @@ def save_collection_association(reflection)
autosave = reflection.options[:autosave]
if records = associated_records_to_validate_or_save(association, @new_record_before_save, autosave)
+ records_to_destroy = []
records.each do |record|
next if record.destroyed?
saved = true
if autosave && record.marked_for_destruction?
- association.proxy.destroy(record)
+ records_to_destroy << record
elsif autosave != false && (@new_record_before_save || record.new_record?)
if autosave
saved = association.insert_record(record, false)
@@ -347,6 +348,10 @@ def save_collection_association(reflection)
raise ActiveRecord::Rollback unless saved
end
+
+ records_to_destroy.each do |record|
+ association.proxy.destroy(record)
+ end
end
# reconstruct the scope now that we know the owner's id
@@ -767,6 +767,16 @@ def destroy(*args)
assert_equal before, @pirate.reload.birds
end
+ def test_when_new_record_a_child_marked_for_destruction_should_not_affect_other_records_from_saving
+ @pirate = @ship.build_pirate(:catchphrase => "Arr' now I shall keep me eye on you matey!") # new record
+
+ 3.times { |i| @pirate.birds.build(:name => "birds_#{i}") }
+ @pirate.birds[1].mark_for_destruction
+ @pirate.save!
+
+ assert_equal 2, @pirate.birds.reload.length
+ end
+
# Add and remove callbacks tests for association collections.
%w{ method proc }.each do |callback_type|
define_method("test_should_run_add_callback_#{callback_type}s_for_has_many") do

0 comments on commit d802a6c

Please sign in to comment.