Merge pull request #1256 from tlynam/fix-ignore-via-hash-creating-version

Skip creating version for timestamp when ignoring attribute with a Hash

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,

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
 

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

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
 

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