[Ledger] Fix N+1 rendering and a pathological query plan on the ledger page

Gemfile

@@ -146,6 +146,7 @@ group :test do
   gem "factory_bot_rails" # Test data
   gem "simplecov", require: false # Code coverage
   gem "webmock"
+  gem "db-query-matchers" # RSpec matchers for asserting SQL query counts (N+1 regression tests)
 end
 
 group :development, :test do

Gemfile.lock

@@ -244,6 +244,9 @@ GEM
     csv (3.3.5)
     dalli (3.2.8)
     date (3.5.1)
+    db-query-matchers (0.15.0)
+      activesupport (>= 4.0)
+      rspec (>= 3.0)
     declarative (0.0.20)
     device_detector (1.1.3)
     diff-lcs (1.6.2)
@@ -759,6 +762,10 @@ GEM
       chunky_png (~> 1.0)
       rqrcode_core (~> 1.0)
     rqrcode_core (1.2.0)
+    rspec (3.13.2)
+      rspec-core (~> 3.13.0)
+      rspec-expectations (~> 3.13.0)
+      rspec-mocks (~> 3.13.0)
     rspec-core (3.13.3)
       rspec-support (~> 3.13.0)
     rspec-expectations (3.13.3)
@@ -983,6 +990,7 @@ DEPENDENCIES
   countries
   country_select (~> 8.0)
   cssbundling-rails (~> 1.4)
+  db-query-matchers
   diffy
   discordrb
   doorkeeper (~> 5.8)

app/controllers/concerns/set_ledger_filters.rb

@@ -33,8 +33,14 @@ def set_ledger_filters
                  else
                    [@ledger]
                  end
-      author_ids = Ledger::Item.where(id: Ledger::Mapping.where(ledger: @ledgers).select(:ledger_item_id)).select(:author_id)
-      @users = User.where(id: author_ids).or(User.where(id: @event.users.select(:id))).with_attached_profile_picture.order(Arel.sql("CONCAT(preferred_name, full_name) ASC"))
+      # Resolved as two plain id lookups unioned in Ruby, rather than
+      # `User.where(id: ...).or(User.where(id: ...))` with each side a
+      # subquery: Postgres was choosing to evaluate both subqueries as a
+      # "hashed SubPlan" filter under a sequential scan of the entire `users`
+      # table instead of an indexed id lookup, on ledgers with many items.
+      author_ids = Ledger::Item.where(id: Ledger::Mapping.where(ledger: @ledgers).select(:ledger_item_id)).distinct.pluck(:author_id)
+      user_ids = (author_ids.compact + @event.users.pluck(:id)).uniq
+      @users = User.where(id: user_ids).with_attached_profile_picture.order(Arel.sql("CONCAT(preferred_name, full_name) ASC"))
 
       if @merchant
         merchant = @event.merchants.find { |merchant| merchant[:id] == @merchant }

app/models/ledger/query.rb

@@ -66,7 +66,16 @@ def execute(ledgers: [], all_ledgers: false)
       # preload, not includes: linked_object is polymorphic, so it can never be
       # JOINed — and includes makes pluck/count attempt exactly that join
       # (EagerLoadPolymorphicError).
-      results.order(pending_first.asc, datetime: :desc, created_at: :desc, id: :desc).preload(:linked_object)
+      #
+      # The nested associations here aren't used by this class — they're what
+      # app/views/ledger/_item.html.erb touches through linked_object (via
+      # Ledger::Item#icon) for a Disbursement, Donation, or CardCharge row.
+      # Preloading them here, once, avoids an N+1 on every render of that
+      # partial; Rails preloads each association only on the linked_object
+      # records whose class actually has it, so this is safe even though no
+      # single row has all four.
+      results.order(pending_first.asc, datetime: :desc, created_at: :desc, id: :desc)
+             .preload(:author, linked_object: [:card_grant, :recurring_donation, :raw_stripe_transactions, :raw_pending_stripe_transaction])
     end
 
     def self.sanitize_query(query_hash)

spec/models/ledger/query_spec.rb

@@ -551,4 +551,32 @@ def create_other_ledger_item(**attrs)
       expect(result.pluck(:id)).to match_array(ids_of(item_a, item_b, item_c, item_d, item_e, item_g))
     end
   end
+
+  describe "preloading" do
+    it "preloads author and linked_object's nested associations so rendering a page doesn't N+1" do
+      author = create(:user)
+      raw_pending = create(:raw_pending_stripe_transaction)
+      card_charge_item = create(:ledger_item, linked_object: raw_pending.card_charge, datetime: Time.current)
+      card_charge_item.update_columns(author_id: author.id)
+      Ledger::Mapping.create!(ledger: test_ledger, ledger_item: card_charge_item, on_primary_ledger: true)
+
+      # execute_query({}) also returns this describe block's shared item_a..item_g
+      # fixtures (mapped onto the same test_ledger in the top-level `before`
+      # hook); only the CardCharge item is relevant to this assertion.
+      result = execute_query({}).to_a
+      reloaded_card_charge_item = result.find { |item| item.id == card_charge_item.id }
+
+      # Covers the CardCharge branch of the preload (a has_many-through plus a
+      # belongs_to). Disbursement's :card_grant and Donation's
+      # :recurring_donation go through the same preload directive in
+      # execute() but aren't separately exercised here.
+      expect {
+        reloaded_card_charge_item.author&.name
+        # icon (app/models/ledger/item.rb) touches these associations for a
+        # CardCharge row; each should already be preloaded.
+        reloaded_card_charge_item.linked_object.raw_stripe_transactions.to_a
+        reloaded_card_charge_item.linked_object.raw_pending_stripe_transaction
+      }.not_to make_database_queries
+    end
+  end
 end

spec/support/db_query_matchers.rb

@@ -0,0 +1,10 @@
+# frozen_string_literal: true
+
+require "db-query-matchers"
+
+DBQueryMatchers.configure do |config|
+  # Schema-introspection queries aren't part of the behavior a query-count
+  # assertion cares about; excluding them matches Rails' own
+  # assert_queries_count default.
+  config.schemaless = true
+end