From 1db88fe4f6dec48b83054dd00c56f70214304047 Mon Sep 17 00:00:00 2001 From: Alex Corey Date: Mon, 6 Jan 2020 09:30:42 -0500 Subject: [PATCH 1/5] Adds Toggle, Variables, user Link and Delete to Inventory Host and Host Details If the user comes to Host details through Inventory Host they will get a Return To Host tab in addition to the others. This PR allows Inventory Host to share many of the same components with Host but does add some complexity to the routing files in Host.jsx --- .../src/components/RoutedTabs/RoutedTabs.jsx | 36 +-- awx/ui_next/src/screens/Host/Host.jsx | 242 +++++++++++----- .../screens/Host/HostDetail/HostDetail.jsx | 137 ++++++++- .../Host/HostDetail/HostDetail.test.jsx | 6 +- .../src/screens/Host/HostList/HostList.jsx | 2 +- .../screens/Host/HostList/HostListItem.jsx | 4 +- .../Host/HostList/HostListItem.test.jsx | 16 +- awx/ui_next/src/screens/Host/Hosts.jsx | 8 +- .../src/screens/Inventory/Inventories.jsx | 40 ++- .../src/screens/Inventory/Inventory.jsx | 29 +- .../InventoryGroup/InventoryGroup.jsx | 9 +- .../InventoryHosts/InventoryHostList.jsx | 226 +++++++++++++++ ...ts.test.jsx => InventoryHostList.test.jsx} | 16 +- .../InventoryHosts/InventoryHosts.jsx | 260 +++--------------- .../screens/Inventory/InventoryHosts/index.js | 2 +- 15 files changed, 668 insertions(+), 365 deletions(-) create mode 100644 awx/ui_next/src/screens/Inventory/InventoryHosts/InventoryHostList.jsx rename awx/ui_next/src/screens/Inventory/InventoryHosts/{InventoryHosts.test.jsx => InventoryHostList.test.jsx} (94%) diff --git a/awx/ui_next/src/components/RoutedTabs/RoutedTabs.jsx b/awx/ui_next/src/components/RoutedTabs/RoutedTabs.jsx index 964806f05e..ca1fd60143 100644 --- a/awx/ui_next/src/components/RoutedTabs/RoutedTabs.jsx +++ b/awx/ui_next/src/components/RoutedTabs/RoutedTabs.jsx @@ -57,23 +57,25 @@ function RoutedTabs(props) { return ( - {tabsArray.map(tab => ( - - {tab.name} - - ) : ( - tab.name - ) - } - /> - ))} + {tabsArray + .filter(tab => tab.isNestedTab || !tab.name.startsWith('Return')) + .map(tab => ( + + {tab.name} + + ) : ( + tab.name + ) + } + /> + ))} ); } diff --git a/awx/ui_next/src/screens/Host/Host.jsx b/awx/ui_next/src/screens/Host/Host.jsx index 7fc96f9ea5..cbd6ff6659 100644 --- a/awx/ui_next/src/screens/Host/Host.jsx +++ b/awx/ui_next/src/screens/Host/Host.jsx @@ -9,6 +9,9 @@ import RoutedTabs from '@components/RoutedTabs'; import ContentError from '@components/ContentError'; import HostFacts from './HostFacts'; import HostDetail from './HostDetail'; +import AlertModal from '@components/AlertModal'; +import ErrorDetail from '@components/ErrorDetail'; + import HostEdit from './HostEdit'; import HostGroups from './HostGroups'; import HostCompletedJobs from './HostCompletedJobs'; @@ -23,8 +26,16 @@ class Host extends Component { hasContentLoading: true, contentError: null, isInitialized: false, + toggleLoading: false, + toggleError: null, + deletionError: false, + isDeleteModalOpen: false, }; this.loadHost = this.loadHost.bind(this); + this.handleHostToggle = this.handleHostToggle.bind(this); + this.handleToggleError = this.handleToggleError.bind(this); + this.handleHostDelete = this.handleHostDelete.bind(this); + this.toggleDeleteModal = this.toggleDeleteModal.bind(this); } async componentDidMount() { @@ -45,15 +56,54 @@ class Host extends Component { } } + toggleDeleteModal() { + const { isDeleteModalOpen } = this.state; + this.setState({ isDeleteModalOpen: !isDeleteModalOpen }); + } + + async handleHostToggle() { + const { host } = this.state; + this.setState({ toggleLoading: true }); + try { + const { data } = await HostsAPI.update(host.id, { + enabled: !host.enabled, + }); + this.setState({ host: data }); + } catch (err) { + this.setState({ toggleError: err }); + } finally { + this.setState({ toggleLoading: null }); + } + } + + async handleHostDelete() { + const { host } = this.state; + const { match, history } = this.props; + + this.setState({ hasContentLoading: true }); + try { + await HostsAPI.destroy(host.id); + this.setState({ hasContentLoading: false }); + history.push(`/inventories/inventory/${match.params.id}/hosts`); + } catch (err) { + this.setState({ deletionError: err }); + } + } + async loadHost() { - const { match, setBreadcrumb } = this.props; - const id = parseInt(match.params.id, 10); + const { match, setBreadcrumb, history, inventory } = this.props; this.setState({ contentError: null, hasContentLoading: true }); try { - const { data } = await HostsAPI.readDetail(id); - setBreadcrumb(data); + const { data } = await HostsAPI.readDetail( + match.params.hostId || match.params.id + ); this.setState({ host: data }); + + if (history.location.pathname.startsWith('/hosts')) { + setBreadcrumb(data); + } + setBreadcrumb(inventory, data); } catch (err) { this.setState({ contentError: err }); } finally { @@ -61,15 +111,44 @@ class Host extends Component { } } + handleToggleError() { + this.setState({ toggleError: false }); + } + render() { const { location, match, history, i18n } = this.props; - - const { host, contentError, hasContentLoading, isInitialized } = this.state; - + const { + deletionError, + host, + isDeleteModalOpen, + toggleError, + hasContentLoading, + toggleLoading, + isInitialized, + contentError, + } = this.state; const tabsArray = [ - { name: i18n._(t`Details`), link: `${match.url}/details`, id: 0 }, - { name: i18n._(t`Facts`), link: `${match.url}/facts`, id: 1 }, - { name: i18n._(t`Groups`), link: `${match.url}/groups`, id: 2 }, + { + name: i18n._(t`Return to Hosts`), + link: `/inventories/inventory/${match.params.id}/hosts`, + id: 99, + isNestedTab: !history.location.pathname.startsWith('/hosts'), + }, + { + name: i18n._(t`Details`), + link: `${match.url}/details`, + id: 0, + }, + { + name: i18n._(t`Facts`), + link: `${match.url}/facts`, + id: 1, + }, + { + name: i18n._(t`Groups`), + link: `${match.url}/groups`, + id: 2, + }, { name: i18n._(t`Completed Jobs`), link: `${match.url}/completed_jobs`, @@ -117,62 +196,99 @@ class Host extends Component { ); } - return ( - - - {cardHeader} - - - {host && ( + <> + + + {cardHeader} + + + {host && ( + } + /> + )} + {host && ( + ( + + )} + /> + )} + {host && ( + } + /> + )} + {host && ( + } + /> + )} + {host && ( + } + /> + )} } + key="not-found" + path="*" + render={() => + !hasContentLoading && ( + + {match.params.id && ( + + {i18n._(`View Host Details`)} + + )} + + ) + } /> - )} - {host && ( - } - /> - )} - {host && ( - } - /> - )} - {host && ( - } - /> - )} - {host && ( - } - /> - )} - - !hasContentLoading && ( - - {match.params.id && ( - - {i18n._(`View Host Details`)} - - )} - - ) - } - /> - , - - - + , + + + + {deletionError && ( + this.setState({ deletionError: false })} + > + {i18n._(t`Failed to delete ${host.name}.`)} + + + )} + ); } } diff --git a/awx/ui_next/src/screens/Host/HostDetail/HostDetail.jsx b/awx/ui_next/src/screens/Host/HostDetail/HostDetail.jsx index db62e11f60..91f8416176 100644 --- a/awx/ui_next/src/screens/Host/HostDetail/HostDetail.jsx +++ b/awx/ui_next/src/screens/Host/HostDetail/HostDetail.jsx @@ -7,14 +7,127 @@ import { Button } from '@patternfly/react-core'; import { CardBody, CardActionsRow } from '@components/Card'; import { DetailList, Detail, UserDateDetail } from '@components/DetailList'; import { VariablesDetail } from '@components/CodeMirrorInput'; +import { Sparkline } from '@components/Sparkline'; +import Switch from '@components/Switch'; function HostDetail({ host, i18n }) { - const { created, description, id, modified, name, summary_fields } = host; +const ActionButtonWrapper = styled.div` + display: flex; + justify-content: flex-end; + margin-top: 20px; + & > :not(:first-child) { + margin-left: 20px; + } +`; +function HostDetail({ + host, + history, + isDeleteModalOpen, + match, + i18n, + toggleError, + toggleLoading, + onHostDelete, + onToggleDeleteModal, + onToggleError, + onHandleHostToggle, +}) { + const { created, description, id, modified, name, summary_fields } = host; + let createdBy = ''; + if (created) { + if (summary_fields.created_by && summary_fields.created_by.username) { + createdBy = ( + + {i18n._(t`${formatDateString(created)} by `)}{' '} + + {summary_fields.created_by.username} + + + ); + } else { + createdBy = formatDateString(created); + } + } + + let modifiedBy = ''; + if (modified) { + if (summary_fields.modified_by && summary_fields.modified_by.username) { + modifiedBy = ( + + {i18n._(t`${formatDateString(modified)} by`)}{' '} + + {summary_fields.modified_by.username} + + + ); + } else { + modifiedBy = formatDateString(modified); + } + } + if (toggleError && !toggleLoading) { + return ( + + {i18n._(t`Failed to toggle host.`)} + + + ); + } + if (isDeleteModalOpen) { + return ( + onToggleDeleteModal()} + > + {i18n._(t`Are you sure you want to delete:`)} +
+ {host.name} + + + + + + + ); + } return ( + + } + label={i18n._(t`Activity`)} + /> {summary_fields.inventory && ( )} - - + + @@ -54,7 +159,11 @@ function HostDetail({ host, i18n }) { diff --git a/awx/ui_next/src/screens/Host/HostDetail/HostDetail.test.jsx b/awx/ui_next/src/screens/Host/HostDetail/HostDetail.test.jsx index 749c512171..5562a5cc05 100644 --- a/awx/ui_next/src/screens/Host/HostDetail/HostDetail.test.jsx +++ b/awx/ui_next/src/screens/Host/HostDetail/HostDetail.test.jsx @@ -50,8 +50,7 @@ describe('', () => { test('should show edit button for users with edit permission', async () => { const wrapper = mountWithContexts(); - // VariablesDetail has two buttons - const editButton = wrapper.find('Button').at(2); + const editButton = wrapper.find('Button[aria-label="edit"]'); expect(editButton.text()).toEqual('Edit'); expect(editButton.prop('to')).toBe('/hosts/1/edit'); }); @@ -61,7 +60,6 @@ describe('', () => { readOnlyHost.summary_fields.user_capabilities.edit = false; const wrapper = mountWithContexts(); await waitForElement(wrapper, 'HostDetail'); - // VariablesDetail has two buttons - expect(wrapper.find('Button').length).toBe(2); + expect(wrapper.find('Button[aria-label="edit"]').length).toBe(0); }); }); diff --git a/awx/ui_next/src/screens/Host/HostList/HostList.jsx b/awx/ui_next/src/screens/Host/HostList/HostList.jsx index 4231ef2cc0..978ff2ba59 100644 --- a/awx/ui_next/src/screens/Host/HostList/HostList.jsx +++ b/awx/ui_next/src/screens/Host/HostList/HostList.jsx @@ -238,7 +238,7 @@ class HostsList extends Component { detailUrl={`${match.url}/${o.id}`} isSelected={selected.some(row => row.id === o.id)} onSelect={() => this.handleSelect(o)} - toggleHost={this.handleHostToggle} + onToggleHost={this.handleHostToggle} toggleLoading={toggleLoading === o.id} /> )} diff --git a/awx/ui_next/src/screens/Host/HostList/HostListItem.jsx b/awx/ui_next/src/screens/Host/HostList/HostListItem.jsx index 9123136749..6e74f9da3b 100644 --- a/awx/ui_next/src/screens/Host/HostList/HostListItem.jsx +++ b/awx/ui_next/src/screens/Host/HostList/HostListItem.jsx @@ -34,7 +34,7 @@ class HostListItem extends React.Component { isSelected, onSelect, detailUrl, - toggleHost, + onToggleHost, toggleLoading, i18n, } = this.props; @@ -93,7 +93,7 @@ class HostListItem extends React.Component { toggleLoading || !host.summary_fields.user_capabilities.edit } - onChange={() => toggleHost(host)} + onChange={() => onToggleHost(host)} aria-label={i18n._(t`Toggle host`)} /> diff --git a/awx/ui_next/src/screens/Host/HostList/HostListItem.test.jsx b/awx/ui_next/src/screens/Host/HostList/HostListItem.test.jsx index d9e5987661..285f71fba4 100644 --- a/awx/ui_next/src/screens/Host/HostList/HostListItem.test.jsx +++ b/awx/ui_next/src/screens/Host/HostList/HostListItem.test.jsx @@ -4,7 +4,7 @@ import { mountWithContexts } from '@testUtils/enzymeHelpers'; import HostsListItem from './HostListItem'; -let toggleHost; +let onToggleHost; const mockHost = { id: 1, @@ -24,7 +24,7 @@ const mockHost = { describe('', () => { beforeEach(() => { - toggleHost = jest.fn(); + onToggleHost = jest.fn(); }); afterEach(() => { @@ -38,7 +38,7 @@ describe('', () => { detailUrl="/host/1" onSelect={() => {}} host={mockHost} - toggleHost={toggleHost} + onToggleHost={onToggleHost} /> ); expect(wrapper.find('PencilAltIcon').exists()).toBeTruthy(); @@ -52,7 +52,7 @@ describe('', () => { detailUrl="/host/1" onSelect={() => {}} host={copyMockHost} - toggleHost={toggleHost} + onToggleHost={onToggleHost} /> ); expect(wrapper.find('PencilAltIcon').exists()).toBeFalsy(); @@ -64,7 +64,7 @@ describe('', () => { detailUrl="/host/1" onSelect={() => {}} host={mockHost} - toggleHost={toggleHost} + onToggleHost={onToggleHost} /> ); wrapper @@ -72,7 +72,7 @@ describe('', () => { .first() .find('input') .simulate('change'); - expect(toggleHost).toHaveBeenCalledWith(mockHost); + expect(onToggleHost).toHaveBeenCalledWith(mockHost); }); test('handles toggle click when host is disabled', () => { @@ -82,7 +82,7 @@ describe('', () => { detailUrl="/host/1" onSelect={() => {}} host={mockHost} - toggleHost={toggleHost} + onToggleHost={onToggleHost} /> ); wrapper @@ -90,6 +90,6 @@ describe('', () => { .first() .find('input') .simulate('change'); - expect(toggleHost).toHaveBeenCalledWith(mockHost); + expect(onToggleHost).toHaveBeenCalledWith(mockHost); }); }); diff --git a/awx/ui_next/src/screens/Host/Hosts.jsx b/awx/ui_next/src/screens/Host/Hosts.jsx index 0a17916223..59bb53bd06 100644 --- a/awx/ui_next/src/screens/Host/Hosts.jsx +++ b/awx/ui_next/src/screens/Host/Hosts.jsx @@ -46,14 +46,17 @@ class Hosts extends Component { }; render() { - const { match, history, location } = this.props; + const { match, history, location, inventory } = this.props; const { breadcrumbConfig } = this.state; return ( - } /> + } + /> ( @@ -64,6 +67,7 @@ class Hosts extends Component { location={location} setBreadcrumb={this.setBreadcrumbConfig} me={me || {}} + inventory={inventory} /> )} diff --git a/awx/ui_next/src/screens/Inventory/Inventories.jsx b/awx/ui_next/src/screens/Inventory/Inventories.jsx index 4253078486..49264336b5 100644 --- a/awx/ui_next/src/screens/Inventory/Inventories.jsx +++ b/awx/ui_next/src/screens/Inventory/Inventories.jsx @@ -27,7 +27,7 @@ class Inventories extends Component { }; } - setBreadCrumbConfig = (inventory, group) => { + setBreadCrumbConfig = (inventory, nestedResource) => { const { i18n } = this.props; if (!inventory) { return; @@ -51,21 +51,39 @@ class Inventories extends Component { [`/inventories/${inventoryKind}/${inventory.id}/completed_jobs`]: i18n._( t`Completed Jobs` ), - [`/inventories/${inventoryKind}/${inventory.id}/hosts`]: i18n._(t`Hosts`), + [`/inventories/${inventoryKind}/${inventory.id}/hosts/${nestedResource && + nestedResource.id}`]: i18n._( + t`${nestedResource && nestedResource.name}` + ), + [`/inventories/${inventoryKind}/${inventory.id}/hosts/${nestedResource && + nestedResource.id}/details`]: i18n._(t`Details`), + [`/inventories/${inventoryKind}/${inventory.id}/hosts/${nestedResource && + nestedResource.id}/edit`]: i18n._(t`Edit Details`), [`/inventories/${inventoryKind}/${inventory.id}/hosts/add`]: i18n._( t`Create New Host` ), - [`/inventories/inventory/${inventory.id}/sources`]: i18n._(t`Sources`), - [`/inventories/inventory/${inventory.id}/groups`]: i18n._(t`Groups`), - [`/inventories/inventory/${inventory.id}/groups/add`]: i18n._( + [`/inventories/${inventoryKind}/${inventory.id}/sources`]: i18n._( + t`Sources` + ), + [`/inventories/${inventoryKind}/${inventory.id}/groups`]: i18n._( + t`Groups` + ), + [`/inventories/${inventoryKind}/${inventory.id}/groups/add`]: i18n._( t`Create New Group` ), - [`/inventories/inventory/${inventory.id}/groups/${group && - group.id}`]: `${group && group.name}`, - [`/inventories/inventory/${inventory.id}/groups/${group && - group.id}/details`]: i18n._(t`Group Details`), - [`/inventories/inventory/${inventory.id}/groups/${group && - group.id}/edit`]: i18n._(t`Edit Details`), + [`/inventories/${inventoryKind}/${inventory.id}/groups/${nestedResource && + nestedResource.id}`]: `${nestedResource && nestedResource.name}`, + [`/inventories/${inventoryKind}/${inventory.id}/groups/${nestedResource && + nestedResource.id}/details`]: i18n._(t`Group Details`), + [`/inventories/${inventoryKind}/${inventory.id}/groups/${nestedResource && + nestedResource.id}/edit`]: i18n._(t`Edit Details`), + [`/inventories/${inventoryKind}/${inventory.id}/hosts`]: i18n._(t`Hosts`), + [`/inventories/${inventoryKind}/${inventory.id}/sources`]: i18n._( + t`Sources` + ), + [`/inventories/${inventoryKind}/${inventory.id}/groups`]: i18n._( + t`Groups` + ), }; this.setState({ breadcrumbConfig }); }; diff --git a/awx/ui_next/src/screens/Inventory/Inventory.jsx b/awx/ui_next/src/screens/Inventory/Inventory.jsx index e632610660..c2cb831331 100644 --- a/awx/ui_next/src/screens/Inventory/Inventory.jsx +++ b/awx/ui_next/src/screens/Inventory/Inventory.jsx @@ -8,14 +8,15 @@ import CardCloseButton from '@components/CardCloseButton'; import ContentError from '@components/ContentError'; import RoutedTabs from '@components/RoutedTabs'; import { ResourceAccessList } from '@components/ResourceAccessList'; +import ContentLoading from '@components/ContentLoading'; import InventoryDetail from './InventoryDetail'; -import InventoryHosts from './InventoryHosts'; -import InventoryHostAdd from './InventoryHostAdd'; + import InventoryGroups from './InventoryGroups'; import InventoryCompletedJobs from './InventoryCompletedJobs'; import InventorySources from './InventorySources'; import { InventoriesAPI } from '@api'; import InventoryEdit from './InventoryEdit'; +import InventoryHosts from './InventoryHosts/InventoryHosts'; function Inventory({ history, i18n, location, match, setBreadcrumb }) { const [contentError, setContentError] = useState(null); @@ -61,10 +62,14 @@ function Inventory({ history, i18n, location, match, setBreadcrumb }) { if ( location.pathname.endsWith('edit') || location.pathname.endsWith('add') || - location.pathname.includes('groups/') + location.pathname.includes('groups/') || + history.location.pathname.includes(`/hosts/`) ) { cardHeader = null; } + if (hasContentLoading) { + return ; + } if (!hasContentLoading && contentError) { return ( @@ -111,9 +116,16 @@ function Inventory({ history, i18n, location, match, setBreadcrumb }) { render={() => } />, } + key="hosts" + path="/inventories/inventory/:id/hosts" + render={() => ( + + )} />, )} />, - } - />, ; diff --git a/awx/ui_next/src/screens/Inventory/InventoryHosts/InventoryHostList.jsx b/awx/ui_next/src/screens/Inventory/InventoryHosts/InventoryHostList.jsx new file mode 100644 index 0000000000..df4d7783fd --- /dev/null +++ b/awx/ui_next/src/screens/Inventory/InventoryHosts/InventoryHostList.jsx @@ -0,0 +1,226 @@ +import React, { useEffect, useState } from 'react'; +import { withRouter } from 'react-router-dom'; +import { withI18n } from '@lingui/react'; +import { t } from '@lingui/macro'; +import { getQSConfig, parseQueryString } from '@util/qs'; +import { InventoriesAPI, HostsAPI } from '@api'; + +import AlertModal from '@components/AlertModal'; +import DataListToolbar from '@components/DataListToolbar'; +import ErrorDetail from '@components/ErrorDetail'; +import PaginatedDataList, { + ToolbarAddButton, + ToolbarDeleteButton, +} from '@components/PaginatedDataList'; +import InventoryHostItem from './InventoryHostItem'; + +const QS_CONFIG = getQSConfig('host', { + page: 1, + page_size: 20, + order_by: 'name', +}); + +function InventoryHostList({ i18n, location, match }) { + const [actions, setActions] = useState(null); + const [contentError, setContentError] = useState(null); + const [deletionError, setDeletionError] = useState(null); + const [hostCount, setHostCount] = useState(0); + const [hosts, setHosts] = useState([]); + const [isLoading, setIsLoading] = useState(true); + const [selected, setSelected] = useState([]); + const [toggleError, setToggleError] = useState(null); + const [toggleLoading, setToggleLoading] = useState(null); + + const fetchHosts = (id, queryString) => { + const params = parseQueryString(QS_CONFIG, queryString); + return InventoriesAPI.readHosts(id, params); + }; + + useEffect(() => { + async function fetchData() { + try { + const [ + { + data: { count, results }, + }, + { + data: { actions: optionActions }, + }, + ] = await Promise.all([ + fetchHosts(match.params.id, location.search), + InventoriesAPI.readOptions(), + ]); + + setHosts(results); + setHostCount(count); + setActions(optionActions); + } catch (error) { + setContentError(error); + } finally { + setIsLoading(false); + } + } + + fetchData(); + }, [match.params.id, location]); + + const handleSelectAll = isSelected => { + setSelected(isSelected ? [...hosts] : []); + }; + + const handleSelect = row => { + if (selected.some(s => s.id === row.id)) { + setSelected(selected.filter(s => s.id !== row.id)); + } else { + setSelected(selected.concat(row)); + } + }; + + const handleDelete = async () => { + setIsLoading(true); + + try { + await Promise.all(selected.map(host => HostsAPI.destroy(host.id))); + } catch (error) { + setDeletionError(error); + } finally { + setSelected([]); + try { + const { + data: { count, results }, + } = await fetchHosts(match.params.id, location.search); + + setHosts(results); + setHostCount(count); + } catch (error) { + setContentError(error); + } finally { + setIsLoading(false); + } + } + }; + + const handleToggle = async hostToToggle => { + setToggleLoading(hostToToggle.id); + + try { + const { data: updatedHost } = await HostsAPI.update(hostToToggle.id, { + enabled: !hostToToggle.enabled, + }); + + setHosts( + hosts.map(host => (host.id === updatedHost.id ? updatedHost : host)) + ); + } catch (error) { + setToggleError(error); + } finally { + setToggleLoading(null); + } + }; + + const canAdd = + actions && Object.prototype.hasOwnProperty.call(actions, 'POST'); + const isAllSelected = selected.length > 0 && selected.length === hosts.length; + + return ( + <> + ( + , + canAdd && ( + + ), + ]} + /> + )} + renderItem={o => ( + row.id === o.id)} + onSelect={() => handleSelect(o)} + toggleHost={handleToggle} + toggleLoading={toggleLoading === o.id} + /> + )} + emptyStateControls={ + canAdd && ( + + ) + } + /> + + {toggleError && !toggleLoading && ( + setToggleError(false)} + > + {i18n._(t`Failed to toggle host.`)} + + + )} + + {deletionError && ( + setDeletionError(null)} + > + {i18n._(t`Failed to delete one or more hosts.`)} + + + )} + + ); +} + +export default withI18n()(withRouter(InventoryHostList)); diff --git a/awx/ui_next/src/screens/Inventory/InventoryHosts/InventoryHosts.test.jsx b/awx/ui_next/src/screens/Inventory/InventoryHosts/InventoryHostList.test.jsx similarity index 94% rename from awx/ui_next/src/screens/Inventory/InventoryHosts/InventoryHosts.test.jsx rename to awx/ui_next/src/screens/Inventory/InventoryHosts/InventoryHostList.test.jsx index 715413c81b..d35d4da489 100644 --- a/awx/ui_next/src/screens/Inventory/InventoryHosts/InventoryHosts.test.jsx +++ b/awx/ui_next/src/screens/Inventory/InventoryHosts/InventoryHostList.test.jsx @@ -2,7 +2,7 @@ import React from 'react'; import { act } from 'react-dom/test-utils'; import { InventoriesAPI, HostsAPI } from '@api'; import { mountWithContexts, waitForElement } from '@testUtils/enzymeHelpers'; -import InventoryHosts from './InventoryHosts'; +import InventoryHostList from './InventoryHostList'; import mockInventory from '../shared/data.inventory.json'; jest.mock('@api'); @@ -62,7 +62,7 @@ const mockHosts = [ }, ]; -describe('', () => { +describe('', () => { let wrapper; beforeEach(async () => { @@ -81,7 +81,7 @@ describe('', () => { }, }); await act(async () => { - wrapper = mountWithContexts(); + wrapper = mountWithContexts(); }); await waitForElement(wrapper, 'ContentLoading', el => el.length === 0); }); @@ -91,7 +91,7 @@ describe('', () => { }); test('initially renders successfully', () => { - expect(wrapper.find('InventoryHosts').length).toBe(1); + expect(wrapper.find('InventoryHostList').length).toBe(1); }); test('should fetch hosts from api and render them in the list', async () => { @@ -261,7 +261,9 @@ describe('', () => { }, }); await act(async () => { - wrapper = mountWithContexts(); + wrapper = mountWithContexts( + + ); }); await waitForElement(wrapper, 'ContentLoading', el => el.length === 0); expect(wrapper.find('ToolbarAddButton').length).toBe(0); @@ -272,7 +274,9 @@ describe('', () => { Promise.reject(new Error()) ); await act(async () => { - wrapper = mountWithContexts(); + wrapper = mountWithContexts( + + ); }); await waitForElement(wrapper, 'ContentError', el => el.length === 1); }); diff --git a/awx/ui_next/src/screens/Inventory/InventoryHosts/InventoryHosts.jsx b/awx/ui_next/src/screens/Inventory/InventoryHosts/InventoryHosts.jsx index 9e96793e3f..c88362686a 100644 --- a/awx/ui_next/src/screens/Inventory/InventoryHosts/InventoryHosts.jsx +++ b/awx/ui_next/src/screens/Inventory/InventoryHosts/InventoryHosts.jsx @@ -1,228 +1,46 @@ -import React, { useEffect, useState } from 'react'; -import { withRouter } from 'react-router-dom'; -import { withI18n } from '@lingui/react'; -import { t } from '@lingui/macro'; -import { getQSConfig, parseQueryString } from '@util/qs'; -import { InventoriesAPI, HostsAPI } from '@api'; +import React from 'react'; +import { Switch, Route, withRouter } from 'react-router-dom'; -import AlertModal from '@components/AlertModal'; -import DataListToolbar from '@components/DataListToolbar'; -import ErrorDetail from '@components/ErrorDetail'; -import PaginatedDataList, { - ToolbarAddButton, - ToolbarDeleteButton, -} from '@components/PaginatedDataList'; -import InventoryHostItem from './InventoryHostItem'; - -const QS_CONFIG = getQSConfig('host', { - page: 1, - page_size: 20, - order_by: 'name', -}); - -function InventoryHosts({ i18n, location, match }) { - const [actions, setActions] = useState(null); - const [contentError, setContentError] = useState(null); - const [deletionError, setDeletionError] = useState(null); - const [hostCount, setHostCount] = useState(0); - const [hosts, setHosts] = useState([]); - const [isLoading, setIsLoading] = useState(true); - const [selected, setSelected] = useState([]); - const [toggleError, setToggleError] = useState(null); - const [toggleLoading, setToggleLoading] = useState(null); - - const fetchHosts = (id, queryString) => { - const params = parseQueryString(QS_CONFIG, queryString); - return InventoriesAPI.readHosts(id, params); - }; - - useEffect(() => { - async function fetchData() { - try { - const [ - { - data: { count, results }, - }, - { - data: { actions: optionActions }, - }, - ] = await Promise.all([ - fetchHosts(match.params.id, location.search), - InventoriesAPI.readOptions(), - ]); - - setHosts(results); - setHostCount(count); - setActions(optionActions); - } catch (error) { - setContentError(error); - } finally { - setIsLoading(false); - } - } - - fetchData(); - }, [match.params.id, location]); - - const handleSelectAll = isSelected => { - setSelected(isSelected ? [...hosts] : []); - }; - - const handleSelect = row => { - if (selected.some(s => s.id === row.id)) { - setSelected(selected.filter(s => s.id !== row.id)); - } else { - setSelected(selected.concat(row)); - } - }; - - const handleDelete = async () => { - setIsLoading(true); - - try { - await Promise.all(selected.map(host => HostsAPI.destroy(host.id))); - } catch (error) { - setDeletionError(error); - } finally { - setSelected([]); - try { - const { - data: { count, results }, - } = await fetchHosts(match.params.id, location.search); - - setHosts(results); - setHostCount(count); - } catch (error) { - setContentError(error); - } finally { - setIsLoading(false); - } - } - }; - - const handleToggle = async hostToToggle => { - setToggleLoading(hostToToggle.id); - - try { - const { data: updatedHost } = await HostsAPI.update(hostToToggle.id, { - enabled: !hostToToggle.enabled, - }); - - setHosts( - hosts.map(host => (host.id === updatedHost.id ? updatedHost : host)) - ); - } catch (error) { - setToggleError(error); - } finally { - setToggleLoading(null); - } - }; - - const canAdd = - actions && Object.prototype.hasOwnProperty.call(actions, 'POST'); - const isAllSelected = selected.length > 0 && selected.length === hosts.length; +import Host from '../../Host/Host'; +import InventoryHostList from './InventoryHostList'; +import HostAdd from '../InventoryHostAdd'; +function InventoryHosts({ match, setBreadcrumb, i18n, inventory }) { return ( - <> - ( - , - canAdd && ( - - ), - ]} - /> - )} - renderItem={o => ( - row.id === o.id)} - onSelect={() => handleSelect(o)} - toggleHost={handleToggle} - toggleLoading={toggleLoading === o.id} - /> - )} - emptyStateControls={ - canAdd && ( - - ) - } + + } /> - - {toggleError && !toggleLoading && ( - setToggleError(false)} - > - {i18n._(t`Failed to toggle host.`)} - - - )} - - {deletionError && ( - setDeletionError(null)} - > - {i18n._(t`Failed to delete one or more hosts.`)} - - - )} - + , + ( + + )} + /> + , + ( + + )} + /> + , + ); } -export default withI18n()(withRouter(InventoryHosts)); +export default withRouter(InventoryHosts); diff --git a/awx/ui_next/src/screens/Inventory/InventoryHosts/index.js b/awx/ui_next/src/screens/Inventory/InventoryHosts/index.js index 6d33814f29..0cb4fe95bc 100644 --- a/awx/ui_next/src/screens/Inventory/InventoryHosts/index.js +++ b/awx/ui_next/src/screens/Inventory/InventoryHosts/index.js @@ -1 +1 @@ -export { default } from './InventoryHosts'; +export { default } from './InventoryHostList'; From 919475a4c7ceb8c9c35d41f34440b57286042652 Mon Sep 17 00:00:00 2001 From: Alex Corey Date: Wed, 8 Jan 2020 09:09:11 -0500 Subject: [PATCH 2/5] Improves NestedTabs, Refactors PR, Adds Delete/DeleteError Functionality to HostDetail --- .../components/AddRole/SelectResourceStep.jsx | 1 + .../src/components/RoutedTabs/RoutedTabs.jsx | 33 +-- awx/ui_next/src/screens/Host/Host.jsx | 271 +++++++----------- .../screens/Host/HostDetail/HostDetail.jsx | 134 ++++----- awx/ui_next/src/screens/Host/Hosts.jsx | 45 ++- .../src/screens/Inventory/Inventories.jsx | 51 ++-- .../src/screens/Inventory/Inventory.jsx | 2 +- .../InventoryGroup/InventoryGroup.jsx | 10 +- .../InventoryGroup/InventoryGroup.test.jsx | 10 +- .../InventoryHosts/InventoryHosts.jsx | 6 +- .../JobTemplateDetail.test.jsx | 2 + 11 files changed, 241 insertions(+), 324 deletions(-) diff --git a/awx/ui_next/src/components/AddRole/SelectResourceStep.jsx b/awx/ui_next/src/components/AddRole/SelectResourceStep.jsx index 64e60c9f21..e208686ab5 100644 --- a/awx/ui_next/src/components/AddRole/SelectResourceStep.jsx +++ b/awx/ui_next/src/components/AddRole/SelectResourceStep.jsx @@ -116,6 +116,7 @@ class SelectResourceStep extends React.Component { name={item[displayKey]} label={item[displayKey]} onSelect={() => onRowClick(item)} + onDeselect={() => {}} /> )} renderToolbar={props => } diff --git a/awx/ui_next/src/components/RoutedTabs/RoutedTabs.jsx b/awx/ui_next/src/components/RoutedTabs/RoutedTabs.jsx index ca1fd60143..1cc90f3995 100644 --- a/awx/ui_next/src/components/RoutedTabs/RoutedTabs.jsx +++ b/awx/ui_next/src/components/RoutedTabs/RoutedTabs.jsx @@ -1,9 +1,8 @@ import React from 'react'; -import { shape, string, number, arrayOf } from 'prop-types'; +import { shape, string, number, arrayOf, node, oneOfType } from 'prop-types'; import { Tab, Tabs as PFTabs } from '@patternfly/react-core'; import { withRouter } from 'react-router-dom'; import styled from 'styled-components'; -import { CaretLeftIcon } from '@patternfly/react-icons'; const Tabs = styled(PFTabs)` --pf-c-tabs__button--PaddingLeft: 20px; @@ -57,25 +56,15 @@ function RoutedTabs(props) { return ( - {tabsArray - .filter(tab => tab.isNestedTab || !tab.name.startsWith('Return')) - .map(tab => ( - - {tab.name} - - ) : ( - tab.name - ) - } - /> - ))} + {tabsArray.map(tab => ( + + ))} ); } @@ -89,7 +78,7 @@ RoutedTabs.propTypes = { shape({ id: number.isRequired, link: string.isRequired, - name: string.isRequired, + name: oneOfType([string.isRequired, node.isRequired]), }) ).isRequired, }; diff --git a/awx/ui_next/src/screens/Host/Host.jsx b/awx/ui_next/src/screens/Host/Host.jsx index cbd6ff6659..203ec74212 100644 --- a/awx/ui_next/src/screens/Host/Host.jsx +++ b/awx/ui_next/src/screens/Host/Host.jsx @@ -2,15 +2,16 @@ import React, { Component } from 'react'; import { withI18n } from '@lingui/react'; import { t } from '@lingui/macro'; import { Switch, Route, withRouter, Redirect, Link } from 'react-router-dom'; -import { Card, PageSection } from '@patternfly/react-core'; +import { Card } from '@patternfly/react-core'; +import { CaretLeftIcon } from '@patternfly/react-icons'; + import { TabbedCardHeader } from '@components/Card'; import CardCloseButton from '@components/CardCloseButton'; import RoutedTabs from '@components/RoutedTabs'; import ContentError from '@components/ContentError'; +import ContentLoading from '@components/ContentLoading'; import HostFacts from './HostFacts'; import HostDetail from './HostDetail'; -import AlertModal from '@components/AlertModal'; -import ErrorDetail from '@components/ErrorDetail'; import HostEdit from './HostEdit'; import HostGroups from './HostGroups'; @@ -26,16 +27,8 @@ class Host extends Component { hasContentLoading: true, contentError: null, isInitialized: false, - toggleLoading: false, - toggleError: null, - deletionError: false, - isDeleteModalOpen: false, }; this.loadHost = this.loadHost.bind(this); - this.handleHostToggle = this.handleHostToggle.bind(this); - this.handleToggleError = this.handleToggleError.bind(this); - this.handleHostDelete = this.handleHostDelete.bind(this); - this.toggleDeleteModal = this.toggleDeleteModal.bind(this); } async componentDidMount() { @@ -56,40 +49,6 @@ class Host extends Component { } } - toggleDeleteModal() { - const { isDeleteModalOpen } = this.state; - this.setState({ isDeleteModalOpen: !isDeleteModalOpen }); - } - - async handleHostToggle() { - const { host } = this.state; - this.setState({ toggleLoading: true }); - try { - const { data } = await HostsAPI.update(host.id, { - enabled: !host.enabled, - }); - this.setState({ host: data }); - } catch (err) { - this.setState({ toggleError: err }); - } finally { - this.setState({ toggleLoading: null }); - } - } - - async handleHostDelete() { - const { host } = this.state; - const { match, history } = this.props; - - this.setState({ hasContentLoading: true }); - try { - await HostsAPI.destroy(host.id); - this.setState({ hasContentLoading: false }); - history.push(`/inventories/inventory/${match.params.id}/hosts`); - } catch (err) { - this.setState({ deletionError: err }); - } - } - async loadHost() { const { match, setBreadcrumb, history, inventory } = this.props; @@ -102,8 +61,9 @@ class Host extends Component { if (history.location.pathname.startsWith('/hosts')) { setBreadcrumb(data); + } else { + setBreadcrumb(inventory, data); } - setBreadcrumb(inventory, data); } catch (err) { this.setState({ contentError: err }); } finally { @@ -111,29 +71,10 @@ class Host extends Component { } } - handleToggleError() { - this.setState({ toggleError: false }); - } - render() { const { location, match, history, i18n } = this.props; - const { - deletionError, - host, - isDeleteModalOpen, - toggleError, - hasContentLoading, - toggleLoading, - isInitialized, - contentError, - } = this.state; + const { host, hasContentLoading, isInitialized, contentError } = this.state; const tabsArray = [ - { - name: i18n._(t`Return to Hosts`), - link: `/inventories/inventory/${match.params.id}/hosts`, - id: 99, - isNestedTab: !history.location.pathname.startsWith('/hosts'), - }, { name: i18n._(t`Details`), link: `${match.url}/details`, @@ -155,7 +96,18 @@ class Host extends Component { id: 3, }, ]; - + if (!history.location.pathname.startsWith('/hosts')) { + tabsArray.unshift({ + name: ( + <> + + {i18n._(t`Back to Hosts`)} + + ), + link: `/inventories/inventory/${match.params.id}/hosts`, + id: 99, + }); + } let cardHeader = ( ; + } + if (!hasContentLoading && contentError) { return ( - - - - {contentError.response.status === 404 && ( - - {i18n._(`Host not found.`)}{' '} - {i18n._(`View all Hosts.`)} - - )} - - - + + + {contentError.response.status === 404 && ( + + {i18n._(`Host not found.`)}{' '} + {i18n._(`View all Hosts.`)} + + )} + + ); } + const redirect = location.pathname.startsWith('/hosts') ? ( + + ) : ( + + ); return ( - <> - - - {cardHeader} - - - {host && ( - } + + {cardHeader} + + {redirect} + {host && ( + ( + this.setState({ host: newHost })} /> )} - {host && ( - ( - + /> + )} + )) + {host && ( + } + /> + )} + {host && ( + } + /> + )} + {host && ( + } + /> + )} + {host && ( + } + /> + )} + + !hasContentLoading && ( + + {match.params.id && ( + + {i18n._(`View Host Details`)} + )} - /> - )} - {host && ( - } - /> - )} - {host && ( - } - /> - )} - {host && ( - } - /> - )} - - !hasContentLoading && ( - - {match.params.id && ( - - {i18n._(`View Host Details`)} - - )} - - ) - } - /> - , - - - - {deletionError && ( - this.setState({ deletionError: false })} - > - {i18n._(t`Failed to delete ${host.name}.`)} - - - )} - + + ) + } + /> + , + + ); } } diff --git a/awx/ui_next/src/screens/Host/HostDetail/HostDetail.jsx b/awx/ui_next/src/screens/Host/HostDetail/HostDetail.jsx index 91f8416176..282edbc471 100644 --- a/awx/ui_next/src/screens/Host/HostDetail/HostDetail.jsx +++ b/awx/ui_next/src/screens/Host/HostDetail/HostDetail.jsx @@ -1,14 +1,18 @@ -import React from 'react'; -import { Link } from 'react-router-dom'; +import React, { useState } from 'react'; +import { Link, withRouter } from 'react-router-dom'; import { withI18n } from '@lingui/react'; import { t } from '@lingui/macro'; import { Host } from '@types'; import { Button } from '@patternfly/react-core'; import { CardBody, CardActionsRow } from '@components/Card'; +import AlertModal from '@components/AlertModal'; +import ErrorDetail from '@components/ErrorDetail'; import { DetailList, Detail, UserDateDetail } from '@components/DetailList'; import { VariablesDetail } from '@components/CodeMirrorInput'; import { Sparkline } from '@components/Sparkline'; +import DeleteButton from '@components/DeleteButton'; import Switch from '@components/Switch'; +import { HostsAPI } from '@api'; function HostDetail({ host, i18n }) { const ActionButtonWrapper = styled.div` @@ -20,92 +24,62 @@ const ActionButtonWrapper = styled.div` } `; -function HostDetail({ - host, - history, - isDeleteModalOpen, - match, - i18n, - toggleError, - toggleLoading, - onHostDelete, - onToggleDeleteModal, - onToggleError, - onHandleHostToggle, -}) { +function HostDetail({ host, history, match, i18n, onUpdateHost }) { const { created, description, id, modified, name, summary_fields } = host; - let createdBy = ''; - if (created) { - if (summary_fields.created_by && summary_fields.created_by.username) { - createdBy = ( - - {i18n._(t`${formatDateString(created)} by `)}{' '} - - {summary_fields.created_by.username} - - - ); - } else { - createdBy = formatDateString(created); - } - } - let modifiedBy = ''; - if (modified) { - if (summary_fields.modified_by && summary_fields.modified_by.username) { - modifiedBy = ( - - {i18n._(t`${formatDateString(modified)} by`)}{' '} - - {summary_fields.modified_by.username} - - - ); - } else { - modifiedBy = formatDateString(modified); + const [isLoading, setIsloading] = useState(false); + const [deletionError, setDeletionError] = useState(false); + const [toggleLoading, setToggleLoading] = useState(false); + const [toggleError, setToggleError] = useState(false); + + const handleHostToggle = async () => { + setToggleLoading(true); + try { + const { data } = await HostsAPI.update(host.id, { + enabled: !host.enabled, + }); + onUpdateHost(data); + } catch (err) { + setToggleError(err); + } finally { + setToggleLoading(false); } - } + }; + + const handleHostDelete = async () => { + setIsloading(true); + try { + await HostsAPI.destroy(host.id); + setIsloading(false); + history.push(`/inventories/inventory/${match.params.id}/hosts`); + } catch (err) { + setDeletionError(err); + } + }; + if (toggleError && !toggleLoading) { return ( setToggleError(false)} > {i18n._(t`Failed to toggle host.`)} ); } - if (isDeleteModalOpen) { + if (!isLoading && deletionError) { return ( onToggleDeleteModal()} + title={i18n._(t`Error!`)} + onClose={() => setDeletionError(false)} > - {i18n._(t`Are you sure you want to delete:`)} -
- {host.name} - - - - - + {i18n._(t`Failed to delete ${host.name}.`)} + ); } @@ -118,7 +92,7 @@ function HostDetail({ labelOff={i18n._(t`Off`)} isChecked={host.enabled} isDisabled={!host.summary_fields.user_capabilities.edit} - onChange={onHandleHostToggle} + onChange={() => handleHostToggle()} aria-label={i18n._(t`Toggle Host`)} /> @@ -145,8 +119,16 @@ function HostDetail({ } /> )} - - + + )} + {summary_fields.user_capabilities && + summary_fields.user_capabilities.delete && ( + handleHostDelete()} + modalTitle={i18n._(t`Delete Host`)} + name={host.name} + /> + )} ); } diff --git a/awx/ui_next/src/screens/Host/Hosts.jsx b/awx/ui_next/src/screens/Host/Hosts.jsx index 59bb53bd06..1ea3e92905 100644 --- a/awx/ui_next/src/screens/Host/Hosts.jsx +++ b/awx/ui_next/src/screens/Host/Hosts.jsx @@ -2,6 +2,7 @@ import React, { Component, Fragment } from 'react'; import { Route, withRouter, Switch } from 'react-router-dom'; import { withI18n } from '@lingui/react'; import { t } from '@lingui/macro'; +import { PageSection } from '@patternfly/react-core'; import { Config } from '@contexts/Config'; import Breadcrumbs from '@components/Breadcrumbs/Breadcrumbs'; @@ -46,35 +47,31 @@ class Hosts extends Component { }; render() { - const { match, history, location, inventory } = this.props; + const { match } = this.props; const { breadcrumbConfig } = this.state; return ( - - } - /> - ( - - {({ me }) => ( - - )} - - )} - /> - } /> - + + + } /> + ( + + {({ me }) => ( + + )} + + )} + /> + } /> + + ); } diff --git a/awx/ui_next/src/screens/Inventory/Inventories.jsx b/awx/ui_next/src/screens/Inventory/Inventories.jsx index 49264336b5..8a17787c30 100644 --- a/awx/ui_next/src/screens/Inventory/Inventories.jsx +++ b/awx/ui_next/src/screens/Inventory/Inventories.jsx @@ -39,51 +39,46 @@ class Inventories extends Component { '/inventories/inventory/add': i18n._(t`Create New Inventory`), '/inventories/smart_inventory/add': i18n._(t`Create New Smart Inventory`), [`/inventories/${inventoryKind}/${inventory.id}`]: `${inventory.name}`, - [`/inventories/${inventoryKind}/${inventory.id}/details`]: i18n._( - t`Details` - ), - [`/inventories/${inventoryKind}/${inventory.id}/edit`]: i18n._( - t`Edit Details` - ), + [`/inventories/${inventoryKind}/${inventory.id}/access`]: i18n._( t`Access` ), [`/inventories/${inventoryKind}/${inventory.id}/completed_jobs`]: i18n._( t`Completed Jobs` ), + [`/inventories/${inventoryKind}/${inventory.id}/details`]: i18n._( + t`Details` + ), + [`/inventories/${inventoryKind}/${inventory.id}/edit`]: i18n._( + t`Edit Details` + ), + [`/inventories/${inventoryKind}/${inventory.id}/groups`]: i18n._( + t`Groups` + ), + [`/inventories/${inventoryKind}/${inventory.id}/hosts`]: i18n._(t`Hosts`), + + [`/inventories/${inventoryKind}/${inventory.id}/sources`]: i18n._( + t`Sources` + ), + + [`/inventories/${inventoryKind}/${inventory.id}/hosts/${nestedResource && + nestedResource.id}/edit`]: i18n._(t`Edit Details`), + [`/inventories/${inventoryKind}/${inventory.id}/hosts/${nestedResource && + nestedResource.id}/details`]: i18n._(t`Host Details`), [`/inventories/${inventoryKind}/${inventory.id}/hosts/${nestedResource && nestedResource.id}`]: i18n._( t`${nestedResource && nestedResource.name}` ), - [`/inventories/${inventoryKind}/${inventory.id}/hosts/${nestedResource && - nestedResource.id}/details`]: i18n._(t`Details`), - [`/inventories/${inventoryKind}/${inventory.id}/hosts/${nestedResource && - nestedResource.id}/edit`]: i18n._(t`Edit Details`), - [`/inventories/${inventoryKind}/${inventory.id}/hosts/add`]: i18n._( - t`Create New Host` - ), - [`/inventories/${inventoryKind}/${inventory.id}/sources`]: i18n._( - t`Sources` - ), - [`/inventories/${inventoryKind}/${inventory.id}/groups`]: i18n._( - t`Groups` - ), + [`/inventories/${inventoryKind}/${inventory.id}/groups/add`]: i18n._( t`Create New Group` ), [`/inventories/${inventoryKind}/${inventory.id}/groups/${nestedResource && - nestedResource.id}`]: `${nestedResource && nestedResource.name}`, + nestedResource.id}/edit`]: i18n._(t`Edit Details`), [`/inventories/${inventoryKind}/${inventory.id}/groups/${nestedResource && nestedResource.id}/details`]: i18n._(t`Group Details`), [`/inventories/${inventoryKind}/${inventory.id}/groups/${nestedResource && - nestedResource.id}/edit`]: i18n._(t`Edit Details`), - [`/inventories/${inventoryKind}/${inventory.id}/hosts`]: i18n._(t`Hosts`), - [`/inventories/${inventoryKind}/${inventory.id}/sources`]: i18n._( - t`Sources` - ), - [`/inventories/${inventoryKind}/${inventory.id}/groups`]: i18n._( - t`Groups` - ), + nestedResource.id}`]: `${nestedResource && nestedResource.name}`, }; this.setState({ breadcrumbConfig }); }; diff --git a/awx/ui_next/src/screens/Inventory/Inventory.jsx b/awx/ui_next/src/screens/Inventory/Inventory.jsx index c2cb831331..edf8e088fc 100644 --- a/awx/ui_next/src/screens/Inventory/Inventory.jsx +++ b/awx/ui_next/src/screens/Inventory/Inventory.jsx @@ -63,7 +63,7 @@ function Inventory({ history, i18n, location, match, setBreadcrumb }) { location.pathname.endsWith('edit') || location.pathname.endsWith('add') || location.pathname.includes('groups/') || - history.location.pathname.includes(`/hosts/`) + location.pathname.includes('hosts/') ) { cardHeader = null; } diff --git a/awx/ui_next/src/screens/Inventory/InventoryGroup/InventoryGroup.jsx b/awx/ui_next/src/screens/Inventory/InventoryGroup/InventoryGroup.jsx index ab8ccace46..eed60f4254 100644 --- a/awx/ui_next/src/screens/Inventory/InventoryGroup/InventoryGroup.jsx +++ b/awx/ui_next/src/screens/Inventory/InventoryGroup/InventoryGroup.jsx @@ -1,8 +1,9 @@ import React, { useEffect, useState } from 'react'; import { t } from '@lingui/macro'; import { withI18n } from '@lingui/react'; - import { Switch, Route, withRouter, Link, Redirect } from 'react-router-dom'; +import { CaretLeftIcon } from '@patternfly/react-icons'; + import { GroupsAPI } from '@api'; import CardCloseButton from '@components/CardCloseButton'; import RoutedTabs from '@components/RoutedTabs'; @@ -40,7 +41,12 @@ function InventoryGroups({ i18n, match, setBreadcrumb, inventory, history }) { const tabsArray = [ { - name: i18n._(t`Return to Groups`), + name: ( + <> + + {i18n._(t`Back to Groups`)} + + ), link: `/inventories/inventory/${inventory.id}/groups`, id: 99, isNestedTab: true, diff --git a/awx/ui_next/src/screens/Inventory/InventoryGroup/InventoryGroup.test.jsx b/awx/ui_next/src/screens/Inventory/InventoryGroup/InventoryGroup.test.jsx index 585d18bc51..b7eb395a64 100644 --- a/awx/ui_next/src/screens/Inventory/InventoryGroup/InventoryGroup.test.jsx +++ b/awx/ui_next/src/screens/Inventory/InventoryGroup/InventoryGroup.test.jsx @@ -14,6 +14,8 @@ GroupsAPI.readDetail.mockResolvedValue({ name: 'Foo', description: 'Bar', variables: 'bizz: buzz', + created: '1/12/2019', + modified: '1/13/2019', summary_fields: { inventory: { id: 1 }, created_by: { id: 1, username: 'Athena' }, @@ -62,10 +64,10 @@ describe('', () => { test('renders successfully', async () => { expect(wrapper.length).toBe(1); }); - test('expect all tabs to exist, including Return to Groups', async () => { - expect(wrapper.find('button[aria-label="Return to Groups"]').length).toBe( - 1 - ); + test('expect all tabs to exist, including Back to Groups', async () => { + expect( + wrapper.find('button[link="/inventories/inventory/1/groups"]').length + ).toBe(1); expect(wrapper.find('button[aria-label="Details"]').length).toBe(1); expect(wrapper.find('button[aria-label="Related Groups"]').length).toBe(1); expect(wrapper.find('button[aria-label="Hosts"]').length).toBe(1); diff --git a/awx/ui_next/src/screens/Inventory/InventoryHosts/InventoryHosts.jsx b/awx/ui_next/src/screens/Inventory/InventoryHosts/InventoryHosts.jsx index c88362686a..cbb0b4d6d3 100644 --- a/awx/ui_next/src/screens/Inventory/InventoryHosts/InventoryHosts.jsx +++ b/awx/ui_next/src/screens/Inventory/InventoryHosts/InventoryHosts.jsx @@ -3,7 +3,7 @@ import { Switch, Route, withRouter } from 'react-router-dom'; import Host from '../../Host/Host'; import InventoryHostList from './InventoryHostList'; -import HostAdd from '../InventoryHostAdd'; +import InventoryHostAdd from '../InventoryHostAdd'; function InventoryHosts({ match, setBreadcrumb, i18n, inventory }) { return ( @@ -11,11 +11,11 @@ function InventoryHosts({ match, setBreadcrumb, i18n, inventory }) { } + render={() => } /> , ( ', () => { playbook: '', id: 1, verbosity: 1, + created: '1/12/2019', + modified: '1/13/2019', summary_fields: { user_capabilities: { edit: true }, created_by: { id: 1, username: 'Joe' }, From 33bc9e63c4115bb0d5ac2852bc36fbcedb54de25 Mon Sep 17 00:00:00 2001 From: Alex Corey Date: Fri, 10 Jan 2020 09:17:00 -0500 Subject: [PATCH 3/5] Addresses Console Errors related to functions and test data Also Adds speecificity to link URLs by add /details for urls that should redireect to details pages instead of them ending in /:id --- .../src/screens/Host/HostDetail/HostDetail.jsx | 14 ++------------ awx/ui_next/src/screens/Host/HostList/HostList.jsx | 2 +- .../Inventory/InventoryGroup/InventoryGroup.jsx | 1 - .../InventoryGroup/InventoryGroup.test.jsx | 2 -- .../JobTemplateDetail/JobTemplateDetail.test.jsx | 2 -- 5 files changed, 3 insertions(+), 18 deletions(-) diff --git a/awx/ui_next/src/screens/Host/HostDetail/HostDetail.jsx b/awx/ui_next/src/screens/Host/HostDetail/HostDetail.jsx index 282edbc471..2ca4fe86ab 100644 --- a/awx/ui_next/src/screens/Host/HostDetail/HostDetail.jsx +++ b/awx/ui_next/src/screens/Host/HostDetail/HostDetail.jsx @@ -14,16 +14,6 @@ import DeleteButton from '@components/DeleteButton'; import Switch from '@components/Switch'; import { HostsAPI } from '@api'; -function HostDetail({ host, i18n }) { -const ActionButtonWrapper = styled.div` - display: flex; - justify-content: flex-end; - margin-top: 20px; - & > :not(:first-child) { - margin-left: 20px; - } -`; - function HostDetail({ host, history, match, i18n, onUpdateHost }) { const { created, description, id, modified, name, summary_fields } = host; @@ -150,7 +140,6 @@ function HostDetail({ host, history, match, i18n, onUpdateHost }) { {i18n._(t`Edit`)} )} - {summary_fields.user_capabilities && summary_fields.user_capabilities.delete && ( )} + ); } @@ -167,4 +157,4 @@ HostDetail.propTypes = { host: Host.isRequired, }; -export default withI18n()(HostDetail); +export default withI18n()(withRouter(HostDetail)); diff --git a/awx/ui_next/src/screens/Host/HostList/HostList.jsx b/awx/ui_next/src/screens/Host/HostList/HostList.jsx index 978ff2ba59..026714728f 100644 --- a/awx/ui_next/src/screens/Host/HostList/HostList.jsx +++ b/awx/ui_next/src/screens/Host/HostList/HostList.jsx @@ -235,7 +235,7 @@ class HostsList extends Component { row.id === o.id)} onSelect={() => this.handleSelect(o)} onToggleHost={this.handleHostToggle} diff --git a/awx/ui_next/src/screens/Inventory/InventoryGroup/InventoryGroup.jsx b/awx/ui_next/src/screens/Inventory/InventoryGroup/InventoryGroup.jsx index eed60f4254..f23455558a 100644 --- a/awx/ui_next/src/screens/Inventory/InventoryGroup/InventoryGroup.jsx +++ b/awx/ui_next/src/screens/Inventory/InventoryGroup/InventoryGroup.jsx @@ -49,7 +49,6 @@ function InventoryGroups({ i18n, match, setBreadcrumb, inventory, history }) { ), link: `/inventories/inventory/${inventory.id}/groups`, id: 99, - isNestedTab: true, }, { name: i18n._(t`Details`), diff --git a/awx/ui_next/src/screens/Inventory/InventoryGroup/InventoryGroup.test.jsx b/awx/ui_next/src/screens/Inventory/InventoryGroup/InventoryGroup.test.jsx index b7eb395a64..9b5afe74bd 100644 --- a/awx/ui_next/src/screens/Inventory/InventoryGroup/InventoryGroup.test.jsx +++ b/awx/ui_next/src/screens/Inventory/InventoryGroup/InventoryGroup.test.jsx @@ -14,8 +14,6 @@ GroupsAPI.readDetail.mockResolvedValue({ name: 'Foo', description: 'Bar', variables: 'bizz: buzz', - created: '1/12/2019', - modified: '1/13/2019', summary_fields: { inventory: { id: 1 }, created_by: { id: 1, username: 'Athena' }, diff --git a/awx/ui_next/src/screens/Template/JobTemplateDetail/JobTemplateDetail.test.jsx b/awx/ui_next/src/screens/Template/JobTemplateDetail/JobTemplateDetail.test.jsx index 033da2b338..a06d1e8db5 100644 --- a/awx/ui_next/src/screens/Template/JobTemplateDetail/JobTemplateDetail.test.jsx +++ b/awx/ui_next/src/screens/Template/JobTemplateDetail/JobTemplateDetail.test.jsx @@ -17,8 +17,6 @@ describe('', () => { playbook: '', id: 1, verbosity: 1, - created: '1/12/2019', - modified: '1/13/2019', summary_fields: { user_capabilities: { edit: true }, created_by: { id: 1, username: 'Joe' }, From dfa578fcde1b0c18cbca0bb455e2094b39dc3d5d Mon Sep 17 00:00:00 2001 From: Alex Corey Date: Mon, 13 Jan 2020 09:40:31 -0500 Subject: [PATCH 4/5] Utilizes React Router Hooks and removes No-op function --- .../components/AddRole/SelectResourceStep.jsx | 2 +- .../screens/Host/HostDetail/HostDetail.jsx | 102 ++++++++++-------- 2 files changed, 59 insertions(+), 45 deletions(-) diff --git a/awx/ui_next/src/components/AddRole/SelectResourceStep.jsx b/awx/ui_next/src/components/AddRole/SelectResourceStep.jsx index e208686ab5..66bbbc634a 100644 --- a/awx/ui_next/src/components/AddRole/SelectResourceStep.jsx +++ b/awx/ui_next/src/components/AddRole/SelectResourceStep.jsx @@ -116,7 +116,7 @@ class SelectResourceStep extends React.Component { name={item[displayKey]} label={item[displayKey]} onSelect={() => onRowClick(item)} - onDeselect={() => {}} + onDeselect={() => onRowClick(item)} /> )} renderToolbar={props => } diff --git a/awx/ui_next/src/screens/Host/HostDetail/HostDetail.jsx b/awx/ui_next/src/screens/Host/HostDetail/HostDetail.jsx index 2ca4fe86ab..8f402ead06 100644 --- a/awx/ui_next/src/screens/Host/HostDetail/HostDetail.jsx +++ b/awx/ui_next/src/screens/Host/HostDetail/HostDetail.jsx @@ -1,5 +1,5 @@ import React, { useState } from 'react'; -import { Link, withRouter } from 'react-router-dom'; +import { Link, useHistory, useParams, useLocation } from 'react-router-dom'; import { withI18n } from '@lingui/react'; import { t } from '@lingui/macro'; import { Host } from '@types'; @@ -14,9 +14,27 @@ import DeleteButton from '@components/DeleteButton'; import Switch from '@components/Switch'; import { HostsAPI } from '@api'; -function HostDetail({ host, history, match, i18n, onUpdateHost }) { - const { created, description, id, modified, name, summary_fields } = host; +function HostDetail({ host, i18n, onUpdateHost }) { + const { + created, + description, + id, + modified, + name, + enabled, + summary_fields: { + inventory, + recent_jobs, + kind, + created_by, + modified_by, + user_capabilities, + }, + } = host; + const history = useHistory(); + const { pathname } = useLocation(); + const { id: inventoryId, hostId: inventoryHostId } = useParams(); const [isLoading, setIsloading] = useState(false); const [deletionError, setDeletionError] = useState(false); const [toggleLoading, setToggleLoading] = useState(false); @@ -25,8 +43,8 @@ function HostDetail({ host, history, match, i18n, onUpdateHost }) { const handleHostToggle = async () => { setToggleLoading(true); try { - const { data } = await HostsAPI.update(host.id, { - enabled: !host.enabled, + const { data } = await HostsAPI.update(id, { + enabled: !enabled, }); onUpdateHost(data); } catch (err) { @@ -39,9 +57,9 @@ function HostDetail({ host, history, match, i18n, onUpdateHost }) { const handleHostDelete = async () => { setIsloading(true); try { - await HostsAPI.destroy(host.id); + await HostsAPI.destroy(id); setIsloading(false); - history.push(`/inventories/inventory/${match.params.id}/hosts`); + history.push(`/inventories/inventory/${inventoryId}/hosts`); } catch (err) { setDeletionError(err); } @@ -68,7 +86,7 @@ function HostDetail({ host, history, match, i18n, onUpdateHost }) { title={i18n._(t`Error!`)} onClose={() => setDeletionError(false)} > - {i18n._(t`Failed to delete ${host.name}.`)} + {i18n._(t`Failed to delete ${name}.`)} ); @@ -76,12 +94,12 @@ function HostDetail({ host, history, match, i18n, onUpdateHost }) { return ( handleHostToggle()} aria-label={i18n._(t`Toggle Host`)} /> @@ -89,22 +107,20 @@ function HostDetail({ host, history, match, i18n, onUpdateHost }) { } + value={} label={i18n._(t`Activity`)} /> - {summary_fields.inventory && ( + {inventory && ( - {summary_fields.inventory.name} + {inventory.name} } /> @@ -112,11 +128,11 @@ function HostDetail({ host, history, match, i18n, onUpdateHost }) { - {summary_fields.user_capabilities && - summary_fields.user_capabilities.edit && ( - - )} - {summary_fields.user_capabilities && - summary_fields.user_capabilities.delete && ( - handleHostDelete()} - modalTitle={i18n._(t`Delete Host`)} - name={host.name} - /> - )} + {user_capabilities && user_capabilities.edit && ( + + )} + {user_capabilities && user_capabilities.delete && ( + handleHostDelete()} + modalTitle={i18n._(t`Delete Host`)} + name={host.name} + /> + )} ); @@ -157,4 +171,4 @@ HostDetail.propTypes = { host: Host.isRequired, }; -export default withI18n()(withRouter(HostDetail)); +export default withI18n()(HostDetail); From 8bfcef01df4b2ae73a476b1d607403578670100d Mon Sep 17 00:00:00 2001 From: Alex Corey Date: Mon, 13 Jan 2020 11:27:23 -0500 Subject: [PATCH 5/5] Fixes Breaedcrumb --- .../src/screens/Host/HostList/HostList.jsx | 140 +++++++++--------- .../src/screens/Inventory/Inventories.jsx | 3 + 2 files changed, 72 insertions(+), 71 deletions(-) diff --git a/awx/ui_next/src/screens/Host/HostList/HostList.jsx b/awx/ui_next/src/screens/Host/HostList/HostList.jsx index 026714728f..36ddd8f0dc 100644 --- a/awx/ui_next/src/screens/Host/HostList/HostList.jsx +++ b/awx/ui_next/src/screens/Host/HostList/HostList.jsx @@ -2,7 +2,7 @@ import React, { Component, Fragment } from 'react'; import { withRouter } from 'react-router-dom'; import { withI18n } from '@lingui/react'; import { t } from '@lingui/macro'; -import { Card, PageSection } from '@patternfly/react-core'; +import { Card } from '@patternfly/react-core'; import { HostsAPI } from '@api'; import AlertModal from '@components/AlertModal'; @@ -180,76 +180,74 @@ class HostsList extends Component { return ( - - - ( - , - canAdd ? ( - - ) : null, - ]} - /> - )} - renderItem={o => ( - row.id === o.id)} - onSelect={() => this.handleSelect(o)} - onToggleHost={this.handleHostToggle} - toggleLoading={toggleLoading === o.id} - /> - )} - emptyStateControls={ - canAdd ? ( - - ) : null - } - /> - - + + ( + , + canAdd ? ( + + ) : null, + ]} + /> + )} + renderItem={o => ( + row.id === o.id)} + onSelect={() => this.handleSelect(o)} + onToggleHost={this.handleHostToggle} + toggleLoading={toggleLoading === o.id} + /> + )} + emptyStateControls={ + canAdd ? ( + + ) : null + } + /> + {toggleError && !toggleLoading && (