From 4d9fc6aaabee2c599af4214f25dca9e6f84c8d34 Mon Sep 17 00:00:00 2001
From: Jared Beck <jared@jaredbeck.com>
Date: Thu, 8 Sep 2016 19:33:14 -0400
Subject: [PATCH 1/2] Clarify interaction between serializer_test and
 model_test

This business with changing Fluxor on the fly is pretty confusing.
It would probably be better to have different models, rather than
changing Fluxor's PT config on the fly.
---
 spec/models/fluxor_spec.rb      | 17 -----------------
 test/dummy/app/models/fluxor.rb |  4 ++++
 test/unit/serializer_test.rb    | 20 ++++++++------------
 3 files changed, 12 insertions(+), 29 deletions(-)
 delete mode 100644 spec/models/fluxor_spec.rb

diff --git a/spec/models/fluxor_spec.rb b/spec/models/fluxor_spec.rb
deleted file mode 100644
index 7dc0e3f88..000000000
--- a/spec/models/fluxor_spec.rb
+++ /dev/null
@@ -1,17 +0,0 @@
-require "rails_helper"
-
-describe Fluxor, type: :model do
-  describe "`be_versioned` matcher" do
-    it { is_expected.to_not be_versioned }
-  end
-
-  describe "Methods" do
-    describe "Class" do
-      describe ".paper_trail.enabled?" do
-        it "returns false" do
-          expect(Fluxor.paper_trail.enabled?).to eq(false)
-        end
-      end
-    end
-  end
-end
diff --git a/test/dummy/app/models/fluxor.rb b/test/dummy/app/models/fluxor.rb
index 68d72306c..ae500b527 100644
--- a/test/dummy/app/models/fluxor.rb
+++ b/test/dummy/app/models/fluxor.rb
@@ -1,3 +1,7 @@
 class Fluxor < ActiveRecord::Base
   belongs_to :widget
+
+  # In test/unit/model_test.rb and test/unit/serializer_test.rb, this is
+  # changed on the fly, which is quite confusing.
+  has_paper_trail
 end
diff --git a/test/unit/serializer_test.rb b/test/unit/serializer_test.rb
index 49e767d2f..fd3fec9c4 100644
--- a/test/unit/serializer_test.rb
+++ b/test/unit/serializer_test.rb
@@ -2,12 +2,16 @@
 require "custom_json_serializer"
 
 class SerializerTest < ActiveSupport::TestCase
+  setup do
+    # Clean up after test/unit/model_test.rb
+    Fluxor.reset_callbacks :create
+    Fluxor.reset_callbacks :update
+    Fluxor.reset_callbacks :destroy
+    Fluxor.instance_eval "has_paper_trail"
+  end
+
   context "YAML Serializer" do
     setup do
-      Fluxor.instance_eval <<-END
-        has_paper_trail
-      END
-
       @fluxor = Fluxor.create name: "Some text."
 
       # this is exactly what PaperTrail serializes
@@ -34,10 +38,6 @@ class SerializerTest < ActiveSupport::TestCase
         config.serializer = PaperTrail::Serializers::JSON
       end
 
-      Fluxor.instance_eval <<-END
-        has_paper_trail
-      END
-
       @fluxor = Fluxor.create name: "Some text."
 
       # this is exactly what PaperTrail serializes
@@ -77,10 +77,6 @@ class SerializerTest < ActiveSupport::TestCase
         config.serializer = CustomJsonSerializer
       end
 
-      Fluxor.instance_eval <<-END
-        has_paper_trail
-      END
-
       @fluxor = Fluxor.create
 
       # this is exactly what PaperTrail serializes

From cfe86bf7201fa7023ea3d931980ab0989d59bf10 Mon Sep 17 00:00:00 2001
From: Jared Beck <jared@jaredbeck.com>
Date: Thu, 8 Sep 2016 18:21:18 -0400
Subject: [PATCH 2/2] Remove deprecated model methods

---
 CHANGELOG.md                       |   2 +
 lib/paper_trail/has_paper_trail.rb | 293 +----------------------------
 lib/paper_trail/model_config.rb    |  22 ++-
 lib/paper_trail/record_trail.rb    |   2 +-
 spec/models/widget_spec.rb         |   2 -
 5 files changed, 20 insertions(+), 301 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 1916c7b6c..9ec2036c5 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -7,6 +7,8 @@ recommendations of [keepachangelog.com](http://keepachangelog.com/).
 
 ### Breaking Changes
 
+- The model methods deprecated in 5.2.0 have been removed. Use paper_trail.x
+  instead of x.
 - `timestamp_field=` removed without replacement. It is no longer configurable. The
   timestamp field in the `versions` table must now be named `created_at`.
 
diff --git a/lib/paper_trail/has_paper_trail.rb b/lib/paper_trail/has_paper_trail.rb
index 943d802e4..d00aece24 100644
--- a/lib/paper_trail/has_paper_trail.rb
+++ b/lib/paper_trail/has_paper_trail.rb
@@ -69,306 +69,15 @@ def has_paper_trail(options = {})
       def paper_trail
         ::PaperTrail::ModelConfig.new(self)
       end
-
-      # @api private
-      def paper_trail_deprecate(new_method, old_method = nil)
-        old = old_method.nil? ? new_method : old_method
-        msg = format("Use paper_trail.%s instead of %s", new_method, old)
-        ::ActiveSupport::Deprecation.warn(msg, caller(2))
-      end
-
-      # @deprecated
-      def paper_trail_on_destroy(*args)
-        paper_trail_deprecate "on_destroy", "paper_trail_on_destroy"
-        paper_trail.on_destroy(*args)
-      end
-
-      # @deprecated
-      def paper_trail_on_update
-        paper_trail_deprecate "on_update", "paper_trail_on_update"
-        paper_trail.on_update
-      end
-
-      # @deprecated
-      def paper_trail_on_create
-        paper_trail_deprecate "on_create", "paper_trail_on_create"
-        paper_trail.on_create
-      end
-
-      # @deprecated
-      def paper_trail_off!
-        paper_trail_deprecate "disable", "paper_trail_off!"
-        paper_trail.disable
-      end
-
-      # @deprecated
-      def paper_trail_on!
-        paper_trail_deprecate "enable", "paper_trail_on!"
-        paper_trail.enable
-      end
-
-      # @deprecated
-      def paper_trail_enabled_for_model?
-        paper_trail_deprecate "enabled?", "paper_trail_enabled_for_model?"
-        paper_trail.enabled?
-      end
-
-      # @deprecated
-      def paper_trail_version_class
-        paper_trail_deprecate "version_class", "paper_trail_version_class"
-        paper_trail.version_class
-      end
     end
 
     # Wrap the following methods in a module so we can include them only in the
     # ActiveRecord models that declare `has_paper_trail`.
     module InstanceMethods
+      # @api public
       def paper_trail
         ::PaperTrail::RecordTrail.new(self)
       end
-
-      # @deprecated
-      def live?
-        self.class.paper_trail_deprecate "live?"
-        paper_trail.live?
-      end
-
-      # @deprecated
-      def paper_trail_originator
-        self.class.paper_trail_deprecate "originator", "paper_trail_originator"
-        paper_trail.originator
-      end
-
-      # @deprecated
-      def originator
-        self.class.paper_trail_deprecate "originator"
-        paper_trail.originator
-      end
-
-      # @deprecated
-      def clear_rolled_back_versions
-        self.class.paper_trail_deprecate "clear_rolled_back_versions"
-        paper_trail.clear_rolled_back_versions
-      end
-
-      # @deprecated
-      def source_version
-        self.class.paper_trail_deprecate "source_version"
-        paper_trail.source_version
-      end
-
-      # @deprecated
-      def version_at(*args)
-        self.class.paper_trail_deprecate "version_at"
-        paper_trail.version_at(*args)
-      end
-
-      # @deprecated
-      def versions_between(start_time, end_time, _reify_options = {})
-        self.class.paper_trail_deprecate "versions_between"
-        paper_trail.versions_between(start_time, end_time)
-      end
-
-      # @deprecated
-      def previous_version
-        self.class.paper_trail_deprecate "previous_version"
-        paper_trail.previous_version
-      end
-
-      # @deprecated
-      def next_version
-        self.class.paper_trail_deprecate "next_version"
-        paper_trail.next_version
-      end
-
-      # @deprecated
-      def paper_trail_enabled_for_model?
-        self.class.paper_trail_deprecate "enabled_for_model?", "paper_trail_enabled_for_model?"
-        paper_trail.enabled_for_model?
-      end
-
-      # @deprecated
-      def without_versioning(method = nil, &block)
-        self.class.paper_trail_deprecate "without_versioning"
-        paper_trail.without_versioning(method, &block)
-      end
-
-      # @deprecated
-      def appear_as_new_record(&block)
-        self.class.paper_trail_deprecate "appear_as_new_record"
-        paper_trail.appear_as_new_record(&block)
-      end
-
-      # @deprecated
-      def whodunnit(value, &block)
-        self.class.paper_trail_deprecate "whodunnit"
-        paper_trail.whodunnit(value, &block)
-      end
-
-      # @deprecated
-      def touch_with_version(name = nil)
-        self.class.paper_trail_deprecate "touch_with_version"
-        paper_trail.touch_with_version(name)
-      end
-
-      # `record_create` is deprecated in favor of `paper_trail.record_create`,
-      # but does not yet print a deprecation warning. When the `after_create`
-      # callback is registered (by ModelConfig#on_create) we still refer to this
-      # method by name, e.g.
-      #
-      #     @model_class.after_create :record_create, if: :save_version?
-      #
-      # instead of using the preferred method `paper_trail.record_create`, e.g.
-      #
-      #     @model_class.after_create { |r| r.paper_trail.record_create if r.save_version?}
-      #
-      # We still register the callback by name so that, if someone calls
-      # `has_paper_trail` twice, the callback will *not* be registered twice.
-      # Our own test suite calls `has_paper_trail` many times for the same
-      # class.
-      #
-      # In the future, perhaps we should require that users only set up
-      # PT once per class.
-      #
-      # @deprecated
-      def record_create
-        paper_trail.record_create
-      end
-
-      # See deprecation comment for `record_create`
-      # @deprecated
-      def record_update(force = nil)
-        paper_trail.record_update(force)
-      end
-
-      # @deprecated
-      def pt_record_object_changes?
-        self.class.paper_trail_deprecate "record_object_changes?", "pt_record_object_changes?"
-        paper_trail.record_object_changes?
-      end
-
-      # @deprecated
-      def pt_recordable_object
-        self.class.paper_trail_deprecate "recordable_object", "pt_recordable_object"
-        paper_trail.recordable_object
-      end
-
-      # @deprecated
-      def pt_recordable_object_changes
-        self.class.paper_trail_deprecate "recordable_object_changes", "pt_recordable_object_changes"
-        paper_trail.recordable_object_changes
-      end
-
-      # @deprecated
-      def changes_for_paper_trail
-        self.class.paper_trail_deprecate "changes", "changes_for_paper_trail"
-        paper_trail.changes
-      end
-
-      # See deprecation comment for `record_create`
-      # @deprecated
-      def clear_version_instance!
-        paper_trail.clear_version_instance
-      end
-
-      # See deprecation comment for `record_create`
-      # @deprecated
-      def reset_timestamp_attrs_for_update_if_needed!
-        paper_trail.reset_timestamp_attrs_for_update_if_needed
-      end
-
-      # See deprecation comment for `record_create`
-      # @deprecated
-      def record_destroy
-        paper_trail.record_destroy
-      end
-
-      # @deprecated
-      def save_associations(version)
-        self.class.paper_trail_deprecate "save_associations"
-        paper_trail.save_associations(version)
-      end
-
-      # @deprecated
-      def save_associations_belongs_to(version)
-        self.class.paper_trail_deprecate "save_associations_belongs_to"
-        paper_trail.save_associations_belongs_to(version)
-      end
-
-      # @deprecated
-      def save_associations_has_and_belongs_to_many(version)
-        self.class.paper_trail_deprecate(
-          "save_associations_habtm",
-          "save_associations_has_and_belongs_to_many"
-        )
-        paper_trail.save_associations_habtm(version)
-      end
-
-      # @deprecated
-      # @api private
-      def reset_transaction_id
-        ::ActiveSupport::Deprecation.warn(
-          "reset_transaction_id is deprecated, use PaperTrail.clear_transaction_id"
-        )
-        PaperTrail.clear_transaction_id
-      end
-
-      # @deprecated
-      # @api private
-      def merge_metadata(data)
-        self.class.paper_trail_deprecate "merge_metadata"
-        paper_trail.merge_metadata(data)
-      end
-
-      # @deprecated
-      def attributes_before_change
-        self.class.paper_trail_deprecate "attributes_before_change"
-        paper_trail.attributes_before_change
-      end
-
-      # @deprecated
-      def object_attrs_for_paper_trail
-        self.class.paper_trail_deprecate "object_attrs_for_paper_trail"
-        paper_trail.object_attrs_for_paper_trail
-      end
-
-      # @deprecated
-      def changed_notably?
-        self.class.paper_trail_deprecate "changed_notably?"
-        paper_trail.changed_notably?
-      end
-
-      # @deprecated
-      def ignored_attr_has_changed?
-        self.class.paper_trail_deprecate "ignored_attr_has_changed?"
-        paper_trail.ignored_attr_has_changed?
-      end
-
-      # @deprecated
-      def notably_changed
-        self.class.paper_trail_deprecate "notably_changed"
-        paper_trail.notably_changed
-      end
-
-      # @deprecated
-      def changed_and_not_ignored
-        self.class.paper_trail_deprecate "changed_and_not_ignored"
-        paper_trail.changed_and_not_ignored
-      end
-
-      # The new method is named "enabled?" for consistency.
-      # @deprecated
-      def paper_trail_switched_on?
-        self.class.paper_trail_deprecate "enabled?", "paper_trail_switched_on?"
-        paper_trail.enabled?
-      end
-
-      # @deprecated
-      # @api private
-      def save_version?
-        self.class.paper_trail_deprecate "save_version?"
-        paper_trail.save_version?
-      end
     end
   end
 end
diff --git a/lib/paper_trail/model_config.rb b/lib/paper_trail/model_config.rb
index 176055068..db402550c 100644
--- a/lib/paper_trail/model_config.rb
+++ b/lib/paper_trail/model_config.rb
@@ -31,7 +31,9 @@ def enabled?
 
     # Adds a callback that records a version after a "create" event.
     def on_create
-      @model_class.after_create :record_create, if: ->(m) { m.paper_trail.save_version? }
+      @model_class.after_create { |r|
+        r.paper_trail.record_create if r.paper_trail.save_version?
+      }
       return if @model_class.paper_trail_options[:on].include?(:create)
       @model_class.paper_trail_options[:on] << :create
     end
@@ -46,8 +48,10 @@ def on_destroy(recording_order = "before")
         ::ActiveSupport::Deprecation.warn(E_CANNOT_RECORD_AFTER_DESTROY)
       end
 
-      @model_class.send "#{recording_order}_destroy", :record_destroy,
-        if: ->(m) { m.paper_trail.save_version? }
+      @model_class.send(
+        "#{recording_order}_destroy",
+        ->(r) { r.paper_trail.record_destroy if r.paper_trail.save_version? }
+      )
 
       return if @model_class.paper_trail_options[:on].include?(:destroy)
       @model_class.paper_trail_options[:on] << :destroy
@@ -55,9 +59,15 @@ def on_destroy(recording_order = "before")
 
     # Adds a callback that records a version after an "update" event.
     def on_update
-      @model_class.before_save :reset_timestamp_attrs_for_update_if_needed!, on: :update
-      @model_class.after_update :record_update, if: ->(m) { m.paper_trail.save_version? }
-      @model_class.after_update :clear_version_instance!
+      @model_class.before_save(on: :update) { |r|
+        r.paper_trail.reset_timestamp_attrs_for_update_if_needed
+      }
+      @model_class.after_update { |r|
+        r.paper_trail.record_update(nil) if r.paper_trail.save_version?
+      }
+      @model_class.after_update { |r|
+        r.paper_trail.clear_version_instance
+      }
       return if @model_class.paper_trail_options[:on].include?(:update)
       @model_class.paper_trail_options[:on] << :update
     end
diff --git a/lib/paper_trail/record_trail.rb b/lib/paper_trail/record_trail.rb
index 82901687d..b421c06d6 100644
--- a/lib/paper_trail/record_trail.rb
+++ b/lib/paper_trail/record_trail.rb
@@ -360,7 +360,7 @@ def touch_with_version(name = nil)
       attributes.each { |column|
         @record.send(:write_attribute, column, current_time)
       }
-      @record.record_update(true) unless will_record_after_update?
+      record_update(true) unless will_record_after_update?
       @record.save!(validate: false)
     end
 
diff --git a/spec/models/widget_spec.rb b/spec/models/widget_spec.rb
index 3023145fc..5fd563fb1 100644
--- a/spec/models/widget_spec.rb
+++ b/spec/models/widget_spec.rb
@@ -238,8 +238,6 @@
       end
 
       describe "#whodunnit" do
-        it { is_expected.to respond_to(:whodunnit) }
-
         context "no block given" do
           it "should raise an error" do
             expect {