Fixes #39339 - use React component for TaskDetails page rendering

app/controllers/foreman_tasks/tasks_controller.rb

@@ -6,11 +6,6 @@ class TasksController < ::ApplicationController
 
     before_action :find_dynflow_task, only: [:unlock, :force_unlock, :cancel, :abort, :cancel_step, :resume]
 
-    def show
-      @task = resource_base.find(params[:id])
-      render :layout => !request.xhr?
-    end
-
     def index
       params[:order] ||= 'started_at DESC'
       respond_with_tasks resource_base

config/routes.rb

@@ -12,7 +12,7 @@
       end
     end
 
-    resources :tasks, :only => [:show] do
+    resources :tasks, :only => [] do
       collection do
         get 'auto_complete_search'
         get '/summary/:recent_timeframe', action: 'summary'
@@ -27,14 +27,15 @@
         post :cancel_step
       end
     end
-    resources :tasks, :only => [:show], constraints: ->(req) { req.format == :csv } do
+    resources :tasks, :only => [], constraints: ->(req) { req.format == :csv } do
       member do
         get :sub_tasks
       end
     end
     resources :tasks, :only => [:index], constraints: ->(req) { req.format == :csv }
 
     match '/tasks', to: '/react#index', via: :get
+    match '/tasks/:id', to: '/react#index', via: :get, :as => :task
     match '/tasks/:id/sub_tasks', to: '/react#index', via: :get
 
     namespace :api do

lib/foreman_tasks/engine.rb

@@ -20,7 +20,7 @@ class Engine < ::Rails::Engine
       require 'foreman/cron'
 
       Foreman::Plugin.register :"foreman-tasks" do
-        requires_foreman '>= 3.19'
+        requires_foreman '>= 5.0'
         register_global_js_file 'global'
         divider :top_menu, :parent => :monitor_menu, :last => true, :caption => N_('Foreman Tasks')
         menu :top_menu, :tasks,
@@ -36,7 +36,7 @@ class Engine < ::Rails::Engine
              :last     => true
 
         security_block :foreman_tasks do |_map|
-          permission :view_foreman_tasks, { :'foreman_tasks/tasks' => [:auto_complete_search, :sub_tasks, :index, :summary, :summary_sub_tasks, :show],
+          permission :view_foreman_tasks, { :'foreman_tasks/tasks' => [:auto_complete_search, :sub_tasks, :index, :summary, :summary_sub_tasks],
                                             :'foreman_tasks/api/tasks' => [:bulk_search, :show, :index, :summary, :summary_sub_tasks, :details, :sub_tasks] }, :resource_type => 'ForemanTasks::Task'
           permission :edit_foreman_tasks, { :'foreman_tasks/tasks' => [:resume, :unlock, :force_unlock, :cancel_step, :cancel, :abort],
                                             :'foreman_tasks/api/tasks' => [:bulk_resume, :bulk_cancel, :bulk_stop] }, :resource_type => 'ForemanTasks::Task'

test/controllers/tasks_controller_test.rb

@@ -82,15 +82,6 @@ def in_taxonomy_scope(organization, location = nil)
         assert_include response.body.lines[1], 'Some action'
       end
 
-      describe 'show' do
-        it 'does not allow user without permissions to see task details' do
-          setup_user('view', 'foreman_tasks', 'owner.id = current_user')
-          get :show, params: { id: FactoryBot.create(:some_task).id },
-                     session: set_session_user(User.current)
-          assert_response :not_found
-        end
-      end
-
       describe 'index' do
         it 'shows duration column' do
           task = ForemanTasks::Task.select_duration.find(FactoryBot.create(:some_task).id)

test/foreman_tasks_test_helper.rb

@@ -9,8 +9,8 @@
 require 'dynflow/testing'
 require 'foreman_tasks/test_helpers'
 
-FactoryBot.definition_file_paths = ["#{ForemanTasks::Engine.root}/test/factories"]
-FactoryBot.find_definitions
+FactoryBot.definition_file_paths << "#{ForemanTasks::Engine.root}/test/factories"
+FactoryBot.reload
 
 ForemanTasks.dynflow.require!
 ForemanTasks.dynflow.config.disable_active_record_actions = true

test/integration/tasks_test.rb

@@ -0,0 +1,17 @@
+# frozen_string_literal: true
+
+require_relative '../test_plugin_helper'
+require 'integration_test_helper'
+
+class TasksIntegrationTest < IntegrationTestWithJavascript
+  test 'does not allow user without permissions to see task details' do
+    setup_user('view', 'foreman_tasks', 'owner.id = current_user')
+    task = FactoryBot.create(:some_task)
+    set_request_user(User.current)
+
+    visit "/foreman_tasks/tasks/#{task.id}"
+
+    assert_selector 'h5', text: /Unable to load task/i
+    assert_no_selector '#task-details-tabs'
+  end
+end

test/test_plugin_helper.rb

@@ -0,0 +1,8 @@
+# frozen_string_literal: true
+
+# This calls the main test_helper in Foreman-core
+require 'test_helper'
+
+# Add plugin to FactoryBot's paths
+FactoryBot.definition_file_paths << File.join(File.dirname(__FILE__), 'factories')
+FactoryBot.reload

webpack/ForemanTasks/Components/TaskDetails/Components/TaskInfo.js

@@ -5,17 +5,12 @@ import {
   GridItem,
   Progress,
   ProgressVariant,
-  Icon,
 } from '@patternfly/react-core';
-import {
-  CheckCircleIcon,
-  ExclamationCircleIcon,
-  ExclamationTriangleIcon,
-  QuestionCircleIcon,
-} from '@patternfly/react-icons';
 import { translate as __ } from 'foremanReact/common/I18n';
 import RelativeDateTime from 'foremanReact/components/common/dates/RelativeDateTime';
 
+import { taskResultIconEl } from '../../common/taskResultIcon';
+
 const isDelayed = ({ startAt, startedAt }) => {
   if (
     startAt == null ||
@@ -35,41 +30,6 @@ const isDelayed = ({ startAt, startedAt }) => {
   return a.getTime() !== b.getTime();
 };
 
-const resultIconEl = (state, result) => {
-  if (state !== 'stopped')
-    return (
-      <Icon>
-        <QuestionCircleIcon />
-      </Icon>
-    );
-  switch (result) {
-    case 'success':
-      return (
-        <Icon status="success">
-          <CheckCircleIcon />
-        </Icon>
-      );
-    case 'error':
-      return (
-        <Icon status="danger">
-          <ExclamationCircleIcon />
-        </Icon>
-      );
-    case 'warning':
-      return (
-        <Icon status="warning">
-          <ExclamationTriangleIcon />
-        </Icon>
-      );
-    default:
-      return (
-        <Icon>
-          <QuestionCircleIcon />
-        </Icon>
-      );
-  }
-};
-
 const progressVariantForResult = result => {
   switch (result) {
     case 'error':
@@ -117,7 +77,7 @@ const TaskInfo = props => {
         title: 'Result',
         value: (
           <span>
-            {resultIconEl(state, result)} {result}
+            {taskResultIconEl(state, result)} {result}
           </span>
         ),
       },

webpack/ForemanTasks/Components/TaskDetails/Components/__tests__/Task.test.js

@@ -1,7 +1,6 @@
 import React from 'react';
 import { render, screen } from '@testing-library/react';
 import '@testing-library/jest-dom';
-import { STATUS } from 'foremanReact/constants';
 
 import Task from '../Task';
 
@@ -29,7 +28,6 @@ describe('Task', () => {
         parentTask="parent-id"
         taskReload
         canEdit
-        status={STATUS.RESOLVED}
         taskProgressToggle={jest.fn()}
         taskReloadStart={jest.fn()}
       />

webpack/ForemanTasks/Components/TaskDetails/Components/__tests__/TaskButtons.test.js

@@ -1,7 +1,6 @@
 import React from 'react';
 import { render, screen, fireEvent } from '@testing-library/react';
 import '@testing-library/jest-dom';
-import { STATUS } from 'foremanReact/constants';
 import { TaskButtons } from '../TaskButtons';
 
 const setUnlockModalOpen = jest.fn();
@@ -169,7 +168,6 @@ describe('TaskButtons', () => {
       resumeTaskRequest,
       taskProgressToggle,
       taskReloadStart,
-      status: STATUS.RESOLVED,
       canEdit: true,
       resumable: true,
       cancellable: true,

webpack/ForemanTasks/Components/TaskDetails/TaskDetails.js

@@ -1,15 +1,17 @@
 import React, { useEffect, useState } from 'react';
 import PropTypes from 'prop-types';
 import { Tabs, Tab, TabTitleText } from '@patternfly/react-core';
-import { translate as __, sprintf } from 'foremanReact/common/I18n';
+import { translate as __ } from 'foremanReact/common/I18n';
 import { STATUS } from 'foremanReact/constants';
-import MessageBox from 'foremanReact/components/common/MessageBox';
+import { usePermissions } from 'foremanReact/common/hooks/Permissions/permissionHooks';
+import { ResourceLoadFailedEmptyState } from 'foremanReact/components/common/EmptyState';
 import Task from './Components/Task';
 import RunningSteps from './Components/RunningSteps';
 import Errors from './Components/Errors';
 import Locks from './Components/Locks';
 import Raw from './Components/Raw';
 import Dependencies from './Components/Dependencies';
+import { TASKS_PATH, VIEW_FOREMAN_TASKS } from './TaskDetailsConstants';
 import { getTaskID } from './TasksDetailsHelper';
 import { TaskSkeleton } from './Components/TaskSkeleton';
 
@@ -26,12 +28,15 @@ const TaskDetails = ({
   cancelStep,
   taskReloadStart,
   taskReloadStop,
-  APIerror,
+  apiStatus,
+  apiErrorMessage,
+  apiErrorCode,
   ...props
 }) => {
   const id = getTaskID();
-  const { taskReload, status, isLoading, result } = props;
+  const { taskReload, isLoading, result } = props;
   const [activeTabKey, setActiveTabKey] = useState(1);
+  const hasViewPermission = usePermissions([VIEW_FOREMAN_TASKS]);
 
   useEffect(() => {
     taskReloadStart(id);
@@ -48,15 +53,25 @@ const TaskDetails = ({
     }
   };
 
-  if (status === STATUS.ERROR) {
+  if (apiStatus === STATUS.ERROR || !hasViewPermission) {
     return (
-      <MessageBox
-        key="task-details-error"
-        icontype="error-circle-o"
-        msg={sprintf(__('Could not receive data: %s'), APIerror?.message)}
+      <ResourceLoadFailedEmptyState
+        resourceLabel={__('task')}
+        resourceId={id}
+        httpStatus={apiErrorCode}
+        errorMessage={apiErrorMessage}
+        viewPermissions={['view_foreman_tasks']}
+        requiredPermissions={['view_foreman_tasks']}
+        ouiaIdPrefix="task-details-empty-state"
+        primaryAction={{
+          label: __('Back to tasks'),
+          url: TASKS_PATH,
+          ouiaId: 'task-details-empty-state-tasks-list',
+        }}
       />
     );
   }
+
   const resumable = executionPlan ? executionPlan.state === 'paused' : false;
   const cancellable = executionPlan ? executionPlan.cancellable : false;
   const lockRecords = locks.concat(links);
@@ -66,13 +81,12 @@ const TaskDetails = ({
     cancellable,
     resumable,
     id,
-    status,
     taskProgressToggle,
     taskReloadStart,
   };
 
   return (
-    <div className="task-details-react well">
+    <div className="task-details-react">
       <Tabs
         id="task-details-tabs"
         ouiaId="task-details-tabs"
@@ -159,8 +173,9 @@ TaskDetails.propTypes = {
   runningSteps: PropTypes.array,
   cancelStep: PropTypes.func.isRequired,
   taskReload: PropTypes.bool.isRequired,
-  status: PropTypes.oneOf(Object.keys(STATUS)),
-  APIerror: PropTypes.object,
+  apiStatus: PropTypes.oneOf(Object.keys(STATUS)),
+  apiErrorMessage: PropTypes.string,
+  apiErrorCode: PropTypes.number,
   taskReloadStop: PropTypes.func.isRequired,
   taskReloadStart: PropTypes.func.isRequired,
   links: PropTypes.array,
@@ -175,8 +190,7 @@ TaskDetails.propTypes = {
 TaskDetails.defaultProps = {
   label: '',
   runningSteps: [],
-  APIerror: null,
-  status: STATUS.PENDING,
+  apiErrorMessage: '',
   links: [],
   dependsOn: [],
   blocks: [],

webpack/ForemanTasks/Components/TaskDetails/TaskDetailsConstants.js

@@ -1,4 +1,5 @@
 export const FOREMAN_TASK_DETAILS = 'FOREMAN_TASK_DETAILS';
 export const FOREMAN_TASK_DETAILS_SUCCESS = 'FOREMAN_TASK_DETAILS_SUCCESS';
-
+export const TASKS_PATH = '/foreman_tasks/tasks';
 export const TASK_STEP_CANCEL = 'TASK_STEP_CANCEL';
+export const VIEW_FOREMAN_TASKS = 'view_foreman_tasks';

webpack/ForemanTasks/Components/TaskDetails/TaskDetailsSelectors.js

@@ -1,7 +1,8 @@
 /* eslint-disable camelcase */
 import {
   selectAPIResponse,
-  selectAPIByKey,
+  selectAPIStatus as selectAPIStatusByKey,
+  selectAPIError as selectAPIErrorByKey,
 } from 'foremanReact/redux/API/APISelectors';
 import { selectDoesIntervalExist } from 'foremanReact/redux/middlewares/IntervalMiddleware/IntervalSelectors';
 import { STATUS } from 'foremanReact/constants';
@@ -101,14 +102,28 @@ export const selectDynflowEnableConsole = state =>
 export const selectCanEdit = state =>
   selectTaskDetailsResponse(state).can_edit || false;
 
-export const selectStatus = state => selectTaskDetailsResponse(state).status;
+export const selectAPIStatus = state =>
+  selectAPIStatusByKey(state, FOREMAN_TASK_DETAILS);
 
 export const selectAPIError = state =>
-  selectTaskDetailsResponse(state)?.APIerror;
+  selectAPIErrorByKey(state, FOREMAN_TASK_DETAILS);
+
+export const selectAPIErrorMessage = state => {
+  const apiError = selectAPIError(state);
+
+  if (apiError?.response?.data?.error) {
+    return apiError.response.data.error.message;
+  }
+
+  return apiError?.message;
+};
+
+export const selectAPIErrorCode = state =>
+  selectAPIError(state)?.response?.status;
 
 export const selectIsLoading = state =>
-  !!selectAPIByKey(state, FOREMAN_TASK_DETAILS).response &&
-  selectStatus(state) === STATUS.PENDING;
+  !Object.keys(selectTaskDetailsResponse(state) ?? {}).length &&
+  selectAPIStatus(state) === STATUS.PENDING;
 
 export const selectDependsOn = state =>
   selectTaskDetailsResponse(state).depends_on || [];

webpack/ForemanTasks/Components/TaskDetails/__tests__/TaskDetails.fixtures.js

@@ -4,5 +4,5 @@ export const minProps = {
   taskReloadStop: jest.fn(),
   taskReloadStart: jest.fn(),
   taskProgressToggle: jest.fn(),
-  status: 'RESOLVED',
+  apiStatus: 'RESOLVED',
 };