diff --git a/app/models/concerns/taxonomix.rb b/app/models/concerns/taxonomix.rb index 5620f37040..d5d73bbecc 100644 --- a/app/models/concerns/taxonomix.rb +++ b/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) diff --git a/app/models/taxonomy.rb b/app/models/taxonomy.rb index 511347291e..178914a994 100644 --- a/app/models/taxonomy.rb +++ b/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" diff --git a/app/models/user.rb b/app/models/user.rb index bbf2c60dd3..a323f09f38 100644 --- a/app/models/user.rb +++ b/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 diff --git a/test/models/taxonomy_test.rb b/test/models/taxonomy_test.rb index 271cc3df6a..806366ce40 100644 --- a/test/models/taxonomy_test.rb +++ b/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" diff --git a/test/models/user_test.rb b/test/models/user_test.rb index cea2a8e26e..77e8f1407d 100644 --- a/test/models/user_test.rb +++ b/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)