diff --git a/activerecord/lib/active_record/nested_attributes.rb b/activerecord/lib/active_record/nested_attributes.rb index 9b4b954427690..b583e62a8292a 100644 --- a/activerecord/lib/active_record/nested_attributes.rb +++ b/activerecord/lib/active_record/nested_attributes.rb @@ -62,10 +62,10 @@ def self.included(base) # accepts_nested_attributes_for :avatar, :allow_destroy => true # end # - # Now, when you add the _delete key to the attributes hash, with a + # Now, when you add the _destroy key to the attributes hash, with a # value that evaluates to +true+, you will destroy the associated model: # - # member.avatar_attributes = { :id => '2', :_delete => '1' } + # member.avatar_attributes = { :id => '2', :_destroy => '1' } # member.avatar.marked_for_destruction? # => true # member.save # member.avatar #=> nil @@ -85,14 +85,14 @@ def self.included(base) # the attribute hash. # # For each hash that does _not_ have an id key a new record will - # be instantiated, unless the hash also contains a _delete key + # be instantiated, unless the hash also contains a _destroy key # that evaluates to +true+. # # params = { :member => { # :name => 'joe', :posts_attributes => [ # { :title => 'Kari, the awesome Ruby documentation browser!' }, # { :title => 'The egalitarian assumption of the modern citizen' }, - # { :title => '', :_delete => '1' } # this will be ignored + # { :title => '', :_destroy => '1' } # this will be ignored # ] # }} # @@ -140,7 +140,7 @@ def self.included(base) # By default the associated records are protected from being destroyed. If # you want to destroy any of the associated records through the attributes # hash, you have to enable it first using the :allow_destroy - # option. This will allow you to also use the _delete key to + # option. This will allow you to also use the _destroy key to # destroy existing records: # # class Member < ActiveRecord::Base @@ -149,7 +149,7 @@ def self.included(base) # end # # params = { :member => { - # :posts_attributes => [{ :id => '2', :_delete => '1' }] + # :posts_attributes => [{ :id => '2', :_destroy => '1' }] # }} # # member.attributes = params['member'] @@ -172,14 +172,14 @@ module ClassMethods # Supported options: # [:allow_destroy] # If true, destroys any members from the attributes hash with a - # _delete key and a value that evaluates to +true+ + # _destroy key and a value that evaluates to +true+ # (eg. 1, '1', true, or 'true'). This option is off by default. # [:reject_if] # Allows you to specify a Proc that checks whether a record should be # built for a certain attribute hash. The hash is passed to the Proc # and the Proc should return either +true+ or +false+. When no Proc - # is specified a record will be built for all attribute hashes that - # do not have a _delete that evaluates to true. + # is specified, a record will be built for all attribute hashes that + # do not have a _destroy value that evaluates to true. # # Examples: # # creates avatar_attributes= @@ -223,15 +223,25 @@ def #{association_name}_attributes=(attributes) # destruction of this association. # # See ActionView::Helpers::FormHelper::fields_for for more info. - def _delete + def _destroy marked_for_destruction? end + # Deal with deprecated _delete. + # + def _delete #:nodoc: + ActiveSupport::Deprecation.warn "_delete is deprecated in nested attributes. Use _destroy instead." + _destroy + end + private # Attribute hash keys that should not be assigned as normal attributes. # These hash keys are nested attributes implementation details. - UNASSIGNABLE_KEYS = %w{ id _delete } + # + # TODO Remove _delete from UNASSIGNABLE_KEYS when deprecation warning are + # removed. + UNASSIGNABLE_KEYS = %w( id _destroy _delete ) # Assigns the given attributes to the association. # @@ -240,7 +250,7 @@ def _delete # record will be built. # # If the given attributes include a matching :id attribute _and_ a - # :_delete key set to a truthy value, then the existing record + # :_destroy 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, allow_destroy) attributes = attributes.stringify_keys @@ -264,7 +274,7 @@ def assign_nested_attributes_for_one_to_one_association(association_name, attrib # Hashes with an :id value matching an existing associated record # will update that record. Hashes without an :id value will build # a new record for the association. Hashes with a matching :id - # value and a :_delete key set to a truthy value will mark the + # value and a :_destroy key set to a truthy value will mark the # matched record for destruction. # # For example: @@ -272,7 +282,7 @@ def assign_nested_attributes_for_one_to_one_association(association_name, attrib # assign_nested_attributes_for_collection_association(:people, { # '1' => { :id => '1', :name => 'Peter' }, # '2' => { :name => 'John' }, - # '3' => { :id => '2', :_delete => true } + # '3' => { :id => '2', :_destroy => true } # }) # # Will update the name of the Person with ID 1, build a new associated @@ -284,7 +294,7 @@ def assign_nested_attributes_for_one_to_one_association(association_name, attrib # assign_nested_attributes_for_collection_association(:people, [ # { :id => '1', :name => 'Peter' }, # { :name => 'John' }, - # { :id => '2', :_delete => true } + # { :id => '2', :_destroy => true } # ]) def assign_nested_attributes_for_collection_association(association_name, attributes_collection, allow_destroy) unless attributes_collection.is_a?(Hash) || attributes_collection.is_a?(Array) @@ -309,25 +319,26 @@ def assign_nested_attributes_for_collection_association(association_name, attrib end # Updates a record with the +attributes+ or marks it for destruction if - # +allow_destroy+ is +true+ and has_delete_flag? returns +true+. + # +allow_destroy+ is +true+ and has_destroy_flag? returns +true+. def assign_to_or_mark_for_destruction(record, attributes, allow_destroy) - if has_delete_flag?(attributes) && allow_destroy + if has_destroy_flag?(attributes) && allow_destroy record.mark_for_destruction else record.attributes = attributes.except(*UNASSIGNABLE_KEYS) end end - # Determines if a hash contains a truthy _delete key. - def has_delete_flag?(hash) - ConnectionAdapters::Column.value_to_boolean hash['_delete'] + # Determines if a hash contains a truthy _destroy key. + def has_destroy_flag?(hash) + ConnectionAdapters::Column.value_to_boolean(hash['_destroy']) || + ConnectionAdapters::Column.value_to_boolean(hash['_delete']) # TODO Remove after deprecation. end # Determines if a new record should be build by checking for - # has_delete_flag? or if a :reject_if proc exists for this + # has_destroy_flag? or if a :reject_if proc exists for this # association and evaluates to +true+. def reject_new_record?(association_name, attributes) - has_delete_flag?(attributes) || + has_destroy_flag?(attributes) || self.class.reject_new_nested_attributes_procs[association_name].try(:call, attributes) end end diff --git a/activerecord/test/cases/nested_attributes_test.rb b/activerecord/test/cases/nested_attributes_test.rb index 7acdf43b2d657..792d6d1944888 100644 --- a/activerecord/test/cases/nested_attributes_test.rb +++ b/activerecord/test/cases/nested_attributes_test.rb @@ -49,15 +49,21 @@ def test_should_disable_allow_destroy_by_default ship = pirate.create_ship(:name => 'Nights Dirty Lightning') assert_no_difference('Ship.count') do - pirate.update_attributes(:ship_attributes => { '_delete' => true }) + pirate.update_attributes(:ship_attributes => { '_destroy' => true }) end end - def test_a_model_should_respond_to_underscore_delete_and_return_if_it_is_marked_for_destruction + def test_a_model_should_respond_to_underscore_destroy_and_return_if_it_is_marked_for_destruction ship = Ship.create!(:name => 'Nights Dirty Lightning') - assert !ship._delete + assert !ship._destroy ship.mark_for_destruction - assert ship._delete + assert ship._destroy + end + + def test_underscore_delete_is_deprecated + ActiveSupport::Deprecation.expects(:warn) + ship = Ship.create!(:name => 'Nights Dirty Lightning') + ship._delete end end @@ -87,9 +93,9 @@ def test_should_build_a_new_record_if_there_is_no_id assert_equal 'Davy Jones Gold Dagger', @pirate.ship.name end - def test_should_not_build_a_new_record_if_there_is_no_id_and_delete_is_truthy + def test_should_not_build_a_new_record_if_there_is_no_id_and_destroy_is_truthy @ship.destroy - @pirate.reload.ship_attributes = { :name => 'Davy Jones Gold Dagger', :_delete => '1' } + @pirate.reload.ship_attributes = { :name => 'Davy Jones Gold Dagger', :_destroy => '1' } assert_nil @pirate.ship end @@ -109,8 +115,8 @@ def test_should_replace_an_existing_record_if_there_is_no_id assert_equal 'Nights Dirty Lightning', @ship.name end - def test_should_not_replace_an_existing_record_if_there_is_no_id_and_delete_is_truthy - @pirate.reload.ship_attributes = { :name => 'Davy Jones Gold Dagger', :_delete => '1' } + def test_should_not_replace_an_existing_record_if_there_is_no_id_and_destroy_is_truthy + @pirate.reload.ship_attributes = { :name => 'Davy Jones Gold Dagger', :_destroy => '1' } assert_equal @ship, @pirate.ship assert_equal 'Nights Dirty Lightning', @pirate.ship.name @@ -137,29 +143,29 @@ def test_should_modify_an_existing_record_if_there_is_a_matching_composite_id assert_equal 'Davy Jones Gold Dagger', @pirate.ship.name end - def test_should_delete_an_existing_record_if_there_is_a_matching_id_and_delete_is_truthy + def test_should_destroy_an_existing_record_if_there_is_a_matching_id_and_destroy_is_truthy @pirate.ship.destroy [1, '1', true, 'true'].each do |truth| @pirate.reload.create_ship(:name => 'Mister Pablo') assert_difference('Ship.count', -1) do - @pirate.update_attribute(:ship_attributes, { :id => @pirate.ship.id, :_delete => truth }) + @pirate.update_attribute(:ship_attributes, { :id => @pirate.ship.id, :_destroy => truth }) end end end - def test_should_not_delete_an_existing_record_if_delete_is_not_truthy + def test_should_not_destroy_an_existing_record_if_destroy_is_not_truthy [nil, '0', 0, 'false', false].each do |not_truth| assert_no_difference('Ship.count') do - @pirate.update_attribute(:ship_attributes, { :id => @pirate.ship.id, :_delete => not_truth }) + @pirate.update_attribute(:ship_attributes, { :id => @pirate.ship.id, :_destroy => not_truth }) end end end - def test_should_not_delete_an_existing_record_if_allow_destroy_is_false + def test_should_not_destroy_an_existing_record_if_allow_destroy_is_false Pirate.accepts_nested_attributes_for :ship, :allow_destroy => false, :reject_if => proc { |attributes| attributes.empty? } assert_no_difference('Ship.count') do - @pirate.update_attribute(:ship_attributes, { :id => @pirate.ship.id, :_delete => '1' }) + @pirate.update_attribute(:ship_attributes, { :id => @pirate.ship.id, :_destroy => '1' }) end Pirate.accepts_nested_attributes_for :ship, :allow_destroy => true, :reject_if => proc { |attributes| attributes.empty? } @@ -182,7 +188,7 @@ def test_should_work_with_update_attributes_as_well def test_should_not_destroy_the_associated_model_until_the_parent_is_saved assert_no_difference('Ship.count') do - @pirate.attributes = { :ship_attributes => { :id => @ship.id, :_delete => '1' } } + @pirate.attributes = { :ship_attributes => { :id => @ship.id, :_destroy => '1' } } end assert_difference('Ship.count', -1) do @pirate.save @@ -213,9 +219,9 @@ def test_should_build_a_new_record_if_there_is_no_id assert_equal 'Arr', @ship.pirate.catchphrase end - def test_should_not_build_a_new_record_if_there_is_no_id_and_delete_is_truthy + def test_should_not_build_a_new_record_if_there_is_no_id_and_destroy_is_truthy @pirate.destroy - @ship.reload.pirate_attributes = { :catchphrase => 'Arr', :_delete => '1' } + @ship.reload.pirate_attributes = { :catchphrase => 'Arr', :_destroy => '1' } assert_nil @ship.pirate end @@ -235,8 +241,8 @@ def test_should_replace_an_existing_record_if_there_is_no_id assert_equal 'Aye', @pirate.catchphrase end - def test_should_not_replace_an_existing_record_if_there_is_no_id_and_delete_is_truthy - @ship.reload.pirate_attributes = { :catchphrase => 'Arr', :_delete => '1' } + def test_should_not_replace_an_existing_record_if_there_is_no_id_and_destroy_is_truthy + @ship.reload.pirate_attributes = { :catchphrase => 'Arr', :_destroy => '1' } assert_equal @pirate, @ship.pirate assert_equal 'Aye', @ship.pirate.catchphrase @@ -263,29 +269,29 @@ def test_should_modify_an_existing_record_if_there_is_a_matching_composite_id assert_equal 'Arr', @ship.pirate.catchphrase end - def test_should_delete_an_existing_record_if_there_is_a_matching_id_and_delete_is_truthy + def test_should_destroy_an_existing_record_if_there_is_a_matching_id_and_destroy_is_truthy @ship.pirate.destroy [1, '1', true, 'true'].each do |truth| @ship.reload.create_pirate(:catchphrase => 'Arr') assert_difference('Pirate.count', -1) do - @ship.update_attribute(:pirate_attributes, { :id => @ship.pirate.id, :_delete => truth }) + @ship.update_attribute(:pirate_attributes, { :id => @ship.pirate.id, :_destroy => truth }) end end end - def test_should_not_delete_an_existing_record_if_delete_is_not_truthy + def test_should_not_destroy_an_existing_record_if_destroy_is_not_truthy [nil, '0', 0, 'false', false].each do |not_truth| assert_no_difference('Pirate.count') do - @ship.update_attribute(:pirate_attributes, { :id => @ship.pirate.id, :_delete => not_truth }) + @ship.update_attribute(:pirate_attributes, { :id => @ship.pirate.id, :_destroy => not_truth }) end end end - def test_should_not_delete_an_existing_record_if_allow_destroy_is_false + def test_should_not_destroy_an_existing_record_if_allow_destroy_is_false Ship.accepts_nested_attributes_for :pirate, :allow_destroy => false, :reject_if => proc { |attributes| attributes.empty? } assert_no_difference('Pirate.count') do - @ship.update_attribute(:pirate_attributes, { :id => @ship.pirate.id, :_delete => '1' }) + @ship.update_attribute(:pirate_attributes, { :id => @ship.pirate.id, :_destroy => '1' }) end Ship.accepts_nested_attributes_for :pirate, :allow_destroy => true, :reject_if => proc { |attributes| attributes.empty? } @@ -301,7 +307,7 @@ def test_should_work_with_update_attributes_as_well def test_should_not_destroy_the_associated_model_until_the_parent_is_saved assert_no_difference('Pirate.count') do - @ship.attributes = { :pirate_attributes => { :id => @ship.pirate.id, '_delete' => true } } + @ship.attributes = { :pirate_attributes => { :id => @ship.pirate.id, '_destroy' => true } } end assert_difference('Pirate.count', -1) { @ship.save } end @@ -369,18 +375,18 @@ def test_should_automatically_build_new_associated_models_for_each_entry_in_a_ha assert_equal 'Privateers Greed', @pirate.send(@association_name).last.name end - def test_should_not_assign_delete_key_to_a_record + def test_should_not_assign_destroy_key_to_a_record assert_nothing_raised ActiveRecord::UnknownAttributeError do - @pirate.send(association_setter, { 'foo' => { '_delete' => '0' }}) + @pirate.send(association_setter, { 'foo' => { '_destroy' => '0' }}) end end - def test_should_ignore_new_associated_records_with_truthy_delete_attribute + def test_should_ignore_new_associated_records_with_truthy_destroy_attribute @pirate.send(@association_name).destroy_all @pirate.reload.attributes = { association_getter => { 'foo' => { :name => 'Grace OMalley' }, - 'bar' => { :name => 'Privateers Greed', '_delete' => '1' } + 'bar' => { :name => 'Privateers Greed', '_destroy' => '1' } } } @@ -432,7 +438,7 @@ def test_should_be_possible_to_destroy_a_record ['1', 1, 'true', true].each do |true_variable| record = @pirate.reload.send(@association_name).create!(:name => 'Grace OMalley') @pirate.send(association_setter, - @alternate_params[association_getter].merge('baz' => { :id => record.id, '_delete' => true_variable }) + @alternate_params[association_getter].merge('baz' => { :id => record.id, '_destroy' => true_variable }) ) assert_difference('@pirate.send(@association_name).count', -1) do @@ -443,7 +449,7 @@ def test_should_be_possible_to_destroy_a_record def test_should_not_destroy_the_associated_model_with_a_non_truthy_argument [nil, '', '0', 0, 'false', false].each do |false_variable| - @alternate_params[association_getter]['foo']['_delete'] = false_variable + @alternate_params[association_getter]['foo']['_destroy'] = false_variable assert_no_difference('@pirate.send(@association_name).count') do @pirate.update_attributes(@alternate_params) end @@ -452,7 +458,7 @@ def test_should_not_destroy_the_associated_model_with_a_non_truthy_argument def test_should_not_destroy_the_associated_model_until_the_parent_is_saved assert_no_difference('@pirate.send(@association_name).count') do - @pirate.send(association_setter, @alternate_params[association_getter].merge('baz' => { :id => @child_1.id, '_delete' => true })) + @pirate.send(association_setter, @alternate_params[association_getter].merge('baz' => { :id => @child_1.id, '_destroy' => true })) end assert_difference('@pirate.send(@association_name).count', -1) { @pirate.save } end