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

3 files changed, 41 insertions, 2 deletions

diff --git a/activerecord/CHANGELOG b/activerecord/CHANGELOG
index dafcdbea52..ada277460d 100644
--- a/activerecord/CHANGELOG
+++ b/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>]
diff --git a/activerecord/lib/active_record/callbacks.rb b/activerecord/lib/active_record/callbacks.rb
index 8e3aac6e36..fb428c3f31 100755
--- a/activerecord/lib/active_record/callbacks.rb
+++ b/activerecord/lib/active_record/callbacks.rb
@@ -345,9 +345,9 @@ module ActiveRecord
           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)
diff --git a/activerecord/test/callbacks_test.rb b/activerecord/test/callbacks_test.rb
index d915b3a875..ea522ca7ac 100644
--- a/activerecord/test/callbacks_test.rb
+++ b/activerecord/test/callbacks_test.rb
@@ -70,6 +70,25 @@ class RecursiveCallbackDeveloper < ActiveRecord::Base
   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 @@ class CallbacksTest < Test::Unit::TestCase
       [ :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)