Merge pull request #614 from Ninigi/master

Enable control over the order of AR callbacks

Author: jaredbeck

CHANGELOG.md

@@ -28,6 +28,9 @@ None
 
 ### Added
 
+- Added callback-methods `paper_trail_update` `paper_trail_create` `paper_trail_destroy`
+  instead of has_paper_trail
+  [#593](https://github.com/airblade/paper_trail/pull/607)
 - Added `unversioned_attributes` option to `reify`.
   [#579](https://github.com/airblade/paper_trail/pull/579)
 

README.md

@@ -330,6 +330,27 @@ a.versions.size                           # 3
 a.versions.last.event                     # 'update'
 ```
 
+### Controlling the Order of AR Callbacks
+
+You can also use the corresponding callback-methods seperately instead of using
+the :on option. If you choose to use the callback-methods, PaperTrail will only
+track the according events - so `paper_trail_on_create` is basically the same as
+`has_paper_trail :on => :create`.
+
+```ruby
+class Article < ActiveRecord::Base
+  has_paper_trail :on => []
+  paper_trail_on_destroy
+  paper_trail_on_update
+  paper_trail_on_create
+end
+```
+
+The `paper_trail_on_destroy` method can be configured to be called `:before` or `:after` the
+destroy event. This can be usefull if you are using a third party tool that alters the
+destroy method (for example paranoia). If you do not pass an argument, it will default
+to after_destroy.
+
 ## Choosing When To Save New Versions
 
 You can choose the conditions when to add new versions with the `if` and

lib/paper_trail/has_paper_trail.rb

@@ -46,6 +46,18 @@ module ClassMethods
       #   column if it exists. Default is true
       #
       def has_paper_trail(options = {})
+        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_model_for_paper_trail(options)
+
+        setup_callbacks_from_options options[:on]
+      end
+
+      def setup_model_for_paper_trail(options = {})
         # Lazily include the instance methods so we don't clutter up
         # any more ActiveRecord models than we have to.
         send :include, InstanceMethods
@@ -60,6 +72,7 @@ def has_paper_trail(options = {})
         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|
@@ -87,26 +100,53 @@ def has_paper_trail(options = {})
             :order      => self.paper_trail_version_class.timestamp_sort_order
         end
 
-        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])
-
-        after_create  :record_create, :if => :save_version? if options_on.include?(:create)
-        if options_on.include?(:update)
-          before_save   :reset_timestamp_attrs_for_update_if_needed!, :on => :update
-          after_update  :record_update, :if => :save_version?
-          after_update  :clear_version_instance!
-        end
-        after_destroy :record_destroy, :if => :save_version? if options_on.include?(:destroy)
-
         # Reset the transaction id when the transaction is closed.
         after_commit :reset_transaction_id
         after_rollback :reset_transaction_id
         after_rollback :clear_rolled_back_versions
       end
 
+      def setup_callbacks_from_options(options_on = [])
+        options_on.each do |option|
+          send "paper_trail_on_#{option}"
+        end
+      end
+
+      # Record version before or after "destroy" event
+      def paper_trail_on_destroy(recording_order = 'after')
+        unless %w[after before].include?(recording_order.to_s)
+          fail ArgumentError, 'recording order can only be "after" or "before"'
+        end
+
+        send "#{recording_order}_destroy",
+             :record_destroy,
+             :if => :save_version?
+
+        return if paper_trail_options[:on].include?(:destroy)
+        paper_trail_options[:on] << :destroy
+      end
+
+      # Record version after "update" event
+      def paper_trail_on_update
+        before_save  :reset_timestamp_attrs_for_update_if_needed!,
+                     :on => :update
+        after_update :record_update,
+                     :if => :save_version?
+        after_update :clear_version_instance!
+
+        return if paper_trail_options[:on].include?(:update)
+        paper_trail_options[:on] << :update
+      end
+
+      # Record version after "create" event
+      def paper_trail_on_create
+        after_create :record_create,
+                     :if => :save_version?
+
+        return if paper_trail_options[:on].include?(:create)
+        paper_trail_options[:on] << :create
+      end
+
       # Switches PaperTrail off for this class.
       def paper_trail_off!
         PaperTrail.enabled_for_model(self, false)

spec/models/animal_spec.rb

@@ -15,5 +15,22 @@
         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

@@ -0,0 +1,97 @@
+require 'rails_helper'
+require 'support/callback_modifier'
+
+describe CallbackModifier, :type => :model do
+  with_versioning do
+    describe 'callback-methods', :versioning => true do
+      describe 'paper_trail_on_destroy' do
+        it 'should add :destroy to paper_trail_options[:on]' do
+          modifier = NoArgDestroyModifier.create!(:some_content => Faker::Lorem.sentence)
+          expect(modifier.paper_trail_options[:on]).to eq [:destroy]
+        end
+
+        context 'when :before' do
+          it 'should create the version before destroy' do
+            modifier = BeforeDestroyModifier.create!(:some_content => Faker::Lorem.sentence)
+            modifier.test_destroy
+            expect(modifier.versions.last.reify).not_to be_flagged_deleted
+          end
+        end
+
+        context 'when :after' do
+          it 'should create the version after destroy' do
+            modifier = AfterDestroyModifier.create!(:some_content => Faker::Lorem.sentence)
+            modifier.test_destroy
+            expect(modifier.versions.last.reify).to be_flagged_deleted
+          end
+        end
+
+        context 'when no argument' do
+          it 'should default to after destroy' do
+            modifier = NoArgDestroyModifier.create!(:some_content => Faker::Lorem.sentence)
+            modifier.test_destroy
+            expect(modifier.versions.last.reify).to be_flagged_deleted
+          end
+        end
+      end
+
+      describe 'paper_trail_on_update' do
+        it 'should add :update to paper_trail_options[:on]' do
+          modifier = UpdateModifier.create!(:some_content => Faker::Lorem.sentence)
+          expect(modifier.paper_trail_options[:on]).to eq [:update]
+        end
+
+        it 'should create a version' do
+          modifier = UpdateModifier.create!(:some_content => Faker::Lorem.sentence)
+          modifier.update_attributes! :some_content => 'modified'
+          expect(modifier.versions.last.event).to eq 'update'
+        end
+      end
+
+      describe 'paper_trail_on_create' do
+        it 'should add :create to paper_trail_options[:on]' do
+          modifier = CreateModifier.create!(:some_content => Faker::Lorem.sentence)
+          expect(modifier.paper_trail_options[:on]).to eq [:create]
+        end
+
+        it 'should create a version' do
+          modifier = CreateModifier.create!(:some_content => Faker::Lorem.sentence)
+          expect(modifier.versions.last.event).to eq 'create'
+        end
+      end
+
+      context 'when no callback-method used' do
+        it 'should set paper_trail_options[:on] to [:create, :update, :destroy]' do
+          modifier = DefaultModifier.create!(:some_content => Faker::Lorem.sentence)
+          expect(modifier.paper_trail_options[:on]).to eq [:create, :update, :destroy]
+        end
+
+        it 'should default to track destroy' do
+          modifier = DefaultModifier.create!(:some_content => Faker::Lorem.sentence)
+          modifier.destroy
+          expect(modifier.versions.last.event).to eq 'destroy'
+        end
+
+        it 'should default to track update' do
+          modifier = DefaultModifier.create!(:some_content => Faker::Lorem.sentence)
+          modifier.update_attributes! :some_content => 'modified'
+          expect(modifier.versions.last.event).to eq 'update'
+        end
+
+        it 'should default to track create' do
+          modifier = DefaultModifier.create!(:some_content => Faker::Lorem.sentence)
+          expect(modifier.versions.last.event).to eq 'create'
+        end
+      end
+
+      context 'when only one callback-method' do
+        it 'does only track the corresponding event' do
+          modifier = CreateModifier.create!(:some_content => Faker::Lorem.sentence)
+          modifier.update_attributes!(:some_content => 'modified')
+          modifier.test_destroy
+          expect(modifier.versions.collect(&:event)).to eq ['create']
+        end
+      end
+    end
+  end
+end

spec/support/callback_modifier.rb

@@ -0,0 +1,28 @@
+class BeforeDestroyModifier < CallbackModifier
+  has_paper_trail :on => []
+  paper_trail_on_destroy :before
+end
+
+class AfterDestroyModifier < CallbackModifier
+  has_paper_trail :on => []
+  paper_trail_on_destroy :after
+end
+
+class NoArgDestroyModifier < CallbackModifier
+  has_paper_trail :on => []
+  paper_trail_on_destroy
+end
+
+class UpdateModifier < CallbackModifier
+  has_paper_trail :on => []
+  paper_trail_on_update
+end
+
+class CreateModifier < CallbackModifier
+  has_paper_trail :on => []
+  paper_trail_on_create
+end
+
+class DefaultModifier < CallbackModifier
+  has_paper_trail
+end

test/dummy/app/models/callback_modifier.rb

@@ -0,0 +1,16 @@
+class CallbackModifier < ActiveRecord::Base
+  has_paper_trail :on => []
+
+  def test_destroy
+    transaction do
+      run_callbacks(:destroy) do
+        self.deleted = true
+        save!
+      end
+    end
+  end
+
+  def flagged_deleted?
+    deleted?
+  end
+end

test/dummy/db/migrate/20110208155312_set_up_test_tables.rb

@@ -212,6 +212,11 @@ def self.up
       t.boolean :scoped, :default => true
     end
 
+    create_table :callback_modifiers, :force => true do |t|
+      t.string  :some_content
+      t.boolean :deleted, :default => false
+    end
+
     create_table :chapters, :force => true do |t|
       t.string :name
     end
@@ -277,5 +282,7 @@ def self.down
     remove_index :version_associations, :column => [:version_id]
     remove_index :version_associations, :name => 'index_version_associations_on_foreign_key'
     drop_table :version_associations
+    drop_table :filter_modifier
+    drop_table :callback_modifiers
   end
 end

test/dummy/db/schema.rb

@@ -55,6 +55,11 @@
     t.boolean "scoped", default: true
   end
 
+  create_table "callback_modifiers", force: :cascade do |t|
+    t.string  "some_content"
+    t.boolean "deleted",      default: false
+  end
+
   create_table "customers", force: :cascade do |t|
     t.string "name"
   end