Make STI .joins() fix more consistent with existing tests

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
         }

lib/paper_trail/events/destroy.rb

@@ -14,7 +14,7 @@ class Destroy < Base
       def data
         data = {
           item_id: @record.id,
-          item_type: @record.class.paper_trail.versions_association_item_type,
+          item_type: @record.class.name,
           event: @record.paper_trail_event || "destroy",
           whodunnit: PaperTrail.request.whodunnit
         }

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
         }

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
@@ -147,20 +139,31 @@ 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
-          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) unless klass.descendants.any?
         end,
         class_name: klass.version_class_name,
         as: :item
       )
 
-      # We override the assocation when STI models are created from a base class
-      # in order to use the right `item_type`.
+      # 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)
       klass.singleton_class.prepend(Module.new {
         def inherited(klass)
           super

lib/paper_trail/record_trail.rb

@@ -84,7 +84,7 @@ 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.save!
       version
     end
@@ -138,7 +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.class.paper_trail.version_class.new(data)
+      version = @record.send(@record.class.versions_association_name).new(data)
       if version.save
         version
       else
@@ -164,7 +164,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

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

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

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
 

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

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

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

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