diff --git a/CHANGELOG.md b/CHANGELOG.md index 4b286a43..00009644 100644 --- a/CHANGELOG.md +++ b/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. diff --git a/lib/paper_trail/has_paper_trail.rb b/lib/paper_trail/has_paper_trail.rb index 84f4d789..e12758e5 100644 --- a/lib/paper_trail/has_paper_trail.rb +++ b/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 diff --git a/spec/models/gadget_spec.rb b/spec/models/gadget_spec.rb new file mode 100644 index 00000000..e1a80367 --- /dev/null +++ b/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 diff --git a/test/dummy/app/models/gadget.rb b/test/dummy/app/models/gadget.rb new file mode 100644 index 00000000..41d7ad39 --- /dev/null +++ b/test/dummy/app/models/gadget.rb @@ -0,0 +1,3 @@ +class Gadget < ActiveRecord::Base + has_paper_trail :ignore => :brand +end diff --git a/test/dummy/db/migrate/20110208155312_set_up_test_tables.rb b/test/dummy/db/migrate/20110208155312_set_up_test_tables.rb index ee61905c..38a90368 100644 --- a/test/dummy/db/migrate/20110208155312_set_up_test_tables.rb +++ b/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 diff --git a/test/unit/model_test.rb b/test/unit/model_test.rb index 07dbd780..90288f54 100644 --- a/test/unit/model_test.rb +++ b/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