mirror of
https://github.com/rails/rails.git
synced 2022-11-09 12:12:34 -05:00
Merge pull request #3329 from armstrjare/autosave_collection_new_record_bug
Autosave association doesn't save all records on a new record for a collection association if there are records marked for destruction
This commit is contained in:
commit
d802a6c275
2 changed files with 16 additions and 1 deletions
|
@ -328,13 +328,14 @@ module ActiveRecord
|
|||
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 @@ module ActiveRecord
|
|||
|
||||
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 @@ class TestDestroyAsPartOfAutosaveAssociation < ActiveRecord::TestCase
|
|||
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
|
||||
|
|
Loading…
Reference in a new issue