From 12b0b26df7560ab5199ba830586864085441508f Mon Sep 17 00:00:00 2001
From: Roque Pinel <repinel@gmail.com>
Date: Sun, 19 Jul 2015 22:00:36 -0400
Subject: [PATCH] Fix state being carried over from previous transaction

This clears the transaction record state when the transaction finishes
with a `:committed` status.

Considering the following example where `name` is a required attribute.
Before we had `new_record?` returning `true` for a persisted record:

```ruby
  author = Author.create! name: 'foo'
  author.name = nil
  author.save        # => false
  author.new_record? # => true
```
---
 .../parameters/parameters_permit_test.rb           |  2 +-
 activerecord/CHANGELOG.md                          | 14 ++++++++++++++
 activerecord/lib/active_record/transactions.rb     |  4 ++++
 activerecord/test/cases/transactions_test.rb       |  7 +++++++
 4 files changed, 26 insertions(+), 1 deletion(-)

diff --git a/actionpack/test/controller/parameters/parameters_permit_test.rb b/actionpack/test/controller/parameters/parameters_permit_test.rb
index 2dd2826196136..9f7d14e85dfd1 100644
--- a/actionpack/test/controller/parameters/parameters_permit_test.rb
+++ b/actionpack/test/controller/parameters/parameters_permit_test.rb
@@ -199,7 +199,7 @@ def assert_filtered_out(params, key)
     assert_equal nil, @params.fetch(:foo) { nil }
   end
 
-  test 'KeyError in fetch block should not be coverd up' do
+  test 'KeyError in fetch block should not be covered up' do
     params = ActionController::Parameters.new
     e = assert_raises(KeyError) do
       params.fetch(:missing_key) { {}.fetch(:also_missing) }
diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md
index aa2438fb3f7f5..d9aa703d19b6c 100644
--- a/activerecord/CHANGELOG.md
+++ b/activerecord/CHANGELOG.md
@@ -1,3 +1,17 @@
+*   Fix state being carried over from previous transaction.
+
+    Considering the following example where `name` is a required attribute.
+    Before we had `new_record?` returning `true` for a persisted record:
+
+        author = Author.create! name: 'foo'
+        author.name = nil
+        author.save        # => false
+        author.new_record? # => true
+
+    Fixes #20824.
+
+    *Roque Pinel*
+
 *   Correctly ignore `mark_for_destruction` when `autosave` isn't set to `true`
     when validating associations.
 
diff --git a/activerecord/lib/active_record/transactions.rb b/activerecord/lib/active_record/transactions.rb
index 267ac26c79b82..887d7a590351d 100644
--- a/activerecord/lib/active_record/transactions.rb
+++ b/activerecord/lib/active_record/transactions.rb
@@ -379,6 +379,10 @@ def with_transaction_returning_status
         raise ActiveRecord::Rollback unless status
       end
       status
+    ensure
+      if @transaction_state && @transaction_state.committed?
+        clear_transaction_record_state
+      end
     end
 
     protected
diff --git a/activerecord/test/cases/transactions_test.rb b/activerecord/test/cases/transactions_test.rb
index 4e163ca4a6444..29a6ec75223e1 100644
--- a/activerecord/test/cases/transactions_test.rb
+++ b/activerecord/test/cases/transactions_test.rb
@@ -175,6 +175,13 @@ def topic.after_save_for_transaction
     assert topic.new_record?, "#{topic.inspect} should be new record"
   end
 
+  def test_transaction_state_is_cleared_when_record_is_persisted
+    author = Author.create! name: 'foo'
+    author.name = nil
+    assert_not author.save
+    assert_not author.new_record?
+  end
+
   def test_update_should_rollback_on_failure
     author = Author.find(1)
     posts_count = author.posts.size