From 3d5a002676b8c355c57f60275c717b2f3b0e00ee Mon Sep 17 00:00:00 2001 From: Marliana Lara Date: Fri, 6 Mar 2020 01:36:38 -0500 Subject: [PATCH] Remove all inventory route logic from Host screens --- awx/ui_next/src/screens/Host/Host.jsx | 123 ++++------------- awx/ui_next/src/screens/Host/Host.test.jsx | 52 +++----- .../screens/Host/HostDetail/HostDetail.jsx | 86 +++++------- .../Host/HostDetail/HostDetail.test.jsx | 126 ++++++++++-------- .../src/screens/Host/HostEdit/HostEdit.jsx | 21 +-- .../screens/Host/HostEdit/HostEdit.test.jsx | 85 +++++++----- awx/ui_next/src/screens/Host/data.host.json | 12 -- 7 files changed, 215 insertions(+), 290 deletions(-) diff --git a/awx/ui_next/src/screens/Host/Host.jsx b/awx/ui_next/src/screens/Host/Host.jsx index 37096fc024..2a8f286bd2 100644 --- a/awx/ui_next/src/screens/Host/Host.jsx +++ b/awx/ui_next/src/screens/Host/Host.jsx @@ -10,7 +10,6 @@ import { useLocation, } from 'react-router-dom'; import { Card, CardActions } from '@patternfly/react-core'; -import { CaretLeftIcon } from '@patternfly/react-icons'; import { TabbedCardHeader } from '@components/Card'; import CardCloseButton from '@components/CardCloseButton'; @@ -24,20 +23,13 @@ import HostEdit from './HostEdit'; import HostGroups from './HostGroups'; import { HostsAPI } from '@api'; -function Host({ inventory, i18n, setBreadcrumb }) { +function Host({ i18n, setBreadcrumb }) { const [host, setHost] = useState(null); const [contentError, setContentError] = useState(null); const [hasContentLoading, setHasContentLoading] = useState(true); const location = useLocation(); - const hostsMatch = useRouteMatch('/hosts/:id'); - const inventoriesMatch = useRouteMatch( - '/inventories/inventory/:id/hosts/:hostId' - ); - const baseUrl = hostsMatch ? hostsMatch.url : inventoriesMatch.url; - const hostListUrl = hostsMatch - ? '/hosts' - : `/inventories/inventory/${inventoriesMatch.params.id}/hosts`; + const match = useRouteMatch('/hosts/:id'); useEffect(() => { (async () => { @@ -45,17 +37,10 @@ function Host({ inventory, i18n, setBreadcrumb }) { setHasContentLoading(true); try { - const hostId = hostsMatch - ? hostsMatch.params.id - : inventoriesMatch.params.hostId; - const { data } = await HostsAPI.readDetail(hostId); - setHost(data); + const { data } = await HostsAPI.readDetail(match.params.id); - if (hostsMatch) { - setBreadcrumb(data); - } else if (inventoriesMatch) { - setBreadcrumb(inventory, data); - } + setHost(data); + setBreadcrumb(data); } catch (error) { setContentError(error); } finally { @@ -67,44 +52,31 @@ function Host({ inventory, i18n, setBreadcrumb }) { const tabsArray = [ { name: i18n._(t`Details`), - link: `${baseUrl}/details`, + link: `${match.url}/details`, id: 0, }, { name: i18n._(t`Facts`), - link: `${baseUrl}/facts`, + link: `${match.url}/facts`, id: 1, }, { name: i18n._(t`Groups`), - link: `${baseUrl}/groups`, + link: `${match.url}/groups`, id: 2, }, { name: i18n._(t`Completed Jobs`), - link: `${baseUrl}/completed_jobs`, + link: `${match.url}/completed_jobs`, id: 3, }, ]; - if (inventoriesMatch) { - tabsArray.unshift({ - name: ( - <> - - {i18n._(t`Back to Hosts`)} - - ), - link: hostListUrl, - id: 99, - }); - } - let cardHeader = ( - + ); @@ -124,7 +96,7 @@ function Host({ inventory, i18n, setBreadcrumb }) { {contentError.response && contentError.response.status === 404 && ( {i18n._(`Host not found.`)}{' '} - {i18n._(`View all Hosts.`)} + {i18n._(`View all Hosts.`)} )} @@ -132,72 +104,35 @@ function Host({ inventory, i18n, setBreadcrumb }) { ); } - const redirect = hostsMatch ? ( - - ) : ( - - ); - return ( {cardHeader} - {redirect} - {host && ( - - setHost(newHost)} - /> - - )} - {host && ( - } - /> - )} - {host && ( - } - /> - )} - {host && ( - } - /> - )} - {host?.id && ( - + + {host && [ + + + , + + + , + + + , + + + , + - - )} + , + ]} !hasContentLoading && ( - + {i18n._(`View Host Details`)} diff --git a/awx/ui_next/src/screens/Host/Host.test.jsx b/awx/ui_next/src/screens/Host/Host.test.jsx index 8d5b83d2ef..64c188fc06 100644 --- a/awx/ui_next/src/screens/Host/Host.test.jsx +++ b/awx/ui_next/src/screens/Host/Host.test.jsx @@ -3,53 +3,41 @@ import { act } from 'react-dom/test-utils'; import { createMemoryHistory } from 'history'; import { HostsAPI } from '@api'; import { mountWithContexts, waitForElement } from '@testUtils/enzymeHelpers'; -import mockDetails from './data.host.json'; +import mockHost from './data.host.json'; import Host from './Host'; jest.mock('@api'); +jest.mock('react-router-dom', () => ({ + ...jest.requireActual('react-router-dom'), + useRouteMatch: () => ({ + url: '/hosts/1', + params: { id: 1 }, + }), +})); + +HostsAPI.readDetail.mockResolvedValue({ + data: { ...mockHost }, +}); describe('', () => { let wrapper; let history; - HostsAPI.readDetail.mockResolvedValue({ - data: { ...mockDetails }, + beforeEach(async () => { + await act(async () => { + wrapper = mountWithContexts( {}} />); + }); }); afterEach(() => { wrapper.unmount(); }); - test('initially renders succesfully', async () => { - history = createMemoryHistory({ - initialEntries: ['/hosts/1/edit'], + test('should render expected tabs', async () => { + const expectedTabs = ['Details', 'Facts', 'Groups', 'Completed Jobs']; + wrapper.find('RoutedTabs li').forEach((tab, index) => { + expect(tab.text()).toEqual(expectedTabs[index]); }); - - await act(async () => { - wrapper = mountWithContexts( {}} />, { - context: { router: { history } }, - }); - }); - await waitForElement(wrapper, 'ContentLoading', el => el.length === 0); - expect(wrapper.find('Host').length).toBe(1); - }); - - test('should render "Back to Hosts" tab when navigating from inventories', async () => { - history = createMemoryHistory({ - initialEntries: ['/inventories/inventory/1/hosts/1'], - }); - await act(async () => { - wrapper = mountWithContexts( {}} />, { - context: { router: { history } }, - }); - }); - await waitForElement(wrapper, 'ContentLoading', el => el.length === 0); - expect( - wrapper - .find('RoutedTabs li') - .first() - .text() - ).toBe('Back to Hosts'); }); test('should show content error when api throws error on initial render', async () => { diff --git a/awx/ui_next/src/screens/Host/HostDetail/HostDetail.jsx b/awx/ui_next/src/screens/Host/HostDetail/HostDetail.jsx index 316ac4e485..2cedbaa6e0 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, useHistory, useParams, useLocation } from 'react-router-dom'; +import { Link, useHistory } from 'react-router-dom'; import { withI18n } from '@lingui/react'; import { t } from '@lingui/macro'; import { Host } from '@types'; @@ -14,42 +14,36 @@ import DeleteButton from '@components/DeleteButton'; import { HostsAPI } from '@api'; import HostToggle from '@components/HostToggle'; -function HostDetail({ host, i18n, onUpdateHost }) { +function HostDetail({ i18n, host }) { const { created, description, id, modified, name, + variables, 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 recentPlaybookJobs = recent_jobs.map(job => ({ ...job, type: 'job' })); + const history = useHistory(); const handleHostDelete = async () => { setIsloading(true); try { await HostsAPI.destroy(id); - setIsloading(false); - const url = pathname.startsWith('/inventories') - ? `/inventories/inventory/${inventoryId}/hosts/` - : `/hosts`; - history.push(url); + history.push('/hosts'); } catch (err) { setDeletionError(err); + } finally { + setIsloading(false); } }; @@ -66,77 +60,71 @@ function HostDetail({ host, i18n, onUpdateHost }) { ); } + + const recentPlaybookJobs = recent_jobs.map(job => ({ ...job, type: 'job' })); return ( - - onUpdateHost({ - ...host, - enabled, - }) - } - css="padding-bottom: 40px" - /> + } label={i18n._(t`Activity`)} + value={} /> - {inventory && ( - - {inventory.name} - - } - /> - )} + + {inventory.name} + + } + /> - {user_capabilities && user_capabilities.edit && ( + {user_capabilities?.edit && ( )} - {user_capabilities && user_capabilities.delete && ( + {user_capabilities?.delete && ( handleHostDelete()} modalTitle={i18n._(t`Delete Host`)} - name={host.name} + name={name} /> )} + {deletionError && ( + setDeletionError(null)} + > + {i18n._(t`Failed to delete host.`)} + + + )} ); } 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 0f1ccc037a..d809bf1732 100644 --- a/awx/ui_next/src/screens/Host/HostDetail/HostDetail.test.jsx +++ b/awx/ui_next/src/screens/Host/HostDetail/HostDetail.test.jsx @@ -1,66 +1,88 @@ import React from 'react'; - +import { act } from 'react-dom/test-utils'; import { mountWithContexts, waitForElement } from '@testUtils/enzymeHelpers'; - import HostDetail from './HostDetail'; +import { HostsAPI } from '@api'; + +import mockHost from '../data.host.json'; jest.mock('@api'); describe('', () => { - const mockHost = { - id: 1, - name: 'Foo', - description: 'Bar', - inventory: 1, - created: '2015-07-07T17:21:26.429745Z', - modified: '2019-08-11T19:47:37.980466Z', - variables: '---', - summary_fields: { - inventory: { - id: 1, - name: 'test inventory', - }, - user_capabilities: { - edit: true, - }, - recent_jobs: [], - }, - }; + let wrapper; - test('initially renders succesfully', () => { - mountWithContexts(); + describe('User has edit permissions', () => { + beforeAll(() => { + wrapper = mountWithContexts(); + }); + + afterAll(() => { + wrapper.unmount(); + }); + + test('should render Details', async () => { + function assertDetail(label, value) { + expect(wrapper.find(`Detail[label="${label}"] dt`).text()).toBe(label); + expect(wrapper.find(`Detail[label="${label}"] dd`).text()).toBe(value); + } + + assertDetail('Name', 'localhost'); + assertDetail('Description', 'a good description'); + assertDetail('Inventory', 'Mikes Inventory'); + assertDetail('Created', '10/28/2019, 9:26:54 PM'); + assertDetail('Last Modified', '10/29/2019, 8:18:41 PM'); + }); + + test('should show edit button for users with edit permission', () => { + const editButton = wrapper.find('Button[aria-label="edit"]'); + expect(editButton.text()).toEqual('Edit'); + expect(editButton.prop('to')).toBe('/hosts/2/edit'); + }); + + test('expected api call is made for delete', async () => { + await act(async () => { + wrapper.find('DeleteButton').invoke('onConfirm')(); + }); + expect(HostsAPI.destroy).toHaveBeenCalledTimes(1); + }); + + test('Error dialog shown for failed deletion', async () => { + HostsAPI.destroy.mockImplementationOnce(() => + Promise.reject(new Error()) + ); + await act(async () => { + wrapper.find('DeleteButton').invoke('onConfirm')(); + }); + await waitForElement( + wrapper, + 'Modal[title="Error!"]', + el => el.length === 1 + ); + await act(async () => { + wrapper.find('Modal[title="Error!"]').invoke('onClose')(); + }); + await waitForElement( + wrapper, + 'Modal[title="Error!"]', + el => el.length === 0 + ); + }); }); - test('should render Details', async () => { - const wrapper = mountWithContexts(); - const testParams = [ - { label: 'Name', value: 'Foo' }, - { label: 'Description', value: 'Bar' }, - { label: 'Inventory', value: 'test inventory' }, - { label: 'Created', value: '7/7/2015, 5:21:26 PM' }, - { label: 'Last Modified', value: '8/11/2019, 7:47:37 PM' }, - ]; - // eslint-disable-next-line no-restricted-syntax - for (const { label, value } of testParams) { - // eslint-disable-next-line no-await-in-loop - const detail = await waitForElement(wrapper, `Detail[label="${label}"]`); - expect(detail.find('dt').text()).toBe(label); - expect(detail.find('dd').text()).toBe(value); - } - }); + describe('User has read-only permissions', () => { + beforeAll(() => { + const readOnlyHost = { ...mockHost }; + readOnlyHost.summary_fields.user_capabilities.edit = false; - test('should show edit button for users with edit permission', async () => { - const wrapper = mountWithContexts(); - const editButton = wrapper.find('Button[aria-label="edit"]'); - expect(editButton.text()).toEqual('Edit'); - expect(editButton.prop('to')).toBe('/hosts/1/edit'); - }); + wrapper = mountWithContexts(); + }); - test('should hide edit button for users without edit permission', async () => { - const readOnlyHost = { ...mockHost }; - readOnlyHost.summary_fields.user_capabilities.edit = false; - const wrapper = mountWithContexts(); - await waitForElement(wrapper, 'HostDetail'); - expect(wrapper.find('Button[aria-label="edit"]').length).toBe(0); + afterAll(() => { + wrapper.unmount(); + }); + + test('should hide edit button for users without edit permission', async () => { + expect(wrapper.find('Button[aria-label="edit"]').length).toBe(0); + }); }); }); diff --git a/awx/ui_next/src/screens/Host/HostEdit/HostEdit.jsx b/awx/ui_next/src/screens/Host/HostEdit/HostEdit.jsx index f6dc82e767..bddb692b14 100644 --- a/awx/ui_next/src/screens/Host/HostEdit/HostEdit.jsx +++ b/awx/ui_next/src/screens/Host/HostEdit/HostEdit.jsx @@ -1,31 +1,14 @@ import React, { useState } from 'react'; import PropTypes from 'prop-types'; -import { useHistory, useRouteMatch } from 'react-router-dom'; +import { useHistory } from 'react-router-dom'; import { CardBody } from '@components/Card'; import HostForm from '@components/HostForm'; import { HostsAPI } from '@api'; -import HostForm from '../shared'; function HostEdit({ host }) { const [formError, setFormError] = useState(null); - const hostsMatch = useRouteMatch('/hosts/:id/edit'); - const inventoriesMatch = useRouteMatch( - '/inventories/inventory/:id/hosts/:hostId/edit' - ); + const detailsUrl = `/hosts/${host.id}/details`; const history = useHistory(); - let detailsUrl; - - if (hostsMatch) { - detailsUrl = `/hosts/${hostsMatch.params.id}/details`; - } - - if (inventoriesMatch) { - const kind = - host.summary_fields.inventory.kind === 'smart' - ? 'smart_inventory' - : 'inventory'; - detailsUrl = `/inventories/${kind}/${inventoriesMatch.params.id}/hosts/${inventoriesMatch.params.hostId}/details`; - } const handleSubmit = async values => { try { diff --git a/awx/ui_next/src/screens/Host/HostEdit/HostEdit.test.jsx b/awx/ui_next/src/screens/Host/HostEdit/HostEdit.test.jsx index 637dd64273..038ee90bbf 100644 --- a/awx/ui_next/src/screens/Host/HostEdit/HostEdit.test.jsx +++ b/awx/ui_next/src/screens/Host/HostEdit/HostEdit.test.jsx @@ -1,49 +1,70 @@ import React from 'react'; +import { act } from 'react-dom/test-utils'; import { createMemoryHistory } from 'history'; import { HostsAPI } from '@api'; import { mountWithContexts } from '@testUtils/enzymeHelpers'; +import mockHost from '../data.host.json'; import HostEdit from './HostEdit'; jest.mock('@api'); describe('', () => { - const mockData = { - id: 1, - name: 'Foo', - description: 'Bar', - inventory: 1, - variables: '---', - summary_fields: { - inventory: { - id: 1, - name: 'test inventory', - }, - }, + let wrapper; + let history; + + const updatedHostData = { + name: 'new name', + description: 'new description', + variables: '---\nfoo: bar', }; - test('handleSubmit should call api update', () => { - const wrapper = mountWithContexts(); - - const updatedHostData = { - name: 'new name', - description: 'new description', - variables: '---\nfoo: bar', - }; - wrapper.find('HostForm').prop('handleSubmit')(updatedHostData); - - expect(HostsAPI.update).toHaveBeenCalledWith(1, updatedHostData); + beforeAll(async () => { + history = createMemoryHistory(); + await act(async () => { + wrapper = mountWithContexts(, { + context: { router: { history } }, + }); + }); }); - test('should navigate to host detail when cancel is clicked', () => { - const history = createMemoryHistory({ - initialEntries: ['/hosts/1/edit'], - }); - const wrapper = mountWithContexts(, { - context: { router: { history } }, - }); + afterAll(() => { + jest.clearAllMocks(); + wrapper.unmount(); + }); - wrapper.find('button[aria-label="Cancel"]').prop('onClick')(); + test('handleSubmit should call api update', async () => { + await act(async () => { + wrapper.find('HostForm').prop('handleSubmit')(updatedHostData); + }); + expect(HostsAPI.update).toHaveBeenCalledWith(2, updatedHostData); + }); - expect(history.location.pathname).toEqual('/hosts/1/details'); + test('should navigate to host detail when cancel is clicked', async () => { + await act(async () => { + wrapper.find('button[aria-label="Cancel"]').prop('onClick')(); + }); + expect(history.location.pathname).toEqual('/hosts/2/details'); + }); + + test('should navigate to host detail after successful submission', async () => { + await act(async () => { + wrapper.find('HostForm').invoke('handleSubmit')(updatedHostData); + }); + expect(wrapper.find('FormSubmitError').length).toBe(0); + expect(history.location.pathname).toEqual('/hosts/2/details'); + }); + + test('failed form submission should show an error message', async () => { + const error = { + response: { + data: { detail: 'An error occurred' }, + }, + }; + HostsAPI.update.mockImplementationOnce(() => Promise.reject(error)); + await act(async () => { + wrapper.find('HostForm').invoke('handleSubmit')(mockHost); + }); + wrapper.update(); + expect(wrapper.find('FormSubmitError').length).toBe(1); }); }); diff --git a/awx/ui_next/src/screens/Host/data.host.json b/awx/ui_next/src/screens/Host/data.host.json index d2ef565610..aacc08f787 100644 --- a/awx/ui_next/src/screens/Host/data.host.json +++ b/awx/ui_next/src/screens/Host/data.host.json @@ -51,18 +51,6 @@ "id": 1, "failed": false }, - "created_by": { - "id": 1, - "username": "admin", - "first_name": "", - "last_name": "" - }, - "modified_by": { - "id": 1, - "username": "admin", - "first_name": "", - "last_name": "" - }, "user_capabilities": { "edit": true, "delete": true