From 8b69b089915f136f53da57af10e9357efda66293 Mon Sep 17 00:00:00 2001 From: Alex Corey Date: Thu, 2 Apr 2020 15:37:40 -0400 Subject: [PATCH 1/3] Adds formik hook functionality to wfjt form --- .../WorkflowJobTemplateAdd.jsx | 14 +- .../WorkflowJobTemplateEdit.jsx | 19 +- .../shared/WorkflowJobTemplateForm.jsx | 678 ++++++++---------- 3 files changed, 330 insertions(+), 381 deletions(-) diff --git a/awx/ui_next/src/screens/Template/WorkflowJobTemplateAdd/WorkflowJobTemplateAdd.jsx b/awx/ui_next/src/screens/Template/WorkflowJobTemplateAdd/WorkflowJobTemplateAdd.jsx index c1efe45572..f69349bb70 100644 --- a/awx/ui_next/src/screens/Template/WorkflowJobTemplateAdd/WorkflowJobTemplateAdd.jsx +++ b/awx/ui_next/src/screens/Template/WorkflowJobTemplateAdd/WorkflowJobTemplateAdd.jsx @@ -12,7 +12,19 @@ function WorkflowJobTemplateAdd() { const [formSubmitError, setFormSubmitError] = useState(null); const handleSubmit = async values => { - const { labels, organizationId, ...remainingValues } = values; + const { + labels, + inventory, + organization, + webhook_credential, + webhookKey, + ...remainingValues + } = values; + remainingValues.inventory = inventory?.id; + remainingValues.organization = organization?.id; + remainingValues.webhook_credential = webhook_credential?.id; + const organizationId = + organization?.id || inventory?.summary_fields?.organization.id || null; try { const { data: { id }, diff --git a/awx/ui_next/src/screens/Template/WorkflowJobTemplateEdit/WorkflowJobTemplateEdit.jsx b/awx/ui_next/src/screens/Template/WorkflowJobTemplateEdit/WorkflowJobTemplateEdit.jsx index b958ff9ff4..0d0d94b7de 100644 --- a/awx/ui_next/src/screens/Template/WorkflowJobTemplateEdit/WorkflowJobTemplateEdit.jsx +++ b/awx/ui_next/src/screens/Template/WorkflowJobTemplateEdit/WorkflowJobTemplateEdit.jsx @@ -11,10 +11,23 @@ function WorkflowJobTemplateEdit({ template, webhook_key }) { const [formSubmitError, setFormSubmitError] = useState(null); const handleSubmit = async values => { - const { labels, ...remainingValues } = values; + const { + labels, + inventory, + organization, + webhook_credential, + webhookKey, + ...remainingValues + } = values; + remainingValues.inventory = inventory?.id; + remainingValues.organization = organization?.id; + remainingValues.webhook_credential = webhook_credential?.id || null; + + const formOrgId = + organization?.id || inventory?.summary_fields?.organization.id || null; try { await Promise.all( - await submitLabels(labels, values.organization, template.organization) + await submitLabels(labels, formOrgId, template.organization) ); await WorkflowJobTemplatesAPI.update(template.id, remainingValues); history.push(`/templates/workflow_job_template/${template.id}/details`); @@ -60,7 +73,7 @@ function WorkflowJobTemplateEdit({ template, webhook_key }) { handleSubmit={handleSubmit} handleCancel={handleCancel} template={template} - webhook_key={webhook_key} + webhookKey={webhook_key} submitError={formSubmitError} /> diff --git a/awx/ui_next/src/screens/Template/shared/WorkflowJobTemplateForm.jsx b/awx/ui_next/src/screens/Template/shared/WorkflowJobTemplateForm.jsx index 1079f9a5dd..673bce96cb 100644 --- a/awx/ui_next/src/screens/Template/shared/WorkflowJobTemplateForm.jsx +++ b/awx/ui_next/src/screens/Template/shared/WorkflowJobTemplateForm.jsx @@ -1,11 +1,11 @@ import React, { useState, useEffect, useCallback } from 'react'; import { t } from '@lingui/macro'; -import { useRouteMatch, useParams } from 'react-router-dom'; +import { useRouteMatch, useParams, withRouter } from 'react-router-dom'; -import { func, shape } from 'prop-types'; +import PropTypes, { shape } from 'prop-types'; import { withI18n } from '@lingui/react'; -import { Formik, Field } from 'formik'; +import { useField, withFormik } from 'formik'; import { Form, FormGroup, @@ -40,38 +40,49 @@ import ContentError from '@components/ContentError'; import CheckboxField from '@components/FormField/CheckboxField'; import LabelSelect from './LabelSelect'; +const urlOrigin = window.location.origin; function WorkflowJobTemplateForm({ handleSubmit, handleCancel, i18n, - template = {}, - webhook_key, submitError, }) { - const urlOrigin = window.location.origin; const { id } = useParams(); const wfjtAddMatch = useRouteMatch('/templates/workflow_job_template/add'); const [hasContentError, setContentError] = useState(null); - const [webhook_url, setWebhookUrl] = useState( - template?.related?.webhook_receiver - ? `${urlOrigin}${template.related.webhook_receiver}` - : '' + + const [organizationField, organizationMeta, organizationHelpers] = useField( + 'organization' ); - const [inventory, setInventory] = useState( - template?.summary_fields?.inventory || null + const [inventoryField, inventoryMeta, inventoryHelpers] = useField( + 'inventory' ); - const [organization, setOrganization] = useState( - template?.summary_fields?.organization || null + const [labelsField, , labelsHelpers] = useField('labels'); + + const [ + webhookServiceField, + webhookServiceMeta, + webhookServiceHelpers, + ] = useField('webhook_service'); + + const [webhookKeyField, webhookKeyMeta, webhookKeyHelpers] = useField( + 'webhookKey' ); - const [webhookCredential, setWebhookCredential] = useState( - template?.summary_fields?.webhook_credential || null + + const [hasWebhooks, setHasWebhooks] = useState( + Boolean(webhookServiceField.value) ); - const [webhookKey, setWebHookKey] = useState(webhook_key); - const [webhookService, setWebHookService] = useState( - template.webhook_service || '' + + const [ + webhookCredentialField, + webhookCredentialMeta, + webhookCredentialHelpers, + ] = useField('webhook_credential'); + + const [webhookUrlField, webhookUrlMeta, webhookUrlHelpers] = useField( + 'webhook_url' ); - const [hasWebhooks, setHasWebhooks] = useState(Boolean(webhookService)); const webhookServiceOptions = [ { @@ -93,6 +104,38 @@ function WorkflowJobTemplateForm({ isDisabled: false, }, ]; + + const storeWebhookValues = webhookServiceValue => { + if ( + webhookServiceValue === webhookServiceMeta.initialValue || + webhookServiceValue === '' + ) { + webhookCredentialHelpers.setValue(webhookCredentialMeta.initialValue); + webhookUrlHelpers.setValue(webhookUrlMeta.initialValue); + webhookServiceHelpers.setValue(webhookServiceMeta.initialValue); + webhookKeyHelpers.setValue(webhookKeyMeta.initialValue); + } else { + webhookCredentialHelpers.setValue(null); + webhookUrlHelpers.setValue( + `${urlOrigin}/api/v2/workflow_job_templates/${id}/${webhookServiceValue}/` + ); + webhookKeyHelpers.setValue( + i18n._(t`a new webhook key will be generated on save.`).toUpperCase() + ); + } + }; + + const handleWebhookEnablement = (enabledWebhooks, webhookServiceValue) => { + if (!enabledWebhooks) { + webhookCredentialHelpers.setValue(null); + webhookServiceHelpers.setValue(''); + webhookUrlHelpers.setValue(''); + webhookKeyHelpers.setValue(''); + } else { + storeWebhookValues(webhookServiceValue); + } + }; + const { request: loadCredentialType, error: contentError, @@ -101,15 +144,15 @@ function WorkflowJobTemplateForm({ } = useRequest( useCallback(async () => { let results; - if (webhookService) { + if (webhookServiceField.value) { results = await CredentialTypesAPI.read({ - namespace: `${webhookService}_token`, + namespace: `${webhookServiceField.value}_token`, }); // TODO: Consider how to handle the situation where the results returns // and empty array, or any of the other values is undefined or null (data, results, id) } return results?.data?.results[0]?.id; - }, [webhookService]) + }, [webhookServiceField.value]) ); useEffect(() => { @@ -124,66 +167,12 @@ function WorkflowJobTemplateForm({ const { data: { webhook_key: key }, } = await WorkflowJobTemplatesAPI.updateWebhookKey(id); - setWebHookKey(key); + webhookKeyHelpers.setValue(key); } catch (err) { setContentError(err); } }; - let initialWebhookKey = webhook_key; - const initialWebhookCredential = template?.summary_fields?.webhook_credential; - - const storeWebhookValues = (form, webhookServiceValue) => { - if ( - webhookServiceValue === form.initialValues.webhook_service || - webhookServiceValue === '' - ) { - form.setFieldValue( - 'webhook_credential', - form.initialValues.webhook_credential - ); - setWebhookCredential(initialWebhookCredential); - - setWebhookUrl( - template?.related?.webhook_receiver - ? `${urlOrigin}${template.related.webhook_receiver}` - : '' - ); - form.setFieldValue('webhook_service', form.initialValues.webhook_service); - setWebHookService(form.initialValues.webhook_service); - - setWebHookKey(initialWebhookKey); - } else { - form.setFieldValue('webhook_credential', null); - setWebhookCredential(null); - - setWebhookUrl( - `${urlOrigin}/api/v2/workflow_job_templates/${template.id}/${webhookServiceValue}/` - ); - - setWebHookKey( - i18n._(t`a new webhook key will be generated on save.`).toUpperCase() - ); - } - }; - - const handleWebhookEnablement = ( - form, - enabledWebhooks, - webhookServiceValue - ) => { - if (!enabledWebhooks) { - initialWebhookKey = webhookKey; - form.setFieldValue('webhook_credential', null); - form.setFieldValue('webhook_service', ''); - setWebhookUrl(''); - setWebHookService(''); - setWebHookKey(''); - } else { - storeWebhookValues(form, webhookServiceValue); - } - }; - if (hasContentError || contentError) { return ; } @@ -193,312 +182,213 @@ function WorkflowJobTemplateForm({ } return ( - { - if (values.webhook_service === '') { - values.webhook_credential = ''; - } - return handleSubmit(values); - }} - initialValues={{ - name: template.name || '', - description: template.description || '', - inventory: template?.summary_fields?.inventory?.id || null, - organization: template?.summary_fields?.organization?.id || null, - labels: template.summary_fields?.labels?.results || [], - extra_vars: template.extra_vars || '---', - limit: template.limit || '', - scm_branch: template.scm_branch || '', - allow_simultaneous: template.allow_simultaneous || false, - webhook_credential: - template?.summary_fields?.webhook_credential?.id || null, - webhook_service: template.webhook_service || '', - ask_limit_on_launch: template.ask_limit_on_launch || false, - ask_inventory_on_launch: template.ask_inventory_on_launch || false, - ask_variables_on_launch: template.ask_variables_on_launch || false, - ask_scm_branch_on_launch: template.ask_scm_branch_on_launch || false, - }} - > - {formik => ( -
- - - - - {({ form }) => ( - { - form.setFieldValue('organization', value?.id || null); - setOrganization(value); - }} - value={organization} - isValid={!form.errors.organization} - /> - )} - - - {({ form }) => ( - - - { - form.setFieldValue('inventory', value?.id || null); - setInventory(value); - form.setFieldValue('organizationId', value?.organization); - }} - /> - - )} - - - - - - - {({ form, field }) => ( - - + + + + { + organizationHelpers.setValue(value || null); + }} + value={organizationField.value} + isValid={!organizationMeta.error} + /> + + + { + inventoryHelpers.setValue(value || null); + }} + /> + + + + + + + - form.setFieldValue('labels', labels)} - onError={err => setContentError(err)} - /> - - )} - - - - - - - - {({ form }) => ( - - {i18n._(t`Enable Webhooks`)} -   - - - } - id="wfjt-enabled-webhooks" - isChecked={ - Boolean(form.values.webhook_service) || hasWebhooks - } - onChange={checked => { - setHasWebhooks(checked); - handleWebhookEnablement(form, checked, webhookService); - }} - /> - )} - - - - - {hasWebhooks && ( - - - {({ form, field }) => ( - - - { - setWebHookService(val); - storeWebhookValues(form, val); - - form.setFieldValue('webhook_service', val); - }} - /> - - )} - - {!wfjtAddMatch && ( - <> - - - - - - {({ form }) => ( - - - - - - - - )} - - - )} - {credTypeId && ( - // TODO: Consider how to handle the situation where the results returns - // an empty array, or any of the other values is undefined or null - // (data, results, id) - - {({ form }) => ( - { - form.setFieldValue( - 'webhook_credential', - value?.id || null - ); - setWebhookCredential(value); - }} - isValid={!form.errors.webhook_credential} - helperTextInvalid={form.errors.webhook_credential} - value={webhookCredential} - /> - )} - - )} - - )} - {submitError && } - - + labelsHelpers.setValue(labels)} + onError={setContentError} + /> + + + + + + + + {i18n._(t`Enable Webhook`)} +   + + + } + id="wfjt-enabled-webhooks" + isChecked={Boolean(webhookServiceField.value) || hasWebhooks} + onChange={checked => { + setHasWebhooks(checked); + handleWebhookEnablement(checked, webhookServiceField.value); + }} + /> + + + {hasWebhooks && ( + + + + { + storeWebhookValues(val); + + webhookServiceHelpers.setValue(val); + }} + /> + + {!wfjtAddMatch && ( + <> + + + + + + + + + + + + + )} + {credTypeId && ( + // TODO: Consider how to handle the situation where the results returns + // an empty array, or any of the other values is undefined or null + // (data, results, id) + { + webhookCredentialHelpers.setValue(value || null); + }} + isValid={!webhookCredentialMeta.error} + helperTextInvalid={webhookCredentialMeta.error} + value={webhookCredentialField.value} + /> + )} + )} -
+ {submitError && } + + ); } WorkflowJobTemplateForm.propTypes = { - handleSubmit: func.isRequired, - handleCancel: func.isRequired, + handleSubmit: PropTypes.func.isRequired, + handleCancel: PropTypes.func.isRequired, submitError: shape({}), }; @@ -506,4 +396,38 @@ WorkflowJobTemplateForm.defaultProps = { submitError: null, }; -export default withI18n()(WorkflowJobTemplateForm); +const FormikApp = withFormik({ + mapPropsToValues({ template = {}, webhookKey }) { + return { + name: template.name || '', + description: template.description || '', + inventory: template?.summary_fields?.inventory || null, + organization: template?.summary_fields?.organization || null, + labels: template.summary_fields?.labels?.results || [], + extra_vars: template.extra_vars || '---', + limit: template.limit || '', + scm_branch: template.scm_branch || '', + allow_simultaneous: template.allow_simultaneous || false, + webhook_credential: template?.summary_fields?.webhook_credential || null, + webhook_service: template.webhook_service || '', + ask_limit_on_launch: template.ask_limit_on_launch || false, + ask_inventory_on_launch: template.ask_inventory_on_launch || false, + ask_variables_on_launch: template.ask_variables_on_launch || false, + ask_scm_branch_on_launch: template.ask_scm_branch_on_launch || false, + webhook_url: template?.related?.webhook_receiver + ? `${urlOrigin}${template.related.webhook_receiver}` + : '', + webhookKey: webhookKey || null, + }; + }, + handleSubmit: async (values, { props, setErrors }) => { + try { + await props.handleSubmit(values); + } catch (errors) { + setErrors(errors); + } + }, +})(WorkflowJobTemplateForm); + +export { WorkflowJobTemplateForm as _WorkflowJobTemplateForm }; +export default withI18n()(withRouter(FormikApp)); From 27e6c2d47d118ad834d707db8003026e763a5e8c Mon Sep 17 00:00:00 2001 From: Alex Corey Date: Thu, 2 Apr 2020 15:38:28 -0400 Subject: [PATCH 2/3] Adds tests --- .../WorkflowJobTemplateAdd.test.jsx | 66 ++++++++++++++--- .../WorkflowJobTemplateEdit.test.jsx | 71 +++++++++++++++---- .../shared/WorkflowJobTemplateForm.test.jsx | 14 ++-- 3 files changed, 120 insertions(+), 31 deletions(-) diff --git a/awx/ui_next/src/screens/Template/WorkflowJobTemplateAdd/WorkflowJobTemplateAdd.test.jsx b/awx/ui_next/src/screens/Template/WorkflowJobTemplateAdd/WorkflowJobTemplateAdd.test.jsx index e60535e4da..c76b71a4f1 100644 --- a/awx/ui_next/src/screens/Template/WorkflowJobTemplateAdd/WorkflowJobTemplateAdd.test.jsx +++ b/awx/ui_next/src/screens/Template/WorkflowJobTemplateAdd/WorkflowJobTemplateAdd.test.jsx @@ -15,9 +15,11 @@ jest.mock('@api/models/Inventories'); describe('', () => { let wrapper; let history; + const handleSubmit = jest.fn(); + const handleCancel = jest.fn(); beforeEach(async () => { WorkflowJobTemplatesAPI.create.mockResolvedValue({ data: { id: 1 } }); - OrganizationsAPI.read.mockResolvedValue({ results: [{ id: 1 }] }); + OrganizationsAPI.read.mockResolvedValue({ data: { results: [{ id: 1 }] } }); LabelsAPI.read.mockResolvedValue({ data: { results: [ @@ -36,7 +38,12 @@ describe('', () => { wrapper = mountWithContexts( } + component={() => ( + + )} />, { context: { @@ -63,16 +70,49 @@ describe('', () => { test('calls workflowJobTemplatesAPI with correct information on submit', async () => { await act(async () => { - await wrapper.find('WorkflowJobTemplateForm').invoke('handleSubmit')({ - name: 'Alex', - labels: [{ name: 'Foo', id: 1 }, { name: 'bar', id: 2 }], - organizationId: 1, + wrapper.find('input#wfjt-name').simulate('change', { + target: { value: 'Alex', name: 'name' }, }); + + wrapper + .find('LabelSelect') + .find('SelectToggle') + .simulate('click'); }); - expect(WorkflowJobTemplatesAPI.create).toHaveBeenCalledWith({ + + wrapper.update(); + + act(() => { + wrapper + .find('SelectOption') + .find('button') + .at(2) + .prop('onClick')(); + }); + + wrapper.update(); + await act(async () => { + wrapper.find('form').simulate('submit'); + }); + await expect(WorkflowJobTemplatesAPI.create).toHaveBeenCalledWith({ name: 'Alex', + allow_simultaneous: false, + ask_inventory_on_launch: false, + ask_limit_on_launch: false, + ask_scm_branch_on_launch: false, + ask_variables_on_launch: false, + description: '', + extra_vars: '---', + inventory: undefined, + limit: '', + organization: undefined, + scm_branch: '', + webhook_credential: undefined, + webhook_service: '', + webhook_url: '', }); - expect(WorkflowJobTemplatesAPI.associateLabel).toHaveBeenCalledTimes(2); + + expect(WorkflowJobTemplatesAPI.associateLabel).toHaveBeenCalledTimes(1); }); test('handleCancel navigates the user to the /templates', async () => { @@ -95,10 +135,16 @@ describe('', () => { WorkflowJobTemplatesAPI.create.mockRejectedValue(error); await act(async () => { - wrapper.find('WorkflowJobTemplateForm').invoke('handleSubmit')({ - name: 'Foo', + wrapper.find('input#wfjt-name').simulate('change', { + target: { value: 'Alex', name: 'name' }, }); }); + + wrapper.update(); + await act(async () => { + wrapper.find('form').simulate('submit'); + }); + expect(WorkflowJobTemplatesAPI.create).toHaveBeenCalled(); wrapper.update(); expect(wrapper.find('WorkflowJobTemplateForm').prop('submitError')).toEqual( diff --git a/awx/ui_next/src/screens/Template/WorkflowJobTemplateEdit/WorkflowJobTemplateEdit.test.jsx b/awx/ui_next/src/screens/Template/WorkflowJobTemplateEdit/WorkflowJobTemplateEdit.test.jsx index 7ec426e096..9acf279c1e 100644 --- a/awx/ui_next/src/screens/Template/WorkflowJobTemplateEdit/WorkflowJobTemplateEdit.test.jsx +++ b/awx/ui_next/src/screens/Template/WorkflowJobTemplateEdit/WorkflowJobTemplateEdit.test.jsx @@ -29,10 +29,15 @@ const mockTemplate = { describe('', () => { let wrapper; let history; + beforeEach(async () => { LabelsAPI.read.mockResolvedValue({ data: { - results: [{ name: 'Label 1', id: 1 }, { name: 'Label 2', id: 2 }], + results: [ + { name: 'Label 1', id: 1 }, + { name: 'Label 2', id: 2 }, + { name: 'Label 3', id: 3 }, + ], }, }); OrganizationsAPI.read.mockResolvedValue({ results: [{ id: 1 }] }); @@ -71,29 +76,67 @@ describe('', () => { }); test('api is called to properly to save the updated template.', async () => { - await act(async () => { - await wrapper.find('WorkflowJobTemplateForm').invoke('handleSubmit')({ - id: 6, - name: 'Alex', - description: 'Apollo and Athena', - inventory: 1, - organization: 1, - labels: [{ name: 'Label 2', id: 2 }, { name: 'Generated Label' }], - scm_branch: 'master', - limit: '5000', - variables: '---', + act(() => { + wrapper.find('input#wfjt-name').simulate('change', { + target: { value: 'Alex', name: 'name' }, }); + wrapper.find('input#wfjt-description').simulate('change', { + target: { value: 'Apollo and Athena', name: 'description' }, + }); + wrapper.find('input#wfjt-description').simulate('change', { + target: { value: 'master', name: 'scm_branch' }, + }); + wrapper.find('input#wfjt-description').simulate('change', { + target: { value: '5000', name: 'limit' }, + }); + wrapper + .find('LabelSelect') + .find('SelectToggle') + .simulate('click'); + }); + + wrapper.update(); + + act(() => { + wrapper + .find('SelectOption') + .find('button') + .at(2) + .prop('onClick')(); + }); + + wrapper.update(); + + act(() => + wrapper + .find('SelectOption') + .find('button') + .at(0) + .prop('onClick')() + ); + + wrapper.update(); + + await act(async () => { + wrapper.find('WorkflowJobTemplateForm').invoke('handleSubmit')(); }); expect(WorkflowJobTemplatesAPI.update).toHaveBeenCalledWith(6, { - id: 6, name: 'Alex', description: 'Apollo and Athena', inventory: 1, organization: 1, scm_branch: 'master', limit: '5000', - variables: '---', + extra_vars: '---', + webhook_credential: null, + webhook_url: '', + webhook_service: '', + allow_simultaneous: false, + ask_inventory_on_launch: false, + ask_limit_on_launch: false, + ask_scm_branch_on_launch: false, + ask_variables_on_launch: false, }); wrapper.update(); await expect(WorkflowJobTemplatesAPI.disassociateLabel).toBeCalledWith(6, { diff --git a/awx/ui_next/src/screens/Template/shared/WorkflowJobTemplateForm.test.jsx b/awx/ui_next/src/screens/Template/shared/WorkflowJobTemplateForm.test.jsx index 3bf15e6fd3..f123abe91b 100644 --- a/awx/ui_next/src/screens/Template/shared/WorkflowJobTemplateForm.test.jsx +++ b/awx/ui_next/src/screens/Template/shared/WorkflowJobTemplateForm.test.jsx @@ -74,7 +74,7 @@ describe('', () => { template={mockTemplate} handleCancel={handleCancel} handleSubmit={handleSubmit} - webhook_key="sdfghjklmnbvcdsew435678iokjhgfd" + webhookKey="sdfghjklmnbvcdsew435678iokjhgfd" /> )} />, @@ -106,13 +106,14 @@ describe('', () => { const fields = [ 'FormField[name="name"]', 'FormField[name="description"]', - 'Field[name="organization"]', - 'Field[name="inventory"]', + 'FormGroup[label="Organization"]', + 'FormGroup[label="Inventory"]', 'FormField[name="limit"]', 'FormField[name="scm_branch"]', - 'Field[name="labels"]', + 'FormGroup[label="Labels"]', 'VariablesField', ]; + const assertField = field => { expect(wrapper.find(`${field}`).length).toBe(1); }; @@ -191,7 +192,7 @@ describe('', () => { ).toBe('sdfghjklmnbvcdsew435678iokjhgfd'); await act(() => wrapper - .find('FormGroup[name="webhook_key"]') + .find('FormGroup[name="webhookKey"]') .find('Button[variant="tertiary"]') .prop('onClick')() ); @@ -201,8 +202,7 @@ describe('', () => { ).toContain('/api/v2/workflow_job_templates/57/gitlab/'); wrapper.update(); - - expect(wrapper.find('Field[name="webhook_service"]').length).toBe(1); + expect(wrapper.find('FormGroup[name="webhook_service"]').length).toBe(1); act(() => wrapper.find('AnsibleSelect').prop('onChange')({}, 'gitlab')); wrapper.update(); From 440691387b8e2036e07db88ee67aada73fcd9608 Mon Sep 17 00:00:00 2001 From: Alex Corey Date: Mon, 13 Apr 2020 14:39:52 -0400 Subject: [PATCH 3/3] Puts webhook key on the template object in WFJTEdit Also adds aria-label to Label Select Options to improve test matchers Improves the name of the template payload in WFJTAdd and WFJTEdit Updates tests including a failing snapshot DeleteConfirmationModal test that was failing in devel --- .../Project/shared/ProjectForm.test.jsx | 2 -- .../JobTemplateEdit/JobTemplateEdit.test.jsx | 2 +- .../screens/Template/WorkflowJobTemplate.jsx | 20 +++++-------------- .../WorkflowJobTemplateAdd.jsx | 14 ++++++------- .../WorkflowJobTemplateAdd.test.jsx | 3 +-- .../WorkflowJobTemplateDetail.jsx | 3 ++- .../WorkflowJobTemplateDetail.test.jsx | 2 +- .../WorkflowJobTemplateEdit.jsx | 15 +++++++------- .../WorkflowJobTemplateEdit.test.jsx | 6 ++---- .../Template/shared/JobTemplateForm.test.jsx | 2 +- .../screens/Template/shared/LabelSelect.jsx | 2 +- .../shared/WorkflowJobTemplateForm.jsx | 12 +++++------ .../shared/WorkflowJobTemplateForm.test.jsx | 8 ++++---- 13 files changed, 38 insertions(+), 53 deletions(-) diff --git a/awx/ui_next/src/screens/Project/shared/ProjectForm.test.jsx b/awx/ui_next/src/screens/Project/shared/ProjectForm.test.jsx index a7680921f2..e2d82c6653 100644 --- a/awx/ui_next/src/screens/Project/shared/ProjectForm.test.jsx +++ b/awx/ui_next/src/screens/Project/shared/ProjectForm.test.jsx @@ -288,8 +288,6 @@ describe('', () => { }); await waitForElement(wrapper, 'ContentLoading', el => el.length === 0); - console.log(wrapper.debug()); - const scmTypeSelect = wrapper.find( 'FormGroup[label="Source Control Credential Type"] FormSelect' ); 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 fef5d89920..8c296b0630 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: { id: 3, summary_fields: { organization: { id: 1 } } }, + project: 3, scm_branch: '', skip_tags: '', summary_fields: { diff --git a/awx/ui_next/src/screens/Template/WorkflowJobTemplate.jsx b/awx/ui_next/src/screens/Template/WorkflowJobTemplate.jsx index f07f51db12..98de6a9432 100644 --- a/awx/ui_next/src/screens/Template/WorkflowJobTemplate.jsx +++ b/awx/ui_next/src/screens/Template/WorkflowJobTemplate.jsx @@ -32,7 +32,6 @@ class WorkflowJobTemplate extends Component { contentError: null, hasContentLoading: true, template: null, - webhook_key: null, isNotifAdmin: false, }; this.createSchedule = this.createSchedule.bind(this); @@ -59,11 +58,9 @@ class WorkflowJobTemplate extends Component { this.setState({ contentError: null }); try { const { data } = await WorkflowJobTemplatesAPI.readDetail(id); + let webhookKey; if (data?.related?.webhook_key) { - const { - data: { webhook_key }, - } = await WorkflowJobTemplatesAPI.readWebhookKey(id); - this.setState({ webhook_key }); + webhookKey = await WorkflowJobTemplatesAPI.readWebhookKey(id); } if (data?.summary_fields?.webhook_credential) { const { @@ -83,7 +80,7 @@ class WorkflowJobTemplate extends Component { }); setBreadcrumb(data); this.setState({ - template: data, + template: { ...data, webhook_key: webhookKey.data.webhook_key }, isNotifAdmin: notifAdminRes.data.results.length > 0, }); } catch (err) { @@ -114,7 +111,6 @@ class WorkflowJobTemplate extends Component { contentError, hasContentLoading, template, - webhook_key, isNotifAdmin, } = this.state; @@ -211,10 +207,7 @@ class WorkflowJobTemplate extends Component { key="wfjt-details" path="/templates/workflow_job_template/:id/details" > - + )} {template && ( @@ -239,10 +232,7 @@ class WorkflowJobTemplate extends Component { key="wfjt-edit" path="/templates/workflow_job_template/:id/edit" > - + )} {template && ( diff --git a/awx/ui_next/src/screens/Template/WorkflowJobTemplateAdd/WorkflowJobTemplateAdd.jsx b/awx/ui_next/src/screens/Template/WorkflowJobTemplateAdd/WorkflowJobTemplateAdd.jsx index f69349bb70..6bfa3d3707 100644 --- a/awx/ui_next/src/screens/Template/WorkflowJobTemplateAdd/WorkflowJobTemplateAdd.jsx +++ b/awx/ui_next/src/screens/Template/WorkflowJobTemplateAdd/WorkflowJobTemplateAdd.jsx @@ -17,18 +17,18 @@ function WorkflowJobTemplateAdd() { inventory, organization, webhook_credential, - webhookKey, - ...remainingValues + webhook_key, + ...templatePayload } = values; - remainingValues.inventory = inventory?.id; - remainingValues.organization = organization?.id; - remainingValues.webhook_credential = webhook_credential?.id; + templatePayload.inventory = inventory?.id; + templatePayload.organization = organization?.id; + templatePayload.webhook_credential = webhook_credential?.id; const organizationId = - organization?.id || inventory?.summary_fields?.organization.id || null; + organization?.id || inventory?.summary_fields?.organization.id; try { const { data: { id }, - } = await WorkflowJobTemplatesAPI.create(remainingValues); + } = await WorkflowJobTemplatesAPI.create(templatePayload); await Promise.all(await submitLabels(id, labels, organizationId)); history.push(`/templates/workflow_job_template/${id}/details`); } catch (err) { diff --git a/awx/ui_next/src/screens/Template/WorkflowJobTemplateAdd/WorkflowJobTemplateAdd.test.jsx b/awx/ui_next/src/screens/Template/WorkflowJobTemplateAdd/WorkflowJobTemplateAdd.test.jsx index c76b71a4f1..ab390f4fcd 100644 --- a/awx/ui_next/src/screens/Template/WorkflowJobTemplateAdd/WorkflowJobTemplateAdd.test.jsx +++ b/awx/ui_next/src/screens/Template/WorkflowJobTemplateAdd/WorkflowJobTemplateAdd.test.jsx @@ -85,8 +85,7 @@ describe('', () => { act(() => { wrapper .find('SelectOption') - .find('button') - .at(2) + .find('button[aria-label="Label 3"]') .prop('onClick')(); }); diff --git a/awx/ui_next/src/screens/Template/WorkflowJobTemplateDetail/WorkflowJobTemplateDetail.jsx b/awx/ui_next/src/screens/Template/WorkflowJobTemplateDetail/WorkflowJobTemplateDetail.jsx index cb0b91ecd0..93aa40a212 100644 --- a/awx/ui_next/src/screens/Template/WorkflowJobTemplateDetail/WorkflowJobTemplateDetail.jsx +++ b/awx/ui_next/src/screens/Template/WorkflowJobTemplateDetail/WorkflowJobTemplateDetail.jsx @@ -25,7 +25,7 @@ import LaunchButton from '@components/LaunchButton'; import Sparkline from '@components/Sparkline'; import { toTitleCase } from '@util/strings'; -function WorkflowJobTemplateDetail({ template, i18n, webhook_key }) { +function WorkflowJobTemplateDetail({ template, i18n }) { const { id, ask_inventory_on_launch, @@ -38,6 +38,7 @@ function WorkflowJobTemplateDetail({ template, i18n, webhook_key }) { summary_fields, related, webhook_credential, + webhook_key, } = template; const urlOrigin = window.location.origin; diff --git a/awx/ui_next/src/screens/Template/WorkflowJobTemplateDetail/WorkflowJobTemplateDetail.test.jsx b/awx/ui_next/src/screens/Template/WorkflowJobTemplateDetail/WorkflowJobTemplateDetail.test.jsx index f693672d20..bfb850c83b 100644 --- a/awx/ui_next/src/screens/Template/WorkflowJobTemplateDetail/WorkflowJobTemplateDetail.test.jsx +++ b/awx/ui_next/src/screens/Template/WorkflowJobTemplateDetail/WorkflowJobTemplateDetail.test.jsx @@ -39,6 +39,7 @@ describe('', () => { user_capabilities: { edit: true, delete: true }, }, webhook_service: 'Github', + webhook_key: 'Foo webhook key', }; beforeEach(async () => { @@ -52,7 +53,6 @@ describe('', () => { component={() => ( {}} /> diff --git a/awx/ui_next/src/screens/Template/WorkflowJobTemplateEdit/WorkflowJobTemplateEdit.jsx b/awx/ui_next/src/screens/Template/WorkflowJobTemplateEdit/WorkflowJobTemplateEdit.jsx index 0d0d94b7de..a860589020 100644 --- a/awx/ui_next/src/screens/Template/WorkflowJobTemplateEdit/WorkflowJobTemplateEdit.jsx +++ b/awx/ui_next/src/screens/Template/WorkflowJobTemplateEdit/WorkflowJobTemplateEdit.jsx @@ -6,7 +6,7 @@ import { getAddedAndRemoved } from '@util/lists'; import { WorkflowJobTemplatesAPI, OrganizationsAPI } from '@api'; import { WorkflowJobTemplateForm } from '../shared'; -function WorkflowJobTemplateEdit({ template, webhook_key }) { +function WorkflowJobTemplateEdit({ template }) { const history = useHistory(); const [formSubmitError, setFormSubmitError] = useState(null); @@ -16,12 +16,12 @@ function WorkflowJobTemplateEdit({ template, webhook_key }) { inventory, organization, webhook_credential, - webhookKey, - ...remainingValues + webhook_key, + ...templatePayload } = values; - remainingValues.inventory = inventory?.id; - remainingValues.organization = organization?.id; - remainingValues.webhook_credential = webhook_credential?.id || null; + templatePayload.inventory = inventory?.id; + templatePayload.organization = organization?.id; + templatePayload.webhook_credential = webhook_credential?.id || null; const formOrgId = organization?.id || inventory?.summary_fields?.organization.id || null; @@ -29,7 +29,7 @@ function WorkflowJobTemplateEdit({ template, webhook_key }) { await Promise.all( await submitLabels(labels, formOrgId, template.organization) ); - await WorkflowJobTemplatesAPI.update(template.id, remainingValues); + await WorkflowJobTemplatesAPI.update(template.id, templatePayload); history.push(`/templates/workflow_job_template/${template.id}/details`); } catch (err) { setFormSubmitError(err); @@ -73,7 +73,6 @@ function WorkflowJobTemplateEdit({ template, webhook_key }) { handleSubmit={handleSubmit} handleCancel={handleCancel} template={template} - webhookKey={webhook_key} submitError={formSubmitError} /> diff --git a/awx/ui_next/src/screens/Template/WorkflowJobTemplateEdit/WorkflowJobTemplateEdit.test.jsx b/awx/ui_next/src/screens/Template/WorkflowJobTemplateEdit/WorkflowJobTemplateEdit.test.jsx index 9acf279c1e..6b2c7be5aa 100644 --- a/awx/ui_next/src/screens/Template/WorkflowJobTemplateEdit/WorkflowJobTemplateEdit.test.jsx +++ b/awx/ui_next/src/screens/Template/WorkflowJobTemplateEdit/WorkflowJobTemplateEdit.test.jsx @@ -100,8 +100,7 @@ describe('', () => { act(() => { wrapper .find('SelectOption') - .find('button') - .at(2) + .find('button[aria-label="Label 3"]') .prop('onClick')(); }); @@ -110,8 +109,7 @@ describe('', () => { act(() => wrapper .find('SelectOption') - .find('button') - .at(0) + .find('button[aria-label="Label 1"]') .prop('onClick')() ); 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 47b9f6b436..383bef39f0 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: { id: 3, summary_fields: { organization: { id: 1 } } }, + project: 3, playbook: 'Baz', type: 'job_template', scm_branch: 'Foo', diff --git a/awx/ui_next/src/screens/Template/shared/LabelSelect.jsx b/awx/ui_next/src/screens/Template/shared/LabelSelect.jsx index 72863cd5f5..a00e90073e 100644 --- a/awx/ui_next/src/screens/Template/shared/LabelSelect.jsx +++ b/awx/ui_next/src/screens/Template/shared/LabelSelect.jsx @@ -47,7 +47,7 @@ function LabelSelect({ value, placeholder, onChange, onError }) { const renderOptions = opts => { return opts.map(option => ( - + {option.name} )); diff --git a/awx/ui_next/src/screens/Template/shared/WorkflowJobTemplateForm.jsx b/awx/ui_next/src/screens/Template/shared/WorkflowJobTemplateForm.jsx index 673bce96cb..2ae30ffed3 100644 --- a/awx/ui_next/src/screens/Template/shared/WorkflowJobTemplateForm.jsx +++ b/awx/ui_next/src/screens/Template/shared/WorkflowJobTemplateForm.jsx @@ -1,6 +1,6 @@ import React, { useState, useEffect, useCallback } from 'react'; import { t } from '@lingui/macro'; -import { useRouteMatch, useParams, withRouter } from 'react-router-dom'; +import { useRouteMatch, useParams } from 'react-router-dom'; import PropTypes, { shape } from 'prop-types'; @@ -67,7 +67,7 @@ function WorkflowJobTemplateForm({ ] = useField('webhook_service'); const [webhookKeyField, webhookKeyMeta, webhookKeyHelpers] = useField( - 'webhookKey' + 'webhook_key' ); const [hasWebhooks, setHasWebhooks] = useState( @@ -339,7 +339,7 @@ function WorkflowJobTemplateForm({ fieldId="wfjt-webhook-key" type="text" id="wfjt-webhook-key" - name="webhookKey" + name="webhook_key" label={i18n._(t`Webhook Key`)} > { @@ -430,4 +430,4 @@ const FormikApp = withFormik({ })(WorkflowJobTemplateForm); export { WorkflowJobTemplateForm as _WorkflowJobTemplateForm }; -export default withI18n()(withRouter(FormikApp)); +export default withI18n()(FormikApp); diff --git a/awx/ui_next/src/screens/Template/shared/WorkflowJobTemplateForm.test.jsx b/awx/ui_next/src/screens/Template/shared/WorkflowJobTemplateForm.test.jsx index f123abe91b..02f721f8a6 100644 --- a/awx/ui_next/src/screens/Template/shared/WorkflowJobTemplateForm.test.jsx +++ b/awx/ui_next/src/screens/Template/shared/WorkflowJobTemplateForm.test.jsx @@ -40,6 +40,7 @@ describe('', () => { related: { webhook_receiver: '/api/v2/workflow_job_templates/57/gitlab/', }, + webhook_key: 'sdfghjklmnbvcdsew435678iokjhgfd', }; beforeEach(async () => { @@ -74,7 +75,6 @@ describe('', () => { template={mockTemplate} handleCancel={handleCancel} handleSubmit={handleSubmit} - webhookKey="sdfghjklmnbvcdsew435678iokjhgfd" /> )} />, @@ -172,7 +172,7 @@ describe('', () => { test('webhooks and enable concurrent jobs functions properly', async () => { act(() => { - wrapper.find('Checkbox[aria-label="Enable Webhooks"]').invoke('onChange')( + wrapper.find('Checkbox[aria-label="Enable Webhook"]').invoke('onChange')( true, { currentTarget: { value: true, type: 'change', checked: true }, @@ -181,7 +181,7 @@ describe('', () => { }); wrapper.update(); expect( - wrapper.find('Checkbox[aria-label="Enable Webhooks"]').prop('isChecked') + wrapper.find('Checkbox[aria-label="Enable Webhook"]').prop('isChecked') ).toBe(true); expect( @@ -192,7 +192,7 @@ describe('', () => { ).toBe('sdfghjklmnbvcdsew435678iokjhgfd'); await act(() => wrapper - .find('FormGroup[name="webhookKey"]') + .find('FormGroup[name="webhook_key"]') .find('Button[variant="tertiary"]') .prop('onClick')() );