Merge pull request #683 from owenr/master

Destroy callback default changed to :before

CHANGELOG.md

@@ -8,6 +8,10 @@
   that PaperTrail no longer adds the `set_paper_trail_whodunnit` before_filter
   for you. Please add this before_filter to your ApplicationController to
   continue recording whodunnit. See the readme for an example.
+- [#683](https://github.com/airblade/paper_trail/pull/683) /
+  [#682](https://github.com/airblade/paper_trail/issues/682) -
+  Destroy callback default changed to :before to accommodate ActiveRecord 5
+  option `belongs_to_required_by_default` and new Rails 5 default.
 
 ### Added
 

README.md

@@ -284,7 +284,7 @@ end
 ```
 
 The `paper_trail_on_destroy` method can be further configured to happen
-`:before` or `:after` the destroy event. By default, it will happen after.
+`:before` or `:after` the destroy event. By default, it will happen before.
 
 ## Choosing When To Save New Versions
 

lib/paper_trail/has_paper_trail.rb

@@ -116,11 +116,22 @@ module PaperTrail
       end
 
       # Record version before or after "destroy" event
-      def paper_trail_on_destroy(recording_order = 'after')
+      def paper_trail_on_destroy(recording_order = 'before')
         unless %w[after before].include?(recording_order.to_s)
           fail ArgumentError, 'recording order can only be "after" or "before"'
         end
 
+        if recording_order == 'after' and
+          Gem::Version.new(ActiveRecord::VERSION::STRING) >= Gem::Version.new("5")
+          if ::ActiveRecord::Base.belongs_to_required_by_default
+            ::ActiveSupport::Deprecation.warn(
+              "paper_trail_on_destroy(:after) is incompatible with ActiveRecord " +
+                "belongs_to_required_by_default and has no effect. Please use :before " +
+                "or disable belongs_to_required_by_default."
+            )
+          end
+        end
+
         send "#{recording_order}_destroy", :record_destroy, :if => :save_version?
 
         return if paper_trail_options[:on].include?(:destroy)

spec/models/callback_modifier_spec.rb

@@ -26,10 +26,10 @@ describe CallbackModifier, :type => :model do
         end
 
         context 'when no argument' do
-          it 'should default to after destroy' do
+          it 'should default to before destroy' do
             modifier = NoArgDestroyModifier.create!(:some_content => FFaker::Lorem.sentence)
             modifier.test_destroy
-            expect(modifier.versions.last.reify).to be_flagged_deleted
+            expect(modifier.versions.last.reify).not_to be_flagged_deleted
           end
         end
       end