legacy complient callback-methods

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 = {})
+    def paper_trail_on_destroy(recording_order = 'after')
-      setup_model_if_necessary options
-      recording_order = options[: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 = {})
+    def paper_trail_on_update
-      setup_model_if_necessary options
+      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 = {})
+    def paper_trail_on_create
-      setup_model_if_necessary options
+      cleanup_callback_chain
+
       after_create :record_create,
                    :if => :save_version?
     end
 
     private
 
-      def setup_model_if_necessary(options)
+      # The cleanup_* methods are only used to provide backward-compatibility
-        return true if model_set_up?
+      # They should be removed as soon as the "traditional" way of using
-
+      # PaperTrail `has_papertrail :on => [...]` with or without the :on option
-        setup_model_for_paper_trail options
+      # and not setting the paper_trail_* methods is no longer supported.
-        @_set_up = true
+      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?
+      def cleanup_create_callbacks
-        @_set_up
+        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

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|

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

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

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:

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

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
+  has_paper_trail
-  # before we can run tests on fake classes inheriting from CallbackModifier
-  has_paper_trail :on => []
 
   def test_destroy
     transaction do