Merge pull request #3340 from surfacedamage/exclude_destroy_in_all_blank_check

Exclude _destroy parameter in :all_blank check (issue #2937)

3 files changed, 16 insertions, 2 deletions

diff --git a/activerecord/CHANGELOG b/activerecord/CHANGELOG
index 0f6a31d679..46076dac61 100644
--- a/activerecord/CHANGELOG
+++ b/activerecord/CHANGELOG
@@ -29,6 +29,11 @@
 
   [Andrés Mejía]
 
+* Fix nested attributes bug where _destroy parameter is taken into account
+  during :reject_if => :all_blank (fixes #2937)
+
+  [Aaron Christy]
+
 *Rails 3.1.1 (October 7, 2011)*
 
 * Add deprecation for the preload_associations method. Fixes #3022.
diff --git a/activerecord/lib/active_record/nested_attributes.rb b/activerecord/lib/active_record/nested_attributes.rb
index 2dbebfcaf8..d2065d701f 100644
--- a/activerecord/lib/active_record/nested_attributes.rb
+++ b/activerecord/lib/active_record/nested_attributes.rb
@@ -220,7 +220,7 @@ module ActiveRecord
     #     validates_presence_of :member
     #   end
     module ClassMethods
-      REJECT_ALL_BLANK_PROC = proc { |attributes| attributes.all? { |_, value| value.blank? } }
+      REJECT_ALL_BLANK_PROC = proc { |attributes| attributes.all? { |key, value| key == '_destroy' || value.blank? } }
 
       # Defines an attributes writer for the specified association(s). If you
       # are using <tt>attr_protected</tt> or <tt>attr_accessible</tt>, then you
@@ -239,7 +239,8 @@ module ActiveRecord
       #   is specified, a record will be built for all attribute hashes that
       #   do not have a <tt>_destroy</tt> value that evaluates to true.
       #   Passing <tt>:all_blank</tt> instead of a Proc will create a proc
-      #   that will reject a record where all the attributes are blank.
+      #   that will reject a record where all the attributes are blank excluding
+      #   any value for _destroy.
       # [:limit]
       #   Allows you to specify the maximum number of the associated records that
       #   can be processed with the nested attributes. If the size of the
diff --git a/activerecord/test/cases/nested_attributes_test.rb b/activerecord/test/cases/nested_attributes_test.rb
index 67a9ed6cd8..2ae9cb4888 100644
--- a/activerecord/test/cases/nested_attributes_test.rb
+++ b/activerecord/test/cases/nested_attributes_test.rb
@@ -45,6 +45,14 @@ class TestNestedAttributesInGeneral < ActiveRecord::TestCase
     end
   end
 
+  def test_should_not_build_a_new_record_using_reject_all_even_if_destroy_is_given
+    pirate = Pirate.create!(:catchphrase => "Don' botharrr talkin' like one, savvy?")
+    pirate.birds_with_reject_all_blank_attributes = [{:name => '', :color => '', :_destroy => '0'}]
+    pirate.save!
+
+    assert pirate.birds_with_reject_all_blank.empty?
+  end
+
   def test_should_not_build_a_new_record_if_reject_all_blank_returns_false
     pirate = Pirate.create!(:catchphrase => "Don' botharrr talkin' like one, savvy?")
     pirate.birds_with_reject_all_blank_attributes = [{:name => '', :color => ''}]