From 18d35ef8b7c0742292e73b46165758820a5f7bde Mon Sep 17 00:00:00 2001
From: Owen Rodda <owen.rodda@gmail.com>
Date: Sat, 2 Jan 2016 11:51:02 -0500
Subject: [PATCH] Change paper_trail_on_destroy default to 'before'

Also warn if paper_trail_on_destroy(:after) is combined with
ActiveRecord belongs_to_required_by_default
---
 CHANGELOG.md                          |  4 ++++
 README.md                             |  2 +-
 lib/paper_trail/has_paper_trail.rb    | 13 ++++++++++++-
 spec/models/callback_modifier_spec.rb |  4 ++--
 4 files changed, 19 insertions(+), 4 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index d1d5e6e4..6a863c4a 100644
--- a/CHANGELOG.md
+++ b/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
 
diff --git a/README.md b/README.md
index 46119627..9daa9a9b 100644
--- a/README.md
+++ b/README.md
@@ -285,7 +285,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
 
diff --git a/lib/paper_trail/has_paper_trail.rb b/lib/paper_trail/has_paper_trail.rb
index 4636e9fb..ac4c8c25 100644
--- a/lib/paper_trail/has_paper_trail.rb
+++ b/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)
diff --git a/spec/models/callback_modifier_spec.rb b/spec/models/callback_modifier_spec.rb
index 4f45d230..65836003 100644
--- a/spec/models/callback_modifier_spec.rb
+++ b/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