close #357; Timestamp attributes for update should get touched if a previous version is re-ified then saved

CHANGELOG.md

@@ -1,12 +1,14 @@
 ## 3.0.2 (Unreleased)
 
+  - [#357](https://github.com/airblade/paper_trail/issues/357) - If a `Version` instance is reified and then persisted at that state,
+    it's timestamp attributes for update should still get `touch`ed.
   - [#351](https://github.com/airblade/paper_trail/pull/351) / [#352](https://github.com/airblade/paper_trail/pull/352) -
     `PaperTrail::Rails::Controller` should hook into all controller types, and should not get loaded unless `ActionController` is.
   - [#346](https://github.com/airblade/paper_trail/pull/346) - `user_for_paper_trail` method should accommodate different types
     for return values from `current_user` method.
   - `PaperTrail::Cleaner.clean_versions!` should group versions by `PaperTrail.timestamp_field` when deciding which ones to
     keep / destroy, instead of always grouping by the `created_at` field.
-  - If a `Version` instance is reified and then re-persisted at that state, it's source version
+  - If a `Version` instance is reified and then persisted at that state, it's source version
     (`model_instance#version_association_name`, usually `model_instance#version`) will get cleared since persisting it causes it to
     become the live instance.
   - If `destroy` actions are tracked for a versioned model, invoking `destroy` on the model will cause the corresponding version that

lib/paper_trail/has_paper_trail.rb

@@ -72,8 +72,11 @@ module PaperTrail
 
         options_on = Array(options[:on]) # so that a single symbol can be passed in without wrapping it in an `Array`
         after_create  :record_create, :if => :save_version? if options_on.empty? || options_on.include?(:create)
-        before_update :record_update, :if => :save_version? if options_on.empty? || options_on.include?(:update)
-        after_update  :clear_version_instance if options_on.empty? || options_on.include?(:update)
+        if options_on.empty? || options_on.include?(:update)
+          before_save   :reset_timestamp_attrs_for_update_if_needed!, :on => :update
+          before_update :record_update, :if => :save_version?
+          after_update  :clear_version_instance!
+        end
         after_destroy :record_destroy, :if => :save_version? if options_on.empty? || options_on.include?(:destroy)
       end
 
@@ -266,12 +269,15 @@ module PaperTrail
 
       def record_update
         if paper_trail_switched_on? && changed_notably?
+          # reset_timestamp_attrs_for_update_if_needed!
+
           object_attrs = object_attrs_for_paper_trail(item_before_change)
           data = {
             :event     => paper_trail_event || 'update',
             :object    => self.class.paper_trail_version_class.object_col_is_json? ? object_attrs : PaperTrail.serializer.dump(object_attrs),
             :whodunnit => PaperTrail.whodunnit
           }
+
           if self.class.paper_trail_version_class.column_names.include?('object_changes')
             data[:object_changes] = self.class.paper_trail_version_class.object_changes_col_is_json? ? changes_for_paper_trail :
               PaperTrail.serializer.dump(changes_for_paper_trail)
@@ -287,10 +293,15 @@ module PaperTrail
       end
 
       # Invoked via `after_update` callback for when a previous version is reified and then saved
-      def clear_version_instance
+      def clear_version_instance!
         send("#{self.class.version_association_name}=", nil)
       end
 
+      def reset_timestamp_attrs_for_update_if_needed!
+        return if self.live? # invoked via callback when a user attempts to persist a reified `Version`
+        timestamp_attributes_for_update_in_model.each { |column| send("reset_#{column}!") }
+      end
+
       def record_destroy
         if paper_trail_switched_on? and not new_record?
           object_attrs = object_attrs_for_paper_trail(item_before_change)

spec/models/widget_spec.rb

@@ -22,6 +22,18 @@ describe Widget do
   end
 
   describe "Callbacks", :versioning => true do
+    describe :before_save do
+      context ':on => :update' do
+        before { widget.update_attributes!(:name => 'Foobar') }
+
+        subject { widget.versions.last.reify }
+
+        it "should reset the value for the timestamp attrs for update so that value gets updated properly" do
+          expect { subject.save }.to change(subject, :updated_at)
+        end
+      end
+    end
+
     describe :after_update do
       before { widget.update_attributes!(:name => 'Foobar') }