From 297f5fcac08cc0aa9e49e6ed12ff5776a31c3b8a Mon Sep 17 00:00:00 2001 From: Ninigi Date: Tue, 15 Sep 2015 15:06:14 +0200 Subject: [PATCH] legacy complient callback-methods --- lib/paper_trail/callbacks.rb | 93 +++++++++++++++------- lib/paper_trail/has_paper_trail.rb | 16 +++- spec/models/animal_spec.rb | 17 ++++ spec/models/callback_modifier_spec.rb | 2 +- spec/spec_helper.rb | 4 +- spec/support/callback_modifier.rb | 15 ++-- test/dummy/app/models/callback_modifier.rb | 4 +- 7 files changed, 108 insertions(+), 43 deletions(-) diff --git a/lib/paper_trail/callbacks.rb b/lib/paper_trail/callbacks.rb index f0ba8dbf..dc0299f7 100644 --- a/lib/paper_trail/callbacks.rb +++ b/lib/paper_trail/callbacks.rb @@ -1,40 +1,34 @@ module PaperTrail module Callbacks - def setup_callbacks_from_options(options_on, options = {}) + # The cleanup_* methods are only used to provide backward-compatibility + # They should be removed as soon as the "traditional" way of using + # PaperTrail `has_papertrail :on => [...]` with or without the :on option + # and not setting the paper_trail_* methods is no longer supported. + def setup_callbacks_from_options(options_on = []) options_on.each do |option| - send "paper_trail_#{option}", options + send "paper_trail_on_#{option}" end + + paper_trail_options[:on] = options_on end # Record version before or after "destroy" event - def paper_trail_destroy(options = {}) - setup_model_if_necessary options - recording_order = options[:recording_order] || 'after' - + def paper_trail_on_destroy(recording_order = 'after') unless %(after before).include?(recording_order.to_s) fail ArgumentError, 'recording order can only be "after" or "before"' end + cleanup_callback_chain + send "#{recording_order}_destroy", :record_destroy, :if => :save_version? end - # Record version after "destroy" event - def paper_trail_after_destroy(options = {}) - options[:recording_order] = :after - paper_trail_destroy options - end - - # Record version before "destroy" event - def paper_trail_before_destroy(options = {}) - options[:recording_order] = :before - paper_trail_destroy options - end - # Record version after "update" event - def paper_trail_update(options = {}) - setup_model_if_necessary options + def paper_trail_on_update + cleanup_callback_chain + before_save :reset_timestamp_attrs_for_update_if_needed!, :on => :update after_update :record_update, @@ -43,23 +37,64 @@ module PaperTrail end # Record version after "create" event - def paper_trail_create(options = {}) - setup_model_if_necessary options + def paper_trail_on_create + cleanup_callback_chain + after_create :record_create, :if => :save_version? end private - def setup_model_if_necessary(options) - return true if model_set_up? - - setup_model_for_paper_trail options - @_set_up = true + # The cleanup_* methods are only used to provide backward-compatibility + # They should be removed as soon as the "traditional" way of using + # PaperTrail `has_papertrail :on => [...]` with or without the :on option + # and not setting the paper_trail_* methods is no longer supported. + def cleanup_callback_chain + on_options = paper_trail_options.try(:delete, :on) || [] + on_options.each do |on_option| + send "cleanup_#{on_option}_callbacks" + end end - def model_set_up? - @_set_up + def cleanup_create_callbacks + callback = + _create_callbacks.find do |cb| + cb.filter.eql?(:record_create) && cb.kind.eql?(:after) + end + + _create_callbacks.delete(callback) + end + + def cleanup_update_callbacks + callback = + _save_callbacks.find do |cb| + cb.filter.eql?(:reset_timestamp_attrs_for_update_if_needed!) && cb.kind.eql?(:before) + end + + _save_callbacks.delete(callback) + + callback = + _update_callbacks.find do |cb| + cb.filter.eql?(:record_update) && cb.kind.eql?(:after) + end + + _update_callbacks.delete(callback) + callback = + _update_callbacks.find do |cb| + cb.filter.eql?(:clear_version_instance!) && cb.kind.eql?(:after) + end + + _update_callbacks.delete(callback) + end + + def cleanup_destroy_callbacks + callback = + _destroy_callbacks.find do |cb| + cb.filter.eql?(:record_destroy) + end + + _destroy_callbacks.delete(callback) end end end diff --git a/lib/paper_trail/has_paper_trail.rb b/lib/paper_trail/has_paper_trail.rb index 65c17aed..bfbb0462 100644 --- a/lib/paper_trail/has_paper_trail.rb +++ b/lib/paper_trail/has_paper_trail.rb @@ -48,15 +48,26 @@ module PaperTrail # column if it exists. Default is true # def has_paper_trail(options = {}) - setup_model_for_paper_trail(options) + # Initializing paper_trail_options with an empty hash before setting + # up the callback-methods is important to make them compatible with the + # traditional has_paper_trail method. + # This can move to setup_model_for_paper_trail, as soon as the + # cleanup_callback and setup_callbacks_from_options method is no longer + # needed. + class_attribute :paper_trail_options + self.paper_trail_options = {} + # TODO: Add a deprecation warning / info to use callback methods instead + # of the :on option? options[:on] ||= [:create, :update, :destroy] # Wrap the :on option in an array if necessary. This allows a single # symbol to be passed in. options_on = Array(options[:on]) - setup_callbacks_from_options options_on, options + setup_callbacks_from_options options_on + + setup_model_for_paper_trail(options) end def setup_model_for_paper_trail(options = {}) @@ -73,7 +84,6 @@ module PaperTrail class_attribute :version_class_name self.version_class_name = options[:class_name] || 'PaperTrail::Version' - class_attribute :paper_trail_options self.paper_trail_options = options.dup [:ignore, :skip, :only].each do |k| diff --git a/spec/models/animal_spec.rb b/spec/models/animal_spec.rb index 88a2c05f..4c4f0183 100644 --- a/spec/models/animal_spec.rb +++ b/spec/models/animal_spec.rb @@ -15,5 +15,22 @@ describe Animal, :type => :model do expect(dog).to be_instance_of(Dog) end end + + context 'with callback-methods' do + context 'when only has_paper_trail set in super class' do + let(:callback_cat) { Cat.create(:name => 'Markus') } + + it 'trails all events' do + callback_cat.update_attributes(:name => 'Billie') + callback_cat.destroy + expect(callback_cat.versions.collect(&:event)).to eq %w(create update destroy) + end + + it 'does not break reify' do + callback_cat.destroy + expect { callback_cat.versions.last.reify }.not_to raise_error + end + end + end end end diff --git a/spec/models/callback_modifier_spec.rb b/spec/models/callback_modifier_spec.rb index 42c5f783..f25e2ad2 100644 --- a/spec/models/callback_modifier_spec.rb +++ b/spec/models/callback_modifier_spec.rb @@ -69,7 +69,7 @@ describe CallbackModifier, :type => :model do modifier = CreateModifier.create!(:some_content => Faker::Lorem.sentence) modifier.update_attributes!(:some_content => 'modified') modifier.test_destroy - expect(modifier.versions.last.event).to eq 'create' + expect(modifier.versions.collect(&:event)).to eq ['create'] end end end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 607474b2..3989951c 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -37,6 +37,8 @@ RSpec.configure do |config| # `true` in RSpec 4. mocks.verify_partial_doubles = true end + config.filter_run :focus + config.run_all_when_everything_filtered = true # The settings below are suggested to provide a good initial experience # with RSpec, but feel free to customize to your heart's content. @@ -45,8 +47,6 @@ RSpec.configure do |config| # to individual examples or groups you care about by tagging them with # `:focus` metadata. When nothing is tagged with `:focus`, all examples # get run. - config.filter_run :focus - config.run_all_when_everything_filtered = true # Limits the available syntax to the non-monkey patched syntax that is recommended. # For more details, see: diff --git a/spec/support/callback_modifier.rb b/spec/support/callback_modifier.rb index b81658b4..14914f0f 100644 --- a/spec/support/callback_modifier.rb +++ b/spec/support/callback_modifier.rb @@ -1,23 +1,28 @@ class BeforeDestroyModifier < CallbackModifier - paper_trail_before_destroy + has_paper_trail + paper_trail_on_destroy :before end class AfterDestroyModifier < CallbackModifier - paper_trail_after_destroy + has_paper_trail + paper_trail_on_destroy :after end class NoArgDestroyModifier < CallbackModifier - paper_trail_destroy + has_paper_trail + paper_trail_on_destroy end class UpdateModifier < CallbackModifier - paper_trail_update + paper_trail_on_update end class CreateModifier < CallbackModifier - paper_trail_create + paper_trail_on_create end class DefaultModifier < CallbackModifier + # Because of the way I set up the destroy method for testing + # has_paper_trail has to be initialized in this model seperately has_paper_trail end diff --git a/test/dummy/app/models/callback_modifier.rb b/test/dummy/app/models/callback_modifier.rb index e2b52dfc..ab97000b 100644 --- a/test/dummy/app/models/callback_modifier.rb +++ b/test/dummy/app/models/callback_modifier.rb @@ -1,7 +1,5 @@ class CallbackModifier < ActiveRecord::Base - # This will not be directly instantiated, but we need to set paper_trail up - # before we can run tests on fake classes inheriting from CallbackModifier - has_paper_trail :on => [] + has_paper_trail def test_destroy transaction do