From fec6597f77ea45c8ae9a26cdf8f49d779f4363a2 Mon Sep 17 00:00:00 2001 From: Ryan Moser Date: Wed, 5 Nov 2014 14:39:44 -0800 Subject: [PATCH 1/5] clear rolled back versions from associated model --- lib/paper_trail/has_paper_trail.rb | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/lib/paper_trail/has_paper_trail.rb b/lib/paper_trail/has_paper_trail.rb index 793ed363..086d6971 100644 --- a/lib/paper_trail/has_paper_trail.rb +++ b/lib/paper_trail/has_paper_trail.rb @@ -78,6 +78,7 @@ module PaperTrail after_update :clear_version_instance! end after_destroy :record_destroy, :if => :save_version? if options_on.empty? || options_on.include?(:destroy) + after_rollback :clear_rolled_back_versions end # Switches PaperTrail off for this class. @@ -180,6 +181,10 @@ module PaperTrail (source_version || send(self.class.versions_association_name).last).try(:whodunnit) end + def clear_rolled_back_versions + send(self.class.versions_association_name).reload + end + # Returns the object (not a Version) as it was at the given timestamp. def version_at(timestamp, reify_options={}) # Because a version stores how its object looked *before* the change, From eaa9809d2020ae0d8af717b567f03b2116ce247d Mon Sep 17 00:00:00 2001 From: Ryan Moser Date: Mon, 10 Nov 2014 16:12:50 -0800 Subject: [PATCH 2/5] add test for after_rollback --- lib/paper_trail/has_paper_trail.rb | 2 ++ spec/models/widget_spec.rb | 20 ++++++++++++++++++++ test/dummy/app/models/widget.rb | 4 ++++ 3 files changed, 26 insertions(+) diff --git a/lib/paper_trail/has_paper_trail.rb b/lib/paper_trail/has_paper_trail.rb index 086d6971..11ad7b04 100644 --- a/lib/paper_trail/has_paper_trail.rb +++ b/lib/paper_trail/has_paper_trail.rb @@ -181,6 +181,8 @@ module PaperTrail (source_version || send(self.class.versions_association_name).last).try(:whodunnit) end + # Invoked after rollbacks to ensure versions records are not created + # for changes that never actually took place def clear_rolled_back_versions send(self.class.versions_association_name).reload end diff --git a/spec/models/widget_spec.rb b/spec/models/widget_spec.rb index bbd87d3a..75ba01a9 100644 --- a/spec/models/widget_spec.rb +++ b/spec/models/widget_spec.rb @@ -85,6 +85,26 @@ describe Widget, :type => :model do expect(widget.version).to eq(widget.versions.last) end end + + describe :after_rollback do + let(:rolled_back_name) { 'Big Moo' } + + before do + widget.transaction do + widget.update_attributes!(name: rolled_back_name) + widget.update_attributes!(name: described_class::EXCLUDED_NAME) + end + rescue ActiveRecord::RecordInvalid + widget.name = nil + widget.save + end + + it 'does not create an event for changes that did not happen' do + widget.versions.map(&:changeset).each do |changeset| + expect(changeset.fetch('name', [])).to_not include(rolled_back_name) + end + end + end end describe "Association", :versioning => true do diff --git a/test/dummy/app/models/widget.rb b/test/dummy/app/models/widget.rb index 68eb40eb..0720a75c 100644 --- a/test/dummy/app/models/widget.rb +++ b/test/dummy/app/models/widget.rb @@ -2,6 +2,10 @@ class Widget < ActiveRecord::Base has_paper_trail has_one :wotsit + EXCLUDED_NAME = 'Biglet' + + validates :name, exclusion: { in: [EXCLUDED_NAME] } + if ::ActiveRecord::VERSION::MAJOR >= 4 # `has_many` syntax for specifying order uses a lambda in Rails 4 has_many :fluxors, lambda { order(:name) } else From e2dee218f6455a684f7358611024f435e7759a0f Mon Sep 17 00:00:00 2001 From: Ryan Moser Date: Mon, 10 Nov 2014 16:20:57 -0800 Subject: [PATCH 3/5] use begin/rescue/end in block --- spec/models/widget_spec.rb | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/spec/models/widget_spec.rb b/spec/models/widget_spec.rb index 75ba01a9..86c8c16a 100644 --- a/spec/models/widget_spec.rb +++ b/spec/models/widget_spec.rb @@ -90,13 +90,15 @@ describe Widget, :type => :model do let(:rolled_back_name) { 'Big Moo' } before do - widget.transaction do - widget.update_attributes!(name: rolled_back_name) - widget.update_attributes!(name: described_class::EXCLUDED_NAME) + begin + widget.transaction do + widget.update_attributes!(name: rolled_back_name) + widget.update_attributes!(name: described_class::EXCLUDED_NAME) + end + rescue ActiveRecord::RecordInvalid + widget.name = nil + widget.save end - rescue ActiveRecord::RecordInvalid - widget.name = nil - widget.save end it 'does not create an event for changes that did not happen' do From 44e433299c7b9f20a599cc6c11d013f34e121dc4 Mon Sep 17 00:00:00 2001 From: Ryan Moser Date: Mon, 10 Nov 2014 17:01:21 -0800 Subject: [PATCH 4/5] add test_after_commit gem --- paper_trail.gemspec | 1 + 1 file changed, 1 insertion(+) diff --git a/paper_trail.gemspec b/paper_trail.gemspec index d1af6e7c..7a105a7d 100644 --- a/paper_trail.gemspec +++ b/paper_trail.gemspec @@ -32,6 +32,7 @@ Gem::Specification.new do |s| s.add_development_dependency 'rspec-rails', '~> 3.1.0' s.add_development_dependency 'generator_spec' s.add_development_dependency 'database_cleaner', '~> 1.2' + s.add_development_dependency 'test_after_commit', '~> 0.4' # JRuby support for the test ENV unless defined?(JRUBY_VERSION) From db4e9fc1e31ff49673a6cc07d10233cb588b113a Mon Sep 17 00:00:00 2001 From: Ben Atkins Date: Mon, 10 Nov 2014 20:32:54 -0500 Subject: [PATCH 5/5] Fix spec for PaperTrail::Model::ClassMethods#after_rollback --- paper_trail.gemspec | 1 - spec/models/widget_spec.rb | 5 +++-- test/dummy/app/models/widget.rb | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/paper_trail.gemspec b/paper_trail.gemspec index 7a105a7d..d1af6e7c 100644 --- a/paper_trail.gemspec +++ b/paper_trail.gemspec @@ -32,7 +32,6 @@ Gem::Specification.new do |s| s.add_development_dependency 'rspec-rails', '~> 3.1.0' s.add_development_dependency 'generator_spec' s.add_development_dependency 'database_cleaner', '~> 1.2' - s.add_development_dependency 'test_after_commit', '~> 0.4' # JRuby support for the test ENV unless defined?(JRUBY_VERSION) diff --git a/spec/models/widget_spec.rb b/spec/models/widget_spec.rb index 86c8c16a..a1ff9aab 100644 --- a/spec/models/widget_spec.rb +++ b/spec/models/widget_spec.rb @@ -92,10 +92,11 @@ describe Widget, :type => :model do before do begin widget.transaction do - widget.update_attributes!(name: rolled_back_name) - widget.update_attributes!(name: described_class::EXCLUDED_NAME) + widget.update_attributes!(:name => rolled_back_name) + widget.update_attributes!(:name => Widget::EXCLUDED_NAME) end rescue ActiveRecord::RecordInvalid + widget.reload widget.name = nil widget.save end diff --git a/test/dummy/app/models/widget.rb b/test/dummy/app/models/widget.rb index 0720a75c..dd2e51aa 100644 --- a/test/dummy/app/models/widget.rb +++ b/test/dummy/app/models/widget.rb @@ -4,7 +4,7 @@ class Widget < ActiveRecord::Base EXCLUDED_NAME = 'Biglet' - validates :name, exclusion: { in: [EXCLUDED_NAME] } + validates :name, :exclusion => { :in => [EXCLUDED_NAME] } if ::ActiveRecord::VERSION::MAJOR >= 4 # `has_many` syntax for specifying order uses a lambda in Rails 4 has_many :fluxors, lambda { order(:name) }