Fixes #39336 - Batch taxonomy subtree lookups to avoid N+1 queries

app/models/concerns/taxonomix.rb

@@ -58,7 +58,18 @@ def used_organization_ids
     end
 
     def get_taxonomy_ids(taxonomy, method)
-      Array(taxonomy).map { |t| t.send(method) + t.ancestor_ids }.flatten.uniq
+      taxonomies = Array(taxonomy)
+      return [] if taxonomies.empty?
+
+      ancestor_ids = taxonomies.flat_map(&:ancestor_ids)
+
+      ids = if method == :subtree_ids
+              Taxonomy.batch_subtree_ids(taxonomies)
+            else
+              taxonomies.flat_map { |t| t.send(method) }
+            end
+
+      (ids + ancestor_ids).uniq
     end
 
     def taxable_ids(loc = which_location, org = which_organization, inner_method = which_ancestry_method)

app/models/taxonomy.rb

@@ -6,6 +6,9 @@ class Taxonomy < ApplicationRecord
   include TopbarCacheExpiry
 
   serialize :ignore_types, Array
+  # Coarse SQL pre-filter on serialized YAML; may over-match (e.g. 'User' matches 'UserProfile').
+  # Callers must verify with ignore? for exactness.
+  scope :potentially_ignoring, ->(type) { where("ignore_types LIKE ?", "%#{sanitize_sql_like(type.to_s.classify)}%") }
 
   before_create :assign_default_templates
   after_create :assign_taxonomy_to_user
@@ -87,6 +90,28 @@ def self.types
     [Organization, Location]
   end
 
+  def self.batch_subtree_ids(taxonomies)
+    return [] if taxonomies.empty?
+
+    raise ArgumentError, "batch_subtree_ids requires persisted records" if taxonomies.any?(&:new_record?)
+
+    klass = taxonomies.first.class
+    raise ArgumentError, "expected all taxonomies to be #{klass}, got: #{taxonomies.map(&:class).uniq.join(', ')}" unless taxonomies.all? { |t| t.is_a?(klass) }
+
+    sql_parts = ["#{klass.table_name}.id IN (?)"]
+    binds = [taxonomies.map(&:id)]
+
+    taxonomies.each do |tax|
+      ca = tax.child_ancestry
+      sql_parts << "#{klass.table_name}.ancestry LIKE ?"
+      binds << "#{sanitize_sql_like(ca)}/%"
+      sql_parts << "#{klass.table_name}.ancestry = ?"
+      binds << ca
+    end
+
+    klass.where(sql_parts.join(' OR '), *binds).order(:id).pluck(:id)
+  end
+
   def self.ignore?(taxable_type)
     current_taxonomies = if current.nil? && User.current.present?
                            # "Any context" - all available taxonomies"

app/models/user.rb

@@ -515,10 +515,11 @@ def taxonomy_ids
   def taxonomy_and_child_ids(taxonomies)
     delay = Rails.env.test? ? 0 : 2.minutes
     Rails.cache.fetch("user/#{id}/taxonomy_and_child_ids/#{taxonomies}", expires_in: delay) do
-      top_level = send(taxonomies) + taxonomies.to_s.classify.constantize.unscoped.select { |tax| tax.ignore?('user') }
-      top_level.each_with_object([]) do |taxonomy, ids|
-        ids.concat taxonomy.subtree_ids
-      end.uniq
+      klass = taxonomies.to_s.classify.constantize
+      top_level = send(taxonomies) + klass.unscoped.potentially_ignoring('user').select { |tax| tax.ignore?('user') }
+      next [] if top_level.empty?
+
+      Taxonomy.batch_subtree_ids(top_level)
     end
   end
 

test/models/taxonomy_test.rb

@@ -64,6 +64,81 @@ class TaxonomyTest < ActiveSupport::TestCase
     end
   end
 
+  test 'batch_subtree_ids returns empty array for empty input' do
+    assert_empty Taxonomy.batch_subtree_ids([])
+  end
+
+  test 'batch_subtree_ids returns subtree for a single taxonomy' do
+    parent = FactoryBot.create(:organization)
+    child = FactoryBot.create(:organization, parent: parent)
+    grandchild = FactoryBot.create(:organization, parent: child)
+
+    result = Taxonomy.batch_subtree_ids([parent])
+    assert_equal [parent.id, child.id, grandchild.id].sort, result
+  end
+
+  test 'batch_subtree_ids returns union of subtrees for multiple taxonomies' do
+    org1 = FactoryBot.create(:organization)
+    org1_child = FactoryBot.create(:organization, parent: org1)
+    org2 = FactoryBot.create(:organization)
+    org2_child = FactoryBot.create(:organization, parent: org2)
+
+    result = Taxonomy.batch_subtree_ids([org1, org2])
+    assert_equal [org1.id, org1_child.id, org2.id, org2_child.id].sort, result
+  end
+
+  test 'batch_subtree_ids handles overlapping subtrees' do
+    parent = FactoryBot.create(:organization)
+    child = FactoryBot.create(:organization, parent: parent)
+    grandchild = FactoryBot.create(:organization, parent: child)
+
+    result = Taxonomy.batch_subtree_ids([parent, child])
+    assert_equal [parent.id, child.id, grandchild.id].sort, result
+  end
+
+  test 'batch_subtree_ids returns deterministic order' do
+    orgs = Array.new(3) { FactoryBot.create(:organization) }
+    result = Taxonomy.batch_subtree_ids(orgs)
+    assert_equal 3, result.size
+    assert_equal result.sort, result
+  end
+
+  test 'potentially_ignoring returns taxonomies that ignore the given type' do
+    ignoring = FactoryBot.create(:organization, ignore_types: ['User'])
+    not_ignoring = FactoryBot.create(:organization, ignore_types: [])
+
+    result = Organization.unscoped.potentially_ignoring('user')
+    assert_includes result, ignoring
+    refute_includes result, not_ignoring
+  end
+
+  test 'batch_subtree_ids matches individual subtree_ids' do
+    parent = FactoryBot.create(:organization)
+    child = FactoryBot.create(:organization, parent: parent)
+    FactoryBot.create(:organization, parent: child)
+    standalone = FactoryBot.create(:organization)
+
+    inputs = [parent, child, standalone]
+    assert_equal inputs.flat_map(&:subtree_ids).uniq.sort,
+      Taxonomy.batch_subtree_ids(inputs)
+  end
+
+  test 'batch_subtree_ids returns only the given node for a leaf taxonomy' do
+    leaf = FactoryBot.create(:organization)
+    assert_equal [leaf.id], Taxonomy.batch_subtree_ids([leaf])
+  end
+
+  test 'batch_subtree_ids raises on mixed taxonomy types' do
+    org = FactoryBot.create(:organization)
+    loc = FactoryBot.create(:location)
+    assert_raises(ArgumentError) { Taxonomy.batch_subtree_ids([org, loc]) }
+  end
+
+  test 'batch_subtree_ids raises on unsaved records' do
+    org = FactoryBot.build(:organization)
+    assert_raises(ArgumentError) { Taxonomy.batch_subtree_ids([org]) }
+  end
+
   test "taxonomy cannot be saved with orphans" do
     location = Location.create :name => "Velky Tynec"
     organization = Organization.create :name => "Olomouc"

test/models/user_test.rb

@@ -967,6 +967,44 @@ def editing_self_helper
     assert_includes user.my_organizations, organization
   end
 
+  test "taxonomy_and_child_ids includes assigned taxonomies and ignoring taxonomies with children" do
+    # Setup user with no default taxonomies
+    user = FactoryBot.create(:user, organizations: [], locations: [])
+
+    # Organizations: assigned + children
+    org_assigned = FactoryBot.create(:organization)
+    org_assigned_child = FactoryBot.create(:organization, parent: org_assigned)
+    user.organizations << org_assigned
+
+    # Organizations: ignoring User + children
+    org_ignoring = FactoryBot.create(:organization, ignore_types: ['User'])
+    org_ignoring_child = FactoryBot.create(:organization, parent: org_ignoring)
+
+    # Locations: mirror the same pattern
+    loc_assigned = FactoryBot.create(:location)
+    loc_assigned_child = FactoryBot.create(:location, parent: loc_assigned)
+    user.locations << loc_assigned
+
+    loc_ignoring = FactoryBot.create(:location, ignore_types: ['User'])
+    loc_ignoring_child = FactoryBot.create(:location, parent: loc_ignoring)
+
+    # Test organizations
+    expected_org_ids = [org_assigned.id, org_assigned_child.id, org_ignoring.id, org_ignoring_child.id].sort
+    assert_equal expected_org_ids, user.organization_and_child_ids.sort
+
+    # Verify it matches batch_subtree_ids (which returns sorted results)
+    batch_result = Taxonomy.batch_subtree_ids([org_assigned, org_ignoring])
+    assert_equal batch_result, user.organization_and_child_ids.sort
+
+    # Test locations
+    expected_loc_ids = [loc_assigned.id, loc_assigned_child.id, loc_ignoring.id, loc_ignoring_child.id].sort
+    assert_equal expected_loc_ids, user.location_and_child_ids.sort
+
+    # Verify it matches batch_subtree_ids (which returns sorted results)
+    batch_result = Taxonomy.batch_subtree_ids([loc_assigned, loc_ignoring])
+    assert_equal batch_result, user.location_and_child_ids.sort
+  end
+
   test "chaging hostgroup should update cache" do
     u = FactoryBot.create(:user)
     g1 = FactoryBot.create(:usergroup)