From caabed6c76eea0db99949f34c234ef1b2657392a Mon Sep 17 00:00:00 2001
From: James Miller <bensie@gmail.com>
Date: Wed, 13 Feb 2013 12:27:06 -0800
Subject: Don't call after_commit when creating through an association and save
 fails, fixes #5802

---
 activerecord/CHANGELOG.md                          |  5 ++++
 activerecord/lib/active_record/transactions.rb     |  5 +++-
 .../test/cases/transaction_callbacks_test.rb       | 27 ++++++++++++++++++++++
 3 files changed, 36 insertions(+), 1 deletion(-)

diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md
index db21d323f6..4fa37cc5c2 100644
--- a/activerecord/CHANGELOG.md
+++ b/activerecord/CHANGELOG.md
@@ -1,5 +1,10 @@
 ## Rails 4.0.0 (unreleased) ##
 
+*   Don't run after_commit callback when creating through an association
+    if saving the record fails.
+
+    *James Miller *
+
 *   Allow store accessors to be overrided like other attribute methods, e.g.:
 
         class User < ActiveRecord::Base
diff --git a/activerecord/lib/active_record/transactions.rb b/activerecord/lib/active_record/transactions.rb
index 4a608e4f7b..4b7a388dc7 100644
--- a/activerecord/lib/active_record/transactions.rb
+++ b/activerecord/lib/active_record/transactions.rb
@@ -286,8 +286,11 @@ module ActiveRecord
     end
 
     # Call the after_commit callbacks
+    #
+    # Ensure that it is not called if the object was never persisted (failed create),
+    # but call it after the commit of a destroyed object
     def committed! #:nodoc:
-      run_callbacks :commit
+      run_callbacks :commit if destroyed? || persisted?
     ensure
       clear_transaction_record_state
     end
diff --git a/activerecord/test/cases/transaction_callbacks_test.rb b/activerecord/test/cases/transaction_callbacks_test.rb
index 869892e33f..fd5651b4e0 100644
--- a/activerecord/test/cases/transaction_callbacks_test.rb
+++ b/activerecord/test/cases/transaction_callbacks_test.rb
@@ -5,9 +5,29 @@ class TransactionCallbacksTest < ActiveRecord::TestCase
   self.use_transactional_fixtures = false
   fixtures :topics
 
+  class ReplyWithCallbacks < ActiveRecord::Base
+    self.table_name = :topics
+
+    belongs_to :topic, foreign_key: "parent_id"
+
+    validates_presence_of :content
+
+    after_commit :do_after_commit, on: :create
+
+    def history
+      @history ||= []
+    end
+
+    def do_after_commit
+      history << :commit_on_create
+    end
+  end
+
   class TopicWithCallbacks < ActiveRecord::Base
     self.table_name = :topics
 
+    has_many :replies, class_name: "ReplyWithCallbacks", foreign_key: "parent_id"
+
     after_commit{|record| record.send(:do_after_commit, nil)}
     after_commit(:on => :create){|record| record.send(:do_after_commit, :create)}
     after_commit(:on => :update){|record| record.send(:do_after_commit, :update)}
@@ -93,6 +113,13 @@ class TransactionCallbacksTest < ActiveRecord::TestCase
     assert_equal [:commit_on_create], @new_record.history
   end
 
+  def test_only_call_after_commit_on_create_after_transaction_commits_for_new_record_if_create_succeeds_creating_through_association
+    topic = TopicWithCallbacks.create!(:title => "New topic", :written_on => Date.today)
+    reply = topic.replies.create
+
+    assert_equal [], reply.history
+  end
+
   def test_call_after_rollback_after_transaction_rollsback
     @first.after_commit_block{|r| r.history << :after_commit}
     @first.after_rollback_block{|r| r.history << :after_rollback}
-- 
cgit v1.2.3