Respect :skip re: timestamp-only updates

When attributes are ignored, either by `:ignore` or by `:skip`,
then during updates, timestamp attributes like `updated_at` should
not be considered notable changes.

[Fixes #569]

lib/paper_trail/has_paper_trail.rb

@@ -469,18 +469,27 @@ def object_attrs_for_paper_trail(attributes_hash)
         attrs
       end
 
-      # This method is invoked in order to determine whether it is appropriate to generate a new version instance.
-      # Because we are now using `after_(create/update/etc)` callbacks, we need to go out of our way to
-      # ensure that during updates timestamp attributes are not acknowledged as a notable changes
-      # to raise false positives when attributes are ignored.
+      # This method determines whether it is appropriate to generate a new
+      # version instance. A timestamp-only update (e.g. only `updated_at`
+      # changed) is considered notable unless an ignored attribute was also
+      # changed.
       def changed_notably?
-        if self.paper_trail_options[:ignore].any? && (changed & self.paper_trail_options[:ignore]).any?
-          (notably_changed - timestamp_attributes_for_update_in_model.map(&:to_s)).any?
+        if ignored_attr_has_changed?
+          timestamps = timestamp_attributes_for_update_in_model.map(&:to_s)
+          (notably_changed - timestamps).any?
         else
           notably_changed.any?
         end
       end
 
+      # An attributed is "ignored" if it is listed in the `:ignore` option
+      # and/or the `:skip` option.  Returns true if an ignored attribute
+      # has changed.
+      def ignored_attr_has_changed?
+        ignored = self.paper_trail_options[:ignore] + self.paper_trail_options[:skip]
+        ignored.any? && (changed & ignored).any?
+      end
+
       def notably_changed
         only = self.paper_trail_options[:only].dup
         # remove Hash arguments and then evaluate whether the attributes (the keys of the hash) should also get pushed into the collection

spec/models/gadget_spec.rb

@@ -45,7 +45,7 @@
                 expect(subject.send(:changed_notably?)).to be true
               end
 
-              it "should not acknowledge ignored attrs and timestamps only" do
+              it "should not acknowledge ignored attr (brand)" do
                 subject.brand = 'Acme'
                 expect(subject.send(:changed_notably?)).to be false
               end

spec/models/skipper_spec.rb

@@ -0,0 +1,17 @@
+require 'rails_helper'
+
+describe Skipper, :type => :model do
+  describe "#update_attributes!", :versioning => true do
+    context "updating a skipped attribute" do
+      let(:t1) { Time.zone.local(2015, 7, 15, 20, 34, 0) }
+      let(:t2) { Time.zone.local(2015, 7, 15, 20, 34, 30) }
+
+      it "should not create a version" do
+        skipper = Skipper.create!(:another_timestamp => t1)
+        expect {
+          skipper.update_attributes!(:another_timestamp => t2)
+        }.to_not change { skipper.versions.length }
+      end
+    end
+  end
+end

test/dummy/app/models/skipper.rb

@@ -0,0 +1,6 @@
+class Skipper < ActiveRecord::Base
+  has_paper_trail(
+    :ignore => [:created_at],
+    :skip => [:another_timestamp]
+  )
+end

test/dummy/db/migrate/20110208155312_set_up_test_tables.rb

@@ -1,5 +1,10 @@
 class SetUpTestTables < ActiveRecord::Migration
   def self.up
+    create_table :skippers, :force => true do |t|
+      t.datetime :another_timestamp
+      t.timestamps :null => true
+    end
+
     create_table :widgets, :force => true do |t|
       t.string    :name
       t.text      :a_text
@@ -209,6 +214,7 @@ def self.up
 
   def self.down
     drop_table :animals
+    drop_table :skippers
     drop_table :not_on_updates
     drop_table :posts
     drop_table :songs