From f4aefaac9da0042835f78bc0341a020a6dbcf5a4 Mon Sep 17 00:00:00 2001 From: kmalyjur Date: Mon, 27 Jan 2025 09:57:16 +0000 Subject: [PATCH 1/6] Fixes #38090 - Add filtering to job invocation hosts table --- .../JobInvocationConstants.js | 16 ++- .../JobInvocationHostTable.js | 124 +++++++++++------- .../JobInvocationHostTableToolbar.js | 63 +++++++++ .../JobInvocationSystemStatusChart.js | 70 ++++++++-- .../__tests__/MainInformation.test.js | 2 +- webpack/JobInvocationDetail/index.js | 12 +- .../TargetingHosts/components/HostStatus.js | 2 +- 7 files changed, 229 insertions(+), 60 deletions(-) create mode 100644 webpack/JobInvocationDetail/JobInvocationHostTableToolbar.js diff --git a/webpack/JobInvocationDetail/JobInvocationConstants.js b/webpack/JobInvocationDetail/JobInvocationConstants.js index a99bad188..1c997b89d 100644 --- a/webpack/JobInvocationDetail/JobInvocationConstants.js +++ b/webpack/JobInvocationDetail/JobInvocationConstants.js @@ -42,6 +42,14 @@ export const STATUS_UPPERCASE = { PENDING: 'PENDING', }; +export const STATUS_TITLES = { + ALL_STATUSES: { id: 'all_statuses', title: __('All statuses') }, + SUCCESS: { id: 'success', title: __('Succeeded') }, + FAILED: { id: 'failed', title: __('Failed') }, + PENDING: { id: 'pending', title: __('In Progress') }, + CANCELLED: { id: 'cancelled', title: __('Cancelled') }, +}; + export const DATE_OPTIONS = { day: 'numeric', month: 'short', @@ -61,7 +69,7 @@ const Columns = () => { return { title: __('Failed'), status: 1 }; case 'planned': return { title: __('Scheduled'), status: 2 }; - case 'running': + case 'running' || 'pending': return { title: __('Pending'), status: 3 }; case 'cancelled': return { title: __('Cancelled'), status: 4 }; @@ -84,13 +92,15 @@ const Columns = () => { wrapper: ({ name }) => ( {name} ), + isSorted: true, weight: 1, }, - groups: { + hostgroup: { title: __('Host group'), wrapper: ({ hostgroup_id, hostgroup_name }) => ( {hostgroup_name} ), + isSorted: true, weight: 2, }, os: { @@ -100,6 +110,7 @@ const Columns = () => { {operatingsystem_name} ), + isSorted: true, weight: 3, }, smart_proxy: { @@ -107,6 +118,7 @@ const Columns = () => { wrapper: ({ smart_proxy_name, smart_proxy_id }) => ( {smart_proxy_name} ), + isSorted: true, weight: 4, }, status: { diff --git a/webpack/JobInvocationDetail/JobInvocationHostTable.js b/webpack/JobInvocationDetail/JobInvocationHostTable.js index 89dd9e548..41f1e3b8d 100644 --- a/webpack/JobInvocationDetail/JobInvocationHostTable.js +++ b/webpack/JobInvocationDetail/JobInvocationHostTable.js @@ -20,7 +20,6 @@ import { useBulkSelect, useUrlParams, } from 'foremanReact/components/PF4/TableIndexPage/Table/TableHooks'; -import Pagination from 'foremanReact/components/Pagination'; import { getControllerSearchProps } from 'foremanReact/constants'; import Columns, { JOB_INVOCATION_HOSTS, @@ -29,17 +28,38 @@ import Columns, { import { TemplateInvocation } from './TemplateInvocation'; import { OpenAlInvocations, PopupAlert } from './OpenAlInvocations'; import { RowActions } from './TemplateInvocationComponents/TemplateActionButtons'; +import JobInvocationHostTableToolbar from './JobInvocationHostTableToolbar'; -const JobInvocationHostTable = ({ id, targeting, finished, autoRefresh }) => { +const JobInvocationHostTable = ({ + id, + targeting, + finished, + autoRefresh, + initialFilter, +}) => { const columns = Columns(); const columnNamesKeys = Object.keys(columns); const apiOptions = { key: JOB_INVOCATION_HOSTS }; + const [selectedFilter, setSelectedFilter] = useState(initialFilter || ''); const { searchParam: urlSearchQuery = '', page: urlPage, per_page: urlPerPage, } = useUrlParams(); - const defaultParams = { search: urlSearchQuery }; + const constructFilter = ( + filter = selectedFilter, + search = urlSearchQuery + ) => { + const baseFilter = `job_invocation.id = ${id}`; + const dropdownFilterClause = + filter && filter !== 'all_statuses' + ? `and job_invocation.result = ${filter}` + : ''; + const searchQueryClause = search ? `and (${search})` : ''; + return `${baseFilter} ${dropdownFilterClause} ${searchQueryClause}`; + }; + + const defaultParams = { search: constructFilter() }; if (urlPage) defaultParams.page = Number(urlPage); if (urlPerPage) defaultParams.per_page = Number(urlPerPage); const [expandedHost, setExpandedHost] = useState([]); @@ -57,30 +77,18 @@ const JobInvocationHostTable = ({ id, targeting, finished, autoRefresh }) => { } ); - const combinedResponse = { - response: { - search: urlSearchQuery, - can_create: false, - results: response?.results || [], - total: response?.total || 0, - per_page: response?.perPage, - page: response?.page, - subtotal: response?.subtotal || 0, - message: response?.message || 'error', - }, - status, - setAPIOptions, - }; - - const { setParamsAndAPI, params } = useSetParamsAndApiAndSearch({ + const { params } = useSetParamsAndApiAndSearch({ defaultParams, apiOptions, - setAPIOptions: combinedResponse.setAPIOptions, + setAPIOptions, }); - const { updateSearchQuery } = useBulkSelect({ + const { updateSearchQuery: updateSearchQueryBulk } = useBulkSelect({ initialSearchQuery: urlSearchQuery, }); + const updateSearchQuery = searchQuery => { + updateSearchQueryBulk(searchQuery); + }; const controller = 'hosts'; const memoDefaultSearchProps = useMemo( @@ -91,6 +99,23 @@ const JobInvocationHostTable = ({ id, targeting, finished, autoRefresh }) => { `/${controller}/auto_complete_search` ); + const wrapSetSelectedFilter = filter => { + const filterSearch = constructFilter(filter); + setAPIOptions(prevOptions => { + if (prevOptions.params.search !== filterSearch) { + return { + ...prevOptions, + params: { + ...prevOptions.params, + search: filterSearch, + }, + }; + } + return prevOptions; + }); + setSelectedFilter(filter); + }; + useEffect(() => { const intervalId = setInterval(() => { if (!finished || autoRefresh) { @@ -108,24 +133,31 @@ const JobInvocationHostTable = ({ id, targeting, finished, autoRefresh }) => { }; }, [finished, autoRefresh, setAPIOptions]); - const onPagination = newPagination => { - setParamsAndAPI({ - ...params, - ...newPagination, - search: urlSearchQuery, - }); + const wrapSetAPIOptions = newAPIOptions => { + setAPIOptions(prevOptions => ({ + ...prevOptions, + params: { + ...prevOptions.params, + ...newAPIOptions.params, + search: constructFilter(undefined, newAPIOptions?.params?.search), + }, + })); }; - const bottomPagination = ( - - ); + const combinedResponse = { + response: { + search: urlSearchQuery, + can_create: false, + results: response?.results || [], + total: response?.total || 0, + per_page: response?.perPage, + page: response?.page, + subtotal: response?.subtotal || 0, + message: response?.message || 'error', + }, + status, + setAPIOptions: wrapSetAPIOptions, + }; const customEmptyState = ( @@ -183,26 +215,30 @@ const JobInvocationHostTable = ({ id, targeting, finished, autoRefresh }) => { creatable={false} replacementResponse={combinedResponse} updateSearchQuery={updateSearchQuery} - customToolbarItems={ + customToolbarItems={[ - } + />, + , + ]} > {}} errorMessage={ @@ -212,7 +248,6 @@ const JobInvocationHostTable = ({ id, targeting, finished, autoRefresh }) => { } isPending={status === STATUS_UPPERCASE.PENDING} isDeleteable={false} - bottomPagination={bottomPagination} childrenOutsideTbody > {results?.map((result, rowIndex) => ( @@ -267,6 +302,7 @@ JobInvocationHostTable.propTypes = { targeting: PropTypes.object.isRequired, finished: PropTypes.bool.isRequired, autoRefresh: PropTypes.bool.isRequired, + initialFilter: PropTypes.string.isRequired, }; JobInvocationHostTable.defaultProps = {}; diff --git a/webpack/JobInvocationDetail/JobInvocationHostTableToolbar.js b/webpack/JobInvocationDetail/JobInvocationHostTableToolbar.js new file mode 100644 index 000000000..8b3601c2c --- /dev/null +++ b/webpack/JobInvocationDetail/JobInvocationHostTableToolbar.js @@ -0,0 +1,63 @@ +import React from 'react'; +import PropTypes from 'prop-types'; +import { translate as __ } from 'foremanReact/common/I18n'; +import { Select, SelectOption, SelectList } from '@patternfly/react-core/next'; // remove "/next" after switching to PF5 +import { MenuToggle, ToolbarItem } from '@patternfly/react-core'; +import { STATUS_TITLES } from './JobInvocationConstants'; + +const JobInvocationHostTableToolbar = ({ + dropdownFilter, + setDropdownFilter, +}) => { + const [isOpen, setIsOpen] = React.useState(false); + const onSelect = (_event, itemId) => { + setDropdownFilter(itemId); + setIsOpen(false); + }; + + const toggle = toggleRef => ( + setIsOpen(!isOpen)} + isExpanded={isOpen} + style={{ + width: '200px', + }} + > + {Object.values(STATUS_TITLES).find(status => status.id === dropdownFilter) + ?.title || __('All statuses')} + + ); + + return ( + + + + ); +}; + +JobInvocationHostTableToolbar.propTypes = { + dropdownFilter: PropTypes.string.isRequired, + setDropdownFilter: PropTypes.func.isRequired, +}; + +export default JobInvocationHostTableToolbar; diff --git a/webpack/JobInvocationDetail/JobInvocationSystemStatusChart.js b/webpack/JobInvocationDetail/JobInvocationSystemStatusChart.js index 0497fc5a5..24c8349d3 100644 --- a/webpack/JobInvocationDetail/JobInvocationSystemStatusChart.js +++ b/webpack/JobInvocationDetail/JobInvocationSystemStatusChart.js @@ -17,18 +17,20 @@ import { Text, } from '@patternfly/react-core'; import { - global_palette_black_600 as canceledColor, + global_palette_black_600 as cancelledColor, global_palette_black_500 as emptyChartDonut, global_palette_red_100 as failedColor, global_palette_blue_300 as inProgressColor, global_palette_green_500 as successedColor, } from '@patternfly/react-tokens'; +import { STATUS_TITLES } from './JobInvocationConstants'; import './JobInvocationDetail.scss'; const JobInvocationSystemStatusChart = ({ data, isAlreadyStarted, formattedStartDate, + onFilterChange, }) => { const { succeeded, @@ -42,7 +44,7 @@ const JobInvocationSystemStatusChart = ({ { title: __('Succeeded:'), count: succeeded, color: successedColor.value }, { title: __('Failed:'), count: failed, color: failedColor.value }, { title: __('In Progress:'), count: pending, color: inProgressColor.value }, - { title: __('Canceled:'), count: cancelled, color: canceledColor.value }, + { title: __('Cancelled:'), count: cancelled, color: cancelledColor.value }, ]; const chartDonutTitle = () => { if (total > 0) return `${succeeded.toString()}/${total}`; @@ -51,6 +53,7 @@ const JobInvocationSystemStatusChart = ({ }; const chartSize = 105; const [legendWidth, setLegendWidth] = useState(270); + const [cursor, setCursor] = useState('default'); // Calculates chart legend width based on its content useEffect(() => { @@ -64,9 +67,19 @@ const JobInvocationSystemStatusChart = ({ } }, [isAlreadyStarted, data]); + const onChartClick = (_evt, { index }) => { + const statusKeys = Object.keys(STATUS_TITLES); + const selectedKey = statusKeys[index + 1]; // first status is ALL_STATUSES + const selectedFilter = selectedKey ? STATUS_TITLES[selectedKey]?.id : null; + + if (onFilterChange && selectedFilter) { + onFilterChange(selectedFilter); + } + }; + return ( <> - + { + setCursor('pointer'); + }, + onMouseOut: () => { + setCursor('default'); + }, + }, + }, + ]} colorScale={ total > 0 ? chartData.map(d => d.color) : [emptyChartDonut.value] } labelComponent={ - + } title={chartDonutTitle} titleComponent={ @@ -97,7 +120,7 @@ const JobInvocationSystemStatusChart = ({ subTitleComponent={ // inline style overrides PatternFly default styling } padding={{ @@ -110,7 +133,7 @@ const JobInvocationSystemStatusChart = ({ height={chartSize} /> - + {__('System status')} @@ -128,6 +151,32 @@ const JobInvocationSystemStatusChart = ({ colorScale={chartData.map(d => d.color)} width={legendWidth} height={chartSize} + events={[ + { + target: 'data', + eventHandlers: { + onClick: onChartClick, + onMouseOver: () => { + setCursor('pointer'); + }, + onMouseOut: () => { + setCursor('default'); + }, + }, + }, + { + target: 'labels', + eventHandlers: { + onClick: onChartClick, + onMouseOver: () => { + setCursor('pointer'); + }, + onMouseOut: () => { + setCursor('default'); + }, + }, + }, + ]} /> ) : ( @@ -148,6 +197,7 @@ JobInvocationSystemStatusChart.propTypes = { data: PropTypes.object.isRequired, isAlreadyStarted: PropTypes.bool.isRequired, formattedStartDate: PropTypes.string, + onFilterChange: PropTypes.func.isRequired, }; JobInvocationSystemStatusChart.defaultProps = { diff --git a/webpack/JobInvocationDetail/__tests__/MainInformation.test.js b/webpack/JobInvocationDetail/__tests__/MainInformation.test.js index bfcc4d158..f959eb540 100644 --- a/webpack/JobInvocationDetail/__tests__/MainInformation.test.js +++ b/webpack/JobInvocationDetail/__tests__/MainInformation.test.js @@ -105,7 +105,7 @@ describe('JobInvocationDetailPage', () => { expect(screen.getByText('Succeeded: 2')).toBeInTheDocument(); expect(screen.getByText('Failed: 4')).toBeInTheDocument(); expect(screen.getByText('In Progress: 0')).toBeInTheDocument(); - expect(screen.getByText('Canceled: 0')).toBeInTheDocument(); + expect(screen.getByText('Cancelled: 0')).toBeInTheDocument(); const informationToCheck = { 'Effective user:': jobInvocationData.effective_user, diff --git a/webpack/JobInvocationDetail/index.js b/webpack/JobInvocationDetail/index.js index 26bfd9da4..22e5bf381 100644 --- a/webpack/JobInvocationDetail/index.js +++ b/webpack/JobInvocationDetail/index.js @@ -1,5 +1,5 @@ import PropTypes from 'prop-types'; -import React, { useEffect } from 'react'; +import React, { useEffect, useState } from 'react'; import { useDispatch, useSelector } from 'react-redux'; import { Divider, @@ -51,6 +51,11 @@ const JobInvocationDetailPage = ({ currentPermissionsUrl, CURRENT_PERMISSIONS ); + const [selectedFilter, setSelectedFilter] = useState(''); + + const handleFilterChange = filter => { + setSelectedFilter(filter); + }; let isAlreadyStarted = false; let formattedStartDate; @@ -79,7 +84,8 @@ const JobInvocationDetailPage = ({ if (task?.id !== undefined) { dispatch(getTask(`${task?.id}`)); } - }, [dispatch, task]); + // eslint-disable-next-line react-hooks/exhaustive-deps + }, [dispatch, task?.id]); const breadcrumbOptions = { breadcrumbItems: [ @@ -113,6 +119,7 @@ const JobInvocationDetailPage = ({ data={items} isAlreadyStarted={isAlreadyStarted} formattedStartDate={formattedStartDate} + onFilterChange={handleFilterChange} /> )} diff --git a/webpack/react_app/components/TargetingHosts/components/HostStatus.js b/webpack/react_app/components/TargetingHosts/components/HostStatus.js index 62b1f2cf1..e6500f831 100644 --- a/webpack/react_app/components/TargetingHosts/components/HostStatus.js +++ b/webpack/react_app/components/TargetingHosts/components/HostStatus.js @@ -17,7 +17,7 @@ const HostStatus = ({ status }) => { {__('Awaiting start')} ); - case 'running': + case 'running' || 'pending': return (
{__('Pending')} From 8b285cbfad4fdf47b0f6f9408577adc93ae6c63b Mon Sep 17 00:00:00 2001 From: Adam Ruzicka Date: Mon, 27 Jan 2025 11:31:13 -0500 Subject: [PATCH 2/6] Show hosts for scheduled jobs Searching inside scheduled jobs doesn't really work, but at least that's consistent with the old implementation. --- app/controllers/api/v2/job_invocations_controller.rb | 5 +++++ .../JobInvocationDetail/JobInvocationHostTable.js | 12 ++++++------ 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/app/controllers/api/v2/job_invocations_controller.rb b/app/controllers/api/v2/job_invocations_controller.rb index 1fb898870..0c5dd04b4 100644 --- a/app/controllers/api/v2/job_invocations_controller.rb +++ b/app/controllers/api/v2/job_invocations_controller.rb @@ -270,6 +270,11 @@ def parent_scope def set_hosts_and_template_invocations @pattern_template_invocations = @job_invocation.pattern_template_invocations.includes(:input_values) @hosts = @job_invocation.targeting.hosts.authorized(:view_hosts, Host) + + unless params[:search].nil? + @hosts = @hosts.joins(:template_invocations) + .where(:template_invocations => { :job_invocation_id => @job_invocation.id}) + end @template_invocations = @job_invocation.template_invocations .where(host: @hosts) .includes(:input_values) diff --git a/webpack/JobInvocationDetail/JobInvocationHostTable.js b/webpack/JobInvocationDetail/JobInvocationHostTable.js index 41f1e3b8d..db96a4355 100644 --- a/webpack/JobInvocationDetail/JobInvocationHostTable.js +++ b/webpack/JobInvocationDetail/JobInvocationHostTable.js @@ -50,16 +50,16 @@ const JobInvocationHostTable = ({ filter = selectedFilter, search = urlSearchQuery ) => { - const baseFilter = `job_invocation.id = ${id}`; const dropdownFilterClause = filter && filter !== 'all_statuses' - ? `and job_invocation.result = ${filter}` - : ''; - const searchQueryClause = search ? `and (${search})` : ''; - return `${baseFilter} ${dropdownFilterClause} ${searchQueryClause}`; + ? `job_invocation.result = ${filter}` + : null; + const parts = [dropdownFilterClause, search]; + return parts.filter((x) => x).map((fragment) => `(${fragment}})`).join(' AND '); }; - const defaultParams = { search: constructFilter() }; + const filter = constructFilter(); + const defaultParams = filter != '' ? { search: filter } : {}; if (urlPage) defaultParams.page = Number(urlPage); if (urlPerPage) defaultParams.per_page = Number(urlPerPage); const [expandedHost, setExpandedHost] = useState([]); From 0dd341438bac8b2b93491e1c5f601c4b0df4018f Mon Sep 17 00:00:00 2001 From: Adam Ruzicka Date: Tue, 28 Jan 2025 04:09:01 -0500 Subject: [PATCH 3/6] Make lints happy --- webpack/JobInvocationDetail/JobInvocationHostTable.js | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/webpack/JobInvocationDetail/JobInvocationHostTable.js b/webpack/JobInvocationDetail/JobInvocationHostTable.js index db96a4355..cd7c2d73c 100644 --- a/webpack/JobInvocationDetail/JobInvocationHostTable.js +++ b/webpack/JobInvocationDetail/JobInvocationHostTable.js @@ -55,11 +55,14 @@ const JobInvocationHostTable = ({ ? `job_invocation.result = ${filter}` : null; const parts = [dropdownFilterClause, search]; - return parts.filter((x) => x).map((fragment) => `(${fragment}})`).join(' AND '); + return parts + .filter(x => x) + .map(fragment => `(${fragment}})`) + .join(' AND '); }; - const filter = constructFilter(); - const defaultParams = filter != '' ? { search: filter } : {}; + const search = constructFilter(); + const defaultParams = search !== '' ? { search } : {}; if (urlPage) defaultParams.page = Number(urlPage); if (urlPerPage) defaultParams.per_page = Number(urlPerPage); const [expandedHost, setExpandedHost] = useState([]); From ba7145bc30cb33e2b9e15bcbaf6df8fffda09b4c Mon Sep 17 00:00:00 2001 From: Adam Ruzicka Date: Tue, 28 Jan 2025 04:58:03 -0500 Subject: [PATCH 4/6] Do not send key to backend --- webpack/JobInvocationDetail/JobInvocationHostTable.js | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/webpack/JobInvocationDetail/JobInvocationHostTable.js b/webpack/JobInvocationDetail/JobInvocationHostTable.js index cd7c2d73c..efc9427d4 100644 --- a/webpack/JobInvocationDetail/JobInvocationHostTable.js +++ b/webpack/JobInvocationDetail/JobInvocationHostTable.js @@ -70,13 +70,7 @@ const JobInvocationHostTable = ({ 'get', `/api/job_invocations/${id}/hosts`, { - params: { - ...defaultParams, - }, - key: JOB_INVOCATION_HOSTS, - handleSuccess: ({ data }) => { - if (data?.results?.length === 1) setExpandedHost([data.results[0].id]); - }, + params: defaultParams, } ); From 9b83abd58af4fb1a2e110965491c21da713517f1 Mon Sep 17 00:00:00 2001 From: Adam Ruzicka Date: Tue, 28 Jan 2025 08:15:50 -0500 Subject: [PATCH 5/6] Fix filtering --- webpack/JobInvocationDetail/JobInvocationHostTable.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/webpack/JobInvocationDetail/JobInvocationHostTable.js b/webpack/JobInvocationDetail/JobInvocationHostTable.js index efc9427d4..dd3cbe8b0 100644 --- a/webpack/JobInvocationDetail/JobInvocationHostTable.js +++ b/webpack/JobInvocationDetail/JobInvocationHostTable.js @@ -57,7 +57,7 @@ const JobInvocationHostTable = ({ const parts = [dropdownFilterClause, search]; return parts .filter(x => x) - .map(fragment => `(${fragment}})`) + .map(fragment => `(${fragment})`) .join(' AND '); }; From 5a57c98f02ca1fd7185da7fbfbcb926d1bd24cea Mon Sep 17 00:00:00 2001 From: kmalyjur Date: Tue, 28 Jan 2025 16:08:33 +0000 Subject: [PATCH 6/6] fix errors --- webpack/JobInvocationDetail/JobInvocationSystemStatusChart.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/webpack/JobInvocationDetail/JobInvocationSystemStatusChart.js b/webpack/JobInvocationDetail/JobInvocationSystemStatusChart.js index 24c8349d3..e65e42078 100644 --- a/webpack/JobInvocationDetail/JobInvocationSystemStatusChart.js +++ b/webpack/JobInvocationDetail/JobInvocationSystemStatusChart.js @@ -197,11 +197,12 @@ JobInvocationSystemStatusChart.propTypes = { data: PropTypes.object.isRequired, isAlreadyStarted: PropTypes.bool.isRequired, formattedStartDate: PropTypes.string, - onFilterChange: PropTypes.func.isRequired, + onFilterChange: PropTypes.func, }; JobInvocationSystemStatusChart.defaultProps = { formattedStartDate: undefined, + onFilterChange: undefined, }; export default JobInvocationSystemStatusChart;