Avoid N+1 queries coming from #version_limit when destroying records

Fixes #1510

Author: davidrunger

CHANGELOG.md

@@ -15,7 +15,9 @@ recommendations of [keepachangelog.com](http://keepachangelog.com/).
 
 ### Fixed
 
-- None
+- [#1511](https://github.com/paper-trail-gem/paper_trail/pull/1511) - Avoid N+1
+  queries when destroying a record with a `has_many` `dependent: :destroy`
+  association where the `has_many` target model is tracked by PaperTrail.
 
 ### Dependencies
 

lib/paper_trail/version_concern.rb

@@ -385,7 +385,7 @@ def sibling_versions
     #
     # @api private
     def version_limit
-      klass = item.class
+      klass = ((respond_to?(:item_subtype) && item_subtype) || item_type).constantize
       if limit_option?(klass)
         klass.paper_trail_options[:limit]
       elsif base_class_limit_option?(klass)

paper_trail.gemspec

@@ -70,6 +70,7 @@ has been destroyed.
   s.add_development_dependency "ffaker", "~> 2.20"
   s.add_development_dependency "generator_spec", "~> 0.9.4"
   s.add_development_dependency "memory_profiler", "~> 1.0.0"
+  s.add_development_dependency "n_plus_one_control", "~> 0.7.2"
 
   # For `spec/dummy_app`. Technically, we only need `actionpack` (as of 2020).
   # However, that might change in the future, and the advantages of specifying a

spec/dummy_app/app/versions/post_version.rb

@@ -2,4 +2,19 @@
 
 class PostVersion < PaperTrail::Version
   self.table_name = "post_versions"
+
+  # BEGIN Hack to simulate a version class that doesn't have the optional `item_subtype` column. >>>
+  def item_subtype
+    raise "This method should not be called!"
+  end
+
+  # We will follow Ruby's `respond_to?` method signature, including an optional boolean parameter.
+  # rubocop:disable Style/OptionalBooleanParameter
+  def respond_to?(method_name, include_private = false)
+    # rubocop:enable Style/OptionalBooleanParameter
+    return false if method_name.to_sym == :item_subtype
+
+    super
+  end
+  # <<< END Hack to simulate a version class that doesn't have the optional `item_subtype` column.
 end

spec/models/order_spec.rb

@@ -11,5 +11,22 @@
 
       expect(customer.versions.count).to(eq(3))
     end
+
+    # We need to use an instance variable to work with the `n_plus_one_control` gem in this spec.
+    # rubocop:disable RSpec/InstanceVariable
+    context "when the record has many associated records (N+1 check)", :n_plus_one do
+      populate do |n|
+        @customer = Customer.create!
+
+        n.times do
+          @customer.orders.create!
+        end
+      end
+
+      it "does not cause an N+1 query" do
+        expect { @customer.destroy! }.to perform_constant_number_of_queries
+      end
+    end
+    # rubocop:enable RSpec/InstanceVariable
   end
 end

spec/spec_helper.rb

@@ -69,6 +69,7 @@
 # by the `Bundler.require` in `config/application.rb`.
 require "paper_trail"
 require "ffaker"
+require "n_plus_one_control/rspec"
 require "rspec/rails"
 require "rails-controller-testing"