From b7a1dcf8891ab8b06c707753fc9702dd4ba3d882 Mon Sep 17 00:00:00 2001
From: Lorin Thwaits <lorint@gmail.com>
Date: Fri, 17 Aug 2018 23:40:25 +0100
Subject: [PATCH 01/12] Minimal changes necessary to support .joins()

---
 lib/paper_trail/events/create.rb             |  2 --
 lib/paper_trail/events/destroy.rb            |  4 +++-
 lib/paper_trail/events/update.rb             |  2 --
 lib/paper_trail/model_config.rb              | 14 ++------------
 lib/paper_trail/record_trail.rb              |  7 ++++---
 lib/paper_trail/version_concern.rb           |  9 +++++++++
 spec/controllers/articles_controller_spec.rb |  4 ++--
 spec/models/json_version_spec.rb             |  6 +++---
 spec/models/on/empty_array_spec.rb           |  4 ++--
 spec/models/pet_spec.rb                      |  2 +-
 spec/models/widget_spec.rb                   |  4 ++--
 spec/paper_trail/cleaner_spec.rb             |  6 +++---
 spec/paper_trail/model_spec.rb               |  2 +-
 13 files changed, 32 insertions(+), 34 deletions(-)

diff --git a/lib/paper_trail/events/create.rb b/lib/paper_trail/events/create.rb
index 1b479e358..76c014b42 100644
--- a/lib/paper_trail/events/create.rb
+++ b/lib/paper_trail/events/create.rb
@@ -13,8 +13,6 @@ class Create < Base
       # @api private
       def data
         data = {
-          item_id: @record.id,
-          item_type: @record.class.paper_trail.versions_association_item_type,
           event: @record.paper_trail_event || "create",
           whodunnit: PaperTrail.request.whodunnit
         }
diff --git a/lib/paper_trail/events/destroy.rb b/lib/paper_trail/events/destroy.rb
index 781771817..9cefc2ea1 100644
--- a/lib/paper_trail/events/destroy.rb
+++ b/lib/paper_trail/events/destroy.rb
@@ -12,9 +12,11 @@ class Destroy < Base
       #
       # @api private
       def data
+        klass = @record.class
         data = {
           item_id: @record.id,
-          item_type: @record.class.paper_trail.versions_association_item_type,
+          item_type: klass.descends_from_active_record? ? klass.name :
+            (@record.respond_to?(klass.inheritance_column) ? klass.name : klass.base_class.name),
           event: @record.paper_trail_event || "destroy",
           whodunnit: PaperTrail.request.whodunnit
         }
diff --git a/lib/paper_trail/events/update.rb b/lib/paper_trail/events/update.rb
index cfeb6d5fb..c2b59b6b7 100644
--- a/lib/paper_trail/events/update.rb
+++ b/lib/paper_trail/events/update.rb
@@ -25,8 +25,6 @@ def initialize(record, in_after_callback, is_touch, force_changes)
       # @api private
       def data
         data = {
-          item_id: @record.id,
-          item_type: @record.class.paper_trail.versions_association_item_type,
           event: @record.paper_trail_event || "update",
           whodunnit: PaperTrail.request.whodunnit
         }
diff --git a/lib/paper_trail/model_config.rb b/lib/paper_trail/model_config.rb
index 6320f6d38..41106f293 100644
--- a/lib/paper_trail/model_config.rb
+++ b/lib/paper_trail/model_config.rb
@@ -110,14 +110,6 @@ def version_class
       @_version_class ||= @model_class.version_class_name.constantize
     end
 
-    def versions_association_item_type
-      if @model_class.descends_from_active_record?
-        @model_class.base_class.name
-      else
-        @model_class.name
-      end
-    end
-
     private
 
     def active_record_gem_version
@@ -150,10 +142,8 @@ def setup_versions_association(klass)
       klass.has_many(
         klass.versions_association_name,
         lambda do
-          relation = order(model.timestamp_sort_order)
-          item_type = klass.paper_trail.send(:versions_association_item_type)
-          relation = relation.unscope(where: :item_type).where(item_type: item_type)
-          relation
+          order!(model.timestamp_sort_order)
+          unscope(where: :item_type).where(item_type: klass.name) if klass.descendants.empty?
         end,
         class_name: klass.version_class_name,
         as: :item
diff --git a/lib/paper_trail/record_trail.rb b/lib/paper_trail/record_trail.rb
index ee26316f5..b50018611 100644
--- a/lib/paper_trail/record_trail.rb
+++ b/lib/paper_trail/record_trail.rb
@@ -84,7 +84,8 @@ def record_create
       # `data_for_create` but PT-AT still does.
       data = event.data.merge(data_for_create)
 
-      version = @record.class.paper_trail.version_class.new(data)
+      version = @record.send(@record.class.versions_association_name).new(data)
+      version.item_type = @record.class.name if @record.class.descends_from_active_record?
       version.save!
       version
     end
@@ -138,7 +139,7 @@ def record_update(force:, in_after_callback:, is_touch:)
       # `data_for_update` but PT-AT still does.
       data = event.data.merge(data_for_update)
 
-      version = @record.class.paper_trail.version_class.new(data)
+      version = @record.send(@record.class.versions_association_name).new(data)
       if version.save
         version
       else
@@ -164,7 +165,7 @@ def record_update_columns(changes)
       # `data_for_update_columns` but PT-AT still does.
       data = event.data.merge(data_for_update_columns)
 
-      version = @record.class.paper_trail.version_class.new(data)
+      version = @record.send(@record.class.versions_association_name).new(data)
       if version.save
         version
       else
diff --git a/lib/paper_trail/version_concern.rb b/lib/paper_trail/version_concern.rb
index 86150e0a0..dd7cbe1bd 100644
--- a/lib/paper_trail/version_concern.rb
+++ b/lib/paper_trail/version_concern.rb
@@ -20,6 +20,7 @@ module VersionConcern
       end
 
       validates_presence_of :event
+      before_update :ensure_item_type_persists_during_create
       after_create :enforce_version_limit!
     end
 
@@ -304,6 +305,14 @@ def object_changes_deserialized
       end
     end
 
+    # Remarkably when doing a #create with a custom item_type, ActiveRecord attempts
+    # to update it back to the base_class.
+    def ensure_item_type_persists_during_create
+      if changes.count == 1 && item_type_changed? && self.event == "create"
+        self.item_type = item_type_was
+      end
+    end
+
     # Enforces the `version_limit`, if set. Default: no limit.
     # @api private
     def enforce_version_limit!
diff --git a/spec/controllers/articles_controller_spec.rb b/spec/controllers/articles_controller_spec.rb
index 1758e15e6..c524eee21 100644
--- a/spec/controllers/articles_controller_spec.rb
+++ b/spec/controllers/articles_controller_spec.rb
@@ -12,7 +12,7 @@
         post :create, params_wrapper(article: { title: "Doh", content: FFaker::Lorem.sentence })
         expect(assigns(:article)).not_to be_nil
         expect(PaperTrail.request.enabled?).to eq(true)
-        expect(assigns(:article).versions.count).to eq(1)
+        expect(assigns(:article).versions.length).to eq(1)
       end
 
       after { PaperTrail.enabled = false }
@@ -23,7 +23,7 @@
         expect(PaperTrail.enabled?).to eq(false)
         post :create, params_wrapper(article: { title: "Doh", content: FFaker::Lorem.sentence })
         expect(PaperTrail.request.enabled?).to eq(false)
-        expect(assigns(:article).versions.count).to eq(0)
+        expect(assigns(:article).versions.length).to eq(0)
       end
     end
   end
diff --git a/spec/models/json_version_spec.rb b/spec/models/json_version_spec.rb
index 750f4d543..42453dde0 100644
--- a/spec/models/json_version_spec.rb
+++ b/spec/models/json_version_spec.rb
@@ -33,11 +33,11 @@
       context "valid arguments", versioning: true do
         it "locates versions according to their `object` contents" do
           fruit = Fruit.create!(name: "apple")
-          expect(fruit.versions.count).to eq(1)
+          expect(fruit.versions.length).to eq(1)
           fruit.update_attributes!(name: "banana", color: "aqua")
-          expect(fruit.versions.count).to eq(2)
+          expect(fruit.versions.length).to eq(2)
           fruit.update_attributes!(name: "coconut", color: "black")
-          expect(fruit.versions.count).to eq(3)
+          expect(fruit.versions.length).to eq(3)
           where_apple = described_class.where_object(name: "apple")
           expect(where_apple.to_sql).to eq(
             <<-SQL.squish
diff --git a/spec/models/on/empty_array_spec.rb b/spec/models/on/empty_array_spec.rb
index fbb6c83cb..b9236336f 100644
--- a/spec/models/on/empty_array_spec.rb
+++ b/spec/models/on/empty_array_spec.rb
@@ -15,9 +15,9 @@ module On
     describe ".paper_trail.update_columns" do
       it "creates a version record" do
         widget = Widget.create
-        assert_equal 1, widget.versions.count
+        assert_equal 1, widget.versions.length
         widget.paper_trail.update_columns(name: "Bugle")
-        assert_equal 2, widget.versions.count
+        assert_equal 2, widget.versions.length
       end
     end
 
diff --git a/spec/models/pet_spec.rb b/spec/models/pet_spec.rb
index 268d35d62..47ff4652c 100644
--- a/spec/models/pet_spec.rb
+++ b/spec/models/pet_spec.rb
@@ -112,7 +112,7 @@
 
     # After creating a bunch of records above, we change the inheritance_column
     # so that we can demonstrate passing hints to the migration generator.
-    xcontext "simulate a historical change to inheritance_column" do
+    context "simulate a historical change to inheritance_column" do
       before do
         Animal.inheritance_column = "species_xyz"
       end
diff --git a/spec/models/widget_spec.rb b/spec/models/widget_spec.rb
index c13982629..df8fe5805 100644
--- a/spec/models/widget_spec.rb
+++ b/spec/models/widget_spec.rb
@@ -249,9 +249,9 @@
   describe "#update", versioning: true do
     it "creates a version record" do
       widget = Widget.create
-      assert_equal 1, widget.versions.count
+      assert_equal 1, widget.versions.length
       widget.update_attributes(name: "Bugle")
-      assert_equal 2, widget.versions.count
+      assert_equal 2, widget.versions.length
     end
   end
 end
diff --git a/spec/paper_trail/cleaner_spec.rb b/spec/paper_trail/cleaner_spec.rb
index 46f7c9bef..78755bf96 100644
--- a/spec/paper_trail/cleaner_spec.rb
+++ b/spec/paper_trail/cleaner_spec.rb
@@ -54,11 +54,11 @@ module PaperTrail
           date = animal.versions.first.created_at.to_date
           animal.update_attribute(:name, FFaker::Name.name)
           expect(PaperTrail::Version.count).to(eq(10))
-          expect(animal.versions.count).to(eq(4))
+          expect(animal.versions.size).to(eq(4))
           expect(animal.paper_trail.versions_between(date, (date + 1.day)).size).to(eq(3))
           PaperTrail.clean_versions!(date: date)
           expect(PaperTrail::Version.count).to(eq(8))
-          expect(animal.versions.count).to(eq(2))
+          expect(animal.versions.reload.size).to(eq(2))
           expect(animal.versions.first.created_at.to_date).to(eq(date))
           # Why use `equal?` here instead of something less strict?
           # Doesn't `to_date` always produce a new date object?
@@ -100,7 +100,7 @@ module PaperTrail
             date = animal.versions.first.created_at.to_date
             expect(PaperTrail::Version.count).to(eq(11))
             [animal, dog].each do |animal|
-              expect(animal.versions.count).to(eq(4))
+              expect(animal.versions.size).to(eq(4))
               expect(animal.versions.between(date, (date + 1.day)).size).to(eq(3))
             end
           end
diff --git a/spec/paper_trail/model_spec.rb b/spec/paper_trail/model_spec.rb
index cf1a8b295..9a17bba91 100644
--- a/spec/paper_trail/model_spec.rb
+++ b/spec/paper_trail/model_spec.rb
@@ -416,7 +416,7 @@
           before { @widget.update_attributes(name: "Ford") }
 
           it "add to its trail" do
-            expect(@widget.versions.count).to(eq((@count + 1)))
+            expect(@widget.versions.length).to(eq((@count + 1)))
           end
         end
       end

From f561e64a42e0877fa3f1b02e69e59956feda150e Mon Sep 17 00:00:00 2001
From: Lorin Thwaits <lorint@gmail.com>
Date: Fri, 17 Aug 2018 23:49:38 +0100
Subject: [PATCH 02/12] A couple cops

---
 lib/paper_trail/events/destroy.rb  | 8 ++++++--
 lib/paper_trail/version_concern.rb | 2 +-
 2 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/lib/paper_trail/events/destroy.rb b/lib/paper_trail/events/destroy.rb
index 9cefc2ea1..10075dddb 100644
--- a/lib/paper_trail/events/destroy.rb
+++ b/lib/paper_trail/events/destroy.rb
@@ -15,8 +15,12 @@ def data
         klass = @record.class
         data = {
           item_id: @record.id,
-          item_type: klass.descends_from_active_record? ? klass.name :
-            (@record.respond_to?(klass.inheritance_column) ? klass.name : klass.base_class.name),
+          item_type:
+            if klass.descends_from_active_record?
+              klass.name
+            else
+              @record.respond_to?(klass.inheritance_column) ? klass.name : klass.base_class.name
+            end,
           event: @record.paper_trail_event || "destroy",
           whodunnit: PaperTrail.request.whodunnit
         }
diff --git a/lib/paper_trail/version_concern.rb b/lib/paper_trail/version_concern.rb
index dd7cbe1bd..24451b051 100644
--- a/lib/paper_trail/version_concern.rb
+++ b/lib/paper_trail/version_concern.rb
@@ -308,7 +308,7 @@ def object_changes_deserialized
     # Remarkably when doing a #create with a custom item_type, ActiveRecord attempts
     # to update it back to the base_class.
     def ensure_item_type_persists_during_create
-      if changes.count == 1 && item_type_changed? && self.event == "create"
+      if changes.count == 1 && item_type_changed? && event == "create"
         self.item_type = item_type_was
       end
     end

From 7489c3f73b4bd08b357078a144a518ef85e86e27 Mon Sep 17 00:00:00 2001
From: Lorin Thwaits <lorint@gmail.com>
Date: Sat, 18 Aug 2018 00:06:15 +0100
Subject: [PATCH 03/12] Stray item_type= in record_create

---
 lib/paper_trail/record_trail.rb | 1 -
 1 file changed, 1 deletion(-)

diff --git a/lib/paper_trail/record_trail.rb b/lib/paper_trail/record_trail.rb
index b50018611..05e75439c 100644
--- a/lib/paper_trail/record_trail.rb
+++ b/lib/paper_trail/record_trail.rb
@@ -85,7 +85,6 @@ def record_create
       data = event.data.merge(data_for_create)
 
       version = @record.send(@record.class.versions_association_name).new(data)
-      version.item_type = @record.class.name if @record.class.descends_from_active_record?
       version.save!
       version
     end

From a8583c3f0989ab18f9caf4eda56df50b8cdfd9f4 Mon Sep 17 00:00:00 2001
From: Lorin Thwaits <lorint@gmail.com>
Date: Sat, 18 Aug 2018 00:46:01 +0100
Subject: [PATCH 04/12] A little simpler logic for reverting to use base_class

---
 lib/paper_trail/events/destroy.rb | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/lib/paper_trail/events/destroy.rb b/lib/paper_trail/events/destroy.rb
index 10075dddb..5f905837f 100644
--- a/lib/paper_trail/events/destroy.rb
+++ b/lib/paper_trail/events/destroy.rb
@@ -16,10 +16,10 @@ def data
         data = {
           item_id: @record.id,
           item_type:
-            if klass.descends_from_active_record?
+            if klass.descends_from_active_record? || @record.respond_to?(klass.inheritance_column)
               klass.name
             else
-              @record.respond_to?(klass.inheritance_column) ? klass.name : klass.base_class.name
+              klass.base_class.name
             end,
           event: @record.paper_trail_event || "destroy",
           whodunnit: PaperTrail.request.whodunnit

From 85b956b4baac52bdf9a97341847218834344c593 Mon Sep 17 00:00:00 2001
From: Lorin Thwaits <lorint@gmail.com>
Date: Sat, 18 Aug 2018 01:26:24 +0100
Subject: [PATCH 05/12] Destroy can refer simply to the real class name

---
 lib/paper_trail/events/destroy.rb | 8 +-------
 1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/lib/paper_trail/events/destroy.rb b/lib/paper_trail/events/destroy.rb
index 5f905837f..a5367bf81 100644
--- a/lib/paper_trail/events/destroy.rb
+++ b/lib/paper_trail/events/destroy.rb
@@ -12,15 +12,9 @@ class Destroy < Base
       #
       # @api private
       def data
-        klass = @record.class
         data = {
           item_id: @record.id,
-          item_type:
-            if klass.descends_from_active_record? || @record.respond_to?(klass.inheritance_column)
-              klass.name
-            else
-              klass.base_class.name
-            end,
+          item_type: @record.class.name,
           event: @record.paper_trail_event || "destroy",
           whodunnit: PaperTrail.request.whodunnit
         }

From cefac8bebd46ad67b07b6f9ce431734fb6ea7f25 Mon Sep 17 00:00:00 2001
From: Lorin Thwaits <lorint@gmail.com>
Date: Sat, 18 Aug 2018 10:40:09 +0100
Subject: [PATCH 06/12] Cleaner way to avoid .create() reverting to the
 base_class

---
 lib/paper_trail/model_config.rb    | 13 +++++++++++--
 lib/paper_trail/version_concern.rb |  9 ---------
 2 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/lib/paper_trail/model_config.rb b/lib/paper_trail/model_config.rb
index 41106f293..f362a29d8 100644
--- a/lib/paper_trail/model_config.rb
+++ b/lib/paper_trail/model_config.rb
@@ -139,16 +139,25 @@ def cannot_record_after_destroy?
     # is "Dog". If `attrs["species"]` is blank, `item_type` is "Animal". See
     # `spec/models/animal_spec.rb`.
     def setup_versions_association(klass)
-      klass.has_many(
+      has_many = klass.has_many(
         klass.versions_association_name,
         lambda do
           order!(model.timestamp_sort_order)
-          unscope(where: :item_type).where(item_type: klass.name) if klass.descendants.empty?
+          unscope(where: :item_type).where(item_type: klass.name) unless klass.descendants.any?
         end,
         class_name: klass.version_class_name,
         as: :item
       )
 
+      # Only for this .versions association override HasManyAssociation#collection?
+      # (that normally always returns true) so it returns false when referring to
+      # a subclassed model that uses STI.  Allows .create() events to not revert
+      # back to base_class at the final stages when Association#creation_attributes
+      # gets called.
+      has_many[klass.versions_association_name.to_s].define_singleton_method(:collection?) do
+        active_record.descends_from_active_record?
+      end
+
       # We override the assocation when STI models are created from a base class
       # in order to use the right `item_type`.
       klass.singleton_class.prepend(Module.new {
diff --git a/lib/paper_trail/version_concern.rb b/lib/paper_trail/version_concern.rb
index 24451b051..86150e0a0 100644
--- a/lib/paper_trail/version_concern.rb
+++ b/lib/paper_trail/version_concern.rb
@@ -20,7 +20,6 @@ module VersionConcern
       end
 
       validates_presence_of :event
-      before_update :ensure_item_type_persists_during_create
       after_create :enforce_version_limit!
     end
 
@@ -305,14 +304,6 @@ def object_changes_deserialized
       end
     end
 
-    # Remarkably when doing a #create with a custom item_type, ActiveRecord attempts
-    # to update it back to the base_class.
-    def ensure_item_type_persists_during_create
-      if changes.count == 1 && item_type_changed? && event == "create"
-        self.item_type = item_type_was
-      end
-    end
-
     # Enforces the `version_limit`, if set. Default: no limit.
     # @api private
     def enforce_version_limit!

From 7372b8fe14817d0695ac0afbd0792ab0151347a6 Mon Sep 17 00:00:00 2001
From: Lorin Thwaits <lorint@gmail.com>
Date: Sat, 18 Aug 2018 11:32:59 +0100
Subject: [PATCH 07/12] Be happy, Rubocop

---
 lib/paper_trail/model_config.rb | 14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/lib/paper_trail/model_config.rb b/lib/paper_trail/model_config.rb
index f362a29d8..5d187b3f6 100644
--- a/lib/paper_trail/model_config.rb
+++ b/lib/paper_trail/model_config.rb
@@ -139,7 +139,7 @@ def cannot_record_after_destroy?
     # is "Dog". If `attrs["species"]` is blank, `item_type` is "Animal". See
     # `spec/models/animal_spec.rb`.
     def setup_versions_association(klass)
-      has_many = klass.has_many(
+      has_manys = klass.has_many(
         klass.versions_association_name,
         lambda do
           order!(model.timestamp_sort_order)
@@ -151,15 +151,19 @@ def setup_versions_association(klass)
 
       # Only for this .versions association override HasManyAssociation#collection?
       # (that normally always returns true) so it returns false when referring to
-      # a subclassed model that uses STI.  Allows .create() events to not revert
+      # a subclassed model that uses STI.  Allows .create() events not to revert
       # back to base_class at the final stages when Association#creation_attributes
       # gets called.
-      has_many[klass.versions_association_name.to_s].define_singleton_method(:collection?) do
+      has_manys[klass.versions_association_name.to_s].define_singleton_method(:collection?) do
         active_record.descends_from_active_record?
       end
 
-      # We override the assocation when STI models are created from a base class
-      # in order to use the right `item_type`.
+      setup_versions_association_when_inheriting(klass)
+    end
+
+    # When STI models are created from a base class, override the otherwise-inherited
+    # has_many :versions in order to use the right `item_type`.
+    def setup_versions_association_when_inheriting(klass)
       klass.singleton_class.prepend(Module.new {
         def inherited(klass)
           super

From 7ce28d82cb0dd3f35b6071eac1aceb83c891ac71 Mon Sep 17 00:00:00 2001
From: Lorin Thwaits <lorint@gmail.com>
Date: Sun, 19 Aug 2018 19:25:25 +0100
Subject: [PATCH 08/12] Override creation_attributes instead of collection?

---
 lib/paper_trail/events/destroy.rb             |  7 +++-
 lib/paper_trail/model_config.rb               | 36 ++++++++++---------
 lib/paper_trail/record_trail.rb               | 27 +++++++++-----
 .../dummy_app/app/models/callback_modifier.rb |  4 +++
 spec/models/management_spec.rb                | 30 ++++++++++++++++
 5 files changed, 77 insertions(+), 27 deletions(-)
 create mode 100644 spec/models/management_spec.rb

diff --git a/lib/paper_trail/events/destroy.rb b/lib/paper_trail/events/destroy.rb
index a5367bf81..96d9ed2dd 100644
--- a/lib/paper_trail/events/destroy.rb
+++ b/lib/paper_trail/events/destroy.rb
@@ -14,7 +14,12 @@ class Destroy < Base
       def data
         data = {
           item_id: @record.id,
-          item_type: @record.class.name,
+          item_type:
+            if @record.respond_to?(@record.class.inheritance_column)
+              @record.class
+            else
+              @record.class.base_class
+            end.name,
           event: @record.paper_trail_event || "destroy",
           whodunnit: PaperTrail.request.whodunnit
         }
diff --git a/lib/paper_trail/model_config.rb b/lib/paper_trail/model_config.rb
index 5d187b3f6..359a142b5 100644
--- a/lib/paper_trail/model_config.rb
+++ b/lib/paper_trail/model_config.rb
@@ -139,31 +139,19 @@ def cannot_record_after_destroy?
     # is "Dog". If `attrs["species"]` is blank, `item_type` is "Animal". See
     # `spec/models/animal_spec.rb`.
     def setup_versions_association(klass)
-      has_manys = klass.has_many(
+      klass.has_many(
         klass.versions_association_name,
         lambda do
           order!(model.timestamp_sort_order)
-          unscope(where: :item_type).where(item_type: klass.name) unless klass.descendants.any?
+          unscope(where: :item_type).where(item_type: klass.name) unless klass.descendants.any? &&
+            !klass.columns.include?(klass.inheritance_column)
         end,
         class_name: klass.version_class_name,
         as: :item
       )
 
-      # Only for this .versions association override HasManyAssociation#collection?
-      # (that normally always returns true) so it returns false when referring to
-      # a subclassed model that uses STI.  Allows .create() events not to revert
-      # back to base_class at the final stages when Association#creation_attributes
-      # gets called.
-      has_manys[klass.versions_association_name.to_s].define_singleton_method(:collection?) do
-        active_record.descends_from_active_record?
-      end
-
-      setup_versions_association_when_inheriting(klass)
-    end
-
-    # When STI models are created from a base class, override the otherwise-inherited
-    # has_many :versions in order to use the right `item_type`.
-    def setup_versions_association_when_inheriting(klass)
+      # When STI models are created from a base class, override the otherwise-inherited
+      # has_many :versions so it will use the right `item_type`.
       klass.singleton_class.prepend(Module.new {
         def inherited(klass)
           super
@@ -172,6 +160,20 @@ def inherited(klass)
       })
     end
 
+    # Allow .create() events not to revert back to base_class at the final stages when
+    # Association#creation_attributes gets called.
+    ActiveRecord::Associations::HasManyAssociation.prepend(Module.new {
+      def creation_attributes
+        if reflection.klass.name == "PaperTrail::Version" &&
+            owner.respond_to?(owner.class.inheritance_column)
+          # For STI records PaperTrail uses the real class name instead of the base_class.
+          { item_type: owner.class.name, item_id: owner[reflection.active_record_primary_key] }
+        else
+          super
+        end
+      end
+    })
+
     def setup_associations(options)
       @model_class.class_attribute :version_association_name
       @model_class.version_association_name = options[:version] || :version
diff --git a/lib/paper_trail/record_trail.rb b/lib/paper_trail/record_trail.rb
index 05e75439c..4c0988c4f 100644
--- a/lib/paper_trail/record_trail.rb
+++ b/lib/paper_trail/record_trail.rb
@@ -138,12 +138,7 @@ def record_update(force:, in_after_callback:, is_touch:)
       # `data_for_update` but PT-AT still does.
       data = event.data.merge(data_for_update)
 
-      version = @record.send(@record.class.versions_association_name).new(data)
-      if version.save
-        version
-      else
-        log_version_errors(version, :update)
-      end
+      update_record(data)
     end
 
     # PT-AT extends this method to add its transaction id.
@@ -164,11 +159,25 @@ def record_update_columns(changes)
       # `data_for_update_columns` but PT-AT still does.
       data = event.data.merge(data_for_update_columns)
 
-      version = @record.send(@record.class.versions_association_name).new(data)
-      if version.save
-        version
+      update_record(data)
+    end
+
+    # Used by both #record_update and #record_update_columns
+    def update_record(data)
+      versions_assoc = @record.send(@record.class.versions_association_name)
+      if @record.respond_to?(@record.class.inheritance_column)
+        # Version item_type will be the real class name
+        version = versions_assoc.new(data)
+        version.save
       else
+        # Version item_type will be the base_class name
+        version = versions_assoc.create(data)
+      end
+
+      if version.errors.any?
         log_version_errors(version, :update)
+      else
+        version
       end
     end
 
diff --git a/spec/dummy_app/app/models/callback_modifier.rb b/spec/dummy_app/app/models/callback_modifier.rb
index 53e7d468e..edb5f1057 100644
--- a/spec/dummy_app/app/models/callback_modifier.rb
+++ b/spec/dummy_app/app/models/callback_modifier.rb
@@ -1,6 +1,10 @@
 # frozen_string_literal: true
 
 class CallbackModifier < ActiveRecord::Base
+  # These guys need to look just a _little_ more like a real STI class in order for the tests to
+  # call modifier.versions().
+  attr_accessor :type
+
   has_paper_trail on: []
 
   def test_destroy
diff --git a/spec/models/management_spec.rb b/spec/models/management_spec.rb
new file mode 100644
index 000000000..228fc1ed4
--- /dev/null
+++ b/spec/models/management_spec.rb
@@ -0,0 +1,30 @@
+# frozen_string_literal: true
+
+require "spec_helper"
+
+# Add a STI-ish class on-the-fly
+class Management < Customer
+end
+
+RSpec.describe Management, type: :model, versioning: true do
+  it { is_expected.to be_versioned }
+
+  it "utilises the base_class for STI classes having no type column" do
+    expect(Management.inheritance_column).to eq("type")
+    expect(Management.columns.map(&:name)).not_to include("type")
+
+    # Create, update, and destroy a Management and a Customer
+    customer1 = Customer.create(name: "Cust 1")
+    customer2 = Management.create(name: "Cust 2")
+    customer1.update(name: "Cust 1a")
+    customer2.update(name: "Cust 2a")
+    customer1.destroy
+    customer2.destroy
+
+    # All versions end up with an `item_type` of Customer
+    cust_pts = PaperTrail::Version.where(item_type: "Customer")
+    mgmt_pts = PaperTrail::Version.where(item_type: "Management")
+    expect(cust_pts.count).to eq(6)
+    expect(mgmt_pts.count).to eq(0)
+  end
+end

From b91ae7acb85ffeb64697907ab42e43f852de9889 Mon Sep 17 00:00:00 2001
From: Lorin Thwaits <lorint@gmail.com>
Date: Sun, 19 Aug 2018 20:13:33 +0100
Subject: [PATCH 09/12] Link to overridden
 HasManyAssociation#creation_attributes

---
 lib/paper_trail/model_config.rb | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/lib/paper_trail/model_config.rb b/lib/paper_trail/model_config.rb
index 359a142b5..7ac11541f 100644
--- a/lib/paper_trail/model_config.rb
+++ b/lib/paper_trail/model_config.rb
@@ -143,8 +143,9 @@ def setup_versions_association(klass)
         klass.versions_association_name,
         lambda do
           order!(model.timestamp_sort_order)
-          unscope(where: :item_type).where(item_type: klass.name) unless klass.descendants.any? &&
-            !klass.columns.include?(klass.inheritance_column)
+          unless klass.descendants.any? && klass.columns.exclude?(klass.inheritance_column)
+            unscope(where: :item_type).where(item_type: klass.name)
+          end
         end,
         class_name: klass.version_class_name,
         as: :item
@@ -161,7 +162,8 @@ def inherited(klass)
     end
 
     # Allow .create() events not to revert back to base_class at the final stages when
-    # Association#creation_attributes gets called.
+    # Association#creation_attributes gets called.  An override for this method from Rails:
+    # https://github.com/rails/rails/blob/v5.2.1/activerecord/lib/active_record/associations/association.rb#L198-L210
     ActiveRecord::Associations::HasManyAssociation.prepend(Module.new {
       def creation_attributes
         if reflection.klass.name == "PaperTrail::Version" &&

From dd5eab9ae4aa88ef10e92d60ec87e93d4954302b Mon Sep 17 00:00:00 2001
From: Lorin Thwaits <lorint@gmail.com>
Date: Wed, 22 Aug 2018 13:27:57 +0100
Subject: [PATCH 10/12] Updates based on Jared's review

---
 lib/paper_trail/model_config.rb               | 40 ++++++++++---------
 .../dummy_app/app/models/callback_modifier.rb |  4 --
 spec/dummy_app/app/models/management.rb       |  6 +++
 .../20110208155312_set_up_test_tables.rb      |  1 +
 spec/models/management_spec.rb                |  4 --
 5 files changed, 29 insertions(+), 26 deletions(-)
 create mode 100644 spec/dummy_app/app/models/management.rb

diff --git a/lib/paper_trail/model_config.rb b/lib/paper_trail/model_config.rb
index 7ac11541f..960091e1f 100644
--- a/lib/paper_trail/model_config.rb
+++ b/lib/paper_trail/model_config.rb
@@ -139,11 +139,15 @@ def cannot_record_after_destroy?
     # is "Dog". If `attrs["species"]` is blank, `item_type` is "Animal". See
     # `spec/models/animal_spec.rb`.
     def setup_versions_association(klass)
-      klass.has_many(
+      has_manys = klass.has_many(
         klass.versions_association_name,
         lambda do
           order!(model.timestamp_sort_order)
-          unless klass.descendants.any? && klass.columns.exclude?(klass.inheritance_column)
+          # Unless it's not a subclassed model
+          unless klass.descendants.any? &&
+              # Or an STI devoid of an `inheritance_column`
+              klass.columns.exclude?(klass.inheritance_column)
+            # Search for Versions based on the real class name
             unscope(where: :item_type).where(item_type: klass.name)
           end
         end,
@@ -151,7 +155,22 @@ def setup_versions_association(klass)
         as: :item
       )
 
-      # When STI models are created from a base class, override the otherwise-inherited
+      # Only for this .versions association override HasManyAssociation#collection?
+      # (that normally always returns true) so it returns false when referring to
+      # a subclassed model that uses STI.  Allows .create() events not to revert
+      # back to base_class at the final stages when Association#creation_attributes
+      # gets called.
+      unless klass.descends_from_active_record?
+        has_manys[klass.versions_association_name.to_s].define_singleton_method(:collection?) do
+          false
+        end
+      end
+
+      setup_versions_association_when_inheriting(klass)
+    end
+
+    def setup_versions_association_when_inheriting(klass)
+      # When STI models are created from another class, override the otherwise-inherited
       # has_many :versions so it will use the right `item_type`.
       klass.singleton_class.prepend(Module.new {
         def inherited(klass)
@@ -161,21 +180,6 @@ def inherited(klass)
       })
     end
 
-    # Allow .create() events not to revert back to base_class at the final stages when
-    # Association#creation_attributes gets called.  An override for this method from Rails:
-    # https://github.com/rails/rails/blob/v5.2.1/activerecord/lib/active_record/associations/association.rb#L198-L210
-    ActiveRecord::Associations::HasManyAssociation.prepend(Module.new {
-      def creation_attributes
-        if reflection.klass.name == "PaperTrail::Version" &&
-            owner.respond_to?(owner.class.inheritance_column)
-          # For STI records PaperTrail uses the real class name instead of the base_class.
-          { item_type: owner.class.name, item_id: owner[reflection.active_record_primary_key] }
-        else
-          super
-        end
-      end
-    })
-
     def setup_associations(options)
       @model_class.class_attribute :version_association_name
       @model_class.version_association_name = options[:version] || :version
diff --git a/spec/dummy_app/app/models/callback_modifier.rb b/spec/dummy_app/app/models/callback_modifier.rb
index edb5f1057..53e7d468e 100644
--- a/spec/dummy_app/app/models/callback_modifier.rb
+++ b/spec/dummy_app/app/models/callback_modifier.rb
@@ -1,10 +1,6 @@
 # frozen_string_literal: true
 
 class CallbackModifier < ActiveRecord::Base
-  # These guys need to look just a _little_ more like a real STI class in order for the tests to
-  # call modifier.versions().
-  attr_accessor :type
-
   has_paper_trail on: []
 
   def test_destroy
diff --git a/spec/dummy_app/app/models/management.rb b/spec/dummy_app/app/models/management.rb
new file mode 100644
index 000000000..50fbd5cc5
--- /dev/null
+++ b/spec/dummy_app/app/models/management.rb
@@ -0,0 +1,6 @@
+# frozen_string_literal: true
+
+# Note that there is no `type` column for this subclassed model, so changes to Management objects
+# should result in Versions which have an item_type of Customer.
+class Management < Customer
+end
diff --git a/spec/dummy_app/db/migrate/20110208155312_set_up_test_tables.rb b/spec/dummy_app/db/migrate/20110208155312_set_up_test_tables.rb
index d102aa90a..869088970 100644
--- a/spec/dummy_app/db/migrate/20110208155312_set_up_test_tables.rb
+++ b/spec/dummy_app/db/migrate/20110208155312_set_up_test_tables.rb
@@ -297,6 +297,7 @@ def up
     end
 
     create_table :callback_modifiers, force: true do |t|
+      t.string  :type
       t.string  :some_content
       t.boolean :deleted, default: false
     end
diff --git a/spec/models/management_spec.rb b/spec/models/management_spec.rb
index 228fc1ed4..6ef5c60a6 100644
--- a/spec/models/management_spec.rb
+++ b/spec/models/management_spec.rb
@@ -2,10 +2,6 @@
 
 require "spec_helper"
 
-# Add a STI-ish class on-the-fly
-class Management < Customer
-end
-
 RSpec.describe Management, type: :model, versioning: true do
   it { is_expected.to be_versioned }
 

From cd8a5ca87986cab3df353e4a9b749e1333d5fd45 Mon Sep 17 00:00:00 2001
From: Lorin Thwaits <lorint@gmail.com>
Date: Wed, 22 Aug 2018 13:38:44 +0100
Subject: [PATCH 11/12] Removed order\!

---
 lib/paper_trail/model_config.rb | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/lib/paper_trail/model_config.rb b/lib/paper_trail/model_config.rb
index 960091e1f..40adfc1ea 100644
--- a/lib/paper_trail/model_config.rb
+++ b/lib/paper_trail/model_config.rb
@@ -142,14 +142,15 @@ def setup_versions_association(klass)
       has_manys = klass.has_many(
         klass.versions_association_name,
         lambda do
-          order!(model.timestamp_sort_order)
+          relation = order(model.timestamp_sort_order)
           # Unless it's not a subclassed model
           unless klass.descendants.any? &&
               # Or an STI devoid of an `inheritance_column`
               klass.columns.exclude?(klass.inheritance_column)
             # Search for Versions based on the real class name
-            unscope(where: :item_type).where(item_type: klass.name)
+            relation = relation.unscope(where: :item_type).where(item_type: klass.name)
           end
+          relation
         end,
         class_name: klass.version_class_name,
         as: :item

From ad549eb4ceaa2a75b3d3059d577275d6f155a72b Mon Sep 17 00:00:00 2001
From: Lorin Thwaits <lorint@gmail.com>
Date: Sun, 26 Aug 2018 11:33:40 +0100
Subject: [PATCH 12/12] Verify that all tests work properly along with the
 store_base_sti_class gem

---
 spec/models/management_spec.rb | 6 ++++++
 spec/models/pet_spec.rb        | 4 ++++
 2 files changed, 10 insertions(+)

diff --git a/spec/models/management_spec.rb b/spec/models/management_spec.rb
index 6ef5c60a6..ff7d51a37 100644
--- a/spec/models/management_spec.rb
+++ b/spec/models/management_spec.rb
@@ -6,6 +6,12 @@
   it { is_expected.to be_versioned }
 
   it "utilises the base_class for STI classes having no type column" do
+    # Specific to the store_base_sti_class gem:
+    if ActiveRecord::Base.respond_to?(:store_base_sti_class) &&
+        !ActiveRecord::Base.store_base_sti_class
+      skip("When using store_base_sti_class it is necessary to implement a type column")
+    end
+
     expect(Management.inheritance_column).to eq("type")
     expect(Management.columns.map(&:name)).not_to include("type")
 
diff --git a/spec/models/pet_spec.rb b/spec/models/pet_spec.rb
index 47ff4652c..44d143db2 100644
--- a/spec/models/pet_spec.rb
+++ b/spec/models/pet_spec.rb
@@ -82,6 +82,10 @@
       expect(versions.third.reify).to be_a(Cat) # Cheshire
       expect(versions.fourth.reify).to be_a(Cat) # Cheshire that was destroyed
 
+      # Properly records "destroy" events according to Issue #37 from the store_base_sti_class gem:
+      #   https://github.com/appfolio/store_base_sti_class/issues/37
+      expect(cat.versions.last.event).to eq("destroy")
+
       # Creating an object from the base class is correctly identified as "Animal"
       expect(versions[5].reify).to be_an(Animal) # Muppets Drummer
       expect(versions[6].reify).to be_an(Animal) # Animal that was destroyed