close #414; Merge branch 'fix-ignored-on-activerecord4'

CHANGELOG.md

@@ -4,7 +4,9 @@
 ```ruby
 PaperTrail::Rails::Engine.eager_load!
 ```
+
   - [#427](https://github.com/airblade/paper_trail/pull/427) - Fix `reify` method in context of model where a column has been removed.
+  - [#414](https://github.com/airblade/paper_trail/issues/414) - Fix functionality `ignore` attributes to `has_paper_trail`.
   - [#399](https://github.com/airblade/paper_trail/pull/399) - Add `:dup` argument for options hash to `reify` which forces a
     new model instance.
   - [#394](https://github.com/airblade/paper_trail/pull/394) - Add RSpec matcher `have_a_version_with` for easier testing.

lib/paper_trail/has_paper_trail.rb

@@ -366,8 +366,16 @@ module PaperTrail
         end
       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.
       def changed_notably?
-        notably_changed.any?
+        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?
+        else
+          notably_changed.any?
+        end
       end
 
       def notably_changed

spec/models/gadget_spec.rb

@@ -0,0 +1,68 @@
+require 'spec_helper'
+
+describe Gadget do
+  it { should be_versioned }
+
+  let(:gadget) { Gadget.create!(:name => 'Wrench', :brand => 'Acme') }
+
+  describe "updates", :versioning => true do
+    it "should generate a version for updates to `name` attribute" do
+      expect { gadget.update_attribute(:name, 'Hammer').to change{gadget.versions.size}.by(1) }
+    end
+
+    it "should ignore for updates to `brand` attribute" do
+      expect { gadget.update_attribute(:brand, 'Stanley') }.to_not change{gadget.versions.size}
+    end
+
+    it "should still generate a version when only the `updated_at` attribute is updated" do
+      expect { gadget.update_attribute(:updated_at, Time.now) }.to change{gadget.versions.size}.by(1)
+    end
+  end
+
+  describe "Methods" do
+    describe "Instance", :versioning => true do
+      describe "private" do
+        describe :changed_notably? do
+          subject { Gadget.new(:created_at => Time.now) }
+
+          # apparently the private methods list in Ruby18 is different than in Ruby19+
+          if RUBY_VERSION.to_f >= 1.9
+            its(:private_methods) { should include(:changed_notably?) }
+          end
+
+          context "create events" do
+            it { subject.send(:changed_notably?).should == true }
+          end
+
+          context "update events" do
+            before { subject.save! }
+
+            context "without update timestamps" do
+              it "should only acknowledge non-ignored attrs" do
+                subject.name = 'Wrench'
+                subject.send(:changed_notably?).should == true
+              end
+
+              it "should not acknowledge ignored attrs and timestamps only" do
+                subject.brand = 'Acme'
+                subject.send(:changed_notably?).should == false
+              end
+            end
+
+            context "with update timestamps" do
+              it "should only acknowledge non-ignored attrs" do
+                subject.name, subject.updated_at = 'Wrench', Time.now
+                subject.send(:changed_notably?).should == true
+              end
+
+              it "should not acknowledge ignored attrs and timestamps only" do
+                subject.brand, subject.updated_at = 'Acme', Time.now
+                subject.send(:changed_notably?).should == false
+              end
+            end
+          end
+        end
+      end
+    end
+  end
+end

test/dummy/app/models/gadget.rb

@@ -0,0 +1,3 @@
+class Gadget < ActiveRecord::Base
+  has_paper_trail :ignore => :brand
+end

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

@@ -116,6 +116,12 @@ class SetUpTestTables < ActiveRecord::Migration
       t.string    :language_code
       t.string    :type
     end
+
+    create_table :gadgets, :force => true do |t|
+      t.string    :name
+      t.string    :brand
+      t.timestamps
+    end
   end
 
   def self.down
@@ -136,5 +142,6 @@ class SetUpTestTables < ActiveRecord::Migration
     drop_table :documents
     drop_table :legacy_widgets
     drop_table :translations
+    drop_table :gadgets
   end
 end

test/unit/model_test.rb

@@ -227,7 +227,7 @@ class HasPaperTrailModelTest < ActiveSupport::TestCase
           'id'         => [nil, @widget.id]
         }
 
-        assert_equal "Time", @widget.versions.last.changeset['updated_at'][1].class.to_s
+        assert_equal Time, @widget.versions.last.changeset['updated_at'][1].class
         assert_equal changes, @widget.versions.last.changeset
       end