Permalink
Browse files

I added this feature so that a Map of changed fields could be retrieved

after a model had been saved. This is useful in the after_save callback
when you need to know what fields changed. At present there is no way
to do this other than have code in the before_save callback that takes
a copy of the changes Map, which I thought was a bit messy.

Example.

  person = Person.find_by_name('bob')
  person.name = 'robert'
  person.changes # => {'name' => ['bob, 'robert']}
  person.save
  person.changes # => {}
  person.previous_changes # => {'name' => ['bob, 'robert']}
  person.reload
  person.previous_changes # => {}

Signed-off-by: Joshua Peek <josh@joshpeek.com>
  • Loading branch information...
1 parent 57f7308 commit 80989437dc1502f9194b0600941b6d70a3efa3b2 @josh-m-sharpe josh-m-sharpe committed with josh Aug 31, 2009
@@ -72,12 +72,26 @@ def changes
changed.inject({}) { |h, attr| h[attr] = attribute_change(attr); h }
end
+ # Map of attributes that were changed when the model was saved.
+ # person.name # => 'bob'
+ # person.name = 'robert'
+ # person.save
+ # person.previous_changes # => {'name' => ['bob, 'robert']}
+ def previous_changes
+ previously_changed_attributes
+ end
+
private
# Map of change <tt>attr => original value</tt>.
def changed_attributes
@changed_attributes ||= {}
end
+ # Map of fields that were changed when the model was saved
+ def previously_changed_attributes
+ @previously_changed || {}
+ end
+
# Handle <tt>*_changed?</tt> for +method_missing+.
def attribute_changed?(attr)
changed_attributes.include?(attr)
@@ -19,19 +19,26 @@ module Dirty
# Attempts to +save+ the record and clears changed attributes if successful.
def save_with_dirty(*args) #:nodoc:
if status = save_without_dirty(*args)
+ @previously_changed = changes
changed_attributes.clear
end
status
end
# Attempts to <tt>save!</tt> the record and clears changed attributes if successful.
def save_with_dirty!(*args) #:nodoc:
- save_without_dirty!(*args).tap { changed_attributes.clear }
+ save_without_dirty!(*args).tap do
+ @previously_changed = changes
+ changed_attributes.clear
+ end
end
# <tt>reload</tt> the record and clears changed attributes.
def reload_with_dirty(*args) #:nodoc:
- reload_without_dirty(*args).tap { changed_attributes.clear }
+ reload_without_dirty(*args).tap do
+ previously_changed_attributes.clear
+ changed_attributes.clear
+ end
end
private
@@ -308,6 +308,84 @@ def test_save_should_not_save_serialized_attribute_with_partial_updates_if_not_p
end
end
+ def test_previous_changes
+ # original values should be in previous_changes
+ pirate = Pirate.new
+
+ assert_equal Hash.new, pirate.previous_changes
+ pirate.catchphrase = "arrr"
+ pirate.save!
+
+ assert_equal 4, pirate.previous_changes.size
+ assert_equal [nil, "arrr"], pirate.previous_changes['catchphrase']
+ assert_equal [nil, pirate.id], pirate.previous_changes['id']
+ assert_nil pirate.previous_changes['updated_on'][0]
+ assert_not_nil pirate.previous_changes['updated_on'][1]
+ assert_nil pirate.previous_changes['created_on'][0]
+ assert_not_nil pirate.previous_changes['created_on'][1]
+ assert !pirate.previous_changes.key?('parrot_id')
+
+ # original values should be in previous_changes
+ pirate = Pirate.new
+
+ assert_equal Hash.new, pirate.previous_changes
+ pirate.catchphrase = "arrr"
+ pirate.save
+
+ assert_equal 4, pirate.previous_changes.size
+ assert_equal [nil, "arrr"], pirate.previous_changes['catchphrase']
+ assert_equal [nil, pirate.id], pirate.previous_changes['id']
+ assert pirate.previous_changes.include?('updated_on')
+ assert pirate.previous_changes.include?('created_on')
+ assert !pirate.previous_changes.key?('parrot_id')
+
+ pirate.catchphrase = "Yar!!"
+ pirate.reload
+ assert_equal Hash.new, pirate.previous_changes
+
+ pirate = Pirate.find_by_catchphrase("arrr")
+ pirate.catchphrase = "Me Maties!"
+ pirate.save!
+
+ assert_equal 2, pirate.previous_changes.size
+ assert_equal ["arrr", "Me Maties!"], pirate.previous_changes['catchphrase']
+ assert_not_nil pirate.previous_changes['updated_on'][0]
+ assert_not_nil pirate.previous_changes['updated_on'][1]
+ assert !pirate.previous_changes.key?('parrot_id')
+ assert !pirate.previous_changes.key?('created_on')
+
+ pirate = Pirate.find_by_catchphrase("Me Maties!")
+ pirate.catchphrase = "Thar She Blows!"
+ pirate.save
+
+ assert_equal 2, pirate.previous_changes.size
+ assert_equal ["Me Maties!", "Thar She Blows!"], pirate.previous_changes['catchphrase']
+ assert_not_nil pirate.previous_changes['updated_on'][0]
+ assert_not_nil pirate.previous_changes['updated_on'][1]
+ assert !pirate.previous_changes.key?('parrot_id')
+ assert !pirate.previous_changes.key?('created_on')
+
+ pirate = Pirate.find_by_catchphrase("Thar She Blows!")
+ pirate.update_attributes(:catchphrase => "Ahoy!")
+
+ assert_equal 2, pirate.previous_changes.size
+ assert_equal ["Thar She Blows!", "Ahoy!"], pirate.previous_changes['catchphrase']
+ assert_not_nil pirate.previous_changes['updated_on'][0]
+ assert_not_nil pirate.previous_changes['updated_on'][1]
+ assert !pirate.previous_changes.key?('parrot_id')
+ assert !pirate.previous_changes.key?('created_on')
+
+ pirate = Pirate.find_by_catchphrase("Ahoy!")
+ pirate.update_attribute(:catchphrase, "Ninjas suck!")
+
+ assert_equal 2, pirate.previous_changes.size
+ assert_equal ["Ahoy!", "Ninjas suck!"], pirate.previous_changes['catchphrase']
+ assert_not_nil pirate.previous_changes['updated_on'][0]
+ assert_not_nil pirate.previous_changes['updated_on'][1]
+ assert !pirate.previous_changes.key?('parrot_id')
+ assert !pirate.previous_changes.key?('created_on')
+ end
+
private
def with_partial_updates(klass, on = true)
old = klass.partial_updates?

3 comments on commit 8098943

Thanks! I had a callback that saved the original attributes to workaround this problem.

wtn replied Sep 9, 2009

Thanks! Only yesterday I was daydreaming about using instance variables and callbacks to solve this problem, but this is better.

Good news. Time now to remove dirty code in models.

Please sign in to comment.