From 7596881838b105dd274595fe412c79a3d4a1cbdf Mon Sep 17 00:00:00 2001 From: Luca Guidi Date: Tue, 10 Mar 2009 12:12:32 +0100 Subject: [PATCH] Make sure autosaved associations run callbacks --- .../associations/association_collection.rb | 36 +++++++++++++++---- .../lib/active_record/autosave_association.rb | 2 +- activerecord/test/cases/nested_attributes_test.rb | 35 +++++++++++++++++++ activerecord/test/models/pirate.rb | 32 +++++++++++++++++ 4 files changed, 96 insertions(+), 9 deletions(-) diff --git a/activerecord/lib/active_record/associations/association_collection.rb b/activerecord/lib/active_record/associations/association_collection.rb index 0fefec1..3b50a1b 100644 --- a/activerecord/lib/active_record/associations/association_collection.rb +++ b/activerecord/lib/active_record/associations/association_collection.rb @@ -197,13 +197,13 @@ module ActiveRecord def delete(*records) records = flatten_deeper(records) records.each { |record| raise_on_type_mismatch(record) } - + transaction do records.each { |record| callback(:before_remove, record) } - - old_records = records.reject {|r| r.new_record? } + + old_records = records.reject { |r| r.new_record? } delete_records(old_records) if old_records.any? - + records.each do |record| @target.delete(record) callback(:after_remove, record) @@ -211,6 +211,29 @@ module ActiveRecord end end + # Destroy +records+ and remove from this association calling +before_remove+ + # and +after_remove+ callbacks. + # + # Note this method will always remove records from database ignoring the + # +:dependent+ option. + def destroy(*records) + records = flatten_deeper(records) + records.each { |record| raise_on_type_mismatch(record) } + + transaction do + records.each { |record| callback(:before_remove, record) } + + old_records = records.reject { |r| r.new_record? } + old_records.each { |record| record.destroy } + + records.each do |record| + callback(:after_remove, record) + end + end + + load_target + end + # Removes all records from this association. Returns +self+ so method calls may be chained. def clear return self if length.zero? # forces load_target if it hasn't happened already @@ -225,10 +248,7 @@ module ActiveRecord end def destroy_all - transaction do - each { |record| record.destroy } - end - + destroy(self) reset_target! end diff --git a/activerecord/lib/active_record/autosave_association.rb b/activerecord/lib/active_record/autosave_association.rb index 1c3d056..7b1e275 100644 --- a/activerecord/lib/active_record/autosave_association.rb +++ b/activerecord/lib/active_record/autosave_association.rb @@ -283,7 +283,7 @@ module ActiveRecord if records = associated_records_to_validate_or_save(association, @new_record_before_save, autosave) records.each do |record| if autosave && record.marked_for_destruction? - record.destroy + association.destroy(record) elsif @new_record_before_save || record.new_record? if autosave association.send(:insert_record, record, false, false) diff --git a/activerecord/test/cases/nested_attributes_test.rb b/activerecord/test/cases/nested_attributes_test.rb index cd6277c..c74aa49 100644 --- a/activerecord/test/cases/nested_attributes_test.rb +++ b/activerecord/test/cases/nested_attributes_test.rb @@ -61,6 +61,41 @@ class TestNestedAttributesInGeneral < ActiveRecord::TestCase end end +class TestNestedAttributesMethodCallbacks < ActiveRecord::TestCase + def test_should_run_adding_callbacks + pirate = Pirate.new(:catchphrase => "Arr", :birds_with_method_callbacks_attributes => [{:name => "Crowe the One-Eyed"}]) + assert_equal ["before_adding_method", "after_adding_method"], pirate.ship_log + end + + def test_should_run_removing_callbacks + pirate = Pirate.create!(:catchphrase => "Arr Arr") + bird = Bird.create!(:name => "Frank Big-hearted") + pirate.birds_with_method_callbacks << bird + pirate.ship_log.clear + + pirate.update_attributes :birds_with_method_callbacks_attributes => [{:id => bird.id, :_delete => true}] + assert_equal ["before_removing_method#{bird.id}", "after_removing_method#{bird.id}"], pirate.ship_log + end +end + +class TestNestedAttributesProcCallbacks < ActiveRecord::TestCase + def test_should_run_adding_callbacks + pirate = Pirate.new(:catchphrase => "To arr is pirate", + :birds_with_proc_callbacks_attributes => [{:name => "General Lee"}]) + assert_equal ["before_adding_proc", "after_adding_proc"], pirate.ship_log + end + + def test_should_run_removing_callbacks + pirate = Pirate.create!(:catchphrase => "To err is human, to arr is pirate") + bird = Bird.create!(:name => "Glowering Maggie") + pirate.birds_with_proc_callbacks << bird + pirate.ship_log.clear + + pirate.update_attributes :birds_with_proc_callbacks_attributes => [{:id => bird.id, :_delete => true}] + assert_equal ["before_removing_proc#{bird.id}", "after_removing_proc#{bird.id}"], pirate.ship_log + end +end + class TestNestedAttributesOnAHasOneAssociation < ActiveRecord::TestCase def setup @pirate = Pirate.create!(:catchphrase => "Don' botharrr talkin' like one, savvy?") diff --git a/activerecord/test/models/pirate.rb b/activerecord/test/models/pirate.rb index 7bc50e0..345908a 100644 --- a/activerecord/test/models/pirate.rb +++ b/activerecord/test/models/pirate.rb @@ -8,9 +8,41 @@ class Pirate < ActiveRecord::Base # These both have :autosave enabled because accepts_nested_attributes_for is used on them. has_one :ship has_many :birds + has_many :birds_with_method_callbacks, :class_name => "Bird", + :before_add => :log_before_add, + :after_add => :log_after_add, + :before_remove => :log_before_remove, + :after_remove => :log_after_remove + has_many :birds_with_proc_callbacks, :class_name => "Bird", + :before_add => proc {|p,b| p.ship_log << "before_adding_proc#{b.id || ''}"}, + :after_add => proc {|p,b| p.ship_log << "after_adding_proc#{b.id || ''}"}, + :before_remove => proc {|p,b| p.ship_log << "before_removing_proc#{b.id}"}, + :after_remove => proc {|p,b| p.ship_log << "after_removing_proc#{b.id}"} accepts_nested_attributes_for :parrots, :birds, :allow_destroy => true, :reject_if => proc { |attributes| attributes.empty? } accepts_nested_attributes_for :ship, :allow_destroy => true, :reject_if => proc { |attributes| attributes.empty? } + accepts_nested_attributes_for :birds_with_method_callbacks, :birds_with_proc_callbacks, :allow_destroy => true validates_presence_of :catchphrase + + def ship_log + @ship_log ||= [] + end + + private + def log_before_add(record) + ship_log << "before_adding_method#{record.id || ''}" + end + + def log_after_add(record) + ship_log << "after_adding_method#{record.id || ''}" + end + + def log_before_remove(record) + ship_log << "before_removing_method#{record.id}" + end + + def log_after_remove(record) + ship_log << "after_removing_method#{record.id}" + end end -- 1.5.4.5