aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorIan White <ian.w.white@gmail.com>2010-05-18 10:28:26 +0100
committerJosé Valim <jose.valim@gmail.com>2010-05-18 16:13:00 +0200
commitb439d85a19c02eefab1ee308fff334ac1049524d (patch)
tree75fa2fa34a6d8ac398d25755d0b84de416500d5c
parentce20b93606195eff1ea2dfeb2aa311130dd9977e (diff)
downloadrails-b439d85a19c02eefab1ee308fff334ac1049524d.tar.gz
rails-b439d85a19c02eefab1ee308fff334ac1049524d.tar.bz2
rails-b439d85a19c02eefab1ee308fff334ac1049524d.zip
Nested records (re: autosave) are now updated even when the intermediate parent record is unchanged [#4242 state:resolved]
Signed-off-by: José Valim <jose.valim@gmail.com>
-rw-r--r--activerecord/lib/active_record/autosave_association.rb23
-rw-r--r--activerecord/test/cases/nested_attributes_test.rb80
-rw-r--r--activerecord/test/models/ship.rb3
-rw-r--r--activerecord/test/models/ship_part.rb2
4 files changed, 106 insertions, 2 deletions
diff --git a/activerecord/lib/active_record/autosave_association.rb b/activerecord/lib/active_record/autosave_association.rb
index fd1082a268..c553e95bad 100644
--- a/activerecord/lib/active_record/autosave_association.rb
+++ b/activerecord/lib/active_record/autosave_association.rb
@@ -214,6 +214,12 @@ module ActiveRecord
@marked_for_destruction
end
+ # Returns whether or not this record has been changed in any way (including whether
+ # any of its nested autosave associations are likewise changed)
+ def changed_for_autosave?
+ new_record? || changed? || marked_for_destruction? || nested_records_changed_for_autosave?
+ end
+
private
# Returns the record for an association collection that should be validated
@@ -223,12 +229,27 @@ module ActiveRecord
if new_record
association
elsif autosave
- association.target.find_all { |record| record.new_record? || record.changed? || record.marked_for_destruction? }
+ association.target.find_all { |record| record.changed_for_autosave? }
else
association.target.find_all { |record| record.new_record? }
end
end
+ # go through nested autosave associations that are loaded in memory (without loading
+ # any new ones), and return true if is changed for autosave
+ def nested_records_changed_for_autosave?
+ self.class.reflect_on_all_autosave_associations.each do |reflection|
+ if association = association_instance_get(reflection.name)
+ if [:belongs_to, :has_one].include?(reflection.macro)
+ return true if association.target && association.target.changed_for_autosave?
+ else
+ association.target.each {|record| return true if record.changed_for_autosave? }
+ end
+ end
+ end
+ false
+ end
+
# Validate the association if <tt>:validate</tt> or <tt>:autosave</tt> is
# turned on for the association specified by +reflection+.
def validate_single_association(reflection)
diff --git a/activerecord/test/cases/nested_attributes_test.rb b/activerecord/test/cases/nested_attributes_test.rb
index fadd62b5a1..57b66fb312 100644
--- a/activerecord/test/cases/nested_attributes_test.rb
+++ b/activerecord/test/cases/nested_attributes_test.rb
@@ -1,6 +1,7 @@
require "cases/helper"
require "models/pirate"
require "models/ship"
+require "models/ship_part"
require "models/bird"
require "models/parrot"
require "models/treasure"
@@ -732,3 +733,82 @@ class TestNestedAttributesWithNonStandardPrimaryKeys < ActiveRecord::TestCase
assert_equal ['Foo', 'Bar'], @owner.pets.map(&:name)
end
end
+
+class TestHasOneAutosaveAssoictaionWhichItselfHasAutosaveAssociations < ActiveRecord::TestCase
+ self.use_transactional_fixtures = false
+
+ def setup
+ @pirate = Pirate.create!(:catchphrase => "My baby takes tha mornin' train!")
+ @ship = @pirate.create_ship(:name => "The good ship Dollypop")
+ @part = @ship.parts.create!(:name => "Mast")
+ @trinket = @part.trinkets.create!(:name => "Necklace")
+ end
+
+ test "when great-grandchild changed in memory, saving parent should save great-grandchild" do
+ @trinket.name = "changed"
+ @pirate.save
+ assert_equal "changed", @trinket.reload.name
+ end
+
+ test "when great-grandchild changed via attributes, saving parent should save great-grandchild" do
+ @pirate.attributes = {:ship_attributes => {:id => @ship.id, :parts_attributes => [{:id => @part.id, :trinkets_attributes => [{:id => @trinket.id, :name => "changed"}]}]}}
+ @pirate.save
+ assert_equal "changed", @trinket.reload.name
+ end
+
+ test "when great-grandchild marked_for_destruction via attributes, saving parent should destroy great-grandchild" do
+ @pirate.attributes = {:ship_attributes => {:id => @ship.id, :parts_attributes => [{:id => @part.id, :trinkets_attributes => [{:id => @trinket.id, :_destroy => true}]}]}}
+ assert_difference('@part.trinkets.count', -1) { @pirate.save }
+ end
+
+ test "when great-grandchild added via attributes, saving parent should create great-grandchild" do
+ @pirate.attributes = {:ship_attributes => {:id => @ship.id, :parts_attributes => [{:id => @part.id, :trinkets_attributes => [{:name => "created"}]}]}}
+ assert_difference('@part.trinkets.count', 1) { @pirate.save }
+ end
+
+ test "when extra records exist for associations, validate (which calls nested_records_changed_for_autosave?) should not load them up" do
+ @trinket.name = "changed"
+ Ship.create!(:pirate => @pirate, :name => "The Black Rock")
+ ShipPart.create!(:ship => @ship, :name => "Stern")
+ assert_no_queries { @pirate.valid? }
+ end
+end
+
+class TestHasManyAutosaveAssoictaionWhichItselfHasAutosaveAssociations < ActiveRecord::TestCase
+ self.use_transactional_fixtures = false
+
+ def setup
+ @ship = Ship.create!(:name => "The good ship Dollypop")
+ @part = @ship.parts.create!(:name => "Mast")
+ @trinket = @part.trinkets.create!(:name => "Necklace")
+ end
+
+ test "when grandchild changed in memory, saving parent should save grandchild" do
+ @trinket.name = "changed"
+ @ship.save
+ assert_equal "changed", @trinket.reload.name
+ end
+
+ test "when grandchild changed via attributes, saving parent should save grandchild" do
+ @ship.attributes = {:parts_attributes => [{:id => @part.id, :trinkets_attributes => [{:id => @trinket.id, :name => "changed"}]}]}
+ @ship.save
+ assert_equal "changed", @trinket.reload.name
+ end
+
+ test "when grandchild marked_for_destruction via attributes, saving parent should destroy grandchild" do
+ @ship.attributes = {:parts_attributes => [{:id => @part.id, :trinkets_attributes => [{:id => @trinket.id, :_destroy => true}]}]}
+ assert_difference('@part.trinkets.count', -1) { @ship.save }
+ end
+
+ test "when grandchild added via attributes, saving parent should create grandchild" do
+ @ship.attributes = {:parts_attributes => [{:id => @part.id, :trinkets_attributes => [{:name => "created"}]}]}
+ assert_difference('@part.trinkets.count', 1) { @ship.save }
+ end
+
+ test "when extra records exist for associations, validate (which calls nested_records_changed_for_autosave?) should not load them up" do
+ @trinket.name = "changed"
+ Ship.create!(:name => "The Black Rock")
+ ShipPart.create!(:ship => @ship, :name => "Stern")
+ assert_no_queries { @ship.valid? }
+ end
+end
diff --git a/activerecord/test/models/ship.rb b/activerecord/test/models/ship.rb
index 75c792d176..3da031946f 100644
--- a/activerecord/test/models/ship.rb
+++ b/activerecord/test/models/ship.rb
@@ -3,8 +3,9 @@ class Ship < ActiveRecord::Base
belongs_to :pirate
belongs_to :update_only_pirate, :class_name => 'Pirate'
- has_many :parts, :class_name => 'ShipPart', :autosave => true
+ has_many :parts, :class_name => 'ShipPart'
+ accepts_nested_attributes_for :parts, :allow_destroy => true
accepts_nested_attributes_for :pirate, :allow_destroy => true, :reject_if => proc { |attributes| attributes.empty? }
accepts_nested_attributes_for :update_only_pirate, :update_only => true
diff --git a/activerecord/test/models/ship_part.rb b/activerecord/test/models/ship_part.rb
index 0a606db239..b6a8a506b4 100644
--- a/activerecord/test/models/ship_part.rb
+++ b/activerecord/test/models/ship_part.rb
@@ -1,5 +1,7 @@
class ShipPart < ActiveRecord::Base
belongs_to :ship
+ has_many :trinkets, :class_name => "Treasure", :as => :looter
+ accepts_nested_attributes_for :trinkets, :allow_destroy => true
validates_presence_of :name
end \ No newline at end of file