Add environment field to Host(group) form

Adds all js necessary to handle the environment field change.

app/controllers/concerns/foreman_puppet_enc/extensions/hostgroups_controller_extensions.rb

@@ -0,0 +1,39 @@
+module ForemanPuppetEnc
+  module Extensions
+    module HostgroupsControllerExtensions
+      extend ActiveSupport::Concern
+
+      PUPPET_AJAX_REQUESTS = %w[environment_selected puppetclass_parameters].freeze
+
+      included do
+        alias_method :ajax_request_for_puppet_hostgroup_extensions, :ajax_request
+        alias_method :taxonomy_scope_for_puppet_hostgroup_extensions, :taxonomy_scope
+
+        before_action :ajax_request_for_puppet_hostgroup_extensions, only: PUPPET_AJAX_REQUESTS
+        before_action :taxonomy_scope_for_puppet_hostgroup_extensions, only: PUPPET_AJAX_REQUESTS
+
+        helper ForemanPuppetEnc::HostsAndHostgroupsHelper
+      end
+
+      def environment_selected
+        env_id = params[:environment_id] || params[:hostgroup][:environment_id]
+        @environment = Environment.find(env_id) if env_id.to_i.positive?
+        return not_found unless @environment
+
+        refresh_hostgroup
+        @hostgroup.environment = @environment if @environment
+
+        @hostgroup.puppetclasses = Puppetclass.where(id: params[:hostgroup][:puppetclass_ids])
+        @hostgroup.config_groups = ConfigGroup.where(id: params[:hostgroup][:config_group_ids])
+        render partial: 'hosts/form_puppet_enc_tab', locals: { obj: @hostgroup, resource_type: :hostgroup }
+      end
+
+      def puppetclass_parameters
+        Taxonomy.as_taxonomy @organization, @location do
+          render partial: 'foreman_puppet_enc/puppetclasses/classes_parameters',
+                 locals: { obj: refresh_hostgroup }
+        end
+      end
+    end
+  end
+end

app/controllers/concerns/foreman_puppet_enc/extensions/hosts_controller_extensions.rb

@@ -24,15 +24,15 @@ module HostsControllerExtensions
 
         set_callback :set_class_variables, :after, :set_puppet_class_variables
 
-        helper ForemanPuppetEnc::HostsHelper
+        helper ForemanPuppetEnc::HostsAndHostgroupsHelper
       end
 
       def hostgroup_or_environment_selected
         refresh_host
         set_class_variables(@host)
         Taxonomy.as_taxonomy @organization, @location do
           if @environment || @hostgroup
-            render partial: 'puppetclasses/class_selection', locals: { obj: @host, resource_type: :host }
+            render partial: 'hosts/form_puppet_enc_tab', locals: { obj: @host, resource_type: :host }
           else
             logger.info 'environment_id or hostgroup_id is required to render puppetclasses'
           end
@@ -41,7 +41,7 @@ def hostgroup_or_environment_selected
 
       def puppetclass_parameters
         Taxonomy.as_taxonomy @organization, @location do
-          render partial: 'puppetclasses/classes_parameters', locals: { obj: refresh_host }
+          render partial: 'foreman_puppet_enc/puppetclasses/classes_parameters', locals: { obj: refresh_host }
         end
       end
 

app/helpers/foreman_puppet_enc/hosts_and_hostgroups_helper.rb

@@ -0,0 +1,87 @@
+module ForemanPuppetEnc
+  module HostsAndHostgroupsHelper
+    UI.register_host_description do
+      multiple_actions_provider :puppet_host_multiple_actions
+    end
+
+    def puppet_host_multiple_actions
+      if ForemanPuppetEnc.extracted_from_core?
+        [{ action: [_('Change Environment'), foreman_puppet_enc.select_multiple_environment_hosts_path], priority: 200 }]
+      else
+        []
+      end
+    end
+
+    def host_puppet_environment_field(form, select_options = {}, html_options = {})
+      select_options = {
+        include_blank: true,
+        disable_button: _(::HostsAndHostgroupsHelper::INHERIT_TEXT),
+        disable_button_enabled: inherited_by_default?(:environment_id, form.object),
+        user_set: user_set?(:environment_id),
+      }.deep_merge(select_options)
+
+      html_options = {
+        data: {
+          url: hostgroup_or_environment_selected_hosts_path,
+          host: {
+            id: form.object.id,
+          },
+        },
+      }.deep_merge(html_options)
+
+      puppet_environment_field(
+        form,
+        accessible_resource(form.object, :environment),
+        select_options,
+        html_options
+      )
+    end
+
+    def hostgroup_puppet_environment_field(form, select_options = {}, html_options = {})
+      select_options = {
+        include_blank: blank_or_inherit_f(form, :environment),
+      }.deep_merge(select_options)
+
+      html_options = {
+        data: {
+          url: environment_selected_hostgroups_path,
+        },
+      }.deep_merge(html_options)
+
+      puppet_environment_field(
+        form,
+        accessible_resource(form.object, :environment),
+        select_options,
+        html_options
+      )
+    end
+
+    def puppet_environment_field(form, environments_choice, select_options = {}, html_options = {})
+      html_options = {
+        onchange: 'tfm.puppetEnc.hostForm.updatePuppetclasses(this)',
+        help_inline: :indicator,
+      }.deep_merge(html_options)
+
+      select_f(
+        form,
+        :environment_id,
+        environments_choice,
+        :id,
+        :to_label,
+        select_options,
+        html_options
+      )
+    end
+
+    def puppetclasses_with_parameters_for(obj)
+      classes = obj.all_puppetclasses
+      ids = classes.reorder(nil).pluck(:id)
+      class_vars = ForemanPuppetEnc::PuppetclassLookupKey.reorder(nil)
+                                                         .joins(:environment_classes)
+                                                         .where(EnvironmentClass.arel_table[:puppetclass_id].in(ids))
+                                                         .distinct
+                                                         .pluck('environment_classes.puppetclass_id')
+      classes.where(id: class_vars)
+    end
+  end
+end

app/views/foreman_puppet_enc/puppetclasses/_classes_in_groups.html.erb

@@ -1,5 +1,5 @@
-<li class="<%= dom_class(klass, 'selected') %>">
-  <%= link_to klass.name, "#", :'data-original-title' => _('belongs to config group'), :rel => "twipsy",
-                               :'data-id' => klass.id, :'data-class-id' => klass.id, :'data-group-class-id' => klass.id,
-                               :'data-host-id' => obj.id %>
+<li id="selected_puppetclass_<%= klass.id %>" class="<%= dom_class(klass, 'selected') %>">
+  <%= link_to klass.name, "#", rel: "twipsy", data: { 'original-title': _('belongs to config group'),
+                                                      id: klass.id, 'class-id': klass.id,
+                                                      'group-class-id': klass.id, 'host-id': obj.id } %>
 </li>

app/views/foreman_puppet_enc/puppetclasses/_selected_classes.html.erb

@@ -1,4 +1,4 @@
-<%= content_tag :li, class: [dom_class(klass, 'selected'), cycle('even', 'odd'), ('unavailable' unless klass.environments.include?(@environment))] do %>
+<%= content_tag :li, id: "selected_puppet_class_#{klass.id}", class: [dom_class(klass, 'selected'), cycle('even', 'odd'), ('unavailable' unless klass.environments.include?(@environment))] do %>
   <%= link_to_remove_puppetclass(klass, obj) %>
   <%= hidden_field_tag "#{resource_type}[puppetclass_ids][]", klass.id %>
 <% end %>

app/views/hosts/_form_puppet_enc_tab.html.erb

@@ -1,14 +1,18 @@
-<% obj = local_assigns[pagelet.opts[:resource_type]] %>
+<% resource_type ||= pagelet.opts[:resource_type] %>
+<% obj ||= local_assigns[resource_type] %>
 <% if @environment.present? || obj.environment.present? %>
   <% if accessible_resource_records(:config_group).exists? %>
-    <%= render 'foreman_puppet_enc/config_groups/config_groups_selection', obj: obj, resource_type: pagelet.opts[:resource_type] %>
+    <%= render 'foreman_puppet_enc/config_groups/config_groups_selection', obj: obj, resource_type: resource_type %>
   <% end %>
 
-  <%= render 'foreman_puppet_enc/puppetclasses/class_selection', obj: obj, resource_type: pagelet.opts[:resource_type] %>
+  <%= render 'foreman_puppet_enc/puppetclasses/class_selection', obj: obj, resource_type: resource_type %>
 <% else %>
   <%= alert(class: alert_class(:info),
             header: _('Notice'),
             text: _('Please select an environment first')) %>
 <% end %>
 
-
+<fieldset id="puppet_klasses_parameters" data-url="<%= url_for(action: :puppetclass_parameters, only_path: true) %>">
+  <h2><%= _('Puppet Class Parameters') %></h2>
+  <%= render 'foreman_puppet_enc/puppetclasses/classes_parameters', obj: obj %>
+</fieldset>

app/views/hosts/foreman_puppet_enc/_form_main_tab_fields.html.erb

@@ -0,0 +1 @@
+<%= public_send("#{pagelet.opts[:resource_type]}_puppet_environment_field", form) %>

lib/foreman_puppet_enc/engine.rb

@@ -18,6 +18,7 @@ class Engine < ::Rails::Engine
       # To stay
       LookupValue.include ForemanPuppetEnc::PuppetLookupValueExtensions
       HostsController.include ForemanPuppetEnc::Extensions::HostsControllerExtensions
+      HostgroupsController.include ForemanPuppetEnc::Extensions::HostgroupsControllerExtensions
     end
 
     rake_tasks do

lib/foreman_puppet_enc/register.rb

@@ -148,6 +148,11 @@
         resource_type: resource_type,
         priority: 100,
         onlyif: (host_onlyif if resource_type == :host)
+
+      context.add_pagelet :main_tab_fields,
+        partial: 'hosts/foreman_puppet_enc/form_main_tab_fields',
+        resource_type: resource_type,
+        priority: 100
     end
   end
 end

test/controllers/foreman_puppet_enc/hostgroups_controller_test.rb

@@ -0,0 +1,64 @@
+require 'test_puppet_enc_helper'
+
+module ForemanPuppetEnc
+  class HostsControllerTest < ActionController::TestCase
+    tests ::HostgroupsController
+
+    describe '#environment_selected' do
+      setup do
+        @environment = FactoryBot.create(:environment)
+        @puppetclass = FactoryBot.create(:puppetclass)
+        @hostgroup = FactoryBot.create(:hostgroup, :environment => @environment)
+        @params = {
+          id: @hostgroup.id,
+          hostgroup: {
+            name: @hostgroup.name,
+            environment_id: "",
+            puppetclass_ids: [@puppetclass.id],
+          },
+        }
+      end
+
+      test "should return the selected puppet classes on environment change" do
+        assert_equal 0, @hostgroup.puppetclasses.length
+
+        post :environment_selected, params: @params, session: set_session_user
+        assert_equal(1, assigns(:hostgroup).puppetclasses.length)
+        assert_include assigns(:hostgroup).puppetclasses, @puppetclass
+      end
+
+      context 'no environment_id param is set' do
+        test 'it will take the hostgroup params environment_id' do
+          other_environment = FactoryBot.create(:environment)
+          @params[:hostgroup][:environment_id] = other_environment.id
+
+          post :environment_selected, params: @params, session: set_session_user
+          assert_equal assigns(:environment), other_environment
+        end
+      end
+
+      test 'should not escape lookup values on environment change' do
+        hostgroup = FactoryBot.create(:hostgroup, :environment => @environment, :puppetclass_ids => [@puppetclass.id])
+        lookup_key = FactoryBot.create(:puppetclass_lookup_key, :as_smart_class_param, :key_type => 'array',
+                                       :default_value => ['a', 'b'], :override => true, :puppetclass => @puppetclass)
+        lookup_value = FactoryBot.create(:lookup_value, :lookup_key => lookup_key, :match => "hostgroup=#{hostgroup.name}", :value => ["c", "d"])
+
+        FactoryBot.create(:environment_class, :puppetclass => @puppetclass, :environment => @environment, :puppetclass_lookup_key => lookup_key)
+
+        # sending exactly what the host form would send which is lookup_value.value_before_type_cast
+        lk = {"lookup_values_attributes" => {lookup_key.id.to_s => {"value" => lookup_value.value_before_type_cast, "id" => lookup_value.id, "lookup_key_id" => lookup_key.id, "_destroy" => false}}}
+
+        params = {
+          hostgroup_id: hostgroup.id,
+          hostgroup: hostgroup.attributes.merge(lk),
+        }
+
+        # environment change calls puppetclass_parameters which caused the extra escaping
+        post :puppetclass_parameters, params: params, session: set_session_user, xhr: true
+
+        # if this was escaped during refresh_host the value in response.body after unescapeHTML would include "[\\\"c\\\",\\\"d\\\"]"
+        assert_includes CGI.unescapeHTML(response.body), "[\"c\",\"d\"]"
+      end
+    end
+  end
+end

test/controllers/foreman_puppet_enc/hosts_controller_test.rb

@@ -43,5 +43,61 @@ class HostsControllerTest < ActionController::TestCase
       assert_equal hostgroup.environment_id, host1.reload.environment_id
       assert_equal hostgroup.environment_id, host2.reload.environment_id
     end
+
+    describe '#hostgroup_or_environment_selected' do
+      let(:hostgroup) { FactoryBot.create(:hostgroup, organizations: [org], locations: [loc]) }
+
+      test 'choosing only one of hostgroup or environment renders classes' do
+        post :hostgroup_or_environment_selected, params: {
+          host_id: nil,
+          host: {
+            environment_id: environment1.id,
+          },
+        }, session: set_session_user, xhr: true
+        assert_response :success
+        assert_template partial: 'hosts/_form_puppet_enc_tab'
+      end
+
+      test 'choosing both hostgroup and environment renders classes' do
+        post :hostgroup_or_environment_selected, params: {
+          host_id: host1.id,
+          host: {
+            environment_id: environment1.id,
+            hostgroup_id: hostgroup.id,
+          },
+        }, session: set_session_user, xhr: true
+        assert_response :success
+        assert_template partial: 'hosts/_form_puppet_enc_tab'
+      end
+
+      test 'should not escape lookup values on environment change' do
+        host = FactoryBot.create(:host, :with_environment, :with_puppetclass)
+
+        host.environment.locations = [host.location]
+        host.environment.organizations = [host.organization]
+
+        lookup_key = FactoryBot.create(:puppetclass_lookup_key, key_type: 'array',
+                                                                default_value: %w[a b],
+                                                                override: true,
+                                                                puppetclass: host.puppetclasses.first)
+        lookup_value = FactoryBot.create(:lookup_value, lookup_key: lookup_key, match: "fqdn=#{host.fqdn}", value: %w[c d])
+
+        # sending exactly what the host form would send which is lookup_value.value_before_type_cast
+        lk_params = { 'lookup_values_attributes' => { lookup_key.id.to_s => { 'value' => lookup_value.value_before_type_cast,
+                                                                              'id' => lookup_value.id,
+                                                                              'lookup_key_id' => lookup_key.id,
+                                                                              '_destroy' => false } } }
+        params = {
+          host_id: host.id,
+          host: host.attributes.merge(lk_params),
+        }
+
+        # environment change calls puppetclass_parameters which caused the extra escaping
+        post :puppetclass_parameters, params: params, session: set_session_user, xhr: true
+
+        # if this was escaped during refresh_host the value in response.body after unescapeHTML would include "[\\\"c\\\",\\\"d\\\"]"
+        assert_includes CGI.unescapeHTML(response.body), '["c","d"]'
+      end
+    end
   end
 end