Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse files

Nested records (re: autosave) are now updated even when the intermedi…

…ate parent record is unchanged [#4242]

Signed-off-by: José Valim <jose.valim@gmail.com>
  • Loading branch information...
commit a5696e36c6b49381e84184fcc2f164285a26a166 1 parent 94878c6
@ianwhite ianwhite authored josevalim committed
View
25 activerecord/lib/active_record/autosave_association.rb
@@ -217,6 +217,12 @@ def marked_for_destruction?
@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
@@ -226,12 +232,27 @@ def associated_records_to_validate_or_save(association, new_record, autosave)
if new_record
association
elsif autosave
- association.target.select { |record| record.new_record? || record.changed? || record.marked_for_destruction? }
+ association.target.select { |record| record.changed_for_autosave? }
else
association.target.select { |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)
View
80 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"
@@ -731,3 +732,82 @@ def test_should_update_existing_records_with_non_standard_primary_key
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
View
3  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
View
4 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
Please sign in to comment.
Something went wrong with that request. Please try again.