From ea4e98c52a015f4e286a1d5accd9dabe94e5cc34 Mon Sep 17 00:00:00 2001 From: Marliana Lara Date: Wed, 20 Nov 2019 13:11:06 -0500 Subject: [PATCH 1/4] Move Switch into shared component directory and update tests --- .../NotificationList.test.jsx | 18 ++--- .../NotificationList/NotificationListItem.jsx | 9 +-- .../NotificationListItem.test.jsx | 15 ++-- .../NotificationListItem.test.jsx.snap | 72 +++++++++---------- awx/ui_next/src/components/Switch/Switch.jsx | 10 +++ awx/ui_next/src/components/Switch/index.js | 1 + .../screens/Host/HostList/HostListItem.jsx | 11 +-- 7 files changed, 60 insertions(+), 76 deletions(-) create mode 100644 awx/ui_next/src/components/Switch/Switch.jsx create mode 100644 awx/ui_next/src/components/Switch/index.js diff --git a/awx/ui_next/src/components/NotificationList/NotificationList.test.jsx b/awx/ui_next/src/components/NotificationList/NotificationList.test.jsx index 3f42d47c04..bbc0a603b8 100644 --- a/awx/ui_next/src/components/NotificationList/NotificationList.test.jsx +++ b/awx/ui_next/src/components/NotificationList/NotificationList.test.jsx @@ -123,8 +123,7 @@ describe('', () => { const items = wrapper.find('NotificationListItem'); items .at(1) - .find('Switch') - .at(1) + .find('PFSwitch[aria-label="Toggle notification success"]') .prop('onChange')(); expect(MockModelAPI.associateNotificationTemplate).toHaveBeenCalledWith( 1, @@ -151,8 +150,7 @@ describe('', () => { const items = wrapper.find('NotificationListItem'); items .at(0) - .find('Switch') - .at(2) + .find('PFSwitch[aria-label="Toggle notification failure"]') .prop('onChange')(); expect(MockModelAPI.associateNotificationTemplate).toHaveBeenCalledWith( 1, @@ -180,8 +178,7 @@ describe('', () => { const items = wrapper.find('NotificationListItem'); items .at(0) - .find('Switch') - .at(0) + .find('PFSwitch[aria-label="Toggle notification start"]') .prop('onChange')(); expect(MockModelAPI.associateNotificationTemplate).toHaveBeenCalledWith( 1, @@ -208,8 +205,7 @@ describe('', () => { const items = wrapper.find('NotificationListItem'); items .at(0) - .find('Switch') - .at(1) + .find('PFSwitch[aria-label="Toggle notification success"]') .prop('onChange')(); expect(MockModelAPI.disassociateNotificationTemplate).toHaveBeenCalledWith( 1, @@ -236,8 +232,7 @@ describe('', () => { const items = wrapper.find('NotificationListItem'); items .at(1) - .find('Switch') - .at(2) + .find('PFSwitch[aria-label="Toggle notification failure"]') .prop('onChange')(); expect(MockModelAPI.disassociateNotificationTemplate).toHaveBeenCalledWith( 1, @@ -264,8 +259,7 @@ describe('', () => { const items = wrapper.find('NotificationListItem'); items .at(2) - .find('Switch') - .at(0) + .find('PFSwitch[aria-label="Toggle notification start"]') .prop('onChange')(); expect(MockModelAPI.disassociateNotificationTemplate).toHaveBeenCalledWith( 1, diff --git a/awx/ui_next/src/components/NotificationList/NotificationListItem.jsx b/awx/ui_next/src/components/NotificationList/NotificationListItem.jsx index f32e8a80e7..be62aa4b0d 100644 --- a/awx/ui_next/src/components/NotificationList/NotificationListItem.jsx +++ b/awx/ui_next/src/components/NotificationList/NotificationListItem.jsx @@ -4,12 +4,12 @@ import { withI18n } from '@lingui/react'; import { t } from '@lingui/macro'; import { Link } from 'react-router-dom'; import { - Switch as PFSwitch, DataListItem, DataListItemRow, DataListItemCells, DataListCell as PFDataListCell, } from '@patternfly/react-core'; +import Switch from '@components/Switch'; import styled from 'styled-components'; @@ -24,13 +24,6 @@ const DataListCell = styled(PFDataListCell)` } `; -const Switch = styled(PFSwitch)` - display: flex; - flex-wrap: no-wrap; - /* workaround PF bug; used in calculating switch width: */ - --pf-c-switch__toggle-icon--Offset: 0.125rem; -`; - function NotificationListItem(props) { const { canToggleNotifications, diff --git a/awx/ui_next/src/components/NotificationList/NotificationListItem.test.jsx b/awx/ui_next/src/components/NotificationList/NotificationListItem.test.jsx index 4646023814..d954bb51b2 100644 --- a/awx/ui_next/src/components/NotificationList/NotificationListItem.test.jsx +++ b/awx/ui_next/src/components/NotificationList/NotificationListItem.test.jsx @@ -89,8 +89,7 @@ describe('', () => { /> ); wrapper - .find('Switch') - .first() + .find('Switch[aria-label="Toggle notification start"]') .find('input') .simulate('change'); expect(toggleNotification).toHaveBeenCalledWith(9000, false, 'started'); @@ -108,8 +107,7 @@ describe('', () => { /> ); wrapper - .find('Switch') - .at(1) + .find('Switch[aria-label="Toggle notification success"]') .find('input') .simulate('change'); expect(toggleNotification).toHaveBeenCalledWith(9000, true, 'success'); @@ -127,8 +125,7 @@ describe('', () => { /> ); wrapper - .find('Switch') - .at(1) + .find('Switch[aria-label="Toggle notification success"]') .find('input') .simulate('change'); expect(toggleNotification).toHaveBeenCalledWith(9000, false, 'success'); @@ -146,8 +143,7 @@ describe('', () => { /> ); wrapper - .find('Switch') - .at(2) + .find('Switch[aria-label="Toggle notification failure"]') .find('input') .simulate('change'); expect(toggleNotification).toHaveBeenCalledWith(9000, true, 'error'); @@ -165,8 +161,7 @@ describe('', () => { /> ); wrapper - .find('Switch') - .at(2) + .find('Switch[aria-label="Toggle notification failure"]') .find('input') .simulate('change'); expect(toggleNotification).toHaveBeenCalledWith(9000, false, 'error'); diff --git a/awx/ui_next/src/components/NotificationList/__snapshots__/NotificationListItem.test.jsx.snap b/awx/ui_next/src/components/NotificationList/__snapshots__/NotificationListItem.test.jsx.snap index 6b8d98afcc..8498fd2b08 100644 --- a/awx/ui_next/src/components/NotificationList/__snapshots__/NotificationListItem.test.jsx.snap +++ b/awx/ui_next/src/components/NotificationList/__snapshots__/NotificationListItem.test.jsx.snap @@ -313,7 +313,7 @@ exports[` initially renders succe className="pf-c-data-list__cell NotificationListItem__DataListCell-w674ng-0 dXsFLF" righthalf="true" > - initially renders succe "$$typeof": Symbol(react.forward_ref), "attrs": Array [], "componentStyle": ComponentStyle { - "componentId": "NotificationListItem__Switch-w674ng-1", + "componentId": "Switch-sc-1xwas62-0", "isStatic": true, - "lastClassName": "hfzRow", + "lastClassName": "eJQXYh", "rules": Array [ "display:flex;flex-wrap:no-wrap;--pf-c-switch__toggle-icon--Offset:0.125rem;", ], }, - "displayName": "NotificationListItem__Switch", + "displayName": "Switch", "foldedComponentIds": Array [], "render": [Function], - "styledComponentId": "NotificationListItem__Switch-w674ng-1", + "styledComponentId": "Switch-sc-1xwas62-0", "target": [Function], "toString": [Function], "warnTooManyClasses": [Function], @@ -354,9 +354,9 @@ exports[` initially renders succe labelOff="Start" onChange={[Function]} > - initially renders succe componentProps={ Object { "aria-label": "Toggle notification start", - "className": "NotificationListItem__Switch-w674ng-1 hfzRow", + "className": "Switch-sc-1xwas62-0 eJQXYh", "id": "notification-9000-started-toggle", "isChecked": false, "isDisabled": false, @@ -382,7 +382,7 @@ exports[` initially renders succe > initially renders succe } > - + - - + initially renders succe "$$typeof": Symbol(react.forward_ref), "attrs": Array [], "componentStyle": ComponentStyle { - "componentId": "NotificationListItem__Switch-w674ng-1", + "componentId": "Switch-sc-1xwas62-0", "isStatic": true, - "lastClassName": "hfzRow", + "lastClassName": "eJQXYh", "rules": Array [ "display:flex;flex-wrap:no-wrap;--pf-c-switch__toggle-icon--Offset:0.125rem;", ], }, - "displayName": "NotificationListItem__Switch", + "displayName": "Switch", "foldedComponentIds": Array [], "render": [Function], - "styledComponentId": "NotificationListItem__Switch-w674ng-1", + "styledComponentId": "Switch-sc-1xwas62-0", "target": [Function], "toString": [Function], "warnTooManyClasses": [Function], @@ -474,9 +474,9 @@ exports[` initially renders succe labelOff="Success" onChange={[Function]} > - initially renders succe componentProps={ Object { "aria-label": "Toggle notification success", - "className": "NotificationListItem__Switch-w674ng-1 hfzRow", + "className": "Switch-sc-1xwas62-0 eJQXYh", "id": "notification-9000-success-toggle", "isChecked": false, "isDisabled": false, @@ -502,7 +502,7 @@ exports[` initially renders succe > initially renders succe } > - + - - + initially renders succe "$$typeof": Symbol(react.forward_ref), "attrs": Array [], "componentStyle": ComponentStyle { - "componentId": "NotificationListItem__Switch-w674ng-1", + "componentId": "Switch-sc-1xwas62-0", "isStatic": true, - "lastClassName": "hfzRow", + "lastClassName": "eJQXYh", "rules": Array [ "display:flex;flex-wrap:no-wrap;--pf-c-switch__toggle-icon--Offset:0.125rem;", ], }, - "displayName": "NotificationListItem__Switch", + "displayName": "Switch", "foldedComponentIds": Array [], "render": [Function], - "styledComponentId": "NotificationListItem__Switch-w674ng-1", + "styledComponentId": "Switch-sc-1xwas62-0", "target": [Function], "toString": [Function], "warnTooManyClasses": [Function], @@ -594,9 +594,9 @@ exports[` initially renders succe labelOff="Failure" onChange={[Function]} > - initially renders succe componentProps={ Object { "aria-label": "Toggle notification failure", - "className": "NotificationListItem__Switch-w674ng-1 hfzRow", + "className": "Switch-sc-1xwas62-0 eJQXYh", "id": "notification-9000-error-toggle", "isChecked": false, "isDisabled": false, @@ -622,7 +622,7 @@ exports[` initially renders succe > initially renders succe } > - + - + diff --git a/awx/ui_next/src/components/Switch/Switch.jsx b/awx/ui_next/src/components/Switch/Switch.jsx new file mode 100644 index 0000000000..f62376e61b --- /dev/null +++ b/awx/ui_next/src/components/Switch/Switch.jsx @@ -0,0 +1,10 @@ +import { Switch } from '@patternfly/react-core'; +import styled from 'styled-components'; + +Switch.displayName = 'PFSwitch'; +export default styled(Switch)` + display: flex; + flex-wrap: no-wrap; + /* workaround PF bug; used in calculating switch width: */ + --pf-c-switch__toggle-icon--Offset: 0.125rem; +`; diff --git a/awx/ui_next/src/components/Switch/index.js b/awx/ui_next/src/components/Switch/index.js new file mode 100644 index 0000000000..ed80f23e5b --- /dev/null +++ b/awx/ui_next/src/components/Switch/index.js @@ -0,0 +1 @@ +export { default } from './Switch'; diff --git a/awx/ui_next/src/screens/Host/HostList/HostListItem.jsx b/awx/ui_next/src/screens/Host/HostList/HostListItem.jsx index 4cd624da01..acb2eb537c 100644 --- a/awx/ui_next/src/screens/Host/HostList/HostListItem.jsx +++ b/awx/ui_next/src/screens/Host/HostList/HostListItem.jsx @@ -6,7 +6,6 @@ import { DataListItem, DataListItemRow, DataListItemCells, - Switch as PFSwitch, Tooltip, } from '@patternfly/react-core'; import { Link } from 'react-router-dom'; @@ -17,18 +16,10 @@ import DataListCell from '@components/DataListCell'; import DataListCheck from '@components/DataListCheck'; import ListActionButton from '@components/ListActionButton'; import { Sparkline } from '@components/Sparkline'; +import Switch from '@components/Switch'; import VerticalSeparator from '@components/VerticalSeparator'; import { Host } from '@types'; -import styled from 'styled-components'; - -const Switch = styled(PFSwitch)` - display: flex; - flex-wrap: no-wrap; - /* workaround PF bug; used in calculating switch width: */ - --pf-c-switch__toggle-icon--Offset: 0.125rem; -`; - class HostListItem extends React.Component { static propTypes = { host: Host.isRequired, From fa144aa98f3a397a249048a203006fdac80acaaa Mon Sep 17 00:00:00 2001 From: Marliana Lara Date: Wed, 20 Nov 2019 13:14:28 -0500 Subject: [PATCH 2/4] Add Inventory Host list and unit tests * Add Inventory Host Add route * Fix host disabled loading switch bug --- awx/ui_next/src/api/models/Inventories.js | 4 + .../src/screens/Host/HostList/HostList.jsx | 8 +- .../src/screens/Host/HostList/index.js | 1 + awx/ui_next/src/screens/Host/Hosts.jsx | 6 +- .../src/screens/Inventory/Inventories.jsx | 3 + .../src/screens/Inventory/Inventory.jsx | 11 +- .../InventoryHostAdd/InventoryHostAdd.jsx | 8 + .../Inventory/InventoryHostAdd/index.js | 1 + .../InventoryHosts/InventoryHostItem.jsx | 105 +++++++ .../InventoryHosts/InventoryHostItem.test.jsx | 80 +++++ .../InventoryHosts/InventoryHosts.jsx | 229 +++++++++++++- .../InventoryHosts/InventoryHosts.test.jsx | 279 ++++++++++++++++++ 12 files changed, 720 insertions(+), 15 deletions(-) create mode 100644 awx/ui_next/src/screens/Inventory/InventoryHostAdd/InventoryHostAdd.jsx create mode 100644 awx/ui_next/src/screens/Inventory/InventoryHostAdd/index.js create mode 100644 awx/ui_next/src/screens/Inventory/InventoryHosts/InventoryHostItem.jsx create mode 100644 awx/ui_next/src/screens/Inventory/InventoryHosts/InventoryHostItem.test.jsx create mode 100644 awx/ui_next/src/screens/Inventory/InventoryHosts/InventoryHosts.test.jsx diff --git a/awx/ui_next/src/api/models/Inventories.js b/awx/ui_next/src/api/models/Inventories.js index cdd30d4e6f..f78a83151e 100644 --- a/awx/ui_next/src/api/models/Inventories.js +++ b/awx/ui_next/src/api/models/Inventories.js @@ -11,6 +11,10 @@ class Inventories extends Base { readAccessList(id, params) { return this.http.get(`${this.baseUrl}${id}/access_list/`, { params }); } + + readHosts(id, params) { + return this.http.get(`${this.baseUrl}${id}/hosts/`, { params }); + } } export default Inventories; diff --git a/awx/ui_next/src/screens/Host/HostList/HostList.jsx b/awx/ui_next/src/screens/Host/HostList/HostList.jsx index 36da7a3535..d5fa4b8f03 100644 --- a/awx/ui_next/src/screens/Host/HostList/HostList.jsx +++ b/awx/ui_next/src/screens/Host/HostList/HostList.jsx @@ -35,7 +35,7 @@ class HostsList extends Component { itemCount: 0, actions: null, toggleError: false, - toggleLoading: false, + toggleLoading: null, }; this.handleSelectAll = this.handleSelectAll.bind(this); @@ -101,7 +101,7 @@ class HostsList extends Component { async handleHostToggle(hostToToggle) { const { hosts } = this.state; - this.setState({ toggleLoading: true }); + this.setState({ toggleLoading: hostToToggle.id }); try { const { data: updatedHost } = await HostsAPI.update(hostToToggle.id, { enabled: !hostToToggle.enabled, @@ -114,7 +114,7 @@ class HostsList extends Component { } catch (err) { this.setState({ toggleError: true }); } finally { - this.setState({ toggleLoading: false }); + this.setState({ toggleLoading: null }); } } @@ -237,7 +237,7 @@ class HostsList extends Component { isSelected={selected.some(row => row.id === o.id)} onSelect={() => this.handleSelect(o)} toggleHost={this.handleHostToggle} - toggleLoading={toggleLoading} + toggleLoading={toggleLoading === o.id} /> )} emptyStateControls={ diff --git a/awx/ui_next/src/screens/Host/HostList/index.js b/awx/ui_next/src/screens/Host/HostList/index.js index e69de29bb2..2e40cbabb7 100644 --- a/awx/ui_next/src/screens/Host/HostList/index.js +++ b/awx/ui_next/src/screens/Host/HostList/index.js @@ -0,0 +1 @@ +export { default } from './HostList'; diff --git a/awx/ui_next/src/screens/Host/Hosts.jsx b/awx/ui_next/src/screens/Host/Hosts.jsx index d647e56c3d..0a17916223 100644 --- a/awx/ui_next/src/screens/Host/Hosts.jsx +++ b/awx/ui_next/src/screens/Host/Hosts.jsx @@ -6,8 +6,8 @@ import { t } from '@lingui/macro'; import { Config } from '@contexts/Config'; import Breadcrumbs from '@components/Breadcrumbs/Breadcrumbs'; -import HostsList from './HostList/HostList'; -import HostAdd from './HostAdd/HostAdd'; +import HostList from './HostList'; +import HostAdd from './HostAdd'; import Host from './Host'; class Hosts extends Component { @@ -69,7 +69,7 @@ class Hosts extends Component { )} /> - } /> + } /> ); diff --git a/awx/ui_next/src/screens/Inventory/Inventories.jsx b/awx/ui_next/src/screens/Inventory/Inventories.jsx index bf669f094d..1d72507c03 100644 --- a/awx/ui_next/src/screens/Inventory/Inventories.jsx +++ b/awx/ui_next/src/screens/Inventory/Inventories.jsx @@ -52,6 +52,9 @@ class Inventories extends Component { t`Completed Jobs` ), [`/inventories/${inventoryKind}/${inventory.id}/hosts`]: i18n._(t`Hosts`), + [`/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`), }; diff --git a/awx/ui_next/src/screens/Inventory/Inventory.jsx b/awx/ui_next/src/screens/Inventory/Inventory.jsx index 44d281f320..54b5ac2298 100644 --- a/awx/ui_next/src/screens/Inventory/Inventory.jsx +++ b/awx/ui_next/src/screens/Inventory/Inventory.jsx @@ -9,6 +9,7 @@ import RoutedTabs from '@components/RoutedTabs'; import { ResourceAccessList } from '@components/ResourceAccessList'; import InventoryDetail from './InventoryDetail'; import InventoryHosts from './InventoryHosts'; +import InventoryHostAdd from './InventoryHostAdd'; import InventoryGroups from './InventoryGroups'; import InventoryCompletedJobs from './InventoryCompletedJobs'; import InventorySources from './InventorySources'; @@ -84,7 +85,10 @@ class Inventory extends Component { ); - if (location.pathname.endsWith('edit')) { + if ( + location.pathname.endsWith('edit') || + location.pathname.endsWith('add') + ) { cardHeader = null; } @@ -134,6 +138,11 @@ class Inventory extends Component { path="/inventories/inventory/:id/edit" render={() => } />, + } + />, Coming soon :); +} + +export default InventoryHostAdd; diff --git a/awx/ui_next/src/screens/Inventory/InventoryHostAdd/index.js b/awx/ui_next/src/screens/Inventory/InventoryHostAdd/index.js new file mode 100644 index 0000000000..56bb7e05ad --- /dev/null +++ b/awx/ui_next/src/screens/Inventory/InventoryHostAdd/index.js @@ -0,0 +1 @@ +export { default } from './InventoryHostAdd'; diff --git a/awx/ui_next/src/screens/Inventory/InventoryHosts/InventoryHostItem.jsx b/awx/ui_next/src/screens/Inventory/InventoryHosts/InventoryHostItem.jsx new file mode 100644 index 0000000000..27bd8fa5f6 --- /dev/null +++ b/awx/ui_next/src/screens/Inventory/InventoryHosts/InventoryHostItem.jsx @@ -0,0 +1,105 @@ +import React from 'react'; +import { string, bool, func } from 'prop-types'; +import { withI18n } from '@lingui/react'; +import { t } from '@lingui/macro'; +import { + DataListItem, + DataListItemRow, + DataListItemCells, + Tooltip, +} from '@patternfly/react-core'; +import { Link } from 'react-router-dom'; +import { PencilAltIcon } from '@patternfly/react-icons'; + +import ActionButtonCell from '@components/ActionButtonCell'; +import DataListCell from '@components/DataListCell'; +import DataListCheck from '@components/DataListCheck'; +import ListActionButton from '@components/ListActionButton'; +import { Sparkline } from '@components/Sparkline'; +import Switch from '@components/Switch'; +import VerticalSeparator from '@components/VerticalSeparator'; +import { Host } from '@types'; + +function InventoryHostItem(props) { + const { + detailUrl, + host, + i18n, + isSelected, + onSelect, + toggleHost, + toggleLoading, + } = props; + + const labelId = `check-action-${host.id}`; + + return ( + + + + + + + {host.name} + + , + + + , + + + toggleHost(host)} + aria-label={i18n._(t`Toggle host`)} + /> + + {host.summary_fields.user_capabilities.edit && ( + + + + + + )} + , + ]} + /> + + + ); +} + +InventoryHostItem.propTypes = { + detailUrl: string.isRequired, + host: Host.isRequired, + isSelected: bool.isRequired, + onSelect: func.isRequired, + toggleHost: func.isRequired, + toggleLoading: bool.isRequired, +}; + +export default withI18n()(InventoryHostItem); diff --git a/awx/ui_next/src/screens/Inventory/InventoryHosts/InventoryHostItem.test.jsx b/awx/ui_next/src/screens/Inventory/InventoryHosts/InventoryHostItem.test.jsx new file mode 100644 index 0000000000..38711ac149 --- /dev/null +++ b/awx/ui_next/src/screens/Inventory/InventoryHosts/InventoryHostItem.test.jsx @@ -0,0 +1,80 @@ +import React from 'react'; +import { mountWithContexts } from '@testUtils/enzymeHelpers'; +import InventoryHostItem from './InventoryHostItem'; + +let toggleHost; + +const mockHost = { + id: 1, + name: 'Host 1', + url: '/api/v2/hosts/1', + inventory: 1, + summary_fields: { + inventory: { + id: 1, + name: 'Inv 1', + }, + user_capabilities: { + edit: true, + }, + }, +}; + +describe.only('', () => { + beforeEach(() => { + toggleHost = jest.fn(); + }); + + afterEach(() => { + jest.clearAllMocks(); + }); + + test('edit button shown to users with edit capabilities', () => { + const wrapper = mountWithContexts( + {}} + host={mockHost} + toggleHost={toggleHost} + toggleLoading={false} + /> + ); + expect(wrapper.find('PencilAltIcon').exists()).toBeTruthy(); + }); + + test('edit button hidden from users without edit capabilities', () => { + const copyMockHost = Object.assign({}, mockHost); + copyMockHost.summary_fields.user_capabilities.edit = false; + const wrapper = mountWithContexts( + {}} + host={copyMockHost} + toggleHost={toggleHost} + toggleLoading={false} + /> + ); + expect(wrapper.find('PencilAltIcon').exists()).toBeFalsy(); + }); + + test('handles toggle click when host is enabled', () => { + const wrapper = mountWithContexts( + {}} + host={mockHost} + toggleHost={toggleHost} + toggleLoading={false} + /> + ); + wrapper + .find('Switch') + .first() + .find('input') + .simulate('change'); + expect(toggleHost).toHaveBeenCalledWith(mockHost); + }); +}); diff --git a/awx/ui_next/src/screens/Inventory/InventoryHosts/InventoryHosts.jsx b/awx/ui_next/src/screens/Inventory/InventoryHosts/InventoryHosts.jsx index ba26d61975..f35d87ae1c 100644 --- a/awx/ui_next/src/screens/Inventory/InventoryHosts/InventoryHosts.jsx +++ b/awx/ui_next/src/screens/Inventory/InventoryHosts/InventoryHosts.jsx @@ -1,10 +1,225 @@ -import React, { Component } from 'react'; -import { CardBody } from '@patternfly/react-core'; +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'; -class InventoryHosts extends Component { - render() { - return Coming soon :); - } +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, inventory }) { + 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(inventory.id, location.search), + InventoriesAPI.readOptions(), + ]); + + setHosts(results); + setHostCount(count); + setActions(optionActions); + } catch (error) { + setContentError(error); + } finally { + setIsLoading(false); + } + } + + fetchData(); + }, [inventory, 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(inventory.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 ? ( + + ) : null, + ]} + /> + )} + renderItem={o => ( + row.id === o.id)} + onSelect={() => handleSelect(o)} + toggleHost={handleToggle} + toggleLoading={toggleLoading === o.id} + /> + )} + emptyStateControls={ + canAdd ? ( + + ) : null + } + /> + + {toggleError && !toggleLoading && ( + setToggleError(false)} + > + {i18n._(t`Failed to toggle host.`)} + + + )} + + {deletionError && ( + setDeletionError(null)} + > + {i18n._(t`Failed to delete one or more hosts.`)} + + + )} + + ); } -export default InventoryHosts; +export default withI18n()(withRouter(InventoryHosts)); diff --git a/awx/ui_next/src/screens/Inventory/InventoryHosts/InventoryHosts.test.jsx b/awx/ui_next/src/screens/Inventory/InventoryHosts/InventoryHosts.test.jsx new file mode 100644 index 0000000000..59e339722d --- /dev/null +++ b/awx/ui_next/src/screens/Inventory/InventoryHosts/InventoryHosts.test.jsx @@ -0,0 +1,279 @@ +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 mockInventory from '../shared/data.inventory.json'; + +jest.mock('@api'); + +const mockHosts = [ + { + id: 1, + name: 'Host 1', + url: '/api/v2/hosts/1', + inventory: 1, + enabled: true, + summary_fields: { + inventory: { + id: 1, + name: 'inv 1', + }, + user_capabilities: { + delete: true, + update: true, + }, + }, + }, + { + id: 2, + name: 'Host 2', + url: '/api/v2/hosts/2', + inventory: 1, + enabled: true, + summary_fields: { + inventory: { + id: 1, + name: 'inv 1', + }, + user_capabilities: { + edit: true, + delete: true, + update: true, + }, + }, + }, + { + id: 3, + name: 'Host 3', + url: '/api/v2/hosts/3', + inventory: 1, + enabled: true, + summary_fields: { + inventory: { + id: 1, + name: 'inv 1', + }, + user_capabilities: { + delete: false, + update: false, + }, + }, + }, +]; + +describe('', () => { + let wrapper; + + beforeEach(async () => { + InventoriesAPI.readHosts.mockResolvedValue({ + data: { + count: mockHosts.length, + results: mockHosts, + }, + }); + InventoriesAPI.readOptions.mockResolvedValue({ + data: { + actions: { + GET: {}, + POST: {}, + }, + }, + }); + await act(async () => { + wrapper = mountWithContexts(); + }); + await waitForElement(wrapper, 'ContentLoading', el => el.length === 0); + }); + + afterEach(() => { + jest.clearAllMocks(); + }); + + test('initially renders successfully', () => { + expect(wrapper.find('InventoryHosts').length).toBe(1); + }); + + test('should fetch hosts from api and render them in the list', async () => { + expect(InventoriesAPI.readHosts).toHaveBeenCalled(); + expect(wrapper.find('InventoryHostItem').length).toBe(3); + }); + + test('should check and uncheck the row item', async () => { + expect( + wrapper.find('PFDataListCheck[id="select-host-1"]').props().checked + ).toBe(false); + + await act(async () => { + wrapper.find('PFDataListCheck[id="select-host-1"]').invoke('onChange')( + true + ); + }); + + wrapper.update(); + expect( + wrapper.find('PFDataListCheck[id="select-host-1"]').props().checked + ).toBe(true); + + await act(async () => { + wrapper.find('PFDataListCheck[id="select-host-1"]').invoke('onChange')( + false + ); + }); + + wrapper.update(); + expect( + wrapper.find('PFDataListCheck[id="select-host-1"]').props().checked + ).toBe(false); + }); + + test('should check all row items when select all is checked', async () => { + wrapper.find('PFDataListCheck').forEach(el => { + expect(el.props().checked).toBe(false); + }); + await act(async () => { + wrapper.find('Checkbox#select-all').invoke('onChange')(true); + }); + wrapper.update(); + wrapper.find('PFDataListCheck').forEach(el => { + expect(el.props().checked).toBe(true); + }); + await act(async () => { + wrapper.find('Checkbox#select-all').invoke('onChange')(false); + }); + wrapper.update(); + wrapper.find('PFDataListCheck').forEach(el => { + expect(el.props().checked).toBe(false); + }); + }); + + test('should call api if host toggle is clicked', async () => { + HostsAPI.update.mockResolvedValueOnce({ + data: { ...mockHosts[1], enabled: false }, + }); + expect(wrapper.find('PFSwitch[id="host-2-toggle"]').props().isChecked).toBe( + true + ); + await act(async () => { + wrapper.find('PFSwitch[id="host-2-toggle"]').invoke('onChange')(); + }); + wrapper.update(); + expect(wrapper.find('PFSwitch[id="host-2-toggle"]').props().isChecked).toBe( + false + ); + expect(HostsAPI.update).toHaveBeenCalledTimes(1); + }); + + test('should show error modal if host is not successfully toggled', async () => { + HostsAPI.update.mockImplementationOnce(() => Promise.reject(new Error())); + await act(async () => { + wrapper.find('PFSwitch[id="host-2-toggle"]').invoke('onChange')(); + }); + wrapper.update(); + await waitForElement( + wrapper, + 'Modal', + el => el.props().isOpen === true && el.props().title === 'Error!' + ); + await act(async () => { + wrapper.find('ModalBoxCloseButton').invoke('onClose')(); + }); + await waitForElement(wrapper, 'Modal', el => el.length === 0); + }); + + test('delete button is disabled if user does not have delete capabilities on a selected host', async () => { + await act(async () => { + wrapper.find('PFDataListCheck[id="select-host-3"]').invoke('onChange')(); + }); + wrapper.update(); + expect(wrapper.find('ToolbarDeleteButton button').props().disabled).toBe( + true + ); + }); + + test('should call api delete hosts for each selected host', async () => { + HostsAPI.destroy = jest.fn(); + await act(async () => { + wrapper.find('PFDataListCheck[id="select-host-1"]').invoke('onChange')(); + }); + wrapper.update(); + await act(async () => { + wrapper.find('ToolbarDeleteButton').invoke('onDelete')(); + }); + wrapper.update(); + expect(HostsAPI.destroy).toHaveBeenCalledTimes(1); + }); + + test('should show error modal when host is not successfully deleted from api', async () => { + HostsAPI.destroy.mockRejectedValue( + new Error({ + response: { + config: { + method: 'delete', + url: '/api/v2/hosts/1', + }, + data: 'An error occurred', + }, + }) + ); + await act(async () => { + wrapper.find('PFDataListCheck[id="select-host-1"]').invoke('onChange')(); + }); + wrapper.update(); + await act(async () => { + wrapper.find('ToolbarDeleteButton').invoke('onDelete')(); + }); + await waitForElement( + wrapper, + 'Modal', + el => el.props().isOpen === true && el.props().title === 'Error!' + ); + await act(async () => { + wrapper.find('ModalBoxCloseButton').invoke('onClose')(); + }); + await waitForElement(wrapper, 'Modal', el => el.length === 0); + }); + + test('should show content error if hosts are not successfully fetched from api', async () => { + InventoriesAPI.readHosts.mockImplementation(() => + Promise.reject(new Error()) + ); + await act(async () => { + wrapper.find('PFDataListCheck[id="select-host-1"]').invoke('onChange')(); + }); + wrapper.update(); + await act(async () => { + wrapper.find('ToolbarDeleteButton').invoke('onDelete')(); + }); + await waitForElement(wrapper, 'ContentError', el => el.length === 1); + }); + + test('should show Add button for users with ability to POST', async () => { + expect(wrapper.find('ToolbarAddButton').length).toBe(1); + }); + + test('should hide Add button for users without ability to POST', async () => { + InventoriesAPI.readOptions.mockResolvedValueOnce({ + data: { + actions: { + GET: {}, + }, + }, + }); + await act(async () => { + wrapper = mountWithContexts(); + }); + await waitForElement(wrapper, 'ContentLoading', el => el.length === 0); + expect(wrapper.find('ToolbarAddButton').length).toBe(0); + }); + + test('should show content error when api throws error on initial render', async () => { + InventoriesAPI.readOptions.mockImplementation(() => + Promise.reject(new Error()) + ); + await act(async () => { + wrapper = mountWithContexts(); + }); + await waitForElement(wrapper, 'ContentError', el => el.length === 1); + }); +}); From faa0802d97016b3b313d2890bffc9a1d9b14032a Mon Sep 17 00:00:00 2001 From: Marliana Lara Date: Wed, 20 Nov 2019 15:56:05 -0500 Subject: [PATCH 3/4] Update breadcrumb and fetch new hosts when url changes --- .../src/screens/Inventory/Inventory.jsx | 306 ++++++++---------- .../src/screens/Inventory/Inventory.test.jsx | 50 ++- .../InventoryHosts/InventoryHosts.jsx | 8 +- .../InventoryHosts/InventoryHosts.test.jsx | 2 +- 4 files changed, 165 insertions(+), 201 deletions(-) diff --git a/awx/ui_next/src/screens/Inventory/Inventory.jsx b/awx/ui_next/src/screens/Inventory/Inventory.jsx index 54b5ac2298..5352b949a4 100644 --- a/awx/ui_next/src/screens/Inventory/Inventory.jsx +++ b/awx/ui_next/src/screens/Inventory/Inventory.jsx @@ -1,4 +1,4 @@ -import React, { Component } from 'react'; +import React, { useEffect, useState } from 'react'; import { t } from '@lingui/macro'; import { withI18n } from '@lingui/react'; import { Card, CardHeader, PageSection } from '@patternfly/react-core'; @@ -16,186 +16,152 @@ import InventorySources from './InventorySources'; import { InventoriesAPI } from '@api'; import InventoryEdit from './InventoryEdit'; -class Inventory extends Component { - constructor(props) { - super(props); +function Inventory({ history, i18n, location, match, setBreadcrumb }) { + const [contentError, setContentError] = useState(null); + const [hasContentLoading, setHasContentLoading] = useState(true); + const [inventory, setInventory] = useState(null); - this.state = { - contentError: null, - hasContentLoading: true, - inventory: null, - }; - this.loadInventory = this.loadInventory.bind(this); - } - - async componentDidMount() { - await this.loadInventory(); - } - - async componentDidUpdate(prevProps) { - const { location, match } = this.props; - const url = `/inventories/inventory/${match.params.id}/`; - - if ( - prevProps.location.pathname.startsWith(url) && - prevProps.location !== location && - location.pathname === `${url}details` - ) { - await this.loadInventory(); - } - } - - async loadInventory() { - const { setBreadcrumb, match } = this.props; - const { id } = match.params; - - this.setState({ contentError: null, hasContentLoading: true }); - try { - const { data } = await InventoriesAPI.readDetail(id); - setBreadcrumb(data); - this.setState({ inventory: data }); - } catch (err) { - this.setState({ contentError: err }); - } finally { - this.setState({ hasContentLoading: false }); - } - } - - render() { - const { history, i18n, location, match } = this.props; - const { contentError, hasContentLoading, inventory } = this.state; - - const tabsArray = [ - { name: i18n._(t`Details`), link: `${match.url}/details`, id: 0 }, - { name: i18n._(t`Access`), link: `${match.url}/access`, id: 1 }, - { name: i18n._(t`Groups`), link: `${match.url}/groups`, id: 2 }, - { name: i18n._(t`Hosts`), link: `${match.url}/hosts`, id: 3 }, - { name: i18n._(t`Sources`), link: `${match.url}/sources`, id: 4 }, - { - name: i18n._(t`Completed Jobs`), - link: `${match.url}/completed_jobs`, - id: 5, - }, - ]; - - let cardHeader = hasContentLoading ? null : ( - - - - - ); - - if ( - location.pathname.endsWith('edit') || - location.pathname.endsWith('add') - ) { - cardHeader = null; + useEffect(() => { + async function fetchData() { + try { + const { data } = await InventoriesAPI.readDetail(match.params.id); + setBreadcrumb(data); + setInventory(data); + } catch (error) { + setContentError(error); + } finally { + setHasContentLoading(false); + } } - if (!hasContentLoading && contentError) { - return ( - - - - {contentError.response.status === 404 && ( - - {i18n._(`Inventory not found.`)}{' '} - - {i18n._(`View all Inventories.`)} - - - )} - - - - ); - } + fetchData(); + }, [match.params.id, setBreadcrumb]); + const tabsArray = [ + { name: i18n._(t`Details`), link: `${match.url}/details`, id: 0 }, + { name: i18n._(t`Access`), link: `${match.url}/access`, id: 1 }, + { name: i18n._(t`Groups`), link: `${match.url}/groups`, id: 2 }, + { name: i18n._(t`Hosts`), link: `${match.url}/hosts`, id: 3 }, + { name: i18n._(t`Sources`), link: `${match.url}/sources`, id: 4 }, + { + name: i18n._(t`Completed Jobs`), + link: `${match.url}/completed_jobs`, + id: 5, + }, + ]; + + let cardHeader = hasContentLoading ? null : ( + + + + + ); + + if (location.pathname.endsWith('edit') || location.pathname.endsWith('add')) { + cardHeader = null; + } + + if (!hasContentLoading && contentError) { return ( - {cardHeader} - - - {inventory && [ - ( - - )} - />, - } - />, - } - />, - ( - - )} - />, - } - />, - } - />, - } - />, - } - />, - - !hasContentLoading && ( - - {match.params.id && ( - - {i18n._(`View Inventory Details`)} - - )} - - ) - } - />, - ]} - + + {contentError.response.status === 404 && ( + + {i18n._(`Inventory not found.`)}{' '} + {i18n._(`View all Inventories.`)} + + )} + ); } + + return ( + + + {cardHeader} + + + {inventory && [ + ( + + )} + />, + } + />, + } + />, + ( + + )} + />, + } + />, + } + />, + } + />, + } + />, + + !hasContentLoading && ( + + {match.params.id && ( + + {i18n._(`View Inventory Details`)} + + )} + + ) + } + />, + ]} + + + + ); } export { Inventory as _Inventory }; diff --git a/awx/ui_next/src/screens/Inventory/Inventory.test.jsx b/awx/ui_next/src/screens/Inventory/Inventory.test.jsx index cff57dfc11..f242b01a7d 100644 --- a/awx/ui_next/src/screens/Inventory/Inventory.test.jsx +++ b/awx/ui_next/src/screens/Inventory/Inventory.test.jsx @@ -1,4 +1,5 @@ import React from 'react'; +import { act } from 'react-dom/test-utils'; import { createMemoryHistory } from 'history'; import { InventoriesAPI } from '@api'; import { mountWithContexts, waitForElement } from '@testUtils/enzymeHelpers'; @@ -12,41 +13,38 @@ InventoriesAPI.readDetail.mockResolvedValue({ }); describe('', () => { - test('initially renders succesfully', async done => { - const wrapper = mountWithContexts( - {}} match={{ params: { id: 1 } }} /> - ); - await waitForElement( - wrapper, - 'Inventory', - el => el.state('hasContentLoading') === true - ); - await waitForElement( - wrapper, - 'Inventory', - el => el.state('hasContentLoading') === false - ); + let wrapper; + + test('initially renders succesfully', async () => { + await act(async () => { + wrapper = mountWithContexts( + {}} match={{ params: { id: 1 } }} /> + ); + }); + await waitForElement(wrapper, 'ContentLoading', el => el.length === 0); await waitForElement(wrapper, '.pf-c-tabs__item', el => el.length === 6); - done(); }); + test('should show content error when user attempts to navigate to erroneous route', async () => { const history = createMemoryHistory({ initialEntries: ['/inventories/inventory/1/foobar'], }); - const wrapper = mountWithContexts( {}} />, { - context: { - router: { - history, - route: { - location: history.location, - match: { - params: { id: 1 }, - url: '/inventories/inventory/1/foobar', - path: '/inventories/inventory/1/foobar', + await act(async () => { + wrapper = mountWithContexts( {}} />, { + context: { + router: { + history, + route: { + location: history.location, + match: { + params: { id: 1 }, + url: '/inventories/inventory/1/foobar', + path: '/inventories/inventory/1/foobar', + }, }, }, }, - }, + }); }); 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 f35d87ae1c..921b78a7a5 100644 --- a/awx/ui_next/src/screens/Inventory/InventoryHosts/InventoryHosts.jsx +++ b/awx/ui_next/src/screens/Inventory/InventoryHosts/InventoryHosts.jsx @@ -20,7 +20,7 @@ const QS_CONFIG = getQSConfig('host', { order_by: 'name', }); -function InventoryHosts({ i18n, location, match, inventory }) { +function InventoryHosts({ i18n, location, match }) { const [actions, setActions] = useState(null); const [contentError, setContentError] = useState(null); const [deletionError, setDeletionError] = useState(null); @@ -47,7 +47,7 @@ function InventoryHosts({ i18n, location, match, inventory }) { data: { actions: optionActions }, }, ] = await Promise.all([ - fetchHosts(inventory.id, location.search), + fetchHosts(match.params.id, location.search), InventoriesAPI.readOptions(), ]); @@ -62,7 +62,7 @@ function InventoryHosts({ i18n, location, match, inventory }) { } fetchData(); - }, [inventory, location]); + }, [match.params.id, location]); const handleSelectAll = isSelected => { setSelected(isSelected ? [...hosts] : []); @@ -88,7 +88,7 @@ function InventoryHosts({ i18n, location, match, inventory }) { try { const { data: { count, results }, - } = await fetchHosts(inventory.id, location.search); + } = await fetchHosts(match.params.id, location.search); setHosts(results); setHostCount(count); diff --git a/awx/ui_next/src/screens/Inventory/InventoryHosts/InventoryHosts.test.jsx b/awx/ui_next/src/screens/Inventory/InventoryHosts/InventoryHosts.test.jsx index 59e339722d..715413c81b 100644 --- a/awx/ui_next/src/screens/Inventory/InventoryHosts/InventoryHosts.test.jsx +++ b/awx/ui_next/src/screens/Inventory/InventoryHosts/InventoryHosts.test.jsx @@ -81,7 +81,7 @@ describe('', () => { }, }); await act(async () => { - wrapper = mountWithContexts(); + wrapper = mountWithContexts(); }); await waitForElement(wrapper, 'ContentLoading', el => el.length === 0); }); From 1b508957382c98af19fd9c27c1d1fa42fd6569be Mon Sep 17 00:00:00 2001 From: Marliana Lara Date: Mon, 25 Nov 2019 11:17:37 -0500 Subject: [PATCH 4/4] Use short circuit operator in favor of ternary conditional --- .../screens/Inventory/InventoryHosts/InventoryHosts.jsx | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/awx/ui_next/src/screens/Inventory/InventoryHosts/InventoryHosts.jsx b/awx/ui_next/src/screens/Inventory/InventoryHosts/InventoryHosts.jsx index 921b78a7a5..b9d2fbeca4 100644 --- a/awx/ui_next/src/screens/Inventory/InventoryHosts/InventoryHosts.jsx +++ b/awx/ui_next/src/screens/Inventory/InventoryHosts/InventoryHosts.jsx @@ -165,12 +165,12 @@ function InventoryHosts({ i18n, location, match }) { itemsToDelete={selected} pluralizedItemName={i18n._(t`Hosts`)} />, - canAdd ? ( + canAdd && ( - ) : null, + ), ]} /> )} @@ -186,12 +186,12 @@ function InventoryHosts({ i18n, location, match }) { /> )} emptyStateControls={ - canAdd ? ( + canAdd && ( - ) : null + ) } />