Fixes #360 - Fix puppetclass keyword search

app/models/concerns/foreman_puppet/extensions/host.rb

@@ -13,20 +13,13 @@ module Host
         include_in_clone puppet: %i[config_groups host_config_groups host_classes]
 
         scoped_search relation: :environment, on: :name, complete_value: true, rename: :environment
-        scoped_search relation: :puppetclasses, on: :name, complete_value: true, rename: :class, only_explicit: true, operators: ['= ', '~ '],
-          ext_method: :search_by_deprecated_class
         scoped_search relation: :puppetclasses, on: :name, complete_value: true, rename: :puppetclass, only_explicit: true, operators: ['= ', '~ '],
           ext_method: :search_by_puppetclass
         scoped_search relation: :config_groups, on: :name, complete_value: true, rename: :config_group, only_explicit: true, operators: ['= ', '~ '],
           ext_method: :search_by_config_group
       end
 
       class_methods do
-        def search_by_deprecated_class(key, operator, value)
-          Foreman::Deprecation.deprecation_warning('3.2', 'search by `class` has been deprecated, please search by `puppetclass` instead')
-          search_by_puppetclass(key, operator, value)
-        end
-
         def search_by_puppetclass(_key, operator, value)
           conditions = sanitize_sql_for_conditions(["puppetclasses.name #{operator} ?", value_to_sql(operator, value)])
           config_group_ids = ForemanPuppet::ConfigGroup.joins(:puppetclasses).where(conditions).pluck(:id)

app/models/concerns/foreman_puppet/extensions/hostgroup.rb

@@ -16,7 +16,12 @@ class << self
 
         # will need through relation to work properly
         scoped_search relation: :environment, on: :name, complete_value: true, rename: :environment, only_explicit: true
-        scoped_search relation: :puppetclasses, on: :name, complete_value: true, rename: :class, only_explicit: true, operators: ['= ', '~ ']
+        scoped_search relation: :puppetclasses, on: :name,
+          complete_value: true,
+          rename: :puppetclass,
+          only_explicit: true,
+          operators: ['= ', '~ '],
+          ext_method: :search_by_puppetclass
         scoped_search relation: :config_groups, on: :name,
           complete_value: true,
           rename: :config_group,
@@ -29,11 +34,22 @@ class << self
       module PatchedClassMethods
         def search_by_config_group(_key, operator, value)
           conditions = sanitize_sql_for_conditions(["config_groups.name #{operator} ?", value_to_sql(operator, value)])
-          hostgroup_ids = ::Hostgroup.unscoped.with_taxonomy_scope.joins(puppet: :config_groups).where(conditions).map(&:subtree_ids).flatten.uniq
+          hostgroup_ids = ::Hostgroup.joins(puppet: :config_groups).where(conditions).map(&:subtree_ids).flatten.uniq
+
+          conds = 'hostgroups.id < 0'
+          conds = "hostgroups.id IN(#{hostgroup_ids.join(',')})" if hostgroup_ids.present?
+
+          { conditions: conds }
+        end
+
+        def search_by_puppetclass(_key, operator, value)
+          conditions = sanitize_sql_for_conditions(["puppetclasses.name #{operator} ?", value_to_sql(operator, value)])
+          hostgroup_ids = ::Hostgroup.joins(puppet: :puppetclasses).where(conditions).map(&:subtree_ids)
+
+          conds = []
+          conds << "hostgroups.id IN (#{hostgroup_ids.join(',')})" if hostgroup_ids.present?
 
-          opts = 'hostgroups.id < 0'
-          opts = "hostgroups.id IN(#{hostgroup_ids.join(',')})" if hostgroup_ids.present?
-          { conditions: opts }
+          { conditions: conds.join(' OR ').presence || 'hostgroups.id < 0' }
         end
       end
     end

app/models/foreman_puppet/config_group.rb

@@ -17,7 +17,7 @@ class ConfigGroup < ApplicationRecord
     validates :name, presence: true, uniqueness: true
 
     scoped_search on: :name, complete_value: true
-    scoped_search relation: :puppetclasses, on: :name, complete_value: true, rename: :class, only_explicit: true, operators: ['= ', '~ ']
+    scoped_search relation: :puppetclasses, on: :name, complete_value: true, rename: :puppetclass, only_explicit: true, operators: ['= ', '~ ']
 
     default_scope -> { order('config_groups.name') }
 

test/models/foreman_puppet/host_test.rb

@@ -48,31 +48,39 @@ class HostTest < ActiveSupport::TestCase
         assert_equal results, results2
       end
 
+      test 'deprecated search by class throws error' do
+        exception = assert_raises(Exception) do
+          host
+          Host.search_for("class = #{host.puppet.puppetclass_names.first}")
+        end
+        assert_equal("Field 'class' not recognized for searching!", exception.message)
+      end
+
       test 'can be found by puppetclass' do
         host
-        result = Host.search_for("class = #{host.puppet.puppetclass_names.first}")
+        result = Host.search_for("puppetclass = #{host.puppet.puppetclass_names.first}")
         assert_includes result, host
       end
 
       test 'search by puppetclass returns only host within that puppetclass' do
         host
         puppetclass = FactoryBot.create(:puppetclass)
-        result = Host.search_for("class = #{puppetclass.name}")
+        result = Host.search_for("puppetclass = #{puppetclass.name}")
         assert_not_includes result, host
       end
 
       test 'search hosts by inherited puppetclass from a hostgroup' do
         hostgroup = FactoryBot.create(:hostgroup, :with_puppet_enc, :with_puppetclass)
         host_with_hg = FactoryBot.create(:host, hostgroup: hostgroup)
-        result = Host.search_for("class = #{hostgroup.puppet.puppetclass_names.first}")
+        result = Host.search_for("puppetclass = #{hostgroup.puppet.puppetclass_names.first}")
         assert_includes result, host_with_hg
       end
 
       test 'can search hosts by inherited puppet class from a parent hostgroup' do
         parent_hg = FactoryBot.create(:hostgroup, :with_puppet_enc, :with_puppetclass)
         hg = FactoryBot.create(:hostgroup, parent: parent_hg)
         host = FactoryBot.create(:host, hostgroup: hg)
-        result = Host.search_for("class = #{parent_hg.puppet.puppetclass_names.first}")
+        result = Host.search_for("puppetclass = #{parent_hg.puppet.puppetclass_names.first}")
         assert_equal 1, result.count
         assert_includes result, host
       end
@@ -82,7 +90,7 @@ class HostTest < ActiveSupport::TestCase
         host = FactoryBot.create(:host, :with_puppet_enc, hostgroup: hostgroup, environment: hostgroup.puppet.environment)
         config_group = FactoryBot.create(:config_group, :with_puppetclass)
         hostgroup.puppet.config_groups << config_group
-        result = Host.search_for("class = #{config_group.puppetclass_names.first}")
+        result = Host.search_for("puppetclass = #{config_group.puppetclass_names.first}")
         assert_equal 1, result.count
         assert_includes result, host
       end