Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Nested records (re: autosave) are now updated even when the intermedi…
…ate parent record is unchanged [rails#4242]

Signed-off-by: José Valim <jose.valim@gmail.com>
  • Loading branch information
ianwhite authored and josevalim committed May 18, 2010
1 parent 94878c6 commit a5696e3
Show file tree
Hide file tree
Showing 4 changed files with 108 additions and 4 deletions.
25 changes: 23 additions & 2 deletions activerecord/lib/active_record/autosave_association.rb
Expand Up @@ -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
Expand All @@ -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)
Expand Down
80 changes: 80 additions & 0 deletions 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"
Expand Down Expand Up @@ -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
3 changes: 2 additions & 1 deletion activerecord/test/models/ship.rb
Expand Up @@ -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

Expand Down
4 changes: 3 additions & 1 deletion 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

0 comments on commit a5696e3

Please sign in to comment.