Permalink
Browse files

Dirty attributes aren't cleared if save fails. [#174 state:resolved]

Signed-off-by: Jeremy Kemper <jeremy@bitsweat.net>
  • Loading branch information...
1 parent a425cd1 commit 593e21d6aedcc05d744be4996bd7180edce57efe @fxn fxn committed with jeremy May 12, 2008
View
16 activerecord/lib/active_record/dirty.rb
@@ -69,19 +69,19 @@ def changes
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
View
21 activerecord/test/cases/dirty_test.rb
@@ -78,7 +78,7 @@ def test_attribute_will_change!
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
@@ -115,6 +115,18 @@ def test_partial_update
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)
old = klass.partial_updates?
@@ -123,4 +135,11 @@ def with_partial_updates(klass, on = true)
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
View
2 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

0 comments on commit 593e21d

Please sign in to comment.