Performance

app/controllers/api/v2/job_invocations_controller.rb

@@ -118,6 +118,7 @@ def output
       add_scoped_search_description_for(JobInvocation)
       param :id, :identifier, :required => true
       def hosts
+        @include_permissions = Foreman::Cast.to_bool(params[:include_permissions])
         set_hosts_and_template_invocations
         @total = @hosts.size
         @hosts = @hosts.search_for(params[:search], :order => params[:order]).paginate(:page => params[:page], :per_page => params[:per_page])
@@ -329,6 +330,22 @@ def set_statuses_and_smart_proxies
         end
         @smart_proxy_id = template_invocations.to_h { |ti| [ti.host_id, ti.smart_proxy_id] }
         @smart_proxy_name = template_invocations.to_h { |ti| [ti.host_id, ti.smart_proxy_name] }
+        return unless @include_permissions
+        @task_by_host = template_invocations.to_h do |ti|
+          task = ti.run_host_job_task
+          [ti.host_id, task]
+        end
+        can_cancel_job_invocations = authorized_for(:permission => :cancel_job_invocations, :auth_object => @job_invocation)
+        can_create_job_invocations = authorized_for(controller: :job_invocations, action: :create)
+        can_execute_on_infra_hosts = User.current.can?(:execute_jobs_on_infrastructure_hosts)
+        @permissions_by_host = hosts.to_h do |host|
+          task = @task_by_host[host.id]
+          [host.id, {
+            :view_foreman_tasks => task && authorized_for(:permission => :view_foreman_tasks, :auth_object => task),
+            :cancel_job_invocations => can_cancel_job_invocations,
+            :execute_jobs => can_create_job_invocations && (!host.infrastructure_host? || can_execute_on_infra_hosts),
+          }]
+        end
       end
     end
   end

app/controllers/job_invocations_controller.rb

@@ -149,33 +149,6 @@ def preview_job_invocations_per_host
     render :json => {:job_invocations => job_invocations}
   end
 
-  def list_jobs_hosts
-    @job_invocation = resource_base.find(params[:id])
-    hosts = @job_invocation.targeting.hosts.authorized(:view_hosts, Host)
-    hosts = hosts.search_for(params[:search])
-    template_invocations_task_by_hosts = {}
-    hosts.each do |host|
-      template_invocation = @job_invocation.template_invocations.find { |template_inv| template_inv.host_id == host.id }
-      next unless template_invocation
-      template_invocation_task = template_invocation.run_host_job_task
-      template_invocations_task_by_hosts[host.id] =
-        {
-          :host_name => host.name,
-          :id => host.id,
-          :task => template_invocation_task.attributes.merge({cancellable: template_invocation_task.cancellable? }),
-          :permissions => {
-            :view_foreman_tasks => authorized_for(:permission => :view_foreman_tasks, :auth_object => template_invocation_task),
-            :cancel_job_invocations => authorized_for(:permission => :cancel_job_invocations, :auth_object => @job_invocation),
-            :execute_jobs => authorized_for(controller: :job_invocations, action: :create) && (!host.infrastructure_host? || User.current.can?(:execute_jobs_on_infrastructure_hosts)),
-          },
-        }
-    end
-
-    render json: {
-      :template_invocations_task_by_hosts => template_invocations_task_by_hosts,
-    }
-  end
-
   private
 
   def action_permission
@@ -186,7 +159,7 @@ def action_permission
         'create'
       when 'cancel'
         'cancel'
-      when 'chart', 'preview_job_invocations_per_host', 'list_jobs_hosts'
+      when 'chart', 'preview_job_invocations_per_host'
         'view'
       else
         super

app/views/api/v2/job_invocations/hosts.json.rabl

@@ -13,3 +13,12 @@ end
 node :smart_proxy_name do |host|
   @smart_proxy_name[host.id]
 end
+
+node(:task, :if => ->(_) { @task_by_host }) do |host|
+  task = @task_by_host[host.id]
+  task && { :id => task.id, :cancellable => task.try(:cancellable?) }
+end
+
+node(:permissions, :if => ->(_) { @permissions_by_host }) do |host|
+  @permissions_by_host[host.id]
+end

app/views/api/v2/job_invocations/main.json.rabl

@@ -34,6 +34,7 @@ end
 
 child :task do
   attributes :id, :state, :started_at
+  node(:cancellable) { |task| task.try(:cancellable?) }
 end
 
 if @template_invocations

config/routes.rb

@@ -35,7 +35,6 @@
       get 'auto_complete_search'
     end
     member do
-      get 'hosts', to: 'job_invocations#list_jobs_hosts'
       post 'cancel'
     end
   end

lib/foreman_remote_execution/plugin.rb

@@ -134,9 +134,9 @@
     permission :lock_job_templates, { :job_templates => [:lock, :unlock] }, :resource_type => 'JobTemplate'
     permission :create_job_invocations, { :job_invocations => [:new, :create, :legacy_create, :refresh, :rerun, :preview_hosts],
                                           'api/v2/job_invocations' => [:create, :rerun] }, :resource_type => 'JobInvocation'
-    permission :view_job_invocations, { :job_invocations => [:index, :chart, :show, :auto_complete_search, :preview_job_invocations_per_host, :list_jobs_hosts], :template_invocations => [:show, :show_template_invocation_by_host],
+    permission :view_job_invocations, { :job_invocations => [:index, :chart, :show, :auto_complete_search, :preview_job_invocations_per_host], :template_invocations => [:show, :show_template_invocation_by_host],
                                         'api/v2/job_invocations' => [:index, :show, :output, :raw_output, :outputs, :hosts] }, :resource_type => 'JobInvocation'
-    permission :view_template_invocations, { :template_invocations => [:show, :template_invocation_preview, :show_template_invocation_by_host], :job_invocations => [:list_jobs_hosts],
+    permission :view_template_invocations, { :template_invocations => [:show, :template_invocation_preview, :show_template_invocation_by_host],
                                             'api/v2/template_invocations' => [:template_invocations], :ui_job_wizard => [:job_invocation] }, :resource_type => 'TemplateInvocation'
     permission :create_template_invocations, {}, :resource_type => 'TemplateInvocation'
     permission :execute_jobs_on_infrastructure_hosts, {}, :resource_type => 'JobInvocation'

test/functional/api/v2/job_invocations_controller_test.rb

@@ -67,6 +67,14 @@ class JobInvocationsControllerTest < ActionController::TestCase
           assert_nil result['template_invocations']
         end
 
+        test 'should include cancellable field in task node' do
+          get :show, params: { :id => @invocation.id }
+          assert_response :success
+          result = ActiveSupport::JSON.decode(@response.body)
+          assert result.key?('task'), 'task node should be present in show response'
+          assert result['task'].key?('cancellable'), 'cancellable field should be present in task node'
+        end
+
         test 'should include job_status per host when host_status=true' do
           invocation = FactoryBot.create(:job_invocation, :with_template, :with_task)
           invocation.template_invocations << FactoryBot.create(:template_invocation, :with_task, :with_host, :job_invocation => invocation)
@@ -284,19 +292,67 @@ class JobInvocationsControllerTest < ActionController::TestCase
       end
 
       describe '#hosts' do
+        setup do
+          @hosts_invocation = FactoryBot.create(:job_invocation, :with_template, :with_task)
+          2.times { @hosts_invocation.template_invocations << FactoryBot.create(:template_invocation, :with_task, :with_host, :job_invocation => @hosts_invocation) }
+          @hosts_invocation.job_category = @hosts_invocation.pattern_template_invocations.first.template.job_category
+          @hosts_invocation.targeting.hosts = @hosts_invocation.template_invocations.map(&:host)
+          @hosts_invocation.save!
+        end
+
         test 'should compute job_status for the paginated subset' do
+          get :hosts, params: { :id => @hosts_invocation.id, :page => 2, :per_page => 1 }
+          assert_response :success
+          result = ActiveSupport::JSON.decode(@response.body)
+          assert_equal 3, result['total']
+          assert_equal 1, result['results'].size
+          assert_equal(['success'], result['results'].map { |r| r['job_status'] })
+        end
+
+        test 'should not include task or permissions nodes without include_permissions' do
+          get :hosts, params: { :id => @hosts_invocation.id }
+          assert_response :success
+          result = ActiveSupport::JSON.decode(@response.body)
+          assert_not_empty result['results']
+          result['results'].each do |host|
+            assert_not host.key?('task'), 'task node should be absent when include_permissions is not set'
+            assert_not host.key?('permissions'), 'permissions node should be absent when include_permissions is not set'
+          end
+        end
+
+        test 'should include task and permissions nodes when include_permissions=true' do
+          get :hosts, params: { :id => @hosts_invocation.id, :include_permissions => true }
+          assert_response :success
+          result = ActiveSupport::JSON.decode(@response.body)
+          assert_not_empty result['results']
+          result['results'].each do |host|
+            assert host.key?('task'), 'task node should be present when include_permissions=true'
+            assert host.key?('permissions'), 'permissions node should be present when include_permissions=true'
+            assert host['task'].key?('id')
+            assert host['task'].key?('cancellable')
+            assert host['permissions'].key?('view_foreman_tasks')
+            assert host['permissions'].key?('cancel_job_invocations')
+            assert host['permissions'].key?('execute_jobs')
+          end
+        end
+
+        test 'should handle nil run_host_job_task with include_permissions=true' do
+          # Create a host without a run_host_job_task (template invocation without :with_task)
           invocation = FactoryBot.create(:job_invocation, :with_template, :with_task)
-          2.times { invocation.template_invocations << FactoryBot.create(:template_invocation, :with_task, :with_host, :job_invocation => invocation) }
+          ti = FactoryBot.create(:template_invocation, :with_host, :job_invocation => invocation)
           invocation.job_category = invocation.pattern_template_invocations.first.template.job_category
-          invocation.targeting.hosts = invocation.template_invocations.map(&:host)
+          invocation.targeting.hosts = [ti.host]
           invocation.save!
 
-          get :hosts, params: { :id => invocation.id, :page => 2, :per_page => 1 }
+          get :hosts, params: { :id => invocation.id, :include_permissions => true }
           assert_response :success
           result = ActiveSupport::JSON.decode(@response.body)
-          assert_equal 3, result['total']
-          assert_equal 1, result['results'].size
-          assert_equal(['success'], result['results'].map { |r| r['job_status'] })
+          assert_not_empty result['results']
+          host_result = result['results'].first
+          assert host_result.key?('task'), 'task node should be present'
+          assert_nil host_result['task'], 'task should be nil when run_host_job_task is absent'
+          assert host_result.key?('permissions'), 'permissions node should be present'
+          refute host_result['permissions']['view_foreman_tasks']
         end
       end
 

webpack/JobInvocationDetail/JobInvocationActions.js

@@ -1,52 +1,57 @@
 import { translate as __, sprintf } from 'foremanReact/common/I18n';
-import { foremanUrl } from 'foremanReact/common/helpers';
 import { addToast } from 'foremanReact/components/ToastsList';
 import { APIActions, get } from 'foremanReact/redux/API';
-import {
-  stopInterval,
-  withInterval,
-} from 'foremanReact/redux/middlewares/IntervalMiddleware';
 import {
   CANCEL_JOB,
   CANCEL_RECURRING_LOGIC,
   CHANGE_ENABLED_RECURRING_LOGIC,
-  GET_TASK,
   JOB_INVOCATION_KEY,
-  UPDATE_JOB,
+  STATUS,
 } from './JobInvocationConstants';
 
-export const getJobInvocation = url => dispatch => {
-  const fetchData = withInterval(
+const FINISHED_STATUSES = [STATUS.FAILED, STATUS.SUCCEEDED, STATUS.CANCELLED];
+
+export const isJobFinished = statusLabel =>
+  FINISHED_STATUSES.includes(statusLabel);
+
+const extractErrorMessage = response =>
+  // eslint-disable-next-line camelcase
+  response?.data?.error?.full_messages?.[0] ||
+  response?.data?.error?.message ||
+  'Unknown error.';
+
+const fetchJobInvocation = (dispatch, url, params = {}, pollTimeoutRef = { current: null }) => {
+  dispatch(
     get({
       key: JOB_INVOCATION_KEY,
-      params: { include_permissions: true, include_hosts: false },
+      params: { include_hosts: false, include_permissions: true, ...params },
       url,
+      handleSuccess: ({ data }) => {
+        if (!isJobFinished(data.status_label)) {
+          pollTimeoutRef.current = setTimeout(
+            () => fetchJobInvocation(dispatch, url, {}, pollTimeoutRef),
+            5000
+          );
+        } else {
+          pollTimeoutRef.current = null;
+        }
+      },
       handleError: () => {
-        dispatch(stopInterval(JOB_INVOCATION_KEY));
+        pollTimeoutRef.current = null;
       },
-      errorToast: ({ response }) =>
-        // eslint-disable-next-line camelcase
-        response?.data?.error?.full_messages?.[0] ||
-        // eslint-disable-next-line camelcase
-        response?.data?.error?.full_messages ||
-        response?.data?.error?.message ||
-        'Error',
-    }),
-    1000
+      errorToast: ({ response }) => extractErrorMessage(response),
+    })
   );
+};
 
-  dispatch(fetchData);
+export const getJobInvocation = (url, pollTimeoutRef) => dispatch => {
+  stopJobInvocationPolling(pollTimeoutRef);
+  fetchJobInvocation(dispatch, url, { include_permissions: true }, pollTimeoutRef);
 };
 
-export const updateJob = jobId => dispatch => {
-  const url = foremanUrl(`/api/job_invocations/${jobId}`);
-  dispatch(
-    APIActions.get({
-      url,
-      key: UPDATE_JOB,
-      params: { include_hosts: false },
-    })
-  );
+export const stopJobInvocationPolling = pollTimeoutRef => {
+  clearTimeout(pollTimeoutRef.current);
+  pollTimeoutRef.current = null;
 };
 
 export const cancelJob = (jobId, force) => dispatch => {
@@ -56,8 +61,8 @@
       : sprintf(__('Trying to cancel the job %s.'), jobId);
   const errorToast = response =>
     force
-      ? sprintf(__(`Could not abort the job %s: ${response}`), jobId)
-      : sprintf(__(`Could not cancel the job %s: ${response}`), jobId);
+      ? sprintf(__('Could not abort the job %s: %s'), jobId, response)
+      : sprintf(__('Could not cancel the job %s: %s'), jobId, response);
   const url = force
     ? `/job_invocations/${jobId}/cancel?force=true`
     : `/job_invocations/${jobId}/cancel`;
@@ -66,13 +71,7 @@
     APIActions.post({
       url,
       key: CANCEL_JOB,
-      errorToast: ({ response }) =>
-        errorToast(
-          // eslint-disable-next-line camelcase
-          response?.data?.error?.full_messages ||
-            response?.data?.error?.message ||
-            'Unknown error.'
-        ),
+      errorToast: ({ response }) => errorToast(extractErrorMessage(response)),
       handleSuccess: () => {
         dispatch(
           addToast({
@@ -81,21 +80,11 @@
             message: infoToast(),
           })
         );
-        dispatch(updateJob(jobId));
       },
     })
   );
 };
 
-export const getTask = taskId => dispatch => {
-  dispatch(
-    get({
-      key: GET_TASK,
-      url: `/foreman_tasks/api/tasks/${taskId}`,
-    })
-  );
-};
-
 export const enableRecurringLogic = (
   recurrenceId,
   enabled,
@@ -122,19 +111,15 @@
       key: CHANGE_ENABLED_RECURRING_LOGIC,
       params: { recurring_logic: { enabled: !enabled } },
       successToast,
-      errorToast: ({ response }) =>
-        errorToast(
-          // eslint-disable-next-line camelcase
-          response?.data?.error?.full_messages ||
-            response?.data?.error?.message ||
-            'Unknown error.'
-        ),
-      handleSuccess: () => dispatch(updateJob(jobId)),
+      errorToast: ({ response }) => errorToast(extractErrorMessage(response)),
+      handleSuccess: () => {
+        fetchJobInvocation(dispatch, `/api/job_invocations/${jobId}`);
+      },
     })
   );
 };
 
-export const cancelRecurringLogic = (recurrenceId, jobId) => dispatch => {
+export const cancelRecurringLogic = recurrenceId => dispatch => {
   const successToast = () =>
     sprintf(__('Recurring logic %s cancelled successfully.'), recurrenceId);
   const errorToast = response =>
@@ -148,14 +133,7 @@
       url,
       key: CANCEL_RECURRING_LOGIC,
       successToast,
-      errorToast: ({ response }) =>
-        errorToast(
-          // eslint-disable-next-line camelcase
-          response?.data?.error?.full_messages ||
-            response?.data?.error?.message ||
-            'Unknown error.'
-        ),
-      handleSuccess: () => dispatch(updateJob(jobId)),
+      errorToast: ({ response }) => errorToast(extractErrorMessage(response)),
     })
   );
 };