From 4b53875a71939ab1bf431070d48cd7f640dba727 Mon Sep 17 00:00:00 2001 From: Marliana Lara Date: Thu, 7 May 2020 08:57:08 -0400 Subject: [PATCH] Clear inv src subform values when source value changes * Test that inv file field resets when project value changes * Remove project and inv file path from API request when type is SCM * Update checkbox tooltip to accept node proptypes * Format option field tooltips --- .../components/FormField/CheckboxField.jsx | 4 +- .../InventorySourceAdd/InventorySourceAdd.jsx | 13 ++++- .../Inventory/shared/InventorySourceForm.jsx | 30 +++++++++-- .../shared/InventorySourceForm.test.jsx | 2 +- .../InventorySourceSubForms/SCMSubForm.jsx | 3 +- .../SCMSubForm.test.jsx | 50 +++++++++++++++++-- .../InventorySourceSubForms/SharedFields.jsx | 41 ++++++++++----- 7 files changed, 116 insertions(+), 27 deletions(-) diff --git a/awx/ui_next/src/components/FormField/CheckboxField.jsx b/awx/ui_next/src/components/FormField/CheckboxField.jsx index a04a78b02c..6a46bcaed3 100644 --- a/awx/ui_next/src/components/FormField/CheckboxField.jsx +++ b/awx/ui_next/src/components/FormField/CheckboxField.jsx @@ -1,5 +1,5 @@ import React from 'react'; -import { string, func } from 'prop-types'; +import { string, func, node } from 'prop-types'; import { useField } from 'formik'; import { Checkbox, Tooltip } from '@patternfly/react-core'; import { QuestionCircleIcon as PFQuestionCircleIcon } from '@patternfly/react-icons'; @@ -40,7 +40,7 @@ CheckboxField.propTypes = { name: string.isRequired, label: string.isRequired, validate: func, - tooltip: string, + tooltip: node, }; CheckboxField.defaultProps = { validate: () => {}, diff --git a/awx/ui_next/src/screens/Inventory/InventorySourceAdd/InventorySourceAdd.jsx b/awx/ui_next/src/screens/Inventory/InventorySourceAdd/InventorySourceAdd.jsx index 11cad37c8f..df12500a91 100644 --- a/awx/ui_next/src/screens/Inventory/InventorySourceAdd/InventorySourceAdd.jsx +++ b/awx/ui_next/src/screens/Inventory/InventorySourceAdd/InventorySourceAdd.jsx @@ -26,12 +26,21 @@ function InventorySourceAdd() { }, [result, history]); const handleSubmit = async form => { - const { credential, source_project, ...remainingForm } = 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, - source_project: source_project?.id || null, inventory: id, + ...sourcePath, + ...sourceProject, ...remainingForm, }); }; diff --git a/awx/ui_next/src/screens/Inventory/shared/InventorySourceForm.jsx b/awx/ui_next/src/screens/Inventory/shared/InventorySourceForm.jsx index eb59ed97c7..3e76f1b223 100644 --- a/awx/ui_next/src/screens/Inventory/shared/InventorySourceForm.jsx +++ b/awx/ui_next/src/screens/Inventory/shared/InventorySourceForm.jsx @@ -1,5 +1,6 @@ import React, { useEffect, useCallback, useContext } from 'react'; -import { Formik, useField } from 'formik'; +import { Formik, useField, useFormikContext } from 'formik'; +import { func, shape } from 'prop-types'; import { withI18n } from '@lingui/react'; import { t } from '@lingui/macro'; import { InventorySourcesAPI } from '@api'; @@ -21,7 +22,8 @@ import { FormColumnLayout, SubFormLayout } from '@components/FormLayout'; import SCMSubForm from './InventorySourceSubForms'; const InventorySourceFormFields = ({ sourceOptions, i18n }) => { - const [sourceField, sourceMeta, sourceHelpers] = useField({ + const { values, initialValues, resetForm } = useFormikContext(); + const [sourceField, sourceMeta] = useField({ name: 'source', validate: required(i18n._(t`Set a value for this field`), i18n), }); @@ -33,6 +35,18 @@ const InventorySourceFormFields = ({ sourceOptions, i18n }) => { key: 'default', }; + const resetSubFormFields = sourceType => { + resetForm({ + values: { + ...initialValues, + name: values.name, + description: values.description, + custom_virtualenv: values.custom_virtualenv, + source: sourceType, + }, + }); + }; + return ( <> { ...sourceOptions, ]} onChange={(event, value) => { - sourceHelpers.setValue(value); + resetSubFormFields(value); }} /> @@ -197,4 +211,14 @@ const InventorySourceForm = ({ ); }; +InventorySourceForm.propTypes = { + onCancel: func.isRequired, + onSubmit: func.isRequired, + submitError: shape({}), +}; + +InventorySourceForm.defaultProps = { + submitError: null, +}; + export default withI18n()(InventorySourceForm); diff --git a/awx/ui_next/src/screens/Inventory/shared/InventorySourceForm.test.jsx b/awx/ui_next/src/screens/Inventory/shared/InventorySourceForm.test.jsx index 2e3dae5c8d..d79cf457c9 100644 --- a/awx/ui_next/src/screens/Inventory/shared/InventorySourceForm.test.jsx +++ b/awx/ui_next/src/screens/Inventory/shared/InventorySourceForm.test.jsx @@ -14,7 +14,7 @@ describe('', () => { data: { count: 0, results: [] }, }); ProjectsAPI.readInventories.mockResolvedValue({ - data: ['foo'], + data: ['foo', 'bar'], }); InventorySourcesAPI.readOptions.mockResolvedValue({ data: { diff --git a/awx/ui_next/src/screens/Inventory/shared/InventorySourceSubForms/SCMSubForm.jsx b/awx/ui_next/src/screens/Inventory/shared/InventorySourceSubForms/SCMSubForm.jsx index 5160c31de4..9b6fbb7ba5 100644 --- a/awx/ui_next/src/screens/Inventory/shared/InventorySourceSubForms/SCMSubForm.jsx +++ b/awx/ui_next/src/screens/Inventory/shared/InventorySourceSubForms/SCMSubForm.jsx @@ -31,8 +31,7 @@ const SCMSubForm = ({ i18n }) => { } = useRequest( useCallback(async projectId => { const { data } = await ProjectsAPI.readInventories(projectId); - data.push('/ (project root)'); - return data; + return [...data, '/ (project root)']; }, []), [] ); diff --git a/awx/ui_next/src/screens/Inventory/shared/InventorySourceSubForms/SCMSubForm.test.jsx b/awx/ui_next/src/screens/Inventory/shared/InventorySourceSubForms/SCMSubForm.test.jsx index ad4d71aaf3..78ec81ad55 100644 --- a/awx/ui_next/src/screens/Inventory/shared/InventorySourceSubForms/SCMSubForm.test.jsx +++ b/awx/ui_next/src/screens/Inventory/shared/InventorySourceSubForms/SCMSubForm.test.jsx @@ -3,9 +3,10 @@ import { act } from 'react-dom/test-utils'; import { mountWithContexts } from '@testUtils/enzymeHelpers'; import { Formik } from 'formik'; import SCMSubForm from './SCMSubForm'; -import { ProjectsAPI } from '@api'; +import { ProjectsAPI, CredentialsAPI } from '@api'; jest.mock('@api/models/Projects'); +jest.mock('@api/models/Credentials'); const initialValues = { credential: null, @@ -23,9 +24,26 @@ const initialValues = { describe('', () => { let wrapper; - + CredentialsAPI.read.mockResolvedValue({ + data: { count: 0, results: [] }, + }); ProjectsAPI.readInventories.mockResolvedValue({ - data: ['foo'], + data: ['foo', 'bar'], + }); + ProjectsAPI.read.mockResolvedValue({ + data: { + count: 2, + results: [ + { + id: 1, + name: 'mock proj one', + }, + { + id: 2, + name: 'mock proj two', + }, + ], + }, }); beforeAll(async () => { @@ -59,10 +77,34 @@ describe('', () => { await act(async () => { wrapper.find('ProjectLookup').invoke('onChange')({ id: 2, - name: 'mock proj', + name: 'mock proj two', }); wrapper.find('ProjectLookup').invoke('onBlur')(); }); expect(ProjectsAPI.readInventories).toHaveBeenCalledWith(2); }); + + test('changing source project should reset source path dropdown', async () => { + expect(wrapper.find('AnsibleSelect#source_path').prop('value')).toEqual(''); + + await act(async () => { + await wrapper.find('AnsibleSelect#source_path').prop('onChange')( + null, + 'bar' + ); + }); + wrapper.update(); + expect(wrapper.find('AnsibleSelect#source_path').prop('value')).toEqual( + 'bar' + ); + + await act(async () => { + wrapper.find('ProjectLookup').invoke('onChange')({ + id: 1, + name: 'mock proj one', + }); + }); + wrapper.update(); + expect(wrapper.find('AnsibleSelect#source_path').prop('value')).toEqual(''); + }); }); diff --git a/awx/ui_next/src/screens/Inventory/shared/InventorySourceSubForms/SharedFields.jsx b/awx/ui_next/src/screens/Inventory/shared/InventorySourceSubForms/SharedFields.jsx index a0285f51c0..b665f792e5 100644 --- a/awx/ui_next/src/screens/Inventory/shared/InventorySourceSubForms/SharedFields.jsx +++ b/awx/ui_next/src/screens/Inventory/shared/InventorySourceSubForms/SharedFields.jsx @@ -63,24 +63,39 @@ export const OptionsField = withI18n()(({ i18n }) => { id="overwrite" name="overwrite" label={i18n._(t`Overwrite`)} - tooltip={i18n._(t`If checked, any hosts and groups that were - previously present on the external source but are now removed - will be removed from the Tower inventory. Hosts and groups - that were not managed by the inventory source will be promoted - to the next manually created group or if there is no manually - created group to promote them into, they will be left in the "all" - default group for the inventory. When not checked, local child - hosts and groups not found on the external source will remain - untouched by the inventory update process.`)} + tooltip={ + <> + {i18n._(t`If checked, any hosts and groups that were + previously present on the external source but are now removed + will be removed from the Tower inventory. Hosts and groups + that were not managed by the inventory source will be promoted + to the next manually created group or if there is no manually + created group to promote them into, they will be left in the "all" + default group for the inventory.`)} +
+
+ {i18n._(t`When not checked, local child + hosts and groups not found on the external source will remain + untouched by the inventory update process.`)} + + } /> + {i18n._(t`If checked, all variables for child groups + and hosts will be removed and replaced by those found + on the external source.`)} +
+
+ {i18n._(t`When not checked, a merge will be performed, + combining local variables with those found on the + external source.`)} + + } />