Fixes #39531 - Filtering hosts with parent_hostgroup does not work correctly

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(/^.*\./, '')
 

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