From fb4343d75e4c2d907efd1cd2bf3ead818cf5865d Mon Sep 17 00:00:00 2001 From: Alex Corey Date: Thu, 26 Mar 2020 13:09:05 -0400 Subject: [PATCH] Removes uncessary formikContext items in favor of useField. Removed OrgId value from formik and get that value from project field Updates tests and type.js to reflect those changes. --- .../JobTemplateAdd/JobTemplateAdd.jsx | 17 +++------ .../JobTemplateAdd/JobTemplateAdd.test.jsx | 5 ++- .../JobTemplateEdit/JobTemplateEdit.jsx | 19 ++++------ .../JobTemplateEdit/JobTemplateEdit.test.jsx | 3 +- .../Template/shared/JobTemplateForm.jsx | 35 +++++++------------ .../Template/shared/JobTemplateForm.test.jsx | 2 +- .../Template/shared/PlaybookSelect.jsx | 11 +----- awx/ui_next/src/types.js | 2 +- 8 files changed, 32 insertions(+), 62 deletions(-) diff --git a/awx/ui_next/src/screens/Template/JobTemplateAdd/JobTemplateAdd.jsx b/awx/ui_next/src/screens/Template/JobTemplateAdd/JobTemplateAdd.jsx index c87d354f8e..201fdb2b3c 100644 --- a/awx/ui_next/src/screens/Template/JobTemplateAdd/JobTemplateAdd.jsx +++ b/awx/ui_next/src/screens/Template/JobTemplateAdd/JobTemplateAdd.jsx @@ -3,7 +3,7 @@ import { useHistory } from 'react-router-dom'; import { Card, PageSection } from '@patternfly/react-core'; import { CardBody } from '@components/Card'; import JobTemplateForm from '../shared/JobTemplateForm'; -import { JobTemplatesAPI, OrganizationsAPI } from '@api'; +import { JobTemplatesAPI } from '@api'; function JobTemplateAdd() { const [formSubmitError, setFormSubmitError] = useState(null); @@ -12,7 +12,6 @@ function JobTemplateAdd() { async function handleSubmit(values) { const { labels, - organizationId, instanceGroups, initialInstanceGroups, credentials, @@ -20,12 +19,13 @@ function JobTemplateAdd() { } = values; setFormSubmitError(null); + remainingValues.project = remainingValues.project.id; try { const { data: { id, type }, } = await JobTemplatesAPI.create(remainingValues); await Promise.all([ - submitLabels(id, labels, organizationId), + submitLabels(id, labels, values.project.summary_fields.organization.id), submitInstanceGroups(id, instanceGroups), submitCredentials(id, credentials), ]); @@ -35,16 +35,7 @@ function JobTemplateAdd() { } } - async function submitLabels(templateId, labels = [], formOrg) { - let orgId = formOrg; - - if (!orgId && labels.length > 0) { - const { - data: { results }, - } = await OrganizationsAPI.read(); - orgId = results[0].id; - } - + async function submitLabels(templateId, labels = [], orgId) { const associationPromises = labels.map(label => JobTemplatesAPI.associateLabel(templateId, label, orgId) ); diff --git a/awx/ui_next/src/screens/Template/JobTemplateAdd/JobTemplateAdd.test.jsx b/awx/ui_next/src/screens/Template/JobTemplateAdd/JobTemplateAdd.test.jsx index 84847af506..504b815d87 100644 --- a/awx/ui_next/src/screens/Template/JobTemplateAdd/JobTemplateAdd.test.jsx +++ b/awx/ui_next/src/screens/Template/JobTemplateAdd/JobTemplateAdd.test.jsx @@ -33,7 +33,7 @@ const jobTemplateData = { limit: '', name: '', playbook: '', - project: 1, + project: { id: 1, summary_fields: { organization: { id: 1 } } }, scm_branch: '', skip_tags: '', timeout: 0, @@ -123,6 +123,7 @@ describe('', () => { wrapper.find('ProjectLookup').invoke('onChange')({ id: 2, name: 'project', + summary_fields: { organization: { id: 1, name: 'Org Foo' } }, }); wrapper.update(); wrapper @@ -161,6 +162,7 @@ describe('', () => { id: 1, type: 'job_template', ...jobTemplateData, + project: jobTemplateData.project.id, }, }); let wrapper; @@ -181,6 +183,7 @@ describe('', () => { wrapper.find('ProjectLookup').invoke('onChange')({ id: 2, name: 'project', + summary_fields: { organization: { id: 1, name: 'Org Foo' } }, }); wrapper.update(); wrapper diff --git a/awx/ui_next/src/screens/Template/JobTemplateEdit/JobTemplateEdit.jsx b/awx/ui_next/src/screens/Template/JobTemplateEdit/JobTemplateEdit.jsx index 07ef9708a7..f653d1082c 100644 --- a/awx/ui_next/src/screens/Template/JobTemplateEdit/JobTemplateEdit.jsx +++ b/awx/ui_next/src/screens/Template/JobTemplateEdit/JobTemplateEdit.jsx @@ -4,7 +4,7 @@ import { withRouter, Redirect } from 'react-router-dom'; import { CardBody } from '@components/Card'; import ContentError from '@components/ContentError'; import ContentLoading from '@components/ContentLoading'; -import { JobTemplatesAPI, OrganizationsAPI, ProjectsAPI } from '@api'; +import { JobTemplatesAPI, ProjectsAPI } from '@api'; import { JobTemplate } from '@types'; import { getAddedAndRemoved } from '@util/lists'; import JobTemplateForm from '../shared/JobTemplateForm'; @@ -97,7 +97,6 @@ class JobTemplateEdit extends Component { const { template, history } = this.props; const { labels, - organizationId, instanceGroups, initialInstanceGroups, credentials, @@ -105,10 +104,14 @@ class JobTemplateEdit extends Component { } = values; this.setState({ formSubmitError: null }); + remainingValues.project = values.project.id; try { await JobTemplatesAPI.update(template.id, remainingValues); await Promise.all([ - this.submitLabels(labels, organizationId), + this.submitLabels( + labels, + values.project.summary_fields.organization.id + ), this.submitInstanceGroups(instanceGroups, initialInstanceGroups), this.submitCredentials(credentials), ]); @@ -118,22 +121,14 @@ class JobTemplateEdit extends Component { } } - async submitLabels(labels = [], formOrgId) { + async submitLabels(labels = [], orgId) { const { template } = this.props; - let orgId = formOrgId; const { added, removed } = getAddedAndRemoved( template.summary_fields.labels.results, labels ); - if (!orgId && added.length > 0) { - const { - data: { results }, - } = await OrganizationsAPI.read(); - orgId = results[0].id; - } - const disassociationPromises = removed.map(label => JobTemplatesAPI.disassociateLabel(template.id, label) ); diff --git a/awx/ui_next/src/screens/Template/JobTemplateEdit/JobTemplateEdit.test.jsx b/awx/ui_next/src/screens/Template/JobTemplateEdit/JobTemplateEdit.test.jsx index 92c47ecf05..fef5d89920 100644 --- a/awx/ui_next/src/screens/Template/JobTemplateEdit/JobTemplateEdit.test.jsx +++ b/awx/ui_next/src/screens/Template/JobTemplateEdit/JobTemplateEdit.test.jsx @@ -35,7 +35,7 @@ const mockJobTemplate = { limit: '', name: 'Foo', playbook: 'Baz', - project: 3, + project: { id: 3, summary_fields: { organization: { id: 1 } } }, scm_branch: '', skip_tags: '', summary_fields: { @@ -239,6 +239,7 @@ describe('', () => { const expected = { ...mockJobTemplate, + project: mockJobTemplate.project.id, ...updatedTemplateData, }; delete expected.summary_fields; diff --git a/awx/ui_next/src/screens/Template/shared/JobTemplateForm.jsx b/awx/ui_next/src/screens/Template/shared/JobTemplateForm.jsx index 10663415a7..9e2b0ae0ce 100644 --- a/awx/ui_next/src/screens/Template/shared/JobTemplateForm.jsx +++ b/awx/ui_next/src/screens/Template/shared/JobTemplateForm.jsx @@ -50,7 +50,6 @@ function JobTemplateForm({ setFieldValue, submitError, i18n, - touched, }) { const [contentError, setContentError] = useState(false); const [project, setProject] = useState(null); @@ -61,23 +60,20 @@ function JobTemplateForm({ Boolean(template?.host_config_key) ); - const formikBag = useFormikContext(); - const formikValues = formikBag.values; - const setFormikFieldValue = formikBag.setFieldValue; + const { values: formikValues } = useFormikContext(); const [jobTypeField, jobTypeMeta, jobTypeHelpers] = useField({ name: 'job_type', validate: required(null, i18n), }); const [, inventoryMeta, inventoryHelpers] = useField('inventory'); - - const [, projectMeta, projectHelpers] = useField({ + const [projectField, projectMeta, projectHelpers] = useField({ name: 'project', validate: () => handleProjectValidation(), }); - const [scmField] = useField('scm_branch'); + const [scmField, , scmHelpers] = useField('scm_branch'); - const [playbookField, , playbookMeta] = useField({ + const [playbookField, playbookMeta, playbookHelpers] = useField({ name: 'playbook', validate: required(i18n._(t`Select a value for this field`), i18n), }); @@ -131,7 +127,7 @@ function JobTemplateForm({ }, [loadRelatedInstanceGroups]); const handleProjectValidation = () => { - if (!project && touched.project) { + if (!project && projectMeta.touched) { return i18n._(t`Select a value for this field`); } if (project && project.status === 'never updated') { @@ -143,11 +139,11 @@ function JobTemplateForm({ const handleProjectUpdate = useCallback( newProject => { setProject(newProject); - setFieldValue('project', newProject.id); - setFieldValue('playbook', 0); - setFieldValue('scm_branch', ''); + projectHelpers.setValue(newProject); + playbookHelpers.setValue(0); + scmHelpers.setValue(''); }, - [setFieldValue, setProject] + [setProject, projectHelpers, playbookHelpers, scmHelpers] ); const jobTypeOptions = [ @@ -239,7 +235,6 @@ function JobTemplateForm({ onBlur={() => inventoryHelpers.setTouched()} onChange={value => { inventoryHelpers.setValue(value.id); - setFormikFieldValue('organizationId', value.organization); setInventory(value); }} required @@ -293,11 +288,10 @@ function JobTemplateForm({ content={i18n._(t`Select the playbook to be executed by this job.`)} /> playbookMeta.setTouched()} + onBlur={() => playbookHelpers.setTouched()} onError={setContentError} /> @@ -581,10 +575,6 @@ const FormikApp = withFormik({ }, } = template; - const hasInventory = summary_fields.inventory - ? summary_fields.inventory.organization_id - : null; - return { ask_credential_on_launch: template.ask_credential_on_launch || false, ask_diff_mode_on_launch: template.ask_diff_mode_on_launch || false, @@ -600,7 +590,7 @@ const FormikApp = withFormik({ description: template.description || '', job_type: template.job_type || 'run', inventory: template.inventory || null, - project: template.project || '', + project: template.project || null, scm_branch: template.scm_branch || '', playbook: template.playbook || '', labels: summary_fields.labels.results || [], @@ -617,7 +607,6 @@ const FormikApp = withFormik({ allow_simultaneous: template.allow_simultaneous || false, use_fact_cache: template.use_fact_cache || false, host_config_key: template.host_config_key || '', - organizationId: hasInventory, initialInstanceGroups: [], instanceGroups: [], credentials: summary_fields.credentials || [], diff --git a/awx/ui_next/src/screens/Template/shared/JobTemplateForm.test.jsx b/awx/ui_next/src/screens/Template/shared/JobTemplateForm.test.jsx index 383bef39f0..47b9f6b436 100644 --- a/awx/ui_next/src/screens/Template/shared/JobTemplateForm.test.jsx +++ b/awx/ui_next/src/screens/Template/shared/JobTemplateForm.test.jsx @@ -14,7 +14,7 @@ describe('', () => { description: 'Bar', job_type: 'run', inventory: 2, - project: 3, + project: { id: 3, summary_fields: { organization: { id: 1 } } }, playbook: 'Baz', type: 'job_template', scm_branch: 'Foo', diff --git a/awx/ui_next/src/screens/Template/shared/PlaybookSelect.jsx b/awx/ui_next/src/screens/Template/shared/PlaybookSelect.jsx index b0e24fe283..226f4836a1 100644 --- a/awx/ui_next/src/screens/Template/shared/PlaybookSelect.jsx +++ b/awx/ui_next/src/screens/Template/shared/PlaybookSelect.jsx @@ -5,15 +5,7 @@ import { t } from '@lingui/macro'; import AnsibleSelect from '@components/AnsibleSelect'; import { ProjectsAPI } from '@api'; -function PlaybookSelect({ - projectId, - isValid, - form, - field, - onBlur, - onError, - i18n, -}) { +function PlaybookSelect({ projectId, isValid, field, onBlur, onError, i18n }) { const [options, setOptions] = useState([]); useEffect(() => { @@ -47,7 +39,6 @@ function PlaybookSelect({ id="template-playbook" data={options} isValid={isValid} - form={form} {...field} onBlur={onBlur} /> diff --git a/awx/ui_next/src/types.js b/awx/ui_next/src/types.js index c1b592cb5a..612a03afc5 100644 --- a/awx/ui_next/src/types.js +++ b/awx/ui_next/src/types.js @@ -70,7 +70,7 @@ export const JobTemplate = shape({ inventory: number, job_type: oneOf(['run', 'check']), playbook: string, - project: number, + project: shape({}), }); export const Inventory = shape({