From a867a32b4e6e4dd2c8d5a0633ac772a74ce88413 Mon Sep 17 00:00:00 2001 From: Alex Corey Date: Mon, 23 Mar 2020 15:18:50 -0400 Subject: [PATCH 1/2] Uses formik hooks for JT Form --- .../Template/shared/JobTemplateForm.jsx | 341 ++++++++---------- 1 file changed, 143 insertions(+), 198 deletions(-) diff --git a/awx/ui_next/src/screens/Template/shared/JobTemplateForm.jsx b/awx/ui_next/src/screens/Template/shared/JobTemplateForm.jsx index 24fc4e91c3..10663415a7 100644 --- a/awx/ui_next/src/screens/Template/shared/JobTemplateForm.jsx +++ b/awx/ui_next/src/screens/Template/shared/JobTemplateForm.jsx @@ -3,7 +3,7 @@ import PropTypes from 'prop-types'; import { withRouter } from 'react-router-dom'; import { withI18n } from '@lingui/react'; import { t } from '@lingui/macro'; -import { withFormik, Field } from 'formik'; +import { withFormik, useField, useFormikContext } from 'formik'; import { Form, FormGroup, @@ -47,7 +47,6 @@ function JobTemplateForm({ validateField, handleCancel, handleSubmit, - handleBlur, setFieldValue, submitError, i18n, @@ -62,6 +61,38 @@ function JobTemplateForm({ Boolean(template?.host_config_key) ); + const formikBag = useFormikContext(); + const formikValues = formikBag.values; + const setFormikFieldValue = formikBag.setFieldValue; + const [jobTypeField, jobTypeMeta, jobTypeHelpers] = useField({ + name: 'job_type', + validate: required(null, i18n), + }); + const [, inventoryMeta, inventoryHelpers] = useField('inventory'); + + const [, projectMeta, projectHelpers] = useField({ + name: 'project', + validate: () => handleProjectValidation(), + }); + + const [scmField] = useField('scm_branch'); + + const [playbookField, , playbookMeta] = useField({ + name: 'playbook', + validate: required(i18n._(t`Select a value for this field`), i18n), + }); + + const [credentialField, , credentialHelpers] = useField('credentials'); + const [labelsField, , labelsHelpers] = useField('labels'); + const [limitField, limitMeta] = useField('limit'); + const [verbosityField] = useField('verbosity'); + const [diffModeField, , diffModeHelpers] = useField('diff_mode'); + const [instanceGroupsField, , instanceGroupsHelpers] = useField( + 'instanceGroups' + ); + const [jobTagsField, , jobTagsHelpers] = useField('job_tags'); + const [skipTagsField, , skipTagsHelpers] = useField('skip_tags'); + const { request: fetchProject, error: projectContentError, @@ -184,26 +215,15 @@ function JobTemplateForm({ test environment setup, and report problems without executing the playbook.`)} > - - {({ form, field }) => { - const isValid = !form.touched.job_type || !form.errors.job_type; - return ( - { - form.setFieldValue('job_type', value); - }} - /> - ); + { + jobTypeHelpers.setValue(value); }} - + /> - - {({ form }) => ( - <> - { - form.setFieldTouched('inventory'); - }} - onChange={value => { - form.setValues({ - ...form.values, - inventory: value.id, - organizationId: value.organization, - }); - setInventory(value); - }} - required - touched={form.touched.inventory} - error={form.errors.inventory} - /> - {(form.touched.inventory || - form.touched.ask_inventory_on_launch) && - form.errors.inventory && ( -
- {form.errors.inventory} -
- )} - + inventoryHelpers.setTouched()} + onChange={value => { + inventoryHelpers.setValue(value.id); + setFormikFieldValue('organizationId', value.organization); + setInventory(value); + }} + required + touched={inventoryMeta.touched} + error={inventoryMeta.error} + /> + {(inventoryMeta.touched || formikValues.ask_inventory_on_launch) && + inventoryMeta.error && ( +
+ {inventoryMeta.error} +
)} - - - {({ form }) => ( - form.setFieldTouched('project')} - tooltip={i18n._(t`Select the project containing the playbook + + projectHelpers.setTouched()} + tooltip={i18n._(t`Select the project containing the playbook you want this job to execute.`)} - isValid={!form.touched.project || !form.errors.project} - helperTextInvalid={form.errors.project} - onChange={handleProjectUpdate} - required - /> - )} - - {project && project.allow_override && ( + isValid={!projectMeta.touched || !projectMeta.error} + helperTextInvalid={projectMeta.error} + onChange={handleProjectUpdate} + required + /> + {project?.allow_override && ( - - {({ field }) => { - return ( - { - field.onChange(event); - }} - /> - ); + { + scmField.onChange(event); }} - + /> )} - - {({ field, form }) => { - const isValid = !form.touched.playbook || !form.errors.playbook; - return ( - - - form.setFieldTouched('playbook')} - onError={setContentError} - /> - - ); - }} - + + playbookMeta.setTouched()} + onError={setContentError} + /> + - - {({ field }) => { - return ( - - setFieldValue('credentials', newCredentials) - } - onError={setContentError} - /> - ); - }} - + + credentialHelpers.setValue(newCredentials) + } + onError={setContentError} + /> - - {({ field }) => ( - - + - setFieldValue('labels', labels)} - onError={setContentError} - /> - - )} - + /> + labelsHelpers.setValue(labels)} + onError={setContentError} + /> + - - {({ form, field }) => { - return ( - { - field.onChange(event); - }} - /> - ); + { + limitField.onChange(event); }} - + /> - - {({ field }) => ( - - )} - + - - {({ form, field }) => { - return ( - - form.setFieldValue(field.name, checked) - } - /> - ); - }} - + diffModeHelpers.setValue(checked)} + /> - - {({ field, form }) => ( - form.setFieldValue(field.name, value)} - tooltip={i18n._(t`Select the Instance Groups for this Organization + instanceGroupsHelpers.setValue(value)} + tooltip={i18n._(t`Select the Instance Groups for this Organization to run on.`)} - /> - )} - + /> - - {({ field, form }) => ( - form.setFieldValue(field.name, value)} - /> - )} - + jobTagsHelpers.setValue(value)} + /> - - {({ field, form }) => ( - form.setFieldValue(field.name, value)} - /> - )} - + skipTagsHelpers.setValue(value)} + /> Date: Thu, 26 Mar 2020 13:09:05 -0400 Subject: [PATCH 2/2] 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({