From dfaad4e44180757d4a34a6c3dceed0ce00810a72 Mon Sep 17 00:00:00 2001
From: nofaralfasi <nalfassi@redhat.com>
Date: Mon, 1 Jan 2024 18:48:53 +0200
Subject: [PATCH] Fixes #36982 - display short hostname on REX job pages

If the `display_fqdn_for_hosts` setting is set to false,
show the short hostname on REX job pages.
---
 app/helpers/job_invocations_helper.rb         |  2 +-
 app/helpers/remote_execution_helper.rb        |  4 ++--
 .../actions/remote_execution/run_host_job.rb  |  9 +++++++--
 .../_preview_hosts_list.html.erb              |  2 +-
 app/views/template_invocations/show.html.erb  |  4 ++--
 webpack/JobWizard/JobWizardSelectors.js       |  9 +++++++--
 webpack/JobWizard/__tests__/fixtures.js       |  6 +++---
 webpack/JobWizard/autofill.js                 | 12 ++++++++----
 .../steps/HostsAndInputs/HostPreviewModal.js  |  6 +++---
 .../steps/HostsAndInputs/SelectGQL.js         |  1 +
 .../steps/HostsAndInputs/SelectedChips.js     | 19 +++++++++++++------
 .../JobWizard/steps/HostsAndInputs/hosts.gql  |  1 +
 .../JobWizard/steps/HostsAndInputs/index.js   |  5 +++++
 .../JobWizard/steps/ReviewDetails/index.js    |  5 +++--
 webpack/JobWizard/steps/form/SearchSelect.js  |  4 +++-
 .../steps/form/__tests__/SelectSearch.test.js |  2 ++
 16 files changed, 62 insertions(+), 29 deletions(-)

diff --git a/app/helpers/job_invocations_helper.rb b/app/helpers/job_invocations_helper.rb
index 7e7e2e186..05889c1b7 100644
--- a/app/helpers/job_invocations_helper.rb
+++ b/app/helpers/job_invocations_helper.rb
@@ -33,7 +33,7 @@ def preview_hosts(template_invocation)
   end
 
   def collapsed_preview(target)
-    title = target.try(:name) || 'N/A'
+    title = target || 'N/A'
     content_tag(:h5,
       :class => "expander collapsed out",
       :data => { :toggle => 'collapse',
diff --git a/app/helpers/remote_execution_helper.rb b/app/helpers/remote_execution_helper.rb
index 573578a63..72700e9b3 100644
--- a/app/helpers/remote_execution_helper.rb
+++ b/app/helpers/remote_execution_helper.rb
@@ -34,7 +34,7 @@ def template_invocation_actions(task, host, job_invocation, template_invocation)
         action: { href: current_host_details_path(host), 'data-method': 'get', id: "#{host.name}-actions-detail" } }
     end
     if authorized_for(controller: :job_invocations, action: :create) && (!host.infrastructure_host? || User.current.can?(:execute_jobs_on_infrastructure_hosts))
-      links << { title: (_('Rerun on %s') % host.name),
+      links << { title: (_('Rerun on %s') % host),
         action: { href: rerun_job_invocation_path(job_invocation, host_ids: [ host.id ]),
                   'data-method': 'get', id: "#{host.name}-actions-rerun" } }
     end
@@ -274,7 +274,7 @@ def targeting_hosts(job_invocation, hosts)
       task = template_invocation.try(:run_host_job_task)
       link_authorized = !task.nil? && authorized_for(hash_for_template_invocation_path(:id => template_invocation).merge(:auth_object => host, :permission => :view_hosts, :authorizer => job_hosts_authorizer))
 
-      { name: host.name,
+      { name: host.to_label,
         link: link_authorized ? template_invocation_path(:id => template_invocation) : '',
         status: template_invocation_status(task, job_invocation.task),
         actions: template_invocation_actions(task, host, job_invocation, template_invocation) }
diff --git a/app/lib/actions/remote_execution/run_host_job.rb b/app/lib/actions/remote_execution/run_host_job.rb
index 0a8d0bf94..2b2a6bc4c 100644
--- a/app/lib/actions/remote_execution/run_host_job.rb
+++ b/app/lib/actions/remote_execution/run_host_job.rb
@@ -28,6 +28,7 @@ def inner_plan(job_invocation, host, template_invocation, proxy_selector, option
         raise _('Could not use any template used in the job invocation') if template_invocation.blank?
         features = template_invocation.template.remote_execution_features.pluck(:label).uniq
         action_subject(host,
+          :host_display_name => host.to_label,
           :job_category => job_invocation.job_category,
           :description => job_invocation.description,
           :job_invocation_id => job_invocation.id,
@@ -101,8 +102,12 @@ def live_output
       end
 
       def humanized_input
-        N_('%{description} on %{host}') % { :host => input[:host].try(:[], :name),
-          :description => input[:description].try(:capitalize) || input[:job_category] }
+        return unless input.present?
+
+        N_('%{description} on %{host}') % {
+          host: input[:host_display_name],
+          description: input[:description].try(:capitalize) || input[:job_category],
+        }
       end
 
       def humanized_name
diff --git a/app/views/job_invocations/_preview_hosts_list.html.erb b/app/views/job_invocations/_preview_hosts_list.html.erb
index c598234b4..c36b66146 100644
--- a/app/views/job_invocations/_preview_hosts_list.html.erb
+++ b/app/views/job_invocations/_preview_hosts_list.html.erb
@@ -7,7 +7,7 @@
 <% if @hosts.any? -%>
   <ul>
     <% @hosts.each do |host| -%>
-        <li><%= link_to h(host.name), current_host_details_path(host), :target => '_blank' %></li>
+        <li><%= link_to h(host), current_host_details_path(host), :target => '_blank' %></li>
     <% end -%>
 
     <% if @additional > 0 -%>
diff --git a/app/views/template_invocations/show.html.erb b/app/views/template_invocations/show.html.erb
index e012fae1c..09ff2c8cc 100644
--- a/app/views/template_invocations/show.html.erb
+++ b/app/views/template_invocations/show.html.erb
@@ -3,7 +3,7 @@
               :url => job_invocation_path(@template_invocation.job_invocation_id) }]
 
 if @host
-  items << { :caption => _('Template Invocation for %s') % @host.name }
+  items << { :caption => _('Template Invocation for %s') % @host }
   breadcrumbs(:resource_url => template_invocations_api_job_invocation_path(@template_invocation.job_invocation_id),
                :name_field => 'host_name',
                :switcher_item_url => template_invocation_path(':id'),
@@ -30,7 +30,7 @@ end
 <% if @host %>
   <% proxy_id = @template_invocation_task.input[:proxy_id] %>
   <h3>
-    <%= _('Target: ') %><%= link_to(@host.name, current_host_details_path(@host)) %>
+    <%= _('Target: ') %><%= link_to(@host, current_host_details_path(@host)) %>
     <% if proxy_id && proxy = SmartProxy.find_by(id: proxy_id) %>
       <%= _('using Smart Proxy') %> <%= link_to(proxy.name, smart_proxy_path(proxy)) %>
     <% end %>
diff --git a/webpack/JobWizard/JobWizardSelectors.js b/webpack/JobWizard/JobWizardSelectors.js
index 0bc3738aa..5fc26ccc1 100644
--- a/webpack/JobWizard/JobWizardSelectors.js
+++ b/webpack/JobWizard/JobWizardSelectors.js
@@ -78,8 +78,13 @@ export const selectHostsResponse = state => selectAPIResponse(state, HOSTS_API);
 export const selectHostCount = state =>
   selectHostsResponse(state).subtotal || 0;
 
-export const selectHosts = state =>
-  (selectHostsResponse(state).results || []).map(host => host.name);
+export const selectHosts = state => {
+  const hosts = selectHostsResponse(state).results || [];
+  return hosts.map(host => ({
+    name: host.name,
+    display_name: host.display_name,
+  }));
+};
 
 export const selectHostsMissingPermissions = state => {
   const hostsResponse = selectHostsResponse(state);
diff --git a/webpack/JobWizard/__tests__/fixtures.js b/webpack/JobWizard/__tests__/fixtures.js
index 3f8a4e192..20ee808c2 100644
--- a/webpack/JobWizard/__tests__/fixtures.js
+++ b/webpack/JobWizard/__tests__/fixtures.js
@@ -227,9 +227,9 @@ export const gqlMock = [
         hosts: {
           totalCount: 3,
           nodes: [
-            { id: 'MDE6SG9zdC0x', name: 'host1' },
-            { id: 'MDE6SG9zdC0y', name: 'host2' },
-            { id: 'MDE6SG9zdC0z', name: 'host3' },
+            { id: 'MDE6SG9zdC0x', name: 'host1', displayName: 'host1' },
+            { id: 'MDE6SG9zdC0y', name: 'host2', displayName: 'host2' },
+            { id: 'MDE6SG9zdC0z', name: 'host3', displayName: 'host3' },
           ],
         },
       },
diff --git a/webpack/JobWizard/autofill.js b/webpack/JobWizard/autofill.js
index 0094338da..6d8feffc8 100644
--- a/webpack/JobWizard/autofill.js
+++ b/webpack/JobWizard/autofill.js
@@ -39,10 +39,14 @@ export const useAutoFill = ({
             handleSuccess: ({ data }) => {
               setSelectedTargets(currentTargets => ({
                 ...currentTargets,
-                hosts: (data.results || []).map(({ id, name }) => ({
-                  id,
-                  name,
-                })),
+                hosts: (data.results || []).map(
+                  // eslint-disable-next-line camelcase
+                  ({ id, name, display_name }) => ({
+                    id,
+                    // eslint-disable-next-line camelcase
+                    name: display_name || name,
+                  })
+                ),
               }));
             },
           })
diff --git a/webpack/JobWizard/steps/HostsAndInputs/HostPreviewModal.js b/webpack/JobWizard/steps/HostsAndInputs/HostPreviewModal.js
index 4b2e53a84..65722718d 100644
--- a/webpack/JobWizard/steps/HostsAndInputs/HostPreviewModal.js
+++ b/webpack/JobWizard/steps/HostsAndInputs/HostPreviewModal.js
@@ -22,16 +22,16 @@ export const HostPreviewModal = ({ isOpen, setIsOpen, searchQuery }) => {
     >
       <List isPlain>
         {hosts.map(host => (
-          <ListItem key={host}>
+          <ListItem key={host.name}>
             <Button
               component="a"
-              href={foremanUrl(`/hosts/${host}`)}
+              href={foremanUrl(`/hosts/${host.name}`)}
               variant="link"
               target="_blank"
               rel="noreferrer"
               isInline
             >
-              {host}
+              {host.display_name}
             </Button>
           </ListItem>
         ))}
diff --git a/webpack/JobWizard/steps/HostsAndInputs/SelectGQL.js b/webpack/JobWizard/steps/HostsAndInputs/SelectGQL.js
index b04d6b4cb..884bc6958 100644
--- a/webpack/JobWizard/steps/HostsAndInputs/SelectGQL.js
+++ b/webpack/JobWizard/steps/HostsAndInputs/SelectGQL.js
@@ -38,6 +38,7 @@ export const useNameSearchGQL = apiKey => {
         data?.[dataName[apiKey]]?.nodes.map(node => ({
           id: decodeId(node.id),
           name: node.name,
+          displayName: node.displayName,
         })) || [],
     },
     loading,
diff --git a/webpack/JobWizard/steps/HostsAndInputs/SelectedChips.js b/webpack/JobWizard/steps/HostsAndInputs/SelectedChips.js
index 59b79fb88..723bbd8a8 100644
--- a/webpack/JobWizard/steps/HostsAndInputs/SelectedChips.js
+++ b/webpack/JobWizard/steps/HostsAndInputs/SelectedChips.js
@@ -4,7 +4,7 @@ import { Chip, ChipGroup, Button } from '@patternfly/react-core';
 import { sprintf, translate as __ } from 'foremanReact/common/I18n';
 import { hostMethods } from '../../JobWizardConstants';
 
-const SelectedChip = ({ selected, setSelected, categoryName }) => {
+const SelectedChip = ({ selected, setSelected, categoryName, setLabel }) => {
   const deleteItem = itemToRemove => {
     setSelected(oldSelected =>
       oldSelected.filter(({ id }) => id !== itemToRemove)
@@ -24,14 +24,14 @@ const SelectedChip = ({ selected, setSelected, categoryName }) => {
           setSelected(() => []);
         }}
       >
-        {selected.map(({ name, id }, index) => (
+        {selected.map((result, index) => (
           <Chip
             key={index}
-            id={`${categoryName}-${id}`}
-            onClick={() => deleteItem(id)}
-            closeBtnAriaLabel={`Remove ${name}`}
+            id={`${categoryName}-${result.id}`}
+            onClick={() => deleteItem(result.id)}
+            closeBtnAriaLabel={`Remove ${result.name}`}
           >
-            {name}
+            {setLabel(result)}
           </Chip>
         ))}
       </ChipGroup>
@@ -49,6 +49,7 @@ export const SelectedChips = ({
   setSelectedHostGroups,
   hostsSearchQuery,
   clearSearch,
+  setLabel,
 }) => {
   const clearAll = () => {
     setSelectedHosts(() => []);
@@ -67,16 +68,19 @@ export const SelectedChips = ({
         selected={selectedHosts}
         categoryName={hostMethods.hosts}
         setSelected={setSelectedHosts}
+        setLabel={setLabel}
       />
       <SelectedChip
         selected={selectedHostCollections}
         categoryName={hostMethods.hostCollections}
         setSelected={setSelectedHostCollections}
+        setLabel={setLabel}
       />
       <SelectedChip
         selected={selectedHostGroups}
         categoryName={hostMethods.hostGroups}
         setSelected={setSelectedHostGroups}
+        setLabel={setLabel}
       />
       <SelectedChip
         selected={
@@ -86,6 +90,7 @@ export const SelectedChips = ({
         }
         categoryName={hostMethods.searchQuery}
         setSelected={clearSearch}
+        setLabel={setLabel}
       />
       {showClear && (
         <Button variant="link" className="clear-chips" onClick={clearAll}>
@@ -105,10 +110,12 @@ SelectedChips.propTypes = {
   setSelectedHostGroups: PropTypes.func.isRequired,
   hostsSearchQuery: PropTypes.string.isRequired,
   clearSearch: PropTypes.func.isRequired,
+  setLabel: PropTypes.func.isRequired,
 };
 
 SelectedChip.propTypes = {
   categoryName: PropTypes.string.isRequired,
   selected: PropTypes.array.isRequired,
   setSelected: PropTypes.func.isRequired,
+  setLabel: PropTypes.func.isRequired,
 };
diff --git a/webpack/JobWizard/steps/HostsAndInputs/hosts.gql b/webpack/JobWizard/steps/HostsAndInputs/hosts.gql
index eddeb1ea1..b4b3b30f8 100644
--- a/webpack/JobWizard/steps/HostsAndInputs/hosts.gql
+++ b/webpack/JobWizard/steps/HostsAndInputs/hosts.gql
@@ -4,6 +4,7 @@ query($search: String!) {
     nodes {
       id
       name
+      displayName
     }
   }
 }
diff --git a/webpack/JobWizard/steps/HostsAndInputs/index.js b/webpack/JobWizard/steps/HostsAndInputs/index.js
index 7f45c942a..080e25ecd 100644
--- a/webpack/JobWizard/steps/HostsAndInputs/index.js
+++ b/webpack/JobWizard/steps/HostsAndInputs/index.js
@@ -104,6 +104,7 @@ const HostsAndInputs = ({
   const dispatch = useDispatch();
 
   const selectedHosts = selected.hosts;
+  const setLabel = result => result.displayName || result.name;
   const setSelectedHosts = newSelected =>
     setSelected(prevSelected => ({
       ...prevSelected,
@@ -191,6 +192,7 @@ const HostsAndInputs = ({
                 apiKey={HOSTS}
                 name="hosts"
                 placeholderText={__('Filter by hosts')}
+                setLabel={setLabel}
               />
             )}
             {hostMethod === hostMethods.hostCollections && (
@@ -201,6 +203,7 @@ const HostsAndInputs = ({
                 name="host collections"
                 url="/katello/api/host_collections?per_page=100"
                 placeholderText={__('Filter by host collections')}
+                setLabel={setLabel}
               />
             )}
             {hostMethod === hostMethods.hostGroups && (
@@ -210,6 +213,7 @@ const HostsAndInputs = ({
                 apiKey={HOST_GROUPS}
                 name="host groups"
                 placeholderText={__('Filter by host groups')}
+                setLabel={setLabel}
               />
             )}
           </InputGroup>
@@ -223,6 +227,7 @@ const HostsAndInputs = ({
           setSelectedHostGroups={setSelectedHostGroups}
           hostsSearchQuery={hostsSearchQuery}
           clearSearch={clearSearch}
+          setLabel={setLabel}
         />
         <Text>
           {__('Apply to')}{' '}
diff --git a/webpack/JobWizard/steps/ReviewDetails/index.js b/webpack/JobWizard/steps/ReviewDetails/index.js
index ae2d862aa..3cb4d6795 100644
--- a/webpack/JobWizard/steps/ReviewDetails/index.js
+++ b/webpack/JobWizard/steps/ReviewDetails/index.js
@@ -79,12 +79,13 @@ const ReviewDetails = ({
 
   const hostsCount = useSelector(selectHostCount);
   const [hostPreviewOpen, setHostPreviewOpen] = useState(false);
+  const NUM_CHIPS = 3;
   const stringHosts = () => {
     if (hosts.length === 0) {
       return __('No Target Hosts');
     }
-    if (hosts.length === 1 || hosts.length === 2) {
-      return hosts.join(', ');
+    if (hosts.length < NUM_CHIPS) {
+      return hosts.map(host => host.display_name).join(', ');
     }
     return (
       <div>
diff --git a/webpack/JobWizard/steps/form/SearchSelect.js b/webpack/JobWizard/steps/form/SearchSelect.js
index fef3985d6..600c28f17 100644
--- a/webpack/JobWizard/steps/form/SearchSelect.js
+++ b/webpack/JobWizard/steps/form/SearchSelect.js
@@ -16,6 +16,7 @@ export const SearchSelect = ({
   apiKey,
   url,
   variant,
+  setLabel,
 }) => {
   const [onSearch, response, isLoading] = useNameSearch(apiKey, url);
   const [isOpen, setIsOpen] = useState(false);
@@ -48,7 +49,7 @@ export const SearchSelect = ({
     ...selectOptions,
     ...Immutable.asMutable(response?.results || [])?.map((result, index) => (
       <SelectOption key={index + 1} value={result.id}>
-        {result.name}
+        {setLabel(result)}
       </SelectOption>
     )),
   ];
@@ -104,6 +105,7 @@ SearchSelect.propTypes = {
   name: PropTypes.string,
   selected: PropTypes.oneOfType([PropTypes.object, PropTypes.array]),
   setSelected: PropTypes.func.isRequired,
+  setLabel: PropTypes.func.isRequired,
   placeholderText: PropTypes.string,
   apiKey: PropTypes.string.isRequired,
   url: PropTypes.string,
diff --git a/webpack/JobWizard/steps/form/__tests__/SelectSearch.test.js b/webpack/JobWizard/steps/form/__tests__/SelectSearch.test.js
index 23d45f830..42d096b9e 100644
--- a/webpack/JobWizard/steps/form/__tests__/SelectSearch.test.js
+++ b/webpack/JobWizard/steps/form/__tests__/SelectSearch.test.js
@@ -7,6 +7,7 @@ const apiKey = 'HOSTS_KEY';
 describe('SearchSelect', () => {
   it('too many', () => {
     const onSearch = jest.fn();
+    const setLabel = jest.fn();
     render(
       <SearchSelect
         selected={['hosts1,host2']}
@@ -19,6 +20,7 @@ describe('SearchSelect', () => {
           { results: ['host1', 'host2', 'host3'], subtotal: 101 },
           false,
         ]}
+        setLabel={setLabel}
       />
     );
     const openSelectbutton = screen.getByRole('button', {