diff options
-rw-r--r-- | activerecord/lib/active_record/nested_attributes.rb | 50 | ||||
-rw-r--r-- | activerecord/test/cases/nested_attributes_test.rb | 41 | ||||
-rw-r--r-- | activerecord/test/models/pirate.rb | 2 | ||||
-rw-r--r-- | activerecord/test/models/ship.rb | 2 |
4 files changed, 48 insertions, 47 deletions
diff --git a/activerecord/lib/active_record/nested_attributes.rb b/activerecord/lib/active_record/nested_attributes.rb index 5143e804d1..ff3a51d5c0 100644 --- a/activerecord/lib/active_record/nested_attributes.rb +++ b/activerecord/lib/active_record/nested_attributes.rb @@ -213,9 +213,9 @@ module ActiveRecord # exception is raised. If omitted, any number associations can be processed. # Note that the :limit option is only applicable to one-to-many associations. # [:update_only] - # Allows to specify that the an existing record can only be updated. - # A new record in only created when there is no existing record. This - # option only works for on-to-one associations and is ignored for + # Allows you to specify that an existing record may only be updated. + # A new record may only be created when there is no existing record. + # This option only works for one-to-one associations and is ignored for # collection associations. This option is off by default. # # Examples: @@ -248,7 +248,7 @@ module ActiveRecord end # def pirate_attributes=(attributes) - # assign_nested_attributes_for_one_to_one_association(:pirate, attributes, false) + # assign_nested_attributes_for_one_to_one_association(:pirate, attributes) # end class_eval %{ def #{association_name}_attributes=(attributes) @@ -289,43 +289,29 @@ module ActiveRecord # Assigns the given attributes to the association. # - # If the given attributes include an <tt>:id</tt> that matches the existing - # record’s id, then the existing record will be modified. Otherwise a new - # record will be built. - # - # If update_only is true, a new record is only created when no object exists, - # otherwise it will be updated - # # If update_only is false and the given attributes include an <tt>:id</tt> # that matches the existing record’s id, then the existing record will be - # modified. Otherwise a new record will be built. + # modified. If update_only is true, a new record is only created when no + # object exists. Otherwise a new record will be built. # - # If the given attributes include a matching <tt>:id</tt> attribute _and_ a - # <tt>:_destroy</tt> key set to a truthy value, then the existing record - # will be marked for destruction. + # If the given attributes include a matching <tt>:id</tt> attribute, or + # update_only is true, and a <tt>:_destroy</tt> key set to a truthy value, + # then the existing record will be marked for destruction. def assign_nested_attributes_for_one_to_one_association(association_name, attributes) options = self.nested_attributes_options[association_name] attributes = attributes.with_indifferent_access + check_existing_record = (options[:update_only] || !attributes['id'].blank?) - if options[:update_only] - if existing_record = send(association_name) - assign_to_or_mark_for_destruction(existing_record, attributes, options[:allow_destroy]) + if check_existing_record && (record = send(association_name)) && + (options[:update_only] || record.id.to_s == attributes['id'].to_s) + assign_to_or_mark_for_destruction(record, attributes, options[:allow_destroy]) + elsif !reject_new_record?(association_name, attributes) + method = "build_#{association_name}" + if respond_to?(method) + send(method, attributes.except(*UNASSIGNABLE_KEYS)) else - unless reject_new_record?(association_name, attributes) - send("build_#{association_name}", attributes.except(*UNASSIGNABLE_KEYS)) - end - end - elsif attributes['id'].blank? - unless reject_new_record?(association_name, attributes) - method = "build_#{association_name}" - if respond_to?(method) - send(method, attributes.except(*UNASSIGNABLE_KEYS)) - else - raise ArgumentError, "Cannot build association #{association_name}. Are you trying to build a polymorphic one-to-one association?" - end + raise ArgumentError, "Cannot build association #{association_name}. Are you trying to build a polymorphic one-to-one association?" end - elsif (existing_record = send(association_name)) && existing_record.id.to_s == attributes['id'].to_s - assign_to_or_mark_for_destruction(existing_record, attributes, options[:allow_destroy]) end end diff --git a/activerecord/test/cases/nested_attributes_test.rb b/activerecord/test/cases/nested_attributes_test.rb index 5e4fc2b8d9..60c5bad225 100644 --- a/activerecord/test/cases/nested_attributes_test.rb +++ b/activerecord/test/cases/nested_attributes_test.rb @@ -247,34 +247,24 @@ class TestNestedAttributesOnAHasOneAssociation < ActiveRecord::TestCase end def test_should_accept_update_only_option - Pirate.accepts_nested_attributes_for :ship, :update_only => true - @pirate.update_attribute(:ship_attributes, { :id => @pirate.ship.id, :name => 'Mayflower' }) - - Pirate.accepts_nested_attributes_for :ship, :allow_destroy => true, :reject_if => proc { |attributes| attributes.empty? } + @pirate.update_attribute(:update_only_ship_attributes, { :id => @pirate.ship.id, :name => 'Mayflower' }) end def test_should_create_new_model_when_nothing_is_there_and_update_only_is_true - Pirate.accepts_nested_attributes_for :ship, :update_only => true @ship.delete - assert_difference('Ship.count', 1) do - @pirate.reload.update_attribute(:ship_attributes, { :name => 'Mayflower' }) + @pirate.reload.update_attribute(:update_only_ship_attributes, { :name => 'Mayflower' }) end - - Pirate.accepts_nested_attributes_for :ship, :allow_destroy => true, :reject_if => proc { |attributes| attributes.empty? } end - def test_should_update_existing_when_update_only_is_true_and_no_id_is_given - Pirate.accepts_nested_attributes_for :ship, :update_only => true + @ship.delete + @ship = @pirate.create_update_only_ship(:name => 'Nights Dirty Lightning') assert_no_difference('Ship.count') do - @pirate.reload.update_attributes(:ship_attributes => { :name => 'Mayflower' }) + @pirate.update_attributes(:update_only_ship_attributes => { :name => 'Mayflower' }) end - assert_equal 'Mayflower', @ship.reload.name - - Pirate.accepts_nested_attributes_for :ship, :allow_destroy => true, :reject_if => proc { |attributes| attributes.empty? } end end @@ -393,6 +383,27 @@ class TestNestedAttributesOnABelongsToAssociation < ActiveRecord::TestCase def test_should_automatically_enable_autosave_on_the_association assert Ship.reflect_on_association(:pirate).options[:autosave] end + + def test_should_accept_update_only_option + @ship.update_attribute(:update_only_pirate_attributes, { :id => @pirate.ship.id, :catchphrase => 'Arr' }) + end + + def test_should_create_new_model_when_nothing_is_there_and_update_only_is_true + @pirate.delete + assert_difference('Pirate.count', 1) do + @ship.reload.update_attribute(:update_only_pirate_attributes, { :catchphrase => 'Arr' }) + end + end + + def test_should_update_existing_when_update_only_is_true_and_no_id_is_given + @pirate.delete + @pirate = @ship.create_update_only_pirate(:catchphrase => 'Aye') + + assert_no_difference('Pirate.count') do + @ship.update_attributes(:update_only_pirate_attributes => { :catchphrase => 'Arr' }) + end + assert_equal 'Arr', @pirate.reload.catchphrase + end end module NestedAttributesOnACollectionAssociationTests diff --git a/activerecord/test/models/pirate.rb b/activerecord/test/models/pirate.rb index f2c05dd48f..88c1634717 100644 --- a/activerecord/test/models/pirate.rb +++ b/activerecord/test/models/pirate.rb @@ -19,6 +19,7 @@ class Pirate < ActiveRecord::Base # These both have :autosave enabled because accepts_nested_attributes_for is used on them. has_one :ship + has_one :update_only_ship, :class_name => 'Ship' has_one :non_validated_ship, :class_name => 'Ship' has_many :birds has_many :birds_with_method_callbacks, :class_name => "Bird", @@ -35,6 +36,7 @@ class Pirate < ActiveRecord::Base 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 :update_only_ship, :update_only => true accepts_nested_attributes_for :parrots_with_method_callbacks, :parrots_with_proc_callbacks, :birds_with_method_callbacks, :birds_with_proc_callbacks, :allow_destroy => true accepts_nested_attributes_for :birds_with_reject_all_blank, :reject_if => :all_blank diff --git a/activerecord/test/models/ship.rb b/activerecord/test/models/ship.rb index 06759d64b8..a96e38ab41 100644 --- a/activerecord/test/models/ship.rb +++ b/activerecord/test/models/ship.rb @@ -2,9 +2,11 @@ class Ship < ActiveRecord::Base self.record_timestamps = false belongs_to :pirate + belongs_to :update_only_pirate, :class_name => 'Pirate' has_many :parts, :class_name => 'ShipPart', :autosave => 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 validates_presence_of :name end |