From f7a94d0b63885bc142d00a0d80994d358f849d49 Mon Sep 17 00:00:00 2001
From: Todd Lynam <TLynam@gmail.com>
Date: Sun, 16 Aug 2020 10:15:09 -1000
Subject: [PATCH] Skip creating version for timestamp when changed attributed
 is ignored via Hash

- It was only ignoring attributes defined as symbols.
- It now ignores when attributes are either defined as symbols or Hashes.
- Consolidates calculation to be shared when determining if changed and not ignored.

Resolves #1240
---
 CHANGELOG.md                                  |  3 +++
 lib/paper_trail/events/base.rb                | 10 +++++++---
 spec/dummy_app/app/models/gadget.rb           |  2 +-
 .../20110208155312_set_up_test_tables.rb      |  1 +
 spec/models/gadget_spec.rb                    | 19 ++++++++++++++++---
 5 files changed, 28 insertions(+), 7 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 610818a4..e6a04266 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -27,6 +27,9 @@ recommendations of [keepachangelog.com](http://keepachangelog.com/).
 - [#1238](https://github.com/paper-trail-gem/paper_trail/pull/1238) -
   Query optimization in `reify`
 
+- [#1256](https://github.com/paper-trail-gem/paper_trail/pull/1256) -
+  Skip version for timestamp when changed attributed is ignored via Hash
+
 ### Dependencies
 
 - Drop support for rails <= 5.1 (reached EOL when 6.0 was released,
diff --git a/lib/paper_trail/events/base.rb b/lib/paper_trail/events/base.rb
index fabde806..9421813a 100644
--- a/lib/paper_trail/events/base.rb
+++ b/lib/paper_trail/events/base.rb
@@ -107,7 +107,7 @@ module PaperTrail
       end
 
       # @api private
-      def changed_and_not_ignored
+      def calculated_ignored_array
         ignore = @record.paper_trail_options[:ignore].dup
         # Remove Hash arguments and then evaluate whether the attributes (the
         # keys of the hash) should also get pushed into the collection.
@@ -117,8 +117,12 @@ module PaperTrail
               ignore << attr if condition.respond_to?(:call) && condition.call(@record)
             }
         end
+      end
+
+      # @api private
+      def changed_and_not_ignored
         skip = @record.paper_trail_options[:skip]
-        (changed_in_latest_version - ignore) - skip
+        (changed_in_latest_version - calculated_ignored_array) - skip
       end
 
       # @api private
@@ -148,7 +152,7 @@ module PaperTrail
       #
       # @api private
       def ignored_attr_has_changed?
-        ignored = @record.paper_trail_options[:ignore] + @record.paper_trail_options[:skip]
+        ignored = calculated_ignored_array + @record.paper_trail_options[:skip]
         ignored.any? && (changed_in_latest_version & ignored).any?
       end
 
diff --git a/spec/dummy_app/app/models/gadget.rb b/spec/dummy_app/app/models/gadget.rb
index 2fcb4b02..9a2f6635 100644
--- a/spec/dummy_app/app/models/gadget.rb
+++ b/spec/dummy_app/app/models/gadget.rb
@@ -1,5 +1,5 @@
 # frozen_string_literal: true
 
 class Gadget < ActiveRecord::Base
-  has_paper_trail ignore: :brand
+  has_paper_trail ignore: [:brand, { color: proc { |obj| obj.color == "Yellow" } }]
 end
diff --git a/spec/dummy_app/db/migrate/20110208155312_set_up_test_tables.rb b/spec/dummy_app/db/migrate/20110208155312_set_up_test_tables.rb
index d59e86fb..3f2becb3 100644
--- a/spec/dummy_app/db/migrate/20110208155312_set_up_test_tables.rb
+++ b/spec/dummy_app/db/migrate/20110208155312_set_up_test_tables.rb
@@ -254,6 +254,7 @@ class SetUpTestTables < ::ActiveRecord::Migration::Current
     create_table :gadgets, force: true do |t|
       t.string    :name
       t.string    :brand
+      t.string    :color
       t.timestamps null: true, limit: 6
     end
 
diff --git a/spec/models/gadget_spec.rb b/spec/models/gadget_spec.rb
index aadbf14c..0e1e90f8 100644
--- a/spec/models/gadget_spec.rb
+++ b/spec/models/gadget_spec.rb
@@ -8,12 +8,25 @@ RSpec.describe Gadget, type: :model do
   it { is_expected.to be_versioned }
 
   describe "updates", versioning: true do
-    it "generates a version for updates to `name` attribute" do
+    it "generates a version for updates" do
       expect { gadget.update_attribute(:name, "Hammer") }.to(change { gadget.versions.size }.by(1))
     end
 
-    it "ignores for updates to `brand` attribute" do
-      expect { gadget.update_attribute(:brand, "Stanley") }.not_to(change { gadget.versions.size })
+    context "ignored via symbol" do
+      it "doesn't generate a version" do
+        expect { gadget.update_attribute(:brand, "Picard") }.not_to(change { gadget.versions.size })
+      end
+    end
+
+    context "ignored via Hash" do
+      it "generates a version when the ignored attribute isn't true" do
+        expect { gadget.update_attribute(:color, "Blue") }.to(change { gadget.versions.size }.by(1))
+        expect(gadget.versions.last.changeset.keys).to eq %w[color updated_at]
+      end
+
+      it "doesn't generate a version when the ignored attribute is true" do
+        expect { gadget.update_attribute(:color, "Yellow") }.not_to(change { gadget.versions.size })
+      end
     end
 
     it "still generates a version when only the `updated_at` attribute is updated" do