Returning false in before_destroy should cancel action. Closes #1829.

git-svn-id: http://svn-commit.rubyonrails.org/rails/trunk@2381 5ecf4fe2-1ee6-0310-87b1-e25e094e27de

activerecord/CHANGELOG

@@ -1,5 +1,7 @@
 *SVN*
 
+* Returning false from before_destroy should cancel the action.  #1829 [Jeremy Huffman]
+
 * Recognize PostgreSQL NOW() default as equivalent to CURRENT_TIMESTAMP or CURRENT_DATE, depending on the column's type.  #2256 [mat <mat@absolight.fr>]
 
 * Extensive documentation for the abstract database adapter.  #2250 [François Beausoleil <fbeausoleil@ftml.net>]

activerecord/lib/active_record/callbacks.rb

@@ -345,9 +345,9 @@ def callback(method)
           return false if result == false
         end
 
-        send(method) if respond_to_without_attributes?(method)
+        result = send(method) if respond_to_without_attributes?(method)
 
-        return true
+        return result
       end
 
       def callbacks_for(method)

activerecord/test/callbacks_test.rb

@@ -70,6 +70,25 @@ def on_after_save
   end
 end
 
+class ImmutableDeveloper < ActiveRecord::Base
+  set_table_name 'developers'
+
+  before_destroy :cancel_destroy
+
+  private
+
+  def cancel_destroy
+    return false
+  end
+end
+
+class ImmutableMethodDeveloper < ActiveRecord::Base
+  set_table_name 'developers'
+
+  def before_destroy    
+    return false
+  end
+end
 
 class CallbacksTest < Test::Unit::TestCase
   fixtures :developers
@@ -282,6 +301,24 @@ def test_delete
       [ :after_initialize,            :block  ],
     ], david.history
   end
+
+  def test_before_destroy_returning_false
+    david = ImmutableDeveloper.find(1)
+    devs = ImmutableDeveloper.find(:all).size
+    assert !david.destroy
+    # cancel_destroy returns false so the destruction should
+    # be cancelled
+    assert_equal ImmutableDeveloper.find(:all).size, devs
+
+    david = ImmutableMethodDeveloper.find(1)
+    devs = ImmutableMethodDeveloper.find(:all).size
+    assert !david.destroy
+    # before_destroy returns false so the destruction should
+    # be cancelled
+    assert_equal ImmutableMethodDeveloper.find(:all).size, devs
+  end
+
+
 
   def test_zzz_callback_returning_false # must be run last since we modify CallbackDeveloper
     david = CallbackDeveloper.find(1)