From 1cbb3daa804a4a63a16c1221f303b3e036c86ba4 Mon Sep 17 00:00:00 2001 From: Soham Majumdar Date: Wed, 15 Jul 2026 21:10:32 +0530 Subject: [PATCH] Fixes #39531 - Filtering hosts with parent_hostgroup does not work correctly --- app/models/concerns/hostext/search.rb | 26 +++- test/models/host_test.rb | 192 ++++++++++++++++++++++++++ 2 files changed, 213 insertions(+), 5 deletions(-) diff --git a/app/models/concerns/hostext/search.rb b/app/models/concerns/hostext/search.rb index b162120b4f..4217f25379 100644 --- a/app/models/concerns/hostext/search.rb +++ b/app/models/concerns/hostext/search.rb @@ -46,7 +46,7 @@ module Search scoped_search :relation => :hostgroup, :on => :title, :complete_value => true, :rename => :hostgroup_fullname scoped_search :relation => :hostgroup, :on => :title, :complete_value => true, :rename => :hostgroup_title, :only_explicit => true scoped_search :relation => :hostgroup, :on => :id, :complete_enabled => false, :rename => :hostgroup_id, :only_explicit => true, :validator => ScopedSearch::Validators::INTEGER - scoped_search :relation => :hostgroup, :on => :title, :complete_value => true, :rename => :parent_hostgroup, :only_explicit => true, :ext_method => :search_by_hostgroup_and_descendants + scoped_search :relation => :hostgroup, :on => :title, :complete_value => true, :rename => :parent_hostgroup, :only_explicit => true, :operators => ['=', '!=', '<>'], :ext_method => :search_by_hostgroup_and_descendants scoped_search :relation => :domain, :on => :name, :complete_value => true, :rename => :domain scoped_search :relation => :domain, :on => :id, :complete_enabled => false, :rename => :domain_id, :only_explicit => true, :validator => ScopedSearch::Validators::INTEGER scoped_search :relation => :realm, :on => :name, :complete_value => true, :rename => :realm @@ -174,18 +174,34 @@ def search_by_user(key, operator, value) end def search_by_hostgroup_and_descendants(key, operator, value) - conditions = sanitize_sql_for_conditions(["hostgroups.title #{operator} ?", value_to_sql(operator, value)]) + negate = parent_hostgroup_search_negated?(operator) + lookup_operator = parent_hostgroup_lookup_operator(operator) + conditions = sanitize_sql_for_conditions(["hostgroups.title #{lookup_operator} ?", value_to_sql(lookup_operator, value)]) # Only one hostgroup (first) is used to determined descendants. Future TODO - alert if result results more than one hostgroup - hostgroup = Hostgroup.unscoped.with_taxonomy_scope.find_by(conditions) + hostgroup = Hostgroup.unscoped.with_taxonomy_scope.where(conditions).first if hostgroup.present? hostgroup_ids = hostgroup.subtree_ids - opts = "hosts.hostgroup_id IN (#{hostgroup_ids.join(',')})" + if hostgroup_ids.blank? + opts = negate ? '1 = 1' : 'hosts.id < 0' + elsif negate + opts = "(hosts.hostgroup_id NOT IN (#{hostgroup_ids.join(',')}) OR hosts.hostgroup_id IS NULL)" + else + opts = "hosts.hostgroup_id IN (#{hostgroup_ids.join(',')})" + end else - opts = "hosts.id < 0" + opts = negate ? '1 = 1' : 'hosts.id < 0' end {:conditions => opts} end + def parent_hostgroup_search_negated?(operator) + ['<>', '!=', 'NOT ILIKE', 'NOT IN'].include?(operator.to_s.strip) + end + + def parent_hostgroup_lookup_operator(operator) + parent_hostgroup_search_negated?(operator) ? '=' : operator + end + def search_by_params(key, operator, value) key_name = key.sub(/^.*\./, '') diff --git a/test/models/host_test.rb b/test/models/host_test.rb index 360dca5566..56a9ae17ee 100644 --- a/test/models/host_test.rb +++ b/test/models/host_test.rb @@ -1784,6 +1784,198 @@ class HostTest < ActiveSupport::TestCase assert_equal hosts.count, 0 end + test "can search hosts excluding a parent hostgroup and its descendants" do + hostgroup = hostgroups(:db) + parent_hostgroup = hostgroups(:common) + hostgroup.parent_id = parent_hostgroup.id + assert hostgroup.save! + + host_in_child = FactoryBot.create(:host, :hostgroup => hostgroup) + host_in_parent = FactoryBot.create(:host, :hostgroup => parent_hostgroup) + host_in_other = FactoryBot.create(:host, :hostgroup => hostgroups(:unusual)) + host_without_group = FactoryBot.create(:host) + + hosts = Host::Managed.search_for("parent_hostgroup != Common") + assert_includes hosts, host_in_other + assert_includes hosts, host_without_group + assert_not_includes hosts, host_in_child + assert_not_includes hosts, host_in_parent + end + + test "can search hosts excluding a parent hostgroup using <> operator" do + hostgroup = hostgroups(:db) + parent_hostgroup = hostgroups(:common) + hostgroup.parent_id = parent_hostgroup.id + assert hostgroup.save! + + host_in_child = FactoryBot.create(:host, :hostgroup => hostgroup) + host_in_parent = FactoryBot.create(:host, :hostgroup => parent_hostgroup) + host_in_other = FactoryBot.create(:host, :hostgroup => hostgroups(:unusual)) + + hosts = Host::Managed.search_for("parent_hostgroup <> Common") + assert_includes hosts, host_in_other + assert_not_includes hosts, host_in_child + assert_not_includes hosts, host_in_parent + end + + test "search_by_hostgroup_and_descendants builds NOT IN for negated operators" do + result = Host::Managed.search_by_hostgroup_and_descendants('parent_hostgroup', '<>', 'Common') + assert_match(/NOT IN/, result[:conditions]) + + result = Host::Managed.search_by_hostgroup_and_descendants('parent_hostgroup', '=', 'Common') + refute_match(/NOT IN/, result[:conditions]) + end + + test "search hosts by non-existing parent hostgroup with != returns all hosts" do + FactoryBot.create(:host, :with_hostgroup) + hosts = Host::Managed.search_for("parent_hostgroup != Nosuchgroup") + assert_equal Host::Managed.count, hosts.count + end + + test "can search hosts excluding a parent hostgroup and its descendants" do + hostgroup = hostgroups(:db) + parent_hostgroup = hostgroups(:common) + hostgroup.parent_id = parent_hostgroup.id + assert hostgroup.save! + + host_in_child = FactoryBot.create(:host, :hostgroup => hostgroup) + host_in_parent = FactoryBot.create(:host, :hostgroup => parent_hostgroup) + host_in_other = FactoryBot.create(:host, :hostgroup => hostgroups(:unusual)) + host_without_group = FactoryBot.create(:host) + + hosts = Host::Managed.search_for("parent_hostgroup != Common") + assert_includes hosts, host_in_other + assert_includes hosts, host_without_group + assert_not_includes hosts, host_in_child + assert_not_includes hosts, host_in_parent + end + + test "can search hosts excluding a parent hostgroup using <> operator" do + hostgroup = hostgroups(:db) + parent_hostgroup = hostgroups(:common) + hostgroup.parent_id = parent_hostgroup.id + assert hostgroup.save! + + host_in_child = FactoryBot.create(:host, :hostgroup => hostgroup) + host_in_parent = FactoryBot.create(:host, :hostgroup => parent_hostgroup) + host_in_other = FactoryBot.create(:host, :hostgroup => hostgroups(:unusual)) + + hosts = Host::Managed.search_for("parent_hostgroup <> Common") + assert_includes hosts, host_in_other + assert_not_includes hosts, host_in_child + assert_not_includes hosts, host_in_parent + end + + test "search_by_hostgroup_and_descendants builds NOT IN for negated operators" do + result = Host::Managed.search_by_hostgroup_and_descendants('parent_hostgroup', '<>', 'Common') + assert_match(/NOT IN/, result[:conditions]) + + result = Host::Managed.search_by_hostgroup_and_descendants('parent_hostgroup', '=', 'Common') + refute_match(/NOT IN/, result[:conditions]) + end + + test "search hosts by non-existing parent hostgroup with != returns all hosts" do + FactoryBot.create(:host, :with_hostgroup) + hosts = Host::Managed.search_for("parent_hostgroup != Nosuchgroup") + assert_equal Host::Managed.count, hosts.count + end + + test "can search hosts excluding a parent hostgroup and its descendants" do + hostgroup = hostgroups(:db) + parent_hostgroup = hostgroups(:common) + hostgroup.parent_id = parent_hostgroup.id + assert hostgroup.save! + + host_in_child = FactoryBot.create(:host, :hostgroup => hostgroup) + host_in_parent = FactoryBot.create(:host, :hostgroup => parent_hostgroup) + host_in_other = FactoryBot.create(:host, :hostgroup => hostgroups(:unusual)) + host_without_group = FactoryBot.create(:host) + + hosts = Host::Managed.search_for("parent_hostgroup != Common") + assert_includes hosts, host_in_other + assert_includes hosts, host_without_group + assert_not_includes hosts, host_in_child + assert_not_includes hosts, host_in_parent + end + + test "can search hosts excluding a parent hostgroup using <> operator" do + hostgroup = hostgroups(:db) + parent_hostgroup = hostgroups(:common) + hostgroup.parent_id = parent_hostgroup.id + assert hostgroup.save! + + host_in_child = FactoryBot.create(:host, :hostgroup => hostgroup) + host_in_parent = FactoryBot.create(:host, :hostgroup => parent_hostgroup) + host_in_other = FactoryBot.create(:host, :hostgroup => hostgroups(:unusual)) + + hosts = Host::Managed.search_for("parent_hostgroup <> Common") + assert_includes hosts, host_in_other + assert_not_includes hosts, host_in_child + assert_not_includes hosts, host_in_parent + end + + test "search_by_hostgroup_and_descendants builds NOT IN for negated operators" do + result = Host::Managed.search_by_hostgroup_and_descendants('parent_hostgroup', '<>', 'Common') + assert_match(/NOT IN/, result[:conditions]) + + result = Host::Managed.search_by_hostgroup_and_descendants('parent_hostgroup', '=', 'Common') + refute_match(/NOT IN/, result[:conditions]) + end + + test "search hosts by non-existing parent hostgroup with != returns all hosts" do + FactoryBot.create(:host, :with_hostgroup) + hosts = Host::Managed.search_for("parent_hostgroup != Nosuchgroup") + assert_equal Host::Managed.count, hosts.count + end + + test "can search hosts excluding a parent hostgroup and its descendants" do + hostgroup = hostgroups(:db) + parent_hostgroup = hostgroups(:common) + hostgroup.parent_id = parent_hostgroup.id + assert hostgroup.save! + + host_in_child = FactoryBot.create(:host, :hostgroup => hostgroup) + host_in_parent = FactoryBot.create(:host, :hostgroup => parent_hostgroup) + host_in_other = FactoryBot.create(:host, :hostgroup => hostgroups(:unusual)) + host_without_group = FactoryBot.create(:host) + + hosts = Host::Managed.search_for("parent_hostgroup != Common") + assert_includes hosts, host_in_other + assert_includes hosts, host_without_group + assert_not_includes hosts, host_in_child + assert_not_includes hosts, host_in_parent + end + + test "can search hosts excluding a parent hostgroup using <> operator" do + hostgroup = hostgroups(:db) + parent_hostgroup = hostgroups(:common) + hostgroup.parent_id = parent_hostgroup.id + assert hostgroup.save! + + host_in_child = FactoryBot.create(:host, :hostgroup => hostgroup) + host_in_parent = FactoryBot.create(:host, :hostgroup => parent_hostgroup) + host_in_other = FactoryBot.create(:host, :hostgroup => hostgroups(:unusual)) + + hosts = Host::Managed.search_for("parent_hostgroup <> Common") + assert_includes hosts, host_in_other + assert_not_includes hosts, host_in_child + assert_not_includes hosts, host_in_parent + end + + test "search_by_hostgroup_and_descendants builds NOT IN for negated operators" do + result = Host::Managed.search_by_hostgroup_and_descendants('parent_hostgroup', '<>', 'Common') + assert_match(/NOT IN/, result[:conditions]) + + result = Host::Managed.search_by_hostgroup_and_descendants('parent_hostgroup', '=', 'Common') + refute_match(/NOT IN/, result[:conditions]) + end + + test "search hosts by non-existing parent hostgroup with != returns all hosts" do + FactoryBot.create(:host, :with_hostgroup) + hosts = Host::Managed.search_for("parent_hostgroup != Nosuchgroup") + assert_equal Host::Managed.count, hosts.count + end + test "can build a hash of all host facts" do host = FactoryBot.create(:host, :with_facts, :fact_count => 22) assert_equal host.facts.keys.length, 22