From da167852012280ce9d8a6e61de52089a0a50b39d Mon Sep 17 00:00:00 2001 From: Keith Grant Date: Tue, 22 Dec 2020 13:27:30 -0800 Subject: [PATCH 1/7] convert JobList to PaginatedTable --- .../src/components/JobList/JobList.jsx | 59 +++----- .../src/components/JobList/JobListItem.jsx | 132 +++++++----------- .../components/JobList/JobListItem.test.jsx | 48 ++++--- .../components/PaginatedTable/HeaderRow.jsx | 15 +- .../PaginatedTable/HeaderRow.test.jsx | 16 +++ 5 files changed, 131 insertions(+), 139 deletions(-) diff --git a/awx/ui_next/src/components/JobList/JobList.jsx b/awx/ui_next/src/components/JobList/JobList.jsx index ead8916fe8..500b3272e4 100644 --- a/awx/ui_next/src/components/JobList/JobList.jsx +++ b/awx/ui_next/src/components/JobList/JobList.jsx @@ -7,7 +7,8 @@ import { Card } from '@patternfly/react-core'; import AlertModal from '../AlertModal'; import DatalistToolbar from '../DataListToolbar'; import ErrorDetail from '../ErrorDetail'; -import PaginatedDataList, { ToolbarDeleteButton } from '../PaginatedDataList'; +import { ToolbarDeleteButton } from '../PaginatedDataList'; +import PaginatedTable, { HeaderRow, HeaderCell } from '../PaginatedTable'; import useRequest, { useDeleteItems, useDismissableError, @@ -27,7 +28,7 @@ import { } from '../../api'; function JobList({ i18n, defaultParams, showTypeColumn = false }) { - const QS_CONFIG = getQSConfig( + const qsConfig = getQSConfig( 'job', { page: 1, @@ -49,7 +50,7 @@ function JobList({ i18n, defaultParams, showTypeColumn = false }) { } = useRequest( useCallback( async () => { - const params = parseQueryString(QS_CONFIG, location.search); + const params = parseQueryString(qsConfig, location.search); const [response, actionsResponse] = await Promise.all([ UnifiedJobsAPI.read({ ...params }), UnifiedJobsAPI.readOptions(), @@ -81,7 +82,7 @@ function JobList({ i18n, defaultParams, showTypeColumn = false }) { // TODO: update QS_CONFIG to be safe for deps array const fetchJobsById = useCallback( async ids => { - const params = parseQueryString(QS_CONFIG, location.search); + const params = parseQueryString(qsConfig, location.search); params.id__in = ids.join(','); const { data } = await UnifiedJobsAPI.read(params); return data.results; @@ -89,7 +90,7 @@ function JobList({ i18n, defaultParams, showTypeColumn = false }) { [location.search] // eslint-disable-line react-hooks/exhaustive-deps ); - const jobs = useWsJobs(results, fetchJobsById, QS_CONFIG); + const jobs = useWsJobs(results, fetchJobsById, qsConfig); const isAllSelected = selected.length === jobs.length && selected.length > 0; @@ -145,7 +146,7 @@ function JobList({ i18n, defaultParams, showTypeColumn = false }) { ); }, [selected]), { - qsConfig: QS_CONFIG, + qsConfig, allItemsSelected: isAllSelected, fetchItems: fetchJobs, } @@ -176,14 +177,13 @@ function JobList({ i18n, defaultParams, showTypeColumn = false }) { return ( <> - + {i18n._(t`Name`)} + {i18n._(t`Status`)} + {showTypeColumn && {i18n._(t`Type`)}} + {i18n._(t`Start Time`)} + + {i18n._(t`Finish Time`)} + + + } toolbarSearchableKeys={searchableKeys} toolbarRelatedSearchableKeys={relatedSearchableKeys} renderToolbar={props => ( @@ -267,7 +252,7 @@ function JobList({ i18n, defaultParams, showTypeColumn = false }) { showSelectAll isAllSelected={isAllSelected} onSelectAll={handleSelectAll} - qsConfig={QS_CONFIG} + qsConfig={qsConfig} additionalControls={[ )} - renderItem={job => ( + renderRow={job => ( - - - - {job.status && } - , - - - - - {job.id} — {job.name} - - - - , - ...(showTypeColumn - ? [ - - {jobTypes[job.type]} - , - ] - : []), - - {job.finished ? formatDateString(job.finished) : ''} - , - ]} - /> - + + + + + + {job.id} — {job.name} + + + + + + {job.status && } + + {showTypeColumn && ( + {jobTypes[job.type]} + )} + {formatDateString(job.started)} + + {job.finished ? formatDateString(job.finished) : ''} + + + - {job.type !== 'system_job' && - job.summary_fields?.user_capabilities?.start ? ( - - - {({ handleRelaunch }) => ( - - )} - - - ) : ( - '' - )} - - - + + {({ handleRelaunch }) => ( + + )} + + + + ); } diff --git a/awx/ui_next/src/components/JobList/JobListItem.test.jsx b/awx/ui_next/src/components/JobList/JobListItem.test.jsx index fc453c3be4..6b9a9c3ae4 100644 --- a/awx/ui_next/src/components/JobList/JobListItem.test.jsx +++ b/awx/ui_next/src/components/JobList/JobListItem.test.jsx @@ -32,7 +32,11 @@ describe('', () => { initialEntries: ['/jobs'], }); wrapper = mountWithContexts( - {}} />, + + + {}} /> + +
, { context: { router: { history } } } ); }); @@ -51,32 +55,40 @@ describe('', () => { test('launch button hidden from users without launch capabilities', () => { wrapper = mountWithContexts( - {}} - isSelected={false} - /> + + + {}} + isSelected={false} + /> + +
); expect(wrapper.find('LaunchButton').length).toBe(0); }); test('should hide type column when showTypeColumn is false', () => { - expect(wrapper.find('DataListCell[aria-label="type"]').length).toBe(0); + expect(wrapper.find('Td[dataLabel="Type"]').length).toBe(0); }); test('should show type column when showTypeColumn is true', () => { wrapper = mountWithContexts( - {}} - /> + + + {}} + /> + +
); - expect(wrapper.find('DataListCell[aria-label="type"]').length).toBe(1); + expect(wrapper.find('Td[dataLabel="Type"]').length).toBe(1); }); }); diff --git a/awx/ui_next/src/components/PaginatedTable/HeaderRow.jsx b/awx/ui_next/src/components/PaginatedTable/HeaderRow.jsx index 14c5c0b8ee..d1914ffae8 100644 --- a/awx/ui_next/src/components/PaginatedTable/HeaderRow.jsx +++ b/awx/ui_next/src/components/PaginatedTable/HeaderRow.jsx @@ -47,12 +47,15 @@ export default function HeaderRow({ qsConfig, children }) { - {React.Children.map(children, child => - React.cloneElement(child, { - onSort, - sortBy, - columnIndex: child.props.sortKey, - }) + {React.Children.map( + children, + child => + child && + React.cloneElement(child, { + onSort, + sortBy, + columnIndex: child.props.sortKey, + }) )} diff --git a/awx/ui_next/src/components/PaginatedTable/HeaderRow.test.jsx b/awx/ui_next/src/components/PaginatedTable/HeaderRow.test.jsx index ec1124c4b7..fa53baac60 100644 --- a/awx/ui_next/src/components/PaginatedTable/HeaderRow.test.jsx +++ b/awx/ui_next/src/components/PaginatedTable/HeaderRow.test.jsx @@ -62,4 +62,20 @@ describe('', () => { const cell = wrapper.find('Th').at(2); expect(cell.prop('sort')).toEqual(null); }); + + test('should handle null children gracefully', async () => { + const nope = false; + const wrapper = mountWithContexts( + + + One + {nope && Hidden} + Two + +
+ ); + + const cells = wrapper.find('Th'); + expect(cells).toHaveLength(3); + }); }); From 8bde6060c4c5ecd68a8fedf32eef5213055e7d84 Mon Sep 17 00:00:00 2001 From: Keith Grant Date: Wed, 23 Dec 2020 14:36:54 -0800 Subject: [PATCH 2/7] add expandable rows to JobList --- .../src/components/JobList/JobList.jsx | 2 +- .../src/components/JobList/JobListItem.jsx | 224 +++++++++++++----- .../components/PaginatedTable/HeaderRow.jsx | 3 +- .../src/screens/Job/JobDetail/JobDetail.jsx | 1 + 4 files changed, 175 insertions(+), 55 deletions(-) diff --git a/awx/ui_next/src/components/JobList/JobList.jsx b/awx/ui_next/src/components/JobList/JobList.jsx index 500b3272e4..0c2ab61a7c 100644 --- a/awx/ui_next/src/components/JobList/JobList.jsx +++ b/awx/ui_next/src/components/JobList/JobList.jsx @@ -234,7 +234,7 @@ function JobList({ i18n, defaultParams, showTypeColumn = false }) { }, ]} headerRow={ - + {i18n._(t`Name`)} {i18n._(t`Status`)} {showTypeColumn && {i18n._(t`Type`)}} diff --git a/awx/ui_next/src/components/JobList/JobListItem.jsx b/awx/ui_next/src/components/JobList/JobListItem.jsx index 3bc2345c75..cfc167f468 100644 --- a/awx/ui_next/src/components/JobList/JobListItem.jsx +++ b/awx/ui_next/src/components/JobList/JobListItem.jsx @@ -1,16 +1,48 @@ -import React from 'react'; +import React, { useState } from 'react'; import { Link } from 'react-router-dom'; import { withI18n } from '@lingui/react'; import { t } from '@lingui/macro'; -import { Button } from '@patternfly/react-core'; -import { Tr, Td } from '@patternfly/react-table'; +import { Button, Chip } from '@patternfly/react-core'; +import { Tr, Td, ExpandableRowContent } from '@patternfly/react-table'; import { RocketIcon } from '@patternfly/react-icons'; import { ActionsTd, ActionItem } from '../PaginatedTable'; import LaunchButton from '../LaunchButton'; import StatusLabel from '../StatusLabel'; +import { DetailList, Detail } from '../DetailList'; +import ChipGroup from '../ChipGroup'; +import CredentialChip from '../CredentialChip'; import { formatDateString } from '../../util/dates'; import { JOB_TYPE_URL_SEGMENTS } from '../../constants'; +const getLaunchedByDetails = ({ summary_fields = {}, related = {} }) => { + const { + created_by: createdBy, + job_template: jobTemplate, + schedule, + } = summary_fields; + const { schedule: relatedSchedule } = related; + + if (!createdBy && !schedule) { + return {}; + } + + let link; + let value; + + if (createdBy) { + link = `/users/${createdBy.id}`; + value = createdBy.username; + } else if (relatedSchedule && jobTemplate) { + link = `/templates/job_template/${jobTemplate.id}/schedules/${schedule.id}`; + value = schedule.name; + } else { + link = null; + value = schedule.name; + } + + return { link, value }; +}; + function JobListItem({ i18n, job, @@ -20,6 +52,7 @@ function JobListItem({ showTypeColumn = false, }) { const labelId = `check-action-${job.id}`; + const [isExpanded, setIsExpanded] = useState(false); const jobTypes = { project_update: i18n._(t`Source Control Update`), @@ -30,57 +63,142 @@ function JobListItem({ workflow_job: i18n._(t`Workflow Job`), }; + const { value: launchedByValue, link: launchedByLink } = + getLaunchedByDetails(job) || {}; + const { credentials, inventory, labels } = job.summary_fields; + return ( - - - - - - - {job.id} — {job.name} - - - - - - {job.status && } - - {showTypeColumn && ( - {jobTypes[job.type]} - )} - {formatDateString(job.started)} - - {job.finished ? formatDateString(job.finished) : ''} - - - - - {({ handleRelaunch }) => ( - - )} - - - - + <> + + setIsExpanded(!isExpanded), + }} + /> + + + + + + {job.id} — {job.name} + + + + + + {job.status && } + + {showTypeColumn && ( + {jobTypes[job.type]} + )} + + {formatDateString(job.started)} + + + {job.finished ? formatDateString(job.finished) : ''} + + + + + {({ handleRelaunch }) => ( + + )} + + + + + + + + + + + + {launchedByValue} + ) : ( + launchedByValue + ) + } + /> + {credentials && credentials.length > 0 && ( + + {credentials.map(c => ( + + ))} + + } + /> + )} + {labels && labels.count > 0 && ( + + {labels.results.map(l => ( + + {l.name} + + ))} + + } + /> + )} + {inventory && ( + + {inventory.name} + + } + /> + )} + + + + + ); } diff --git a/awx/ui_next/src/components/PaginatedTable/HeaderRow.jsx b/awx/ui_next/src/components/PaginatedTable/HeaderRow.jsx index d1914ffae8..b284104053 100644 --- a/awx/ui_next/src/components/PaginatedTable/HeaderRow.jsx +++ b/awx/ui_next/src/components/PaginatedTable/HeaderRow.jsx @@ -12,7 +12,7 @@ const Th = styled(PFTh)` --pf-c-table--cell--Overflow: initial; `; -export default function HeaderRow({ qsConfig, children }) { +export default function HeaderRow({ qsConfig, isExpandable, children }) { const location = useLocation(); const history = useHistory(); @@ -46,6 +46,7 @@ export default function HeaderRow({ qsConfig, children }) { return ( + {isExpandable && } {React.Children.map( children, diff --git a/awx/ui_next/src/screens/Job/JobDetail/JobDetail.jsx b/awx/ui_next/src/screens/Job/JobDetail/JobDetail.jsx index 9255b27af1..50841bf95f 100644 --- a/awx/ui_next/src/screens/Job/JobDetail/JobDetail.jsx +++ b/awx/ui_next/src/screens/Job/JobDetail/JobDetail.jsx @@ -360,3 +360,4 @@ JobDetail.propTypes = { }; export default withI18n()(JobDetail); +export { getLaunchedByDetails }; From 2c17d56acdd2ca5c274b65055cbc2d473c87fc9a Mon Sep 17 00:00:00 2001 From: Keith Grant Date: Wed, 23 Dec 2020 14:58:00 -0800 Subject: [PATCH 3/7] create LaunchedByDetail to consolidate shared code between detail & list --- .../DetailList/LaunchedByDetail.jsx | 51 +++++++++++++++++++ .../src/components/DetailList/index.js | 1 + .../src/components/JobList/JobListItem.jsx | 44 +--------------- .../src/screens/Job/JobDetail/JobDetail.jsx | 45 +--------------- 4 files changed, 56 insertions(+), 85 deletions(-) create mode 100644 awx/ui_next/src/components/DetailList/LaunchedByDetail.jsx diff --git a/awx/ui_next/src/components/DetailList/LaunchedByDetail.jsx b/awx/ui_next/src/components/DetailList/LaunchedByDetail.jsx new file mode 100644 index 0000000000..6c451a3e51 --- /dev/null +++ b/awx/ui_next/src/components/DetailList/LaunchedByDetail.jsx @@ -0,0 +1,51 @@ +import React from 'react'; +import { Link } from 'react-router-dom'; +import { t } from '@lingui/macro'; +import Detail from './Detail'; + +const getLaunchedByDetails = ({ summary_fields = {}, related = {} }) => { + const { + created_by: createdBy, + job_template: jobTemplate, + schedule, + } = summary_fields; + const { schedule: relatedSchedule } = related; + + if (!createdBy && !schedule) { + return {}; + } + + let link; + let value; + + if (createdBy) { + link = `/users/${createdBy.id}`; + value = createdBy.username; + } else if (relatedSchedule && jobTemplate) { + link = `/templates/job_template/${jobTemplate.id}/schedules/${schedule.id}`; + value = schedule.name; + } else { + link = null; + value = schedule.name; + } + + return { link, value }; +}; + +export default function LaunchedByDetail({ job, i18n }) { + const { value: launchedByValue, link: launchedByLink } = + getLaunchedByDetails(job) || {}; + + return ( + {launchedByValue} + ) : ( + launchedByValue + ) + } + /> + ); +} diff --git a/awx/ui_next/src/components/DetailList/index.js b/awx/ui_next/src/components/DetailList/index.js index d5e2ccd8a4..a393fe72a0 100644 --- a/awx/ui_next/src/components/DetailList/index.js +++ b/awx/ui_next/src/components/DetailList/index.js @@ -4,6 +4,7 @@ export { default as DeletedDetail } from './DeletedDetail'; export { default as UserDateDetail } from './UserDateDetail'; export { default as DetailBadge } from './DetailBadge'; export { default as ArrayDetail } from './ArrayDetail'; +export { default as LaunchedByDetail } from './LaunchedByDetail'; /* NOTE: CodeDetail cannot be imported here, as it causes circular dependencies in testing environment. Import it directly from diff --git a/awx/ui_next/src/components/JobList/JobListItem.jsx b/awx/ui_next/src/components/JobList/JobListItem.jsx index cfc167f468..94e0185fdd 100644 --- a/awx/ui_next/src/components/JobList/JobListItem.jsx +++ b/awx/ui_next/src/components/JobList/JobListItem.jsx @@ -8,41 +8,12 @@ import { RocketIcon } from '@patternfly/react-icons'; import { ActionsTd, ActionItem } from '../PaginatedTable'; import LaunchButton from '../LaunchButton'; import StatusLabel from '../StatusLabel'; -import { DetailList, Detail } from '../DetailList'; +import { DetailList, Detail, LaunchedByDetail } from '../DetailList'; import ChipGroup from '../ChipGroup'; import CredentialChip from '../CredentialChip'; import { formatDateString } from '../../util/dates'; import { JOB_TYPE_URL_SEGMENTS } from '../../constants'; -const getLaunchedByDetails = ({ summary_fields = {}, related = {} }) => { - const { - created_by: createdBy, - job_template: jobTemplate, - schedule, - } = summary_fields; - const { schedule: relatedSchedule } = related; - - if (!createdBy && !schedule) { - return {}; - } - - let link; - let value; - - if (createdBy) { - link = `/users/${createdBy.id}`; - value = createdBy.username; - } else if (relatedSchedule && jobTemplate) { - link = `/templates/job_template/${jobTemplate.id}/schedules/${schedule.id}`; - value = schedule.name; - } else { - link = null; - value = schedule.name; - } - - return { link, value }; -}; - function JobListItem({ i18n, job, @@ -63,8 +34,6 @@ function JobListItem({ workflow_job: i18n._(t`Workflow Job`), }; - const { value: launchedByValue, link: launchedByLink } = - getLaunchedByDetails(job) || {}; const { credentials, inventory, labels } = job.summary_fields; return ( @@ -141,16 +110,7 @@ function JobListItem({ label={i18n._(t`Finished`)} value={formatDateString(job.started)} /> - {launchedByValue} - ) : ( - launchedByValue - ) - } - /> + {credentials && credentials.length > 0 && ( { - const { - created_by: createdBy, - job_template: jobTemplate, - schedule, - } = summary_fields; - const { schedule: relatedSchedule } = related; - - if (!createdBy && !schedule) { - return null; - } - - let link; - let value; - - if (createdBy) { - link = `/users/${createdBy.id}`; - value = createdBy.username; - } else if (relatedSchedule && jobTemplate) { - link = `/templates/job_template/${jobTemplate.id}/schedules/${schedule.id}`; - value = schedule.name; - } else { - link = null; - value = schedule.name; - } - - return { link, value }; -}; - function JobDetail({ job, i18n }) { const { created_by, @@ -106,9 +78,6 @@ function JobDetail({ job, i18n }) { workflow_job: i18n._(t`Workflow Job`), }; - const { value: launchedByValue, link: launchedByLink } = - getLaunchedByDetails(job) || {}; - const deleteJob = async () => { try { switch (job.type) { @@ -196,16 +165,7 @@ function JobDetail({ job, i18n }) { /> )} - {launchedByValue} - ) : ( - launchedByValue - ) - } - /> + {inventory && ( Date: Tue, 26 Jan 2021 14:18:10 -0800 Subject: [PATCH 4/7] add id to expanded job rows --- awx/ui_next/src/components/JobList/JobListItem.jsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/awx/ui_next/src/components/JobList/JobListItem.jsx b/awx/ui_next/src/components/JobList/JobListItem.jsx index 94e0185fdd..2e677db9bd 100644 --- a/awx/ui_next/src/components/JobList/JobListItem.jsx +++ b/awx/ui_next/src/components/JobList/JobListItem.jsx @@ -97,7 +97,7 @@ function JobListItem({ - + From 2e7b53cf9054b5822dfa69ba5f8bf8f256573f90 Mon Sep 17 00:00:00 2001 From: Keith Grant Date: Tue, 26 Jan 2021 14:22:50 -0800 Subject: [PATCH 5/7] remove job data duplicated in expanded view --- awx/ui_next/src/components/JobList/JobListItem.jsx | 8 -------- 1 file changed, 8 deletions(-) diff --git a/awx/ui_next/src/components/JobList/JobListItem.jsx b/awx/ui_next/src/components/JobList/JobListItem.jsx index 2e677db9bd..a84b1b0ec5 100644 --- a/awx/ui_next/src/components/JobList/JobListItem.jsx +++ b/awx/ui_next/src/components/JobList/JobListItem.jsx @@ -102,14 +102,6 @@ function JobListItem({ - - {credentials && credentials.length > 0 && ( Date: Wed, 27 Jan 2021 08:36:30 -0800 Subject: [PATCH 6/7] Add id to table sort headers --- .../src/components/PaginatedTable/HeaderRow.jsx | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/awx/ui_next/src/components/PaginatedTable/HeaderRow.jsx b/awx/ui_next/src/components/PaginatedTable/HeaderRow.jsx index b284104053..a70ca52232 100644 --- a/awx/ui_next/src/components/PaginatedTable/HeaderRow.jsx +++ b/awx/ui_next/src/components/PaginatedTable/HeaderRow.jsx @@ -41,6 +41,7 @@ export default function HeaderRow({ qsConfig, isExpandable, children }) { index: sortKey || qsConfig.defaultParams?.order_by, direction: params.order_by?.startsWith('-') ? 'desc' : 'asc', }; + const idPrefix = `${qsConfig.namespace}-table-sort`; // empty first Th aligns with checkboxes in table rows return ( @@ -56,6 +57,7 @@ export default function HeaderRow({ qsConfig, isExpandable, children }) { onSort, sortBy, columnIndex: child.props.sortKey, + idPrefix, }) )} @@ -63,7 +65,14 @@ export default function HeaderRow({ qsConfig, isExpandable, children }) { ); } -export function HeaderCell({ sortKey, onSort, sortBy, columnIndex, children }) { +export function HeaderCell({ + sortKey, + onSort, + sortBy, + columnIndex, + idPrefix, + children, +}) { const sort = sortKey ? { onSort: (event, key, order) => onSort(sortKey, order), @@ -71,5 +80,9 @@ export function HeaderCell({ sortKey, onSort, sortBy, columnIndex, children }) { columnIndex, } : null; - return {children}; + return ( + + {children} + + ); } From 0feeaf045336cdc9c2bcfcf6cc41f051427acc13 Mon Sep 17 00:00:00 2001 From: Keith Grant Date: Wed, 27 Jan 2021 09:26:19 -0800 Subject: [PATCH 7/7] fix select boxes in JobList --- awx/ui_next/src/components/JobList/JobList.jsx | 3 ++- awx/ui_next/src/components/JobList/JobListItem.jsx | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/awx/ui_next/src/components/JobList/JobList.jsx b/awx/ui_next/src/components/JobList/JobList.jsx index 0c2ab61a7c..f21e8bb15b 100644 --- a/awx/ui_next/src/components/JobList/JobList.jsx +++ b/awx/ui_next/src/components/JobList/JobList.jsx @@ -268,13 +268,14 @@ function JobList({ i18n, defaultParams, showTypeColumn = false }) { ]} /> )} - renderRow={job => ( + renderRow={(job, index) => ( handleSelect(job)} isSelected={selected.some(row => row.id === job.id)} + rowIndex={index} /> )} /> diff --git a/awx/ui_next/src/components/JobList/JobListItem.jsx b/awx/ui_next/src/components/JobList/JobListItem.jsx index a84b1b0ec5..6e2bff2bf4 100644 --- a/awx/ui_next/src/components/JobList/JobListItem.jsx +++ b/awx/ui_next/src/components/JobList/JobListItem.jsx @@ -51,8 +51,8 @@ function JobListItem({ rowIndex, isSelected, onSelect, - disable: false, }} + dataLabel={i18n._(t`Select`)} />