diff --git a/app/controllers/foreman_tasks/tasks_controller.rb b/app/controllers/foreman_tasks/tasks_controller.rb index 678495a8b..3f21aeac1 100644 --- a/app/controllers/foreman_tasks/tasks_controller.rb +++ b/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 diff --git a/app/views/foreman_tasks/tasks/show.html.erb b/app/views/foreman_tasks/tasks/show.html.erb deleted file mode 100644 index ad454da05..000000000 --- a/app/views/foreman_tasks/tasks/show.html.erb +++ /dev/null @@ -1,18 +0,0 @@ -<% stylesheet 'foreman_tasks/foreman_tasks' %> -<% content_for(:javascripts) do %> - <%= webpacked_plugins_js_for :'foreman-tasks' %> -<% end %> -<% content_for(:stylesheets) do %> - <%= webpacked_plugins_css_for :'foreman-tasks' %> -<% end %> - -<% title _("Details of %s task") % @task.to_s %> - -<%= breadcrumbs( - items: breadcrumb_items, - name_field: 'action', - resource_url: foreman_tasks_api_tasks_path, - switcher_item_url: foreman_tasks_task_path(:id => ':id') -) %> - -<%= react_component('TaskDetails') %> diff --git a/config/routes.rb b/config/routes.rb index eb44fdc71..dbac28a98 100644 --- a/config/routes.rb +++ b/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,7 +27,7 @@ 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 @@ -35,6 +35,7 @@ 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 diff --git a/lib/foreman_tasks/engine.rb b/lib/foreman_tasks/engine.rb index 07268cf36..6b8b7a608 100644 --- a/lib/foreman_tasks/engine.rb +++ b/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' diff --git a/test/controllers/tasks_controller_test.rb b/test/controllers/tasks_controller_test.rb index dd3c684ba..a96c9d30d 100644 --- a/test/controllers/tasks_controller_test.rb +++ b/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) diff --git a/test/foreman_tasks_test_helper.rb b/test/foreman_tasks_test_helper.rb index bbd2526c0..7e75a2a28 100644 --- a/test/foreman_tasks_test_helper.rb +++ b/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 diff --git a/test/integration/tasks_test.rb b/test/integration/tasks_test.rb new file mode 100644 index 000000000..8d21af2fa --- /dev/null +++ b/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 diff --git a/test/test_plugin_helper.rb b/test/test_plugin_helper.rb new file mode 100644 index 000000000..5bd928eac --- /dev/null +++ b/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 diff --git a/webpack/ForemanTasks/Components/TaskDetails/Components/TaskInfo.js b/webpack/ForemanTasks/Components/TaskDetails/Components/TaskInfo.js index a206c1043..3c4a6145b 100644 --- a/webpack/ForemanTasks/Components/TaskDetails/Components/TaskInfo.js +++ b/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 ( - - - - ); - switch (result) { - case 'success': - return ( - - - - ); - case 'error': - return ( - - - - ); - case 'warning': - return ( - - - - ); - default: - return ( - - - - ); - } -}; - const progressVariantForResult = result => { switch (result) { case 'error': @@ -117,7 +77,7 @@ const TaskInfo = props => { title: 'Result', value: ( - {resultIconEl(state, result)} {result} + {taskResultIconEl(state, result)} {result} ), }, diff --git a/webpack/ForemanTasks/Components/TaskDetails/Components/__tests__/Task.test.js b/webpack/ForemanTasks/Components/TaskDetails/Components/__tests__/Task.test.js index 4ef7727d2..2ca7caf8a 100644 --- a/webpack/ForemanTasks/Components/TaskDetails/Components/__tests__/Task.test.js +++ b/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()} /> diff --git a/webpack/ForemanTasks/Components/TaskDetails/Components/__tests__/TaskButtons.test.js b/webpack/ForemanTasks/Components/TaskDetails/Components/__tests__/TaskButtons.test.js index ad3acd5d3..b0ecd89d8 100644 --- a/webpack/ForemanTasks/Components/TaskDetails/Components/__tests__/TaskButtons.test.js +++ b/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, diff --git a/webpack/ForemanTasks/Components/TaskDetails/TaskDetails.js b/webpack/ForemanTasks/Components/TaskDetails/TaskDetails.js index 7a14446bd..cab955487 100644 --- a/webpack/ForemanTasks/Components/TaskDetails/TaskDetails.js +++ b/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 ( - ); } + 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 ( -
+
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 || []; diff --git a/webpack/ForemanTasks/Components/TaskDetails/__tests__/TaskDetails.fixtures.js b/webpack/ForemanTasks/Components/TaskDetails/__tests__/TaskDetails.fixtures.js index 84bb4c07b..209bfcccc 100644 --- a/webpack/ForemanTasks/Components/TaskDetails/__tests__/TaskDetails.fixtures.js +++ b/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', }; diff --git a/webpack/ForemanTasks/Components/TaskDetails/__tests__/TaskDetails.test.js b/webpack/ForemanTasks/Components/TaskDetails/__tests__/TaskDetails.test.js index 8e8cad961..412f6787a 100644 --- a/webpack/ForemanTasks/Components/TaskDetails/__tests__/TaskDetails.test.js +++ b/webpack/ForemanTasks/Components/TaskDetails/__tests__/TaskDetails.test.js @@ -1,38 +1,125 @@ import React from 'react'; import { render, screen } from '@testing-library/react'; import '@testing-library/jest-dom'; +import { configureStore } from '@reduxjs/toolkit'; +import { Provider } from 'react-redux'; +import { createMemoryHistory } from 'history'; +import { Router } from 'react-router-dom'; import TaskDetails from '../TaskDetails'; import { minProps } from './TaskDetails.fixtures'; +import { VIEW_FOREMAN_TASKS } from '../TaskDetailsConstants'; + +const mockUseForemanPermissions = jest.fn( + () => new Set(['view_foreman_tasks']) +); + +jest.mock('foremanReact/Root/Context/ForemanContext', () => ({ + ...jest.requireActual('foremanReact/Root/Context/ForemanContext'), + useForemanPermissions: (...args) => mockUseForemanPermissions(...args), +})); delete window.location; window.location = new URL( 'https://foreman.com/foreman_tasks/tasks/a15dd820-32f1-4ced-9ab7-c0fab8234c47/' ); +const store = configureStore({ reducer: state => state || {} }); + +function renderTaskDetails(props = {}) { + const history = createMemoryHistory(); + + return render( + + + + + + ); +} + describe('TaskDetails', () => { - it('shows error message when status is ERROR', () => { - render( - + beforeEach(() => { + mockUseForemanPermissions.mockImplementation( + () => new Set(['view_foreman_tasks']) ); + }); + + it(`shows ResourceLoadFailedEmptyState when ${VIEW_FOREMAN_TASKS} is absent`, () => { + mockUseForemanPermissions.mockImplementation(() => new Set()); + + renderTaskDetails(); + + expect( + screen.getByRole('heading', { name: /permission denied/i }) + ).toBeInTheDocument(); + expect(screen.getByText(VIEW_FOREMAN_TASKS)).toBeInTheDocument(); + expect( + screen.getByRole('button', { name: /back to tasks/i }) + ).toBeInTheDocument(); + expect(screen.queryByRole('tab', { name: /^task$/i })).not.toBeInTheDocument(); + }); + + it('shows ResourceLoadFailedEmptyState when apiStatus is ERROR', () => { + renderTaskDetails({ + apiStatus: 'ERROR', + apiErrorMessage: 'some-error', + }); + + expect( + screen.getByRole('heading', { name: /unable to load task/i }) + ).toBeInTheDocument(); + expect( + screen.getByText('Server returned: some-error') + ).toBeInTheDocument(); + expect(screen.queryByRole('tab', { name: /^task$/i })).not.toBeInTheDocument(); + }); + + it('shows permission denied when apiErrorCode is 403', () => { + renderTaskDetails({ + apiStatus: 'ERROR', + apiErrorCode: 403, + apiErrorMessage: 'Forbidden', + }); + + expect( + screen.getByRole('heading', { name: /permission denied/i }) + ).toBeInTheDocument(); + expect(screen.getByText('view_foreman_tasks')).toBeInTheDocument(); + expect( + screen.getByText('Server returned: Forbidden') + ).toBeInTheDocument(); + expect(screen.queryByRole('tab', { name: /^task$/i })).not.toBeInTheDocument(); + }); + + it('shows not found messaging when apiErrorCode is 404', () => { + renderTaskDetails({ + apiStatus: 'ERROR', + apiErrorCode: 404, + apiErrorMessage: 'Task missing', + }); + + expect( + screen.getByRole('heading', { name: /unable to load task/i }) + ).toBeInTheDocument(); + expect( + screen.getByText(/could not be found/i) + ).toBeInTheDocument(); expect( - screen.getByText(/could not receive data: some-error/i) + screen.getByText('Server returned: Task missing') ).toBeInTheDocument(); + expect(screen.queryByRole('tab', { name: /^task$/i })).not.toBeInTheDocument(); }); it('shows skeleton while loading on the Task tab', () => { - const { container } = render(); + const { container } = renderTaskDetails({ isLoading: true }); expect( container.querySelector('.react-loading-skeleton') ).toBeInTheDocument(); }); it('renders six tabs with expected labels', () => { - render(); + renderTaskDetails(); expect(document.getElementById('task-details-tabs')).toBeInTheDocument(); expect(screen.getByRole('tab', { name: /^task$/i })).toBeInTheDocument(); expect( diff --git a/webpack/ForemanTasks/Components/TaskDetails/__tests__/TaskDetailsSelectors.test.js b/webpack/ForemanTasks/Components/TaskDetails/__tests__/TaskDetailsSelectors.test.js new file mode 100644 index 000000000..dd199f678 --- /dev/null +++ b/webpack/ForemanTasks/Components/TaskDetails/__tests__/TaskDetailsSelectors.test.js @@ -0,0 +1,81 @@ +import { STATUS } from 'foremanReact/constants'; + +import { FOREMAN_TASK_DETAILS } from '../TaskDetailsConstants'; +import { + selectAPIError, + selectAPIErrorCode, + selectAPIErrorMessage, +} from '../TaskDetailsSelectors'; + +const axiosErrorWithApiBody = { + message: 'Request failed with status code 404', + response: { + status: 404, + data: { + error: { + message: 'Task not found', + }, + }, + }, +}; + +const plainError = { + message: 'Network Error', +}; + +const forbiddenError = { + message: 'Request failed with status code 403', + response: { + status: 403, + data: { + error: { + message: 'Forbidden', + }, + }, + }, +}; + +const buildState = (response, status = STATUS.ERROR) => ({ + API: { + [FOREMAN_TASK_DETAILS]: { + payload: {}, + response, + status, + }, + }, +}); + +describe('TaskDetailsSelectors - API error selectors', () => { + it('selectAPIError returns null when API status is not ERROR', () => { + expect(selectAPIError(buildState({}, STATUS.RESOLVED))).toBeNull(); + }); + + it('selectAPIError returns the stored response when API status is ERROR', () => { + expect(selectAPIError(buildState(plainError))).toEqual(plainError); + }); + + it('selectAPIErrorMessage prefers the API error body message', () => { + expect( + selectAPIErrorMessage(buildState(axiosErrorWithApiBody)) + ).toBe('Task not found'); + }); + + it('selectAPIErrorMessage falls back to the top-level error message', () => { + expect(selectAPIErrorMessage(buildState(plainError))).toBe('Network Error'); + }); + + it('selectAPIErrorMessage returns undefined when there is no API error', () => { + expect( + selectAPIErrorMessage(buildState({}, STATUS.RESOLVED)) + ).toBeUndefined(); + }); + + it('selectAPIErrorCode returns the HTTP status from the error response', () => { + expect(selectAPIErrorCode(buildState(axiosErrorWithApiBody))).toBe(404); + expect(selectAPIErrorCode(buildState(forbiddenError))).toBe(403); + }); + + it('selectAPIErrorCode returns undefined when the error response has no status', () => { + expect(selectAPIErrorCode(buildState(plainError))).toBeUndefined(); + }); +}); diff --git a/webpack/ForemanTasks/Components/TaskDetails/index.js b/webpack/ForemanTasks/Components/TaskDetails/index.js index dda6d4064..58c0ed7c1 100644 --- a/webpack/ForemanTasks/Components/TaskDetails/index.js +++ b/webpack/ForemanTasks/Components/TaskDetails/index.js @@ -33,8 +33,9 @@ import { selectExternalId, selectDynflowEnableConsole, selectCanEdit, - selectStatus, - selectAPIError, + selectAPIStatus, + selectAPIErrorMessage, + selectAPIErrorCode, selectIsLoading, selectDependsOn, selectBlocks, @@ -70,8 +71,9 @@ const mapStateToProps = state => ({ externalId: selectExternalId(state), dynflowEnableConsole: selectDynflowEnableConsole(state), canEdit: selectCanEdit(state), - status: selectStatus(state), - APIerror: selectAPIError(state), + apiStatus: selectAPIStatus(state), + apiErrorMessage: selectAPIErrorMessage(state), + apiErrorCode: selectAPIErrorCode(state), isLoading: selectIsLoading(state), dependsOn: selectDependsOn(state), blocks: selectBlocks(state), diff --git a/webpack/ForemanTasks/Components/common/taskResultIcon.js b/webpack/ForemanTasks/Components/common/taskResultIcon.js new file mode 100644 index 000000000..2af4b8185 --- /dev/null +++ b/webpack/ForemanTasks/Components/common/taskResultIcon.js @@ -0,0 +1,53 @@ +import React from 'react'; +import { Icon } from '@patternfly/react-core'; +import { + CheckCircleIcon, + ExclamationCircleIcon, + ExclamationTriangleIcon, + QuestionCircleIcon, +} from '@patternfly/react-icons'; +import { translate as __ } from 'foremanReact/common/I18n'; + +/** + * Icon reflecting task state/result (aligned with TaskInfo / tasks table). + * + * @param {string} state Dynflow task state (e.g. stopped, running). + * @param {string} [result] Result when stopped (success, error, warning, …). + * @returns {React.ReactElement} + */ +export const taskResultIconEl = (state, result) => { + if (state && state !== 'stopped') { + return ( + + + + ); + } + + switch (result) { + case 'success': + return ( + + + + ); + case 'error': + return ( + + + + ); + case 'warning': + return ( + + + + ); + default: + return ( + + + + ); + } +}; diff --git a/webpack/ForemanTasks/Routes/ShowTaskDetails/TaskDetailsPage.js b/webpack/ForemanTasks/Routes/ShowTaskDetails/TaskDetailsPage.js new file mode 100644 index 000000000..31187a2e6 --- /dev/null +++ b/webpack/ForemanTasks/Routes/ShowTaskDetails/TaskDetailsPage.js @@ -0,0 +1,74 @@ +import React from 'react'; +import PropTypes from 'prop-types'; +import { useSelector } from 'react-redux'; +import { Flex, FlexItem, TextContent, Text } from '@patternfly/react-core'; +import PageLayout from 'foremanReact/routes/common/PageLayout/PageLayout'; +import { translate as __, sprintf } from 'foremanReact/common/I18n'; +import TaskDetails from '../../Components/TaskDetails'; +import { + selectAction, + selectResult, + selectState, +} from '../../Components/TaskDetails/TaskDetailsSelectors'; +import { taskResultIconEl } from '../../Components/common/taskResultIcon'; + +const TaskDetailsPage = props => { + const { id } = props.match.params; + const action = useSelector(selectAction); + const taskState = useSelector(selectState); + const taskResult = useSelector(selectResult); + const headerText = action + ? sprintf(__('Details of %s task'), action) + : __('Task Details'); + + const header = ( + + + + {headerText} + + + + {taskResultIconEl(taskState, taskResult)} + + + ); + + return ( + + + + ); +}; + +TaskDetailsPage.propTypes = { + match: PropTypes.shape({ + params: PropTypes.shape({ + id: PropTypes.string.isRequired, + }).isRequired, + }).isRequired, +}; + +export default TaskDetailsPage; diff --git a/webpack/ForemanTasks/Routes/ShowTaskDetails/__tests__/TaskDetailsPage.test.js b/webpack/ForemanTasks/Routes/ShowTaskDetails/__tests__/TaskDetailsPage.test.js new file mode 100644 index 000000000..0e1d040ec --- /dev/null +++ b/webpack/ForemanTasks/Routes/ShowTaskDetails/__tests__/TaskDetailsPage.test.js @@ -0,0 +1,265 @@ +import React from 'react'; +import { render, screen, within } from '@testing-library/react'; +import '@testing-library/jest-dom'; +import { combineReducers, configureStore } from '@reduxjs/toolkit'; +import { createMemoryHistory } from 'history'; +import { Router } from 'react-router-dom'; +import { IntlProvider } from 'react-intl'; +import { Provider } from 'react-redux'; + +import breadcrumbBarReducer from 'foremanReact/components/BreadcrumbBar/BreadcrumbBarReducer'; +import { STATUS } from 'foremanReact/constants'; +import intervalsReducer from 'foremanReact/redux/middlewares/IntervalMiddleware/IntervalReducer'; + +import { FOREMAN_TASK_DETAILS, VIEW_FOREMAN_TASKS } from '../../../Components/TaskDetails/TaskDetailsConstants'; +import TaskDetailsPage from '../TaskDetailsPage'; + +const mockUseForemanPermissions = jest.fn( + () => new Set(['view_foreman_tasks']) +); + +jest.mock('foremanReact/Root/Context/ForemanContext', () => ({ + ...jest.requireActual('foremanReact/Root/Context/ForemanContext'), + useForemanPermissions: (...args) => mockUseForemanPermissions(...args), +})); + +const TASK_DETAILS_TITLE_ROW_OUIA_ID = 'foreman-tasks-task-details-title-row'; + +const routerPropsBase = { + history: { push: jest.fn(), replace: jest.fn(), go: jest.fn() }, + location: { + pathname: '/foreman_tasks/tasks/task-route-id', + search: '', + hash: '', + state: undefined, + }, +}; + +const matchDefault = { + params: { id: 'task-route-id' }, + path: '/foreman_tasks/tasks/:id', + url: '/foreman_tasks/tasks/task-route-id', + isExact: true, +}; + +const baseTaskPayload = { + action: '', + input: [], + output: {}, + locks: [], + links: [], + depends_on: [], + blocks: [], + failed_steps: [], + running_steps: [], + execution_plan: {}, + state: 'running', + result: '', +}; + +const createStoreForTaskPayload = overrides => ({ + API: { + [FOREMAN_TASK_DETAILS]: { + response: { ...baseTaskPayload, ...overrides }, + status: STATUS.RESOLVED, + payload: {}, + }, + }, +}); + +const rootReducer = combineReducers({ + API: (state = {}, action) => state, + intervals: intervalsReducer, + breadcrumbBar: breadcrumbBarReducer, + foremanTasks: (state = {}, action) => state, +}); + +const renderPage = (apiPayloadOverrides = {}, propsOverrides = {}) => { + const history = createMemoryHistory({ + initialEntries: [`/foreman_tasks/tasks/${matchDefault.params.id}`], + }); + const store = configureStore({ + reducer: rootReducer, + preloadedState: createStoreForTaskPayload(apiPayloadOverrides), + middleware: getDefaultMiddleware => + getDefaultMiddleware({ + immutableCheck: false, + serializableCheck: false, + }), + }); + + window.history.pushState( + {}, + '', + `/foreman_tasks/tasks/${matchDefault.params.id}` + ); + + return render( + + + + + + + + ); +}; + +const breadcrumbTitleHeadings = () => + screen.getAllByRole('heading', { level: 1 }).filter( + heading => heading.getAttribute('data-ouia-component-id') === 'breadcrumb_title' + ); + +/** + * Title row (`customHeader` root `Flex`): same OUIA pattern as `Locks.test.js`. + */ +const taskDetailsTitleRegion = container => { + const el = container.querySelector( + `[data-ouia-component-id="${TASK_DETAILS_TITLE_ROW_OUIA_ID}"]` + ); + + expect(el).toBeTruthy(); + + return el; +}; + +describe('TaskDetailsPage', () => { + beforeEach(() => { + mockUseForemanPermissions.mockImplementation(() => new Set(['view_foreman_tasks'])); + }); + + describe('permissions', () => { + it(`renders the task details chrome when ${VIEW_FOREMAN_TASKS} is present`, () => { + mockUseForemanPermissions.mockImplementation( + () => new Set(['edit_foreman_tasks', VIEW_FOREMAN_TASKS]) + ); + + renderPage({ action: 'Run job' }); + + expect( + screen.getByRole('navigation', { name: 'Breadcrumb' }) + ).toBeInTheDocument(); + expect( + screen.getByRole('heading', { + level: 1, + name: /Details of Run job task/, + }) + ).toBeInTheDocument(); + expect(screen.queryByRole('heading', { name: /Permission denied/i })).not.toBeInTheDocument(); + expect(mockUseForemanPermissions).toHaveBeenCalled(); + }); + + it(`shows ResourceLoadFailedEmptyState and lists ${VIEW_FOREMAN_TASKS} when it is absent`, () => { + mockUseForemanPermissions.mockImplementation(() => new Set()); + + renderPage({ action: 'Hidden task' }); + + expect( + screen.getByRole('heading', { name: /Permission denied/i }) + ).toBeInTheDocument(); + expect( + screen.getByText( + /You do not have permission to view the task with id task-route-id/ + ) + ).toBeInTheDocument(); + expect(screen.getByText(VIEW_FOREMAN_TASKS)).toBeInTheDocument(); + expect( + screen.getByRole('button', { name: /Back to tasks/i }) + ).toBeInTheDocument(); + expect( + screen.getByRole('navigation', { name: 'Breadcrumb' }) + ).toBeInTheDocument(); + expect(screen.queryByRole('tab', { name: /^task$/i })).not.toBeInTheDocument(); + }); + + it('denies access when user only has edit_foreman_tasks without view', () => { + mockUseForemanPermissions.mockImplementation( + () => new Set(['edit_foreman_tasks']) + ); + + renderPage({}); + + expect( + screen.getByRole('heading', { name: /Permission denied/i }) + ).toBeInTheDocument(); + expect(screen.getByText(VIEW_FOREMAN_TASKS)).toBeInTheDocument(); + expect( + screen.getByRole('navigation', { name: 'Breadcrumb' }) + ).toBeInTheDocument(); + expect(screen.queryByRole('tab', { name: /^task$/i })).not.toBeInTheDocument(); + }); + }); + + + const expectToolbarHeadingText = substring => { + const headings = breadcrumbTitleHeadings(); + + expect(headings.length).toBeGreaterThan(0); + + headings.forEach(heading => { + expect(heading).toHaveTextContent(substring); + }); + }; + + it('shows generic title and breadcrumb from route id when action is unset', () => { + const page = renderPage({}); + expectToolbarHeadingText('Task Details'); + + expect(screen.getByRole('navigation', { name: 'Breadcrumb' })).toHaveTextContent( + 'Tasks' + ); + expect( + within(screen.getByRole('navigation', { name: 'Breadcrumb' })).getByText( + /task-route-id/ + ) + ).toBeInTheDocument(); + + const titleRegion = taskDetailsTitleRegion(page.container); + + expect( + within(titleRegion).getAllByRole('img', { hidden: true }).length + ).toBeGreaterThan(0); + expect(titleRegion.querySelector('[class*="danger"]')).toBeNull(); + }); + + it('uses task action for title and breadcrumb when loaded', () => { + renderPage({ action: 'Refresh hosts' }); + + expectToolbarHeadingText('Details of Refresh hosts task'); + + expect(screen.getByRole('link', { name: /^Tasks$/ })).toHaveAttribute( + 'href', + '/foreman_tasks/tasks' + ); + expect( + within(screen.getByRole('navigation', { name: 'Breadcrumb' })).getByText( + 'Refresh hosts' + ) + ).toBeInTheDocument(); + }); + + it('shows error status styling in the heading when task is stopped with error result', () => { + const page = renderPage({ + action: 'Some action', + state: 'stopped', + result: 'error', + }); + + expectToolbarHeadingText('Details of Some action task'); + + expect( + taskDetailsTitleRegion(page.container).querySelector('[class*="danger"]') + ).toBeTruthy(); + + expect( + within(screen.getByRole('navigation', { name: 'Breadcrumb' })).getByText( + 'Some action' + ) + ).toBeInTheDocument(); + }); +}); diff --git a/webpack/Routes/routes.js b/webpack/Routes/routes.js index 01c9f9301..c5e786412 100644 --- a/webpack/Routes/routes.js +++ b/webpack/Routes/routes.js @@ -1,5 +1,6 @@ import React from 'react'; import TasksTableIndexPage from '../ForemanTasks/Components/TasksTable/TasksIndexPage'; +import TaskDetailsPage from '../ForemanTasks/Routes/ShowTaskDetails/TaskDetailsPage'; const ForemanTasksRoutes = [ { @@ -7,6 +8,11 @@ const ForemanTasksRoutes = [ exact: true, render: props => , }, + { + path: '/foreman_tasks/tasks/:id', + exact: true, + render: props => , + }, { path: '/foreman_tasks/tasks/:id/sub_tasks', exact: true, diff --git a/webpack/Routes/routes.test.js b/webpack/Routes/routes.test.js index 44877a679..62553363d 100644 --- a/webpack/Routes/routes.test.js +++ b/webpack/Routes/routes.test.js @@ -12,6 +12,14 @@ jest.mock( } ); +jest.mock( + '../ForemanTasks/Routes/ShowTaskDetails/TaskDetailsPage', + () => + function TaskDetailsPageStub() { + return
; + } +); + const routerProps = { history: { push: jest.fn(), replace: jest.fn(), go: jest.fn() }, location: { @@ -34,6 +42,7 @@ describe('ForemanTasks routes', () => { ForemanTasksRoutes.map(({ path, exact }) => ({ path, exact })) ).toEqual([ { path: '/foreman_tasks/tasks', exact: true }, + { path: '/foreman_tasks/tasks/:id', exact: true }, { path: '/foreman_tasks/tasks/:id/sub_tasks', exact: true }, ]); }); @@ -53,9 +62,9 @@ describe('ForemanTasks routes', () => { ...routerProps, match: { ...routerProps.match, - params: { id: '7' }, - path: '/foreman_tasks/tasks/:id/sub_tasks', - url: '/foreman_tasks/tasks/7/sub_tasks', + params: { id: '99' }, + path: '/foreman_tasks/tasks/:id', + url: '/foreman_tasks/tasks/99', }, }, ]; @@ -63,8 +72,10 @@ describe('ForemanTasks routes', () => { ForemanTasksRoutes.forEach((route, index) => { const { unmount } = render(route.render(propsByIndex[index])); - if (index === 2) { - expect(screen.getByTestId('show-task-stub')).toBeInTheDocument(); + if (index === 1) { + expect( + screen.getByTestId('task-details-page-stub') + ).toBeInTheDocument(); } else { expect( screen.getByTestId('tasks-table-index-stub')