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]
This commit is contained in:
parent
5bad080ebc
commit
40bf20beb5
|
@ -469,18 +469,27 @@ module PaperTrail
|
|||
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
|
||||
|
|
|
@ -45,7 +45,7 @@ describe Gadget, :type => :model do
|
|||
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
|
||||
|
|
|
@ -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
|
|
@ -0,0 +1,6 @@
|
|||
class Skipper < ActiveRecord::Base
|
||||
has_paper_trail(
|
||||
:ignore => [:created_at],
|
||||
:skip => [:another_timestamp]
|
||||
)
|
||||
end
|
|
@ -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 @@ class SetUpTestTables < ActiveRecord::Migration
|
|||
|
||||
def self.down
|
||||
drop_table :animals
|
||||
drop_table :skippers
|
||||
drop_table :not_on_updates
|
||||
drop_table :posts
|
||||
drop_table :songs
|
||||
|
|
Loading…
Reference in New Issue