Merge pull request #6916 from nixocio/ui_issue_6742

Use Webhook Fields in WFJT Form

Reviewed-by: https://github.com/apps/softwarefactory-project-zuul

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) {

awx/ui_next/src/screens/Template/WorkflowJobTemplate.test.jsx

@@ -32,6 +32,7 @@ describe('<WorkflowJobTemplate/>', () => {
         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' },

awx/ui_next/src/screens/Template/shared/JobTemplateForm.jsx

@@ -512,9 +512,7 @@ function JobTemplateForm({
                         {i18n._(t`Enable Webhook`)}
                          
                         <FieldTooltip
-                          content={i18n._(
+                          content={i18n._(t`Enable webhook for this template.`)}
-                            t`Enable webhook for this workflow job template.`
-                          )}
                         />
                       </span>
                     }
@@ -542,7 +540,10 @@ function JobTemplateForm({
                 </FormCheckboxLayout>
               </FormGroup>
             </FormFullWidthLayout>
-            <WebhookSubForm enableWebhooks={enableWebhooks} />
+            <WebhookSubForm
+              enableWebhooks={enableWebhooks}
+              templateType={template.type}
+            />
             {allowCallbacks && (
               <>
                 {callbackUrl && (

awx/ui_next/src/screens/Template/shared/JobTemplateForm.test.jsx

@@ -36,7 +36,7 @@ describe('<JobTemplateForm />', () => {
         { 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('<JobTemplateForm />', () => {
     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();
 

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 () => {

awx/ui_next/src/screens/Template/shared/WebhookSubForm.test.jsx

@@ -11,7 +11,7 @@ import WebhookSubForm from './WebhookSubForm';
 
 jest.mock('@api');
 
-describe('<WebhooksSubForm />', () => {
+describe('<WebhookSubForm />', () => {
   let wrapper;
   let history;
   const initialValues = {
@@ -31,7 +31,7 @@ describe('<WebhooksSubForm />', () => {
       wrapper = mountWithContexts(
         <Route path="templates/:templateType/:id/edit">
           <Formik initialValues={initialValues}>
-            <WebhookSubForm enableWebhooks />
+            <WebhookSubForm enableWebhooks templateType="job_template" />
           </Formik>
         </Route>,
         {
@@ -50,6 +50,7 @@ describe('<WebhooksSubForm />', () => {
   });
   afterEach(() => {
     jest.clearAllMocks();
+    wrapper.unmount();
   });
   test('mounts properly', () => {
     expect(wrapper.length).toBe(1);
@@ -99,7 +100,7 @@ describe('<WebhooksSubForm />', () => {
               webhook_key: 'A NEW WEBHOOK KEY WILL BE GENERATED ON SAVE.',
             }}
           >
-            <WebhookSubForm enableWebhooks />
+            <WebhookSubForm enableWebhooks templateType="job_template" />
           </Formik>
         </Route>,
         {
@@ -121,4 +122,39 @@ describe('<WebhooksSubForm />', () => {
         .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(
+        <Route path="templates/:templateType/:id/edit">
+          <Formik initialValues={{ ...initialValues, webhook_url }}>
+            <WebhookSubForm
+              enableWebhooks
+              templateType="workflow_job_template"
+            />
+          </Formik>
+        </Route>,
+        {
+          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);
+  });
 });

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 {
+import { Form, FormGroup, Checkbox } from '@patternfly/react-core';
-  Form,
-  FormGroup,
-  InputGroup,
-  Button,
-  TextInput,
-  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 [
+  const [enableWebhooks, setEnableWebhooks] = useState(
-    webhookServiceField,
+    Boolean(template.webhook_service)
-    webhookServiceMeta,
-    webhookServiceHelpers,
-  ] = useField('webhook_service');
-
-  const [webhookKeyField, webhookKeyMeta, webhookKeyHelpers] = useField(
-    'webhook_key'
   );
 
-  const [hasWebhooks, setHasWebhooks] = useState(
+  if (hasContentError) {
-    Boolean(webhookServiceField.value)
+    return <ContentError error={hasContentError} />;
-  );
-
-  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 <ContentError error={contentError || hasContentError} />;
-  }
-
-  if (contentLoading) {
-    return <ContentLoading />;
   }
 
   return (
@@ -232,7 +105,7 @@ function WorkflowJobTemplateForm({
         />
         <FormField
           type="text"
-          label={i18n._(t`SCM Branch`)}
+          label={i18n._(t`Source Control Branch`)}
           tooltip={i18n._(
             t`Select a branch for the workflow. This branch is applied to all job template nodes that prompt for a branch.`
           )}
@@ -274,16 +147,15 @@ function WorkflowJobTemplateForm({
               &nbsp;
               <FieldTooltip
                 content={i18n._(
-                  t`Enable webhook for this workflow job template.`
+                  t`Enable Webhook for this workflow job template.`
                 )}
               />
             </span>
           }
           id="wfjt-enabled-webhooks"
-          isChecked={Boolean(webhookServiceField.value) || hasWebhooks}
+          isChecked={enableWebhooks}
           onChange={checked => {
-            setHasWebhooks(checked);
+            setEnableWebhooks(checked);
-            handleWebhookEnablement(checked, webhookServiceField.value);
           }}
         />
         <CheckboxField
@@ -295,92 +167,10 @@ function WorkflowJobTemplateForm({
           label={i18n._(t`Enable Concurrent Jobs`)}
         />
       </FormCheckboxLayout>
-      {hasWebhooks && (
+      <WebhookSubForm
-        <FormColumnLayout>
+        enableWebhooks={enableWebhooks}
-          <FormGroup
+        templateType={template.type}
-            name="webhook_service"
+      />
-            fieldId="webhook_service"
-            helperTextInvalid={webhookServiceMeta.error}
-            isValid={!(webhookServiceMeta.touched || webhookServiceMeta.error)}
-            label={i18n._(t`Webhook Service`)}
-          >
-            <FieldTooltip content={i18n._(t`Select a webhook service`)} />
-            <AnsibleSelect
-              id="webhook_service"
-              data={webhookServiceOptions}
-              value={webhookServiceField.value}
-              onChange={(event, val) => {
-                storeWebhookValues(val);
-
-                webhookServiceHelpers.setValue(val);
-              }}
-            />
-          </FormGroup>
-          {!wfjtAddMatch && (
-            <>
-              <FormGroup
-                type="text"
-                fieldId="wfjt-webhookURL"
-                label={i18n._(t`Webhook URL`)}
-                id="wfjt-webhook-url"
-                name="webhook_url"
-              >
-                <FieldTooltip
-                  content={i18n._(
-                    t`Webhook services can launch jobs with this workflow job template by making a POST request to this URL.`
-                  )}
-                />
-                <TextInput
-                  aria-label={i18n._(t`Webhook URL`)}
-                  value={webhookUrlField.value}
-                  isReadOnly
-                />
-              </FormGroup>
-              <FormGroup
-                fieldId="wfjt-webhook-key"
-                type="text"
-                id="wfjt-webhook-key"
-                name="webhook_key"
-                label={i18n._(t`Webhook Key`)}
-              >
-                <FieldTooltip
-                  content={i18n._(
-                    t`Webhook services can use this as a shared secret.`
-                  )}
-                />
-                <InputGroup>
-                  <TextInput
-                    isReadOnly
-                    aria-label="wfjt-webhook-key"
-                    value={webhookKeyField.value}
-                  />
-                  <Button variant="tertiary" onClick={changeWebhookKey}>
-                    <SyncAltIcon />
-                  </Button>
-                </InputGroup>
-              </FormGroup>
-            </>
-          )}
-          {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)
-            <CredentialLookup
-              label={i18n._(t`Webhook Credential`)}
-              tooltip={i18n._(
-                t`Optionally select the credential to use to send status updates back to the webhook service.`
-              )}
-              credentialTypeId={credTypeId}
-              onChange={value => {
-                webhookCredentialHelpers.setValue(value || null);
-              }}
-              isValid={!webhookCredentialMeta.error}
-              helperTextInvalid={webhookCredentialMeta.error}
-              value={webhookCredentialField.value}
-            />
-          )}
-        </FormColumnLayout>
-      )}
       {submitError && <FormSubmitError error={submitError} />}
       <FormActionGroup onCancel={handleCancel} onSubmit={handleSubmit} />
     </Form>
@@ -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({

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('<WorkflowJobTemplateForm/>', () => {
     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('<WorkflowJobTemplateForm/>', () => {
       wrapper.find('input[aria-label="wfjt-webhook-key"]').prop('value')
     ).toBe('sdfghjklmnbvcdsew435678iokjhgfd');
     await act(() =>
-      wrapper
+      wrapper.find('Button[aria-label="Update webhook key"]').prop('onClick')()
-        .find('FormGroup[name="webhook_key"]')
-        .find('Button[variant="tertiary"]')
-        .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);
-
+    expect(wrapper.find('AnsibleSelect#webhook_service').length).toBe(1);
-    act(() => wrapper.find('AnsibleSelect').prop('onChange')({}, 'gitlab'));
+    await act(async () => {
+      wrapper.find('AnsibleSelect#webhook_service').prop('onChange')(
+        {},
+        'gitlab'
+      );
+    });
     wrapper.update();
-
+    expect(wrapper.find('AnsibleSelect#webhook_service').prop('value')).toBe(
-    expect(wrapper.find('AnsibleSelect').prop('value')).toBe('gitlab');
+      'gitlab'
+    );
   });
 
   test('handleSubmit is called on submit button click', async () => {

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,