diff --git a/awx/ui_next/src/components/Lookup/CredentialLookup.jsx b/awx/ui_next/src/components/Lookup/CredentialLookup.jsx index 2562b81f50..558b4fc039 100644 --- a/awx/ui_next/src/components/Lookup/CredentialLookup.jsx +++ b/awx/ui_next/src/components/Lookup/CredentialLookup.jsx @@ -1,4 +1,4 @@ -import React, { useEffect, useState } from 'react'; +import React, { useEffect, useState, useRef } from 'react'; import { bool, func, node, number, string, oneOfType } from 'prop-types'; import { withRouter } from 'react-router-dom'; import { withI18n } from '@lingui/react'; @@ -35,6 +35,15 @@ function CredentialLookup({ const [credentials, setCredentials] = useState([]); const [count, setCount] = useState(0); const [error, setError] = useState(null); + const isMounted = useRef(null); + + useEffect(() => { + isMounted.current = true; + return () => { + isMounted.current = false; + }; + }, []); + useEffect(() => { (async () => { const params = parseQueryString(QS_CONFIG, history.location.search); @@ -49,10 +58,12 @@ function CredentialLookup({ const { data } = await CredentialsAPI.read( mergeParams(params, { ...typeIdParams, ...typeKindParams }) ); - setCredentials(data.results); - setCount(data.count); + if (isMounted.current) { + setCredentials(data.results); + setCount(data.count); + } } catch (err) { - if (setError) { + if (isMounted.current) { setError(err); } } diff --git a/awx/ui_next/src/components/Lookup/ProjectLookup.jsx b/awx/ui_next/src/components/Lookup/ProjectLookup.jsx index a45a5b3429..4f97117da2 100644 --- a/awx/ui_next/src/components/Lookup/ProjectLookup.jsx +++ b/awx/ui_next/src/components/Lookup/ProjectLookup.jsx @@ -1,4 +1,4 @@ -import React, { useState, useEffect } from 'react'; +import React, { useState, useEffect, useRef } from 'react'; import { node, string, func, bool } from 'prop-types'; import { withRouter } from 'react-router-dom'; import { withI18n } from '@lingui/react'; @@ -32,19 +32,31 @@ function ProjectLookup({ const [projects, setProjects] = useState([]); const [count, setCount] = useState(0); const [error, setError] = useState(null); + const isMounted = useRef(null); + + useEffect(() => { + isMounted.current = true; + return () => { + isMounted.current = false; + }; + }, []); useEffect(() => { (async () => { const params = parseQueryString(QS_CONFIG, history.location.search); try { const { data } = await ProjectsAPI.read(params); - setProjects(data.results); - setCount(data.count); - if (data.count === 1) { - onChange(data.results[0]); + if (isMounted.current) { + setProjects(data.results); + setCount(data.count); + if (data.count === 1) { + onChange(data.results[0]); + } } } catch (err) { - setError(err); + if (isMounted.current) { + setError(err); + } } })(); }, [onChange, history.location]); diff --git a/awx/ui_next/src/screens/Inventory/Inventories.jsx b/awx/ui_next/src/screens/Inventory/Inventories.jsx index d1a46ca8de..c4c18e76cf 100644 --- a/awx/ui_next/src/screens/Inventory/Inventories.jsx +++ b/awx/ui_next/src/screens/Inventory/Inventories.jsx @@ -19,9 +19,9 @@ class Inventories extends Component { this.state = { breadcrumbConfig: { '/inventories': i18n._(t`Inventories`), - '/inventories/inventory/add': i18n._(t`Create New Inventory`), + '/inventories/inventory/add': i18n._(t`Create new inventory`), '/inventories/smart_inventory/add': i18n._( - t`Create New Smart Inventory` + t`Create new smart inventory` ), }, }; @@ -43,42 +43,43 @@ class Inventories extends Component { const breadcrumbConfig = { '/inventories': i18n._(t`Inventories`), - '/inventories/inventory/add': i18n._(t`Create New Inventory`), - '/inventories/smart_inventory/add': i18n._(t`Create New Smart Inventory`), + '/inventories/inventory/add': i18n._(t`Create new inventory`), + '/inventories/smart_inventory/add': i18n._(t`Create new smart inventory`), [inventoryPath]: `${inventory.name}`, [`${inventoryPath}/access`]: i18n._(t`Access`), - [`${inventoryPath}/completed_jobs`]: i18n._(t`Completed Jobs`), + [`${inventoryPath}/completed_jobs`]: i18n._(t`Completed jobs`), [`${inventoryPath}/details`]: i18n._(t`Details`), - [`${inventoryPath}/edit`]: i18n._(t`Edit Details`), + [`${inventoryPath}/edit`]: i18n._(t`Edit details`), [inventoryHostsPath]: i18n._(t`Hosts`), - [`${inventoryHostsPath}/add`]: i18n._(t`Create New Host`), + [`${inventoryHostsPath}/add`]: i18n._(t`Create new host`), [`${inventoryHostsPath}/${nested?.id}`]: `${nested?.name}`, - [`${inventoryHostsPath}/${nested?.id}/edit`]: i18n._(t`Edit Details`), + [`${inventoryHostsPath}/${nested?.id}/edit`]: i18n._(t`Edit details`), [`${inventoryHostsPath}/${nested?.id}/details`]: i18n._(t`Host Details`), [`${inventoryHostsPath}/${nested?.id}/completed_jobs`]: i18n._( - t`Completed Jobs` + t`Completed jobs` ), [`${inventoryHostsPath}/${nested?.id}/facts`]: i18n._(t`Facts`), [`${inventoryHostsPath}/${nested?.id}/groups`]: i18n._(t`Groups`), [inventoryGroupsPath]: i18n._(t`Groups`), - [`${inventoryGroupsPath}/add`]: i18n._(t`Create New Group`), + [`${inventoryGroupsPath}/add`]: i18n._(t`Create new group`), [`${inventoryGroupsPath}/${nested?.id}`]: `${nested?.name}`, - [`${inventoryGroupsPath}/${nested?.id}/edit`]: i18n._(t`Edit Details`), + [`${inventoryGroupsPath}/${nested?.id}/edit`]: i18n._(t`Edit details`), [`${inventoryGroupsPath}/${nested?.id}/details`]: i18n._( - t`Group Details` + t`Group details` ), [`${inventoryGroupsPath}/${nested?.id}/nested_hosts`]: i18n._(t`Hosts`), [`${inventoryGroupsPath}/${nested?.id}/nested_hosts/add`]: i18n._( - t`Create New Host` + t`Create new host` ), [`${inventorySourcesPath}`]: i18n._(t`Sources`), - [`${inventorySourcesPath}/add`]: i18n._(t`Create New Source`), + [`${inventorySourcesPath}/add`]: i18n._(t`Create new source`), [`${inventorySourcesPath}/${nested?.id}`]: `${nested?.name}`, [`${inventorySourcesPath}/${nested?.id}/details`]: i18n._(t`Details`), + [`${inventorySourcesPath}/${nested?.id}/edit`]: i18n._(t`Edit details`), }; this.setState({ breadcrumbConfig }); }; diff --git a/awx/ui_next/src/screens/Inventory/InventorySource/InventorySource.jsx b/awx/ui_next/src/screens/Inventory/InventorySource/InventorySource.jsx index 1b88478b55..96b0b27fff 100644 --- a/awx/ui_next/src/screens/Inventory/InventorySource/InventorySource.jsx +++ b/awx/ui_next/src/screens/Inventory/InventorySource/InventorySource.jsx @@ -20,6 +20,7 @@ import ContentError from '../../../components/ContentError'; import ContentLoading from '../../../components/ContentLoading'; import RoutedTabs from '../../../components/RoutedTabs'; import InventorySourceDetail from '../InventorySourceDetail'; +import InventorySourceEdit from '../InventorySourceEdit'; function InventorySource({ i18n, inventory, setBreadcrumb }) { const location = useLocation(); @@ -38,7 +39,7 @@ function InventorySource({ i18n, inventory, setBreadcrumb }) { useEffect(() => { fetchSource(); - }, [fetchSource, match.params.sourceId]); + }, [fetchSource, location.pathname]); useEffect(() => { if (inventory && source) { @@ -104,6 +105,12 @@ function InventorySource({ i18n, inventory, setBreadcrumb }) { > + + + diff --git a/awx/ui_next/src/screens/Inventory/InventorySourceAdd/InventorySourceAdd.test.jsx b/awx/ui_next/src/screens/Inventory/InventorySourceAdd/InventorySourceAdd.test.jsx index 8d26c48560..c19c70e12f 100644 --- a/awx/ui_next/src/screens/Inventory/InventorySourceAdd/InventorySourceAdd.test.jsx +++ b/awx/ui_next/src/screens/Inventory/InventorySourceAdd/InventorySourceAdd.test.jsx @@ -1,7 +1,10 @@ import React from 'react'; import { act } from 'react-dom/test-utils'; import { createMemoryHistory } from 'history'; -import { mountWithContexts } from '../../../../testUtils/enzymeHelpers'; +import { + mountWithContexts, + waitForElement, +} from '../../../../testUtils/enzymeHelpers'; import InventorySourceAdd from './InventorySourceAdd'; import { InventorySourcesAPI, ProjectsAPI } from '../../../api'; @@ -75,6 +78,7 @@ describe('', () => { context: { config }, }); }); + await waitForElement(wrapper, 'ContentLoading', el => el.length === 0); expect(wrapper.find('FormGroup[label="Name"]')).toHaveLength(1); expect(wrapper.find('FormGroup[label="Description"]')).toHaveLength(1); expect(wrapper.find('FormGroup[label="Source"]')).toHaveLength(1); diff --git a/awx/ui_next/src/screens/Inventory/InventorySourceDetail/InventorySourceDetail.jsx b/awx/ui_next/src/screens/Inventory/InventorySourceDetail/InventorySourceDetail.jsx index 8c790a8b71..e95cc448f1 100644 --- a/awx/ui_next/src/screens/Inventory/InventorySourceDetail/InventorySourceDetail.jsx +++ b/awx/ui_next/src/screens/Inventory/InventorySourceDetail/InventorySourceDetail.jsx @@ -228,7 +228,7 @@ function InventorySourceDetail({ inventorySource, i18n }) { diff --git a/awx/ui_next/src/screens/Inventory/InventorySourceDetail/InventorySourceDetail.test.jsx b/awx/ui_next/src/screens/Inventory/InventorySourceDetail/InventorySourceDetail.test.jsx index 1eeab58dca..ca13caaf5e 100644 --- a/awx/ui_next/src/screens/Inventory/InventorySourceDetail/InventorySourceDetail.test.jsx +++ b/awx/ui_next/src/screens/Inventory/InventorySourceDetail/InventorySourceDetail.test.jsx @@ -88,7 +88,7 @@ describe('InventorySourceDetail', () => { const editButton = wrapper.find('Button[aria-label="edit"]'); expect(editButton.text()).toEqual('Edit'); expect(editButton.prop('to')).toBe( - '/inventories/inventory/2/source/123/edit' + '/inventories/inventory/2/sources/123/edit' ); expect(wrapper.find('DeleteButton')).toHaveLength(1); }); diff --git a/awx/ui_next/src/screens/Inventory/InventorySourceEdit/InventorySourceEdit.jsx b/awx/ui_next/src/screens/Inventory/InventorySourceEdit/InventorySourceEdit.jsx new file mode 100644 index 0000000000..5dce012503 --- /dev/null +++ b/awx/ui_next/src/screens/Inventory/InventorySourceEdit/InventorySourceEdit.jsx @@ -0,0 +1,68 @@ +import React, { useCallback, useEffect } from 'react'; +import { useHistory, useParams } from 'react-router-dom'; +import { Card } from '@patternfly/react-core'; +import { CardBody } from '../../../components/Card'; +import useRequest from '../../../util/useRequest'; +import { InventorySourcesAPI } from '../../../api'; +import InventorySourceForm from '../shared/InventorySourceForm'; + +function InventorySourceEdit({ source }) { + const history = useHistory(); + const { id } = useParams(); + const detailsUrl = `/inventories/inventory/${id}/sources/${source.id}/details`; + + const { error, request, result } = useRequest( + useCallback( + async values => { + const { data } = await InventorySourcesAPI.replace(source.id, values); + return data; + }, + [source.id] + ), + null + ); + + useEffect(() => { + if (result) { + history.push(detailsUrl); + } + }, [result, detailsUrl, history]); + + const handleSubmit = async form => { + const { credential, source_path, source_project, ...remainingForm } = form; + + const sourcePath = {}; + const sourceProject = {}; + if (form.source === 'scm') { + sourcePath.source_path = + source_path === '/ (project root)' ? '' : source_path; + sourceProject.source_project = source_project.id; + } + await request({ + credential: credential?.id || null, + inventory: id, + ...sourcePath, + ...sourceProject, + ...remainingForm, + }); + }; + + const handleCancel = () => { + history.push(detailsUrl); + }; + + return ( + + + + + + ); +} + +export default InventorySourceEdit; diff --git a/awx/ui_next/src/screens/Inventory/InventorySourceEdit/InventorySourceEdit.test.jsx b/awx/ui_next/src/screens/Inventory/InventorySourceEdit/InventorySourceEdit.test.jsx new file mode 100644 index 0000000000..c37b72c9f6 --- /dev/null +++ b/awx/ui_next/src/screens/Inventory/InventorySourceEdit/InventorySourceEdit.test.jsx @@ -0,0 +1,147 @@ +import React from 'react'; +import { act } from 'react-dom/test-utils'; +import { createMemoryHistory } from 'history'; +import { + mountWithContexts, + waitForElement, +} from '../../../../testUtils/enzymeHelpers'; +import InventorySourceEdit from './InventorySourceEdit'; +import { CredentialsAPI, InventorySourcesAPI, ProjectsAPI } from '../../../api'; + +jest.mock('../../../api/models/Projects'); +jest.mock('../../../api/models/Credentials'); +jest.mock('../../../api/models/InventorySources'); +jest.mock('react-router-dom', () => ({ + ...jest.requireActual('react-router-dom'), + useParams: () => ({ + id: 1, + }), +})); + +describe('', () => { + let wrapper; + let history; + const mockInvSrc = { + id: 23, + description: 'bar', + inventory: 1, + name: 'foo', + overwrite: false, + overwrite_vars: false, + source: 'scm', + source_path: 'mock/file.sh', + source_project: { id: 999 }, + source_vars: '---↵', + update_cache_timeout: 0, + update_on_launch: false, + update_on_project_update: false, + verbosity: 1, + }; + InventorySourcesAPI.readOptions.mockResolvedValue({ + data: { + actions: { + GET: { + source: { + choices: [ + ['file', 'File, Directory or Script'], + ['scm', 'Sourced from a Project'], + ['ec2', 'Amazon EC2'], + ['gce', 'Google Compute Engine'], + ['azure_rm', 'Microsoft Azure Resource Manager'], + ['vmware', 'VMware vCenter'], + ['satellite6', 'Red Hat Satellite 6'], + ['cloudforms', 'Red Hat CloudForms'], + ['openstack', 'OpenStack'], + ['rhv', 'Red Hat Virtualization'], + ['tower', 'Ansible Tower'], + ['custom', 'Custom Script'], + ], + }, + }, + }, + }, + }); + InventorySourcesAPI.replace.mockResolvedValue({ + data: { + ...mockInvSrc, + }, + }); + ProjectsAPI.readInventories.mockResolvedValue({ + data: [], + }); + CredentialsAPI.read.mockResolvedValue({ + data: { count: 0, results: [] }, + }); + ProjectsAPI.read.mockResolvedValue({ + data: { + count: 2, + results: [ + { + id: 1, + name: 'mock proj one', + }, + { + id: 2, + name: 'mock proj two', + }, + ], + }, + }); + + beforeAll(async () => { + history = createMemoryHistory(); + await act(async () => { + wrapper = mountWithContexts(, { + context: { router: { history } }, + }); + }); + await waitForElement(wrapper, 'ContentLoading', el => el.length === 0); + }); + + afterAll(() => { + jest.clearAllMocks(); + wrapper.unmount(); + }); + + test('handleSubmit should call api update', async () => { + expect(InventorySourcesAPI.replace).toHaveBeenCalledTimes(0); + await act(async () => { + wrapper.find('InventorySourceForm').invoke('onSubmit')(mockInvSrc); + }); + expect(InventorySourcesAPI.replace).toHaveBeenCalledTimes(1); + }); + + test('should navigate to inventory source detail after successful submission', () => { + expect(wrapper.find('FormSubmitError').length).toBe(0); + expect(history.location.pathname).toEqual( + '/inventories/inventory/1/sources/23/details' + ); + }); + + test('should navigate to inventory sources list when cancel is clicked', async () => { + await act(async () => { + wrapper.find('button[aria-label="Cancel"]').invoke('onClick')(); + }); + expect(history.location.pathname).toEqual( + '/inventories/inventory/1/sources/23/details' + ); + }); + + test('unsuccessful form submission should show an error message', async () => { + const error = { + response: { + data: { detail: 'An error occurred' }, + }, + }; + InventorySourcesAPI.replace.mockImplementation(() => Promise.reject(error)); + await act(async () => { + wrapper = mountWithContexts(); + }); + expect(wrapper.find('FormSubmitError').length).toBe(0); + await act(async () => { + wrapper.find('InventorySourceForm').invoke('onSubmit')({}); + }); + wrapper.update(); + expect(wrapper.find('FormSubmitError').length).toBe(1); + }); +}); diff --git a/awx/ui_next/src/screens/Inventory/InventorySourceEdit/index.js b/awx/ui_next/src/screens/Inventory/InventorySourceEdit/index.js new file mode 100644 index 0000000000..b63d354049 --- /dev/null +++ b/awx/ui_next/src/screens/Inventory/InventorySourceEdit/index.js @@ -0,0 +1 @@ +export { default } from './InventorySourceEdit'; diff --git a/awx/ui_next/src/screens/Inventory/shared/InventorySourceForm.jsx b/awx/ui_next/src/screens/Inventory/shared/InventorySourceForm.jsx index 53f51a8f05..5dfe2656e6 100644 --- a/awx/ui_next/src/screens/Inventory/shared/InventorySourceForm.jsx +++ b/awx/ui_next/src/screens/Inventory/shared/InventorySourceForm.jsx @@ -133,23 +133,24 @@ const InventorySourceForm = ({ i18n, onCancel, onSubmit, + source, submitError = null, }) => { const initialValues = { - credential: null, - custom_virtualenv: '', - description: '', - name: '', - overwrite: false, - overwrite_vars: false, - source: '', - source_path: '', - source_project: null, - source_vars: '---\n', - update_cache_timeout: 0, - update_on_launch: false, - update_on_project_update: false, - verbosity: 1, + credential: source?.summary_fields?.credential || null, + custom_virtualenv: source?.custom_virtualenv || '', + description: source?.description || '', + name: source?.name || '', + overwrite: source?.overwrite || false, + overwrite_vars: source?.overwrite_vars || false, + source: source?.source || '', + source_path: source?.source_path || '', + source_project: source?.summary_fields?.source_project || null, + source_vars: source?.source_vars || '---\n', + update_cache_timeout: source?.update_cache_timeout || 0, + update_on_launch: source?.update_on_launch || false, + update_on_project_update: source?.update_on_project_update || false, + verbosity: source?.verbosity || 1, }; const { @@ -172,21 +173,21 @@ const InventorySourceForm = ({ }; }); }, []), - [] + null ); useEffect(() => { fetchSourceOptions(); }, [fetchSourceOptions]); - if (isSourceOptionsLoading) { - return ; - } - if (sourceOptionsError) { return ; } + if (!sourceOptions || isSourceOptionsLoading) { + return ; + } + return ( { [] ); + useEffect(() => { + if (projectField.value?.id) { + fetchSourcePath(projectField.value.id); + } + }, [fetchSourcePath, projectField.value]); + const handleProjectUpdate = useCallback( value => { sourcePathHelpers.setValue('');