From 9aef57003a8d9a5df905f4361df560b13f647df3 Mon Sep 17 00:00:00 2001 From: Keith Grant Date: Wed, 19 Feb 2020 16:00:11 -0800 Subject: [PATCH] use HostToggle in HostDetail; update tests --- .../src/components/AlertModal/AlertModal.jsx | 10 +- .../screens/Host/HostDetail/HostDetail.jsx | 49 ++------ .../src/screens/Host/HostList/HostList.jsx | 17 +-- .../screens/Host/HostList/HostList.test.jsx | 113 +++++++++--------- .../screens/Host/HostList/HostListItem.jsx | 2 +- .../Host/HostList/HostListItem.test.jsx | 37 +----- .../Host/{HostList => shared}/HostToggle.jsx | 10 +- .../screens/Host/shared/HostToggle.test.jsx | 92 ++++++++++++++ 8 files changed, 183 insertions(+), 147 deletions(-) rename awx/ui_next/src/screens/Host/{HostList => shared}/HostToggle.jsx (90%) create mode 100644 awx/ui_next/src/screens/Host/shared/HostToggle.test.jsx diff --git a/awx/ui_next/src/components/AlertModal/AlertModal.jsx b/awx/ui_next/src/components/AlertModal/AlertModal.jsx index f600d77745..8b47a8d3f9 100644 --- a/awx/ui_next/src/components/AlertModal/AlertModal.jsx +++ b/awx/ui_next/src/components/AlertModal/AlertModal.jsx @@ -16,7 +16,13 @@ const Header = styled.div` } `; -export default ({ isOpen = null, title, variant, children, ...props }) => { +export default function AlertModal({ + isOpen = null, + title, + variant, + children, + ...props +}) { const variantIcons = { danger: , error: , @@ -44,4 +50,4 @@ export default ({ isOpen = null, title, variant, children, ...props }) => { {children} ); -}; +} diff --git a/awx/ui_next/src/screens/Host/HostDetail/HostDetail.jsx b/awx/ui_next/src/screens/Host/HostDetail/HostDetail.jsx index c6c1814667..7e03abc736 100644 --- a/awx/ui_next/src/screens/Host/HostDetail/HostDetail.jsx +++ b/awx/ui_next/src/screens/Host/HostDetail/HostDetail.jsx @@ -3,7 +3,7 @@ import { Link, useHistory, useParams, useLocation } from 'react-router-dom'; import { withI18n } from '@lingui/react'; import { t } from '@lingui/macro'; import { Host } from '@types'; -import { Button, Switch } from '@patternfly/react-core'; +import { Button } from '@patternfly/react-core'; import { CardBody, CardActionsRow } from '@components/Card'; import AlertModal from '@components/AlertModal'; import ErrorDetail from '@components/ErrorDetail'; @@ -12,6 +12,7 @@ import { VariablesDetail } from '@components/CodeMirrorInput'; import Sparkline from '@components/Sparkline'; import DeleteButton from '@components/DeleteButton'; import { HostsAPI } from '@api'; +import HostToggle from '../shared/HostToggle'; function HostDetail({ host, i18n, onUpdateHost }) { const { @@ -20,7 +21,6 @@ function HostDetail({ host, i18n, onUpdateHost }) { id, modified, name, - enabled, summary_fields: { inventory, recent_jobs, @@ -36,25 +36,9 @@ function HostDetail({ host, i18n, onUpdateHost }) { const { id: inventoryId, hostId: inventoryHostId } = useParams(); const [isLoading, setIsloading] = useState(false); const [deletionError, setDeletionError] = useState(false); - const [toggleLoading, setToggleLoading] = useState(false); - const [toggleError, setToggleError] = useState(false); const recentPlaybookJobs = recent_jobs.map(job => ({ ...job, type: 'job' })); - const handleHostToggle = async () => { - setToggleLoading(true); - try { - const { data } = await HostsAPI.update(id, { - enabled: !enabled, - }); - onUpdateHost(data); - } catch (err) { - setToggleError(err); - } finally { - setToggleLoading(false); - } - }; - const handleHostDelete = async () => { setIsloading(true); try { @@ -66,19 +50,6 @@ function HostDetail({ host, i18n, onUpdateHost }) { } }; - if (toggleError && !toggleLoading) { - return ( - setToggleError(false)} - > - {i18n._(t`Failed to toggle host.`)} - - - ); - } if (!isLoading && deletionError) { return ( - + onUpdateHost({ + ...host, + enabled, + }) + } css="padding-bottom: 40px" - id={`host-${id}-toggle`} - label={i18n._(t`On`)} - labelOff={i18n._(t`Off`)} - isChecked={enabled} - isDisabled={!user_capabilities.edit} - onChange={() => handleHostToggle()} - aria-label={i18n._(t`Toggle Host`)} /> diff --git a/awx/ui_next/src/screens/Host/HostList/HostList.jsx b/awx/ui_next/src/screens/Host/HostList/HostList.jsx index 90f9f3e00a..61c3fa6ab4 100644 --- a/awx/ui_next/src/screens/Host/HostList/HostList.jsx +++ b/awx/ui_next/src/screens/Host/HostList/HostList.jsx @@ -1,5 +1,5 @@ import React, { Fragment, useState, useEffect, useCallback } from 'react'; -import { useLocation, useParams } from 'react-router-dom'; +import { useLocation, useRouteMatch } from 'react-router-dom'; import { withI18n } from '@lingui/react'; import { t } from '@lingui/macro'; import { Card } from '@patternfly/react-core'; @@ -25,7 +25,7 @@ const QS_CONFIG = getQSConfig('host', { function HostList({ i18n }) { const location = useLocation(); - const match = useParams(); + const match = useRouteMatch(); const [selected, setSelected] = useState([]); const { @@ -146,13 +146,13 @@ function HostList({ i18n }) { ]} /> )} - renderItem={o => ( + renderItem={host => ( row.id === o.id)} - onSelect={() => handleSelect(o)} + key={host.id} + host={host} + detailUrl={`${match.url}/${host.id}/details`} + isSelected={selected.some(row => row.id === host.id)} + onSelect={() => handleSelect(host)} /> )} emptyStateControls={ @@ -177,4 +177,5 @@ function HostList({ i18n }) { ); } +export { HostList as _HostList }; export default withI18n()(HostList); diff --git a/awx/ui_next/src/screens/Host/HostList/HostList.test.jsx b/awx/ui_next/src/screens/Host/HostList/HostList.test.jsx index 7328b31c9c..8f62f91018 100644 --- a/awx/ui_next/src/screens/Host/HostList/HostList.test.jsx +++ b/awx/ui_next/src/screens/Host/HostList/HostList.test.jsx @@ -1,9 +1,10 @@ import React from 'react'; +import { act } from 'react-dom/test-utils'; import { HostsAPI } from '@api'; import { mountWithContexts, waitForElement } from '@testUtils/enzymeHelpers'; import { sleep } from '@testUtils/testUtils'; -import HostsList, { _HostsList } from './HostList'; +import HostList, { _HostList } from './HostList'; jest.mock('@api'); @@ -68,7 +69,15 @@ const mockHosts = [ }, ]; -describe('', () => { +function waitForLoaded(wrapper) { + return waitForElement( + wrapper, + 'HostList', + el => el.find('ContentLoading').length === 0 + ); +} + +describe('', () => { beforeEach(() => { HostsAPI.read.mockResolvedValue({ data: { @@ -91,38 +100,33 @@ describe('', () => { jest.clearAllMocks(); }); - test('initially renders successfully', () => { - mountWithContexts( - - ); + test('initially renders successfully', async () => { + await act(async () => { + mountWithContexts( + + ); + }); }); - test('Hosts are retrieved from the api and the components finishes loading', async done => { - const loadHosts = jest.spyOn(_HostsList.prototype, 'loadHosts'); - const wrapper = mountWithContexts(); - await waitForElement( - wrapper, - 'HostsList', - el => el.state('hasContentLoading') === true - ); - expect(loadHosts).toHaveBeenCalled(); - await waitForElement( - wrapper, - 'HostsList', - el => el.state('hasContentLoading') === false - ); - done(); + test.only('Hosts are retrieved from the api and the components finishes loading', async () => { + let wrapper; + await act(async () => { + wrapper = mountWithContexts(); + }); + await waitForLoaded(wrapper); + expect(HostsAPI.read).toHaveBeenCalled(); + expect(wrapper.find('HostListItem')).toHaveLength(3); }); - test('handleSelect is called when a host list item is selected', async done => { - const handleSelect = jest.spyOn(_HostsList.prototype, 'handleSelect'); - const wrapper = mountWithContexts(); + test('handleSelect is called when a host list item is selected', async () => { + const handleSelect = jest.spyOn(_HostList.prototype, 'handleSelect'); + const wrapper = mountWithContexts(); await waitForElement( wrapper, - 'HostsList', + 'HostList', el => el.state('hasContentLoading') === false ); await wrapper @@ -133,18 +137,17 @@ describe('', () => { expect(handleSelect).toBeCalled(); await waitForElement( wrapper, - 'HostsList', + 'HostList', el => el.state('selected').length === 1 ); - done(); }); - test('handleSelectAll is called when select all checkbox is clicked', async done => { - const handleSelectAll = jest.spyOn(_HostsList.prototype, 'handleSelectAll'); - const wrapper = mountWithContexts(); + test('handleSelectAll is called when select all checkbox is clicked', async () => { + const handleSelectAll = jest.spyOn(_HostList.prototype, 'handleSelectAll'); + const wrapper = mountWithContexts(); await waitForElement( wrapper, - 'HostsList', + 'HostList', el => el.state('hasContentLoading') === false ); wrapper @@ -154,15 +157,14 @@ describe('', () => { expect(handleSelectAll).toBeCalled(); await waitForElement( wrapper, - 'HostsList', + 'HostList', el => el.state('selected').length === 3 ); - done(); }); - test('delete button is disabled if user does not have delete capabilities on a selected host', async done => { - const wrapper = mountWithContexts(); - wrapper.find('HostsList').setState({ + test('delete button is disabled if user does not have delete capabilities on a selected host', async () => { + const wrapper = mountWithContexts(); + wrapper.find('HostList').setState({ hosts: mockHosts, itemCount: 3, isInitialized: true, @@ -173,7 +175,7 @@ describe('', () => { 'ToolbarDeleteButton * button', el => el.getDOMNode().disabled === false ); - wrapper.find('HostsList').setState({ + wrapper.find('HostList').setState({ selected: mockHosts, }); await waitForElement( @@ -181,13 +183,12 @@ describe('', () => { 'ToolbarDeleteButton * button', el => el.getDOMNode().disabled === true ); - done(); }); test('api is called to delete hosts for each selected host.', () => { HostsAPI.destroy = jest.fn(); - const wrapper = mountWithContexts(); - wrapper.find('HostsList').setState({ + const wrapper = mountWithContexts(); + wrapper.find('HostList').setState({ hosts: mockHosts, itemCount: 2, isInitialized: true, @@ -198,7 +199,7 @@ describe('', () => { expect(HostsAPI.destroy).toHaveBeenCalledTimes(2); }); - test('error is shown when host not successfully deleted from api', async done => { + test('error is shown when host not successfully deleted from api', async () => { HostsAPI.destroy.mockRejectedValue( new Error({ response: { @@ -210,8 +211,8 @@ describe('', () => { }, }) ); - const wrapper = mountWithContexts(); - wrapper.find('HostsList').setState({ + const wrapper = mountWithContexts(); + wrapper.find('HostList').setState({ hosts: mockHosts, itemCount: 1, isInitialized: true, @@ -226,27 +227,24 @@ describe('', () => { 'Modal', el => el.props().isOpen === true && el.props().title === 'Error!' ); - - done(); }); - test('Add button shown for users without ability to POST', async done => { - const wrapper = mountWithContexts(); + test('Add button shown for users without ability to POST', async () => { + const wrapper = mountWithContexts(); await waitForElement( wrapper, - 'HostsList', + 'HostList', el => el.state('hasContentLoading') === true ); await waitForElement( wrapper, - 'HostsList', + 'HostList', el => el.state('hasContentLoading') === false ); expect(wrapper.find('ToolbarAddButton').length).toBe(1); - done(); }); - test('Add button hidden for users without ability to POST', async done => { + test('Add button hidden for users without ability to POST', async () => { HostsAPI.readOptions.mockResolvedValue({ data: { actions: { @@ -254,18 +252,17 @@ describe('', () => { }, }, }); - const wrapper = mountWithContexts(); + const wrapper = mountWithContexts(); await waitForElement( wrapper, - 'HostsList', + 'HostList', el => el.state('hasContentLoading') === true ); await waitForElement( wrapper, - 'HostsList', + 'HostList', el => el.state('hasContentLoading') === false ); expect(wrapper.find('ToolbarAddButton').length).toBe(0); - done(); }); }); diff --git a/awx/ui_next/src/screens/Host/HostList/HostListItem.jsx b/awx/ui_next/src/screens/Host/HostList/HostListItem.jsx index 3d59e29532..3652f2e52b 100644 --- a/awx/ui_next/src/screens/Host/HostList/HostListItem.jsx +++ b/awx/ui_next/src/screens/Host/HostList/HostListItem.jsx @@ -17,8 +17,8 @@ import { PencilAltIcon } from '@patternfly/react-icons'; import Sparkline from '@components/Sparkline'; import { Host } from '@types'; -import HostToggle from './HostToggle'; import styled from 'styled-components'; +import HostToggle from '../shared/HostToggle'; const DataListAction = styled(_DataListAction)` align-items: center; 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 7b7b7f4385..3605a62546 100644 --- a/awx/ui_next/src/screens/Host/HostList/HostListItem.test.jsx +++ b/awx/ui_next/src/screens/Host/HostList/HostListItem.test.jsx @@ -1,5 +1,4 @@ import React from 'react'; - import { mountWithContexts } from '@testUtils/enzymeHelpers'; import HostsListItem from './HostListItem'; @@ -44,6 +43,7 @@ describe('', () => { ); 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; @@ -58,39 +58,4 @@ describe('', () => { ); expect(wrapper.find('PencilAltIcon').exists()).toBeFalsy(); }); - test('handles toggle click when host is enabled', () => { - const wrapper = mountWithContexts( - {}} - host={mockHost} - onToggleHost={onToggleHost} - /> - ); - wrapper - .find('Switch') - .first() - .find('input') - .simulate('change'); - expect(onToggleHost).toHaveBeenCalledWith(mockHost); - }); - - test('handles toggle click when host is disabled', () => { - const wrapper = mountWithContexts( - {}} - host={mockHost} - onToggleHost={onToggleHost} - /> - ); - wrapper - .find('Switch') - .first() - .find('input') - .simulate('change'); - expect(onToggleHost).toHaveBeenCalledWith(mockHost); - }); }); diff --git a/awx/ui_next/src/screens/Host/HostList/HostToggle.jsx b/awx/ui_next/src/screens/Host/shared/HostToggle.jsx similarity index 90% rename from awx/ui_next/src/screens/Host/HostList/HostToggle.jsx rename to awx/ui_next/src/screens/Host/shared/HostToggle.jsx index 343eb712dd..eefdb76fe9 100644 --- a/awx/ui_next/src/screens/Host/HostList/HostToggle.jsx +++ b/awx/ui_next/src/screens/Host/shared/HostToggle.jsx @@ -7,7 +7,7 @@ import ErrorDetail from '@components/ErrorDetail'; import useRequest from '@util/useRequest'; import { HostsAPI } from '@api'; -function HostToggle({ host, i18n }) { +function HostToggle({ host, onToggle, className, i18n }) { const [isEnabled, setIsEnabled] = useState(host.enabled); const [showError, setShowError] = useState(false); @@ -24,8 +24,11 @@ function HostToggle({ host, i18n }) { useEffect(() => { if (result !== isEnabled) { setIsEnabled(result); + if (onToggle) { + onToggle(result); + } } - }, [result, isEnabled]); + }, [result, isEnabled, onToggle]); useEffect(() => { if (error) { @@ -44,6 +47,7 @@ function HostToggle({ host, i18n }) { position="top" > {showError && error && !isLoading && ( setShowError(false)} diff --git a/awx/ui_next/src/screens/Host/shared/HostToggle.test.jsx b/awx/ui_next/src/screens/Host/shared/HostToggle.test.jsx new file mode 100644 index 0000000000..cef60fb34e --- /dev/null +++ b/awx/ui_next/src/screens/Host/shared/HostToggle.test.jsx @@ -0,0 +1,92 @@ +import React from 'react'; +import { act } from 'react-dom/test-utils'; +import { HostsAPI } from '@api'; +import { mountWithContexts } from '@testUtils/enzymeHelpers'; +import HostToggle from './HostToggle'; + +jest.mock('@api'); + +const mockHost = { + 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, + }, + recent_jobs: [], + }, +}; + +describe('', () => { + test('should should toggle off', async () => { + const onToggle = jest.fn(); + const wrapper = mountWithContexts( + + ); + expect(wrapper.find('Switch').prop('isChecked')).toEqual(true); + + await act(async () => { + wrapper.find('Switch').invoke('onChange')(); + }); + expect(HostsAPI.update).toHaveBeenCalledWith(1, { + enabled: false, + }); + wrapper.update(); + expect(wrapper.find('Switch').prop('isChecked')).toEqual(false); + expect(onToggle).toHaveBeenCalledWith(false); + }); + + test('should should toggle on', async () => { + const onToggle = jest.fn(); + const wrapper = mountWithContexts( + + ); + expect(wrapper.find('Switch').prop('isChecked')).toEqual(false); + + await act(async () => { + wrapper.find('Switch').invoke('onChange')(); + }); + expect(HostsAPI.update).toHaveBeenCalledWith(1, { + enabled: true, + }); + wrapper.update(); + expect(wrapper.find('Switch').prop('isChecked')).toEqual(true); + expect(onToggle).toHaveBeenCalledWith(true); + }); + + test('should show error modal', async () => { + HostsAPI.update.mockImplementation(() => { + throw new Error('nope'); + }); + const wrapper = mountWithContexts(); + expect(wrapper.find('Switch').prop('isChecked')).toEqual(true); + + await act(async () => { + wrapper.find('Switch').invoke('onChange')(); + }); + wrapper.update(); + const modal = wrapper.find('AlertModal'); + expect(modal).toHaveLength(1); + expect(modal.prop('isOpen')).toEqual(true); + + act(() => { + modal.invoke('onClose')(); + }); + wrapper.update(); + expect(wrapper.find('AlertModal')).toHaveLength(0); + }); +});