From 0b8aabbd16b3517d3a95d6c956c4431943ef9d43 Mon Sep 17 00:00:00 2001 From: nixocio Date: Mon, 27 Apr 2020 14:34:25 -0400 Subject: [PATCH] Use Webhook Fields in WFJT Form Update WorkflowJobTemplateForm to use `WebhookSubForm`. Also, update related unit-tests. closes: https://github.com/ansible/awx/issues/6742 --- .../screens/Template/WorkflowJobTemplate.jsx | 4 +- .../Template/WorkflowJobTemplate.test.jsx | 1 + .../Template/shared/JobTemplateForm.jsx | 9 +- .../Template/shared/JobTemplateForm.test.jsx | 4 +- .../Template/shared/WebhookSubForm.jsx | 19 +- .../Template/shared/WebhookSubForm.test.jsx | 42 ++- .../shared/WorkflowJobTemplateForm.jsx | 253 ++---------------- .../shared/WorkflowJobTemplateForm.test.jsx | 22 +- awx/ui_next/src/types.js | 6 + 9 files changed, 106 insertions(+), 254 deletions(-) diff --git a/awx/ui_next/src/screens/Template/WorkflowJobTemplate.jsx b/awx/ui_next/src/screens/Template/WorkflowJobTemplate.jsx index ba4530a673..700cee8f76 100644 --- a/awx/ui_next/src/screens/Template/WorkflowJobTemplate.jsx +++ b/awx/ui_next/src/screens/Template/WorkflowJobTemplate.jsx @@ -59,7 +59,7 @@ class WorkflowJobTemplate extends Component { try { const { data } = await WorkflowJobTemplatesAPI.readDetail(id); let webhookKey; - if (data?.related?.webhook_key) { + if (data?.webhook_service && data?.related?.webhook_key) { webhookKey = await WorkflowJobTemplatesAPI.readWebhookKey(id); } if (data?.summary_fields?.webhook_credential) { @@ -80,7 +80,7 @@ class WorkflowJobTemplate extends Component { }); setBreadcrumb(data); this.setState({ - template: { ...data, webhook_key: webhookKey.data.webhook_key }, + template: { ...data, webhook_key: webhookKey?.data.webhook_key }, isNotifAdmin: notifAdminRes.data.results.length > 0, }); } catch (err) { diff --git a/awx/ui_next/src/screens/Template/WorkflowJobTemplate.test.jsx b/awx/ui_next/src/screens/Template/WorkflowJobTemplate.test.jsx index ae5dc41fa9..66840a331e 100644 --- a/awx/ui_next/src/screens/Template/WorkflowJobTemplate.test.jsx +++ b/awx/ui_next/src/screens/Template/WorkflowJobTemplate.test.jsx @@ -32,6 +32,7 @@ describe('', () => { created: '2015-07-07T17:21:26.429745Z', modified: '2019-08-11T19:47:37.980466Z', extra_vars: '', + webhook_service: 'github', summary_fields: { webhook_credential: { id: 1234567, name: 'Foo Webhook Credential' }, created_by: { id: 1, username: 'Athena' }, diff --git a/awx/ui_next/src/screens/Template/shared/JobTemplateForm.jsx b/awx/ui_next/src/screens/Template/shared/JobTemplateForm.jsx index 20dab80514..48c4caeb33 100644 --- a/awx/ui_next/src/screens/Template/shared/JobTemplateForm.jsx +++ b/awx/ui_next/src/screens/Template/shared/JobTemplateForm.jsx @@ -512,9 +512,7 @@ function JobTemplateForm({ {i18n._(t`Enable Webhook`)}   } @@ -542,7 +540,10 @@ function JobTemplateForm({ - + {allowCallbacks && ( <> {callbackUrl && ( 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 bb2d8be6e2..430bd42984 100644 --- a/awx/ui_next/src/screens/Template/shared/JobTemplateForm.test.jsx +++ b/awx/ui_next/src/screens/Template/shared/JobTemplateForm.test.jsx @@ -36,7 +36,7 @@ describe('', () => { { id: 2, kind: 'ssh', name: 'Bar' }, ], }, - related: { webhook_receiver: '/api/v2/workflow_job_templates/57/gitlab/' }, + related: { webhook_receiver: '/api/v2/job_templates/57/gitlab/' }, webhook_key: 'webhook key', webhook_service: 'github', webhook_credential: 7, @@ -273,7 +273,7 @@ describe('', () => { expect(JobTemplatesAPI.updateWebhookKey).toBeCalledWith('1'); expect( wrapper.find('TextInputBase[aria-label="Webhook URL"]').prop('value') - ).toContain('/api/v2/workflow_job_templates/57/gitlab/'); + ).toContain('/api/v2/job_templates/57/gitlab/'); wrapper.update(); diff --git a/awx/ui_next/src/screens/Template/shared/WebhookSubForm.jsx b/awx/ui_next/src/screens/Template/shared/WebhookSubForm.jsx index 7211990c90..d38c1644ea 100644 --- a/awx/ui_next/src/screens/Template/shared/WebhookSubForm.jsx +++ b/awx/ui_next/src/screens/Template/shared/WebhookSubForm.jsx @@ -17,10 +17,15 @@ import { FormColumnLayout } from '@components/FormLayout'; import { CredentialLookup } from '@components/Lookup'; import AnsibleSelect from '@components/AnsibleSelect'; import { FieldTooltip } from '@components/FormField'; -import { JobTemplatesAPI, CredentialTypesAPI } from '@api'; +import { + JobTemplatesAPI, + WorkflowJobTemplatesAPI, + CredentialTypesAPI, +} from '@api'; + +function WebhookSubForm({ i18n, enableWebhooks, templateType }) { + const { id } = useParams(); -function WebhookSubForm({ i18n, enableWebhooks }) { - const { id, templateType } = useParams(); const { pathname } = useLocation(); const { origin } = document.location; @@ -83,11 +88,15 @@ function WebhookSubForm({ i18n, enableWebhooks }) { const { request: fetchWebhookKey, error: webhookKeyError } = useRequest( useCallback(async () => { + const updateWebhookKey = + templateType === 'job_template' + ? JobTemplatesAPI.updateWebhookKey(id) + : WorkflowJobTemplatesAPI.updateWebhookKey(id); const { data: { webhook_key: key }, - } = await JobTemplatesAPI.updateWebhookKey(id); + } = await updateWebhookKey; webhookKeyHelpers.setValue(key); - }, [webhookKeyHelpers, id]) + }, [webhookKeyHelpers, id, templateType]) ); const changeWebhookKey = async () => { diff --git a/awx/ui_next/src/screens/Template/shared/WebhookSubForm.test.jsx b/awx/ui_next/src/screens/Template/shared/WebhookSubForm.test.jsx index e23afb136b..c867ac43fe 100644 --- a/awx/ui_next/src/screens/Template/shared/WebhookSubForm.test.jsx +++ b/awx/ui_next/src/screens/Template/shared/WebhookSubForm.test.jsx @@ -11,7 +11,7 @@ import WebhookSubForm from './WebhookSubForm'; jest.mock('@api'); -describe('', () => { +describe('', () => { let wrapper; let history; const initialValues = { @@ -31,7 +31,7 @@ describe('', () => { wrapper = mountWithContexts( - + , { @@ -50,6 +50,7 @@ describe('', () => { }); afterEach(() => { jest.clearAllMocks(); + wrapper.unmount(); }); test('mounts properly', () => { expect(wrapper.length).toBe(1); @@ -99,7 +100,7 @@ describe('', () => { webhook_key: 'A NEW WEBHOOK KEY WILL BE GENERATED ON SAVE.', }} > - + , { @@ -121,4 +122,39 @@ describe('', () => { .prop('isDisabled') ).toBe(true); }); + + test('test whether the workflow template type is part of the webhook url', async () => { + let newWrapper; + const webhook_url = '/api/v2/workflow_job_templates/42/github/'; + await act(async () => { + newWrapper = mountWithContexts( + + + + + , + { + context: { + router: { + history, + route: { + location: { + pathname: 'templates/workflow_job_template/51/edit', + }, + match: { + params: { id: 51, templateType: 'workflow_job_template' }, + }, + }, + }, + }, + } + ); + }); + expect( + newWrapper.find('TextInputBase[aria-label="Webhook URL"]').prop('value') + ).toContain(webhook_url); + }); }); diff --git a/awx/ui_next/src/screens/Template/shared/WorkflowJobTemplateForm.jsx b/awx/ui_next/src/screens/Template/shared/WorkflowJobTemplateForm.jsx index 17151531fb..6d884d7516 100644 --- a/awx/ui_next/src/screens/Template/shared/WorkflowJobTemplateForm.jsx +++ b/awx/ui_next/src/screens/Template/shared/WorkflowJobTemplateForm.jsx @@ -1,26 +1,13 @@ -import React, { useState, useEffect, useCallback } from 'react'; +import React, { useState } from 'react'; import { t } from '@lingui/macro'; -import { useRouteMatch, useParams } from 'react-router-dom'; import PropTypes, { shape } from 'prop-types'; import { withI18n } from '@lingui/react'; import { useField, withFormik } from 'formik'; -import { - Form, - FormGroup, - InputGroup, - Button, - TextInput, - Checkbox, -} from '@patternfly/react-core'; +import { Form, FormGroup, Checkbox } from '@patternfly/react-core'; import { required } from '@util/validators'; -import { SyncAltIcon } from '@patternfly/react-icons'; -import AnsibleSelect from '@components/AnsibleSelect'; -import { WorkflowJobTemplatesAPI, CredentialTypesAPI } from '@api'; - -import useRequest from '@util/useRequest'; import FormField, { FieldTooltip, FormSubmitError, @@ -30,26 +17,25 @@ import { FormFullWidthLayout, FormCheckboxLayout, } from '@components/FormLayout'; -import ContentLoading from '@components/ContentLoading'; import OrganizationLookup from '@components/Lookup/OrganizationLookup'; -import CredentialLookup from '@components/Lookup/CredentialLookup'; import { InventoryLookup } from '@components/Lookup'; import { VariablesField } from '@components/CodeMirrorInput'; import FormActionGroup from '@components/FormActionGroup'; import ContentError from '@components/ContentError'; import CheckboxField from '@components/FormField/CheckboxField'; import LabelSelect from './LabelSelect'; +import WebhookSubForm from './WebhookSubForm'; +import { WorkFlowJobTemplate } from '@types'; const urlOrigin = window.location.origin; + function WorkflowJobTemplateForm({ + template, handleSubmit, handleCancel, i18n, submitError, }) { - const { id } = useParams(); - const wfjtAddMatch = useRouteMatch('/templates/workflow_job_template/add'); - const [hasContentError, setContentError] = useState(null); const [organizationField, organizationMeta, organizationHelpers] = useField( @@ -60,125 +46,12 @@ function WorkflowJobTemplateForm({ ); const [labelsField, , labelsHelpers] = useField('labels'); - const [ - webhookServiceField, - webhookServiceMeta, - webhookServiceHelpers, - ] = useField('webhook_service'); - - const [webhookKeyField, webhookKeyMeta, webhookKeyHelpers] = useField( - 'webhook_key' + const [enableWebhooks, setEnableWebhooks] = useState( + Boolean(template.webhook_service) ); - const [hasWebhooks, setHasWebhooks] = useState( - Boolean(webhookServiceField.value) - ); - - const [ - webhookCredentialField, - webhookCredentialMeta, - webhookCredentialHelpers, - ] = useField('webhook_credential'); - - const [webhookUrlField, webhookUrlMeta, webhookUrlHelpers] = useField( - 'webhook_url' - ); - - const webhookServiceOptions = [ - { - value: '', - key: '', - label: i18n._(t`Choose a Webhook Service`), - isDisabled: true, - }, - { - value: 'github', - key: 'github', - label: i18n._(t`GitHub`), - isDisabled: false, - }, - { - value: 'gitlab', - key: 'gitlab', - label: i18n._(t`GitLab`), - 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, - contentLoading, - result: credTypeId, - } = useRequest( - useCallback(async () => { - let results; - if (webhookServiceField.value) { - results = await CredentialTypesAPI.read({ - 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; - }, [webhookServiceField.value]) - ); - - useEffect(() => { - loadCredentialType(); - }, [loadCredentialType]); - - // TODO: Convert this function below to useRequest. Might want to create a new - // webhookkey component that handles all of that api calls. Will also need - // to move this api call out of WorkflowJobTemplate.jsx and add it to workflowJobTemplateDetai.jsx - const changeWebhookKey = async () => { - try { - const { - data: { webhook_key: key }, - } = await WorkflowJobTemplatesAPI.updateWebhookKey(id); - webhookKeyHelpers.setValue(key); - } catch (err) { - setContentError(err); - } - }; - - if (hasContentError || contentError) { - return ; - } - - if (contentLoading) { - return ; + if (hasContentError) { + return ; } return ( @@ -232,7 +105,7 @@ function WorkflowJobTemplateForm({ /> } id="wfjt-enabled-webhooks" - isChecked={Boolean(webhookServiceField.value) || hasWebhooks} + isChecked={enableWebhooks} onChange={checked => { - setHasWebhooks(checked); - handleWebhookEnablement(checked, webhookServiceField.value); + setEnableWebhooks(checked); }} /> - {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 && } @@ -388,6 +178,7 @@ function WorkflowJobTemplateForm({ } WorkflowJobTemplateForm.propTypes = { + template: WorkFlowJobTemplate, handleSubmit: PropTypes.func.isRequired, handleCancel: PropTypes.func.isRequired, submitError: shape({}), @@ -395,6 +186,12 @@ WorkflowJobTemplateForm.propTypes = { WorkflowJobTemplateForm.defaultProps = { submitError: null, + template: { + name: '', + description: '', + inventory: undefined, + project: undefined, + }, }; const FormikApp = withFormik({ 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 02f721f8a6..d3dcbef066 100644 --- a/awx/ui_next/src/screens/Template/shared/WorkflowJobTemplateForm.test.jsx +++ b/awx/ui_next/src/screens/Template/shared/WorkflowJobTemplateForm.test.jsx @@ -13,6 +13,7 @@ import { InventoriesAPI, } from '@api'; +jest.mock('@api/models/CredentialTypes'); jest.mock('@api/models/WorkflowJobTemplates'); jest.mock('@api/models/Labels'); jest.mock('@api/models/Organizations'); @@ -183,7 +184,6 @@ describe('', () => { expect( wrapper.find('Checkbox[aria-label="Enable Webhook"]').prop('isChecked') ).toBe(true); - expect( wrapper.find('input[aria-label="wfjt-webhook-key"]').prop('readOnly') ).toBe(true); @@ -191,23 +191,25 @@ describe('', () => { wrapper.find('input[aria-label="wfjt-webhook-key"]').prop('value') ).toBe('sdfghjklmnbvcdsew435678iokjhgfd'); await act(() => - wrapper - .find('FormGroup[name="webhook_key"]') - .find('Button[variant="tertiary"]') - .prop('onClick')() + wrapper.find('Button[aria-label="Update webhook key"]').prop('onClick')() ); expect(WorkflowJobTemplatesAPI.updateWebhookKey).toBeCalledWith('6'); expect( wrapper.find('TextInputBase[aria-label="Webhook URL"]').prop('value') ).toContain('/api/v2/workflow_job_templates/57/gitlab/'); - wrapper.update(); expect(wrapper.find('FormGroup[name="webhook_service"]').length).toBe(1); - - act(() => wrapper.find('AnsibleSelect').prop('onChange')({}, 'gitlab')); + expect(wrapper.find('AnsibleSelect#webhook_service').length).toBe(1); + await act(async () => { + wrapper.find('AnsibleSelect#webhook_service').prop('onChange')( + {}, + 'gitlab' + ); + }); wrapper.update(); - - expect(wrapper.find('AnsibleSelect').prop('value')).toBe('gitlab'); + expect(wrapper.find('AnsibleSelect#webhook_service').prop('value')).toBe( + 'gitlab' + ); }); test('handleSubmit is called on submit button click', async () => { diff --git a/awx/ui_next/src/types.js b/awx/ui_next/src/types.js index c1b592cb5a..e46021a5b6 100644 --- a/awx/ui_next/src/types.js +++ b/awx/ui_next/src/types.js @@ -73,6 +73,12 @@ export const JobTemplate = shape({ project: number, }); +export const WorkFlowJobTemplate = shape({ + name: string.isRequired, + description: string, + inventory: number, +}); + export const Inventory = shape({ id: number.isRequired, name: string,