From b4cd820322ba0380aeda75354704c2fcf2fd8fb7 Mon Sep 17 00:00:00 2001
From: Michael Lovitt <michael@lovitt.net>
Date: Fri, 9 Jan 2009 18:09:50 -0500
Subject: [PATCH] Fixed broken after_save callback; was being called when before_create was canceled or before_update was canceled

---
 activerecord/lib/active_record/callbacks.rb |    5 ++-
 activerecord/test/cases/callbacks_test.rb   |   44 +++++++++++++++++++++++++--
 2 files changed, 44 insertions(+), 5 deletions(-)

diff --git a/activerecord/lib/active_record/callbacks.rb b/activerecord/lib/active_record/callbacks.rb
index 42bfe34..9f5384d 100644
--- a/activerecord/lib/active_record/callbacks.rb
+++ b/activerecord/lib/active_record/callbacks.rb
@@ -219,8 +219,9 @@ module ActiveRecord
     def after_save()  end
     def create_or_update_with_callbacks #:nodoc:
       return false if callback(:before_save) == false
-      result = create_or_update_without_callbacks
-      callback(:after_save)
+      if result = create_or_update_without_callbacks
+        callback(:after_save)
+      end
       result
     end
     private :create_or_update_with_callbacks
diff --git a/activerecord/test/cases/callbacks_test.rb b/activerecord/test/cases/callbacks_test.rb
index 11830a2..8d357da 100644
--- a/activerecord/test/cases/callbacks_test.rb
+++ b/activerecord/test/cases/callbacks_test.rb
@@ -121,9 +121,19 @@ end
 
 class CallbackCancellationDeveloper < ActiveRecord::Base
   set_table_name 'developers'
-  def before_create
-    false
-  end
+  
+  attr_reader   :after_save_called, :after_create_called, :after_update_called, :after_destroy_called
+  attr_accessor :cancel_before_save, :cancel_before_create, :cancel_before_update, :cancel_before_destroy
+  
+  def before_save;    !@cancel_before_save;    end
+  def before_create;  !@cancel_before_create;  end
+  def before_update;  !@cancel_before_update;  end
+  def before_destroy; !@cancel_before_destroy; end
+  
+  def after_save;      @after_save_called    = true; end
+  def after_update;    @after_update_called  = true; end
+  def after_create;    @after_create_called  = true; end
+  def after_destroy;   @after_destroy_called = true; end
 end
 
 class CallbacksTest < ActiveRecord::TestCase
@@ -349,19 +359,47 @@ class CallbacksTest < ActiveRecord::TestCase
     assert !david.valid?
     assert !david.save
     assert_raises(ActiveRecord::RecordInvalid) { david.save! }
+    
+    someone = CallbackCancellationDeveloper.find(1)
+    someone.cancel_before_save = true
+    assert someone.valid?
+    assert !someone.save
+    assert_save_callbacks_not_called(someone)
   end
 
   def test_before_create_returning_false
     someone = CallbackCancellationDeveloper.new
+    someone.cancel_before_create = true
     assert someone.valid?
     assert !someone.save
+    assert_save_callbacks_not_called(someone)
+  end
+
+  def test_before_update_returning_false
+    someone = CallbackCancellationDeveloper.find(1)
+    someone.cancel_before_update = true
+    assert someone.valid?
+    assert !someone.save
+    assert_save_callbacks_not_called(someone)
   end
 
   def test_before_destroy_returning_false
     david = ImmutableDeveloper.find(1)
     assert !david.destroy
     assert_not_nil ImmutableDeveloper.find_by_id(1)
+    
+    someone = CallbackCancellationDeveloper.find(1)
+    someone.cancel_before_destroy = true
+    assert !someone.destroy
+    assert !someone.after_destroy_called
+  end
+  
+  def assert_save_callbacks_not_called(someone)
+    assert !someone.after_save_called
+    assert !someone.after_create_called
+    assert !someone.after_update_called    
   end
+  private :assert_save_callbacks_not_called
 
   def test_zzz_callback_returning_false # must be run last since we modify CallbackDeveloper
     david = CallbackDeveloper.find(1)
-- 
1.5.6