From 1f981220bd51082f49c4417cb604fc2a0b2dcdb6 Mon Sep 17 00:00:00 2001
From: Xavier Noria <fxn@hashref.com>
Date: Tue, 13 May 2008 00:46:57 +0200
Subject: [PATCH] dirty objects no longer clear changed attributes after a save failure

---
 activerecord/lib/active_record/dirty.rb |   16 ++++++++--------
 activerecord/test/cases/dirty_test.rb   |   21 ++++++++++++++++++++-
 activerecord/test/models/pirate.rb      |    2 ++
 3 files changed, 30 insertions(+), 9 deletions(-)

diff --git a/activerecord/lib/active_record/dirty.rb b/activerecord/lib/active_record/dirty.rb
index c6d89e3..6034963 100644
--- a/activerecord/lib/active_record/dirty.rb
+++ b/activerecord/lib/active_record/dirty.rb
@@ -69,19 +69,19 @@ module ActiveRecord
       changed.inject({}) { |h, attr| h[attr] = attribute_change(attr); h }
     end
 
-
-    # Clear changed attributes after they are saved.
+    # Attempts to +save+ the record and clears changed attributes if successful.
     def save_with_dirty(*args) #:nodoc:
-      save_without_dirty(*args)
-    ensure
-      changed_attributes.clear
+      if status = save_without_dirty(*args)
+        changed_attributes.clear
+      end
+      status
     end
 
-    # Clear changed attributes after they are saved.
+    # Attempts to <tt>save!</tt> the record and clears changed attributes if successful.
     def save_with_dirty!(*args) #:nodoc:
-      save_without_dirty!(*args)
-    ensure
+      status = save_without_dirty!(*args)
       changed_attributes.clear
+      status
     end
 
     private
diff --git a/activerecord/test/cases/dirty_test.rb b/activerecord/test/cases/dirty_test.rb
index 7412e63..0f917fd 100644
--- a/activerecord/test/cases/dirty_test.rb
+++ b/activerecord/test/cases/dirty_test.rb
@@ -78,7 +78,7 @@ class DirtyTest < ActiveRecord::TestCase
   end
 
   def test_association_assignment_changes_foreign_key
-    pirate = Pirate.create!
+    pirate = Pirate.create!(:catchphrase => 'jarl')
     pirate.parrot = Parrot.create!
     assert pirate.changed?
     assert_equal %w(parrot_id), pirate.changed
@@ -114,6 +114,18 @@ class DirtyTest < ActiveRecord::TestCase
       assert_not_equal old_updated_on, pirate.reload.updated_on
     end
   end
+  
+  def test_changed_attributes_should_be_preserved_if_save_failure
+    pirate = Pirate.new
+    pirate.parrot_id = 1
+    assert !pirate.save
+    check_pirate_after_save_failure(pirate)
+    
+    pirate = Pirate.new
+    pirate.parrot_id = 1
+    assert_raises(ActiveRecord::RecordInvalid) { pirate.save! }
+    check_pirate_after_save_failure(pirate)
+  end
 
   private
     def with_partial_updates(klass, on = true)
@@ -123,4 +135,11 @@ class DirtyTest < ActiveRecord::TestCase
     ensure
       klass.partial_updates = old
     end
+    
+    def check_pirate_after_save_failure(pirate)
+      assert pirate.changed?
+      assert pirate.parrot_id_changed?
+      assert_equal %w(parrot_id), pirate.changed
+      assert_nil pirate.parrot_id_was
+    end      
 end
diff --git a/activerecord/test/models/pirate.rb b/activerecord/test/models/pirate.rb
index bb4d02c..43358aa 100644
--- a/activerecord/test/models/pirate.rb
+++ b/activerecord/test/models/pirate.rb
@@ -4,4 +4,6 @@ class Pirate < ActiveRecord::Base
   has_many :treasures, :as => :looter
 
   has_many :treasure_estimates, :through => :treasures, :source => :price_estimates
+  
+  validates_presence_of :catchphrase
 end
-- 
1.5.4.1