Adds error handling to submit labels, prevents uncessary api call

The unecessary api call is for the webhook credential id.  If there is no
webhook service we do not want the api to make a call for get the webhook credential
type id.

awx/ui_next/src/components/CodeMirrorInput/VariablesField.jsx

@@ -5,9 +5,8 @@ import { t } from '@lingui/macro';
 import { useField } from 'formik';
 import styled from 'styled-components';
 import { Split, SplitItem } from '@patternfly/react-core';
-import { CheckboxField } from '@components/FormField';
+import { CheckboxField, FieldTooltip } from '@components/FormField';
 import { yamlToJson, jsonToYaml, isJson } from '@util/yaml';
-import { FieldTooltip } from '@components/FormField';
 import CodeMirrorInput from './CodeMirrorInput';
 import YamlJsonToggle from './YamlJsonToggle';
 import { JSON_MODE, YAML_MODE } from './constants';

awx/ui_next/src/screens/Template/WorkflowJobTemplateAdd/WorkflowJobTemplateAdd.jsx

@@ -17,33 +17,28 @@ function WorkflowJobTemplateAdd() {
       const {
         data: { id },
       } = await WorkflowJobTemplatesAPI.create(remainingValues);
-      await Promise.all([submitLabels(id, labels, organizationId, values)]);
+      await submitLabels(id, labels, organizationId);
       history.push(`/templates/workflow_job_template/${id}/details`);
     } catch (err) {
       setFormSubmitError(err);
     }
   };
 
-  const submitLabels = async (
-    templateId,
-    labels = [],
-    organizationId,
-    values
-  ) => {
-    if (!organizationId && !values.organization) {
+  const submitLabels = async (templateId, labels = [], organizationId) => {
+    if (!organizationId) {
       try {
         const {
           data: { results },
         } = await OrganizationsAPI.read();
         organizationId = results[0].id;
       } catch (err) {
-        setFormSubmitError(err);
+        throw err;
       }
     }
     const associatePromises = labels.map(label =>
       WorkflowJobTemplatesAPI.associateLabel(templateId, label, organizationId)
     );
-    return Promise.all([...associatePromises]);
+    return [...associatePromises];
   };
 
   const handleCancel = () => {

awx/ui_next/src/screens/Template/WorkflowJobTemplateEdit/WorkflowJobTemplateEdit.jsx

@@ -13,46 +13,38 @@ function WorkflowJobTemplateEdit({ template, webhook_key }) {
   const handleSubmit = async values => {
     const { labels, ...remainingValues } = values;
     try {
+      await submitLabels(labels, values.organization, template.organization);
       await WorkflowJobTemplatesAPI.update(template.id, remainingValues);
-      await Promise.all([submitLabels(labels, values.organization)]);
       history.push(`/templates/workflow_job_template/${template.id}/details`);
     } catch (err) {
       setFormSubmitError(err);
     }
   };
 
-  const submitLabels = async (labels = [], orgId) => {
+  const submitLabels = async (labels = [], formOrgId, templateOrgId) => {
     const { added, removed } = getAddedAndRemoved(
       template.summary_fields.labels.results,
       labels
     );
-    if (!orgId && !template.organization) {
+    let orgId = formOrgId || templateOrgId;
+    if (!orgId) {
       try {
         const {
           data: { results },
         } = await OrganizationsAPI.read();
         orgId = results[0].id;
       } catch (err) {
-        setFormSubmitError(err);
+        throw err;
       }
     }
 
-    const disassociationPromises = removed.map(label =>
+    const disassociationPromises = await removed.map(label =>
       WorkflowJobTemplatesAPI.disassociateLabel(template.id, label)
     );
-
-    const associationPromises = added.map(label => {
-      return WorkflowJobTemplatesAPI.associateLabel(
-        template.id,
-        label,
-        orgId || template.organization
-      );
-    });
-
-    const results = await Promise.all([
-      ...disassociationPromises,
-      ...associationPromises,
-    ]);
+    const associationPromises = await added.map(label =>
+      WorkflowJobTemplatesAPI.associateLabel(template.id, label, orgId)
+    );
+    const results = [...disassociationPromises, ...associationPromises];
     return results;
   };
 

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

@@ -1,12 +1,14 @@
 import React from 'react';
 import { Route } from 'react-router-dom';
 import { act } from 'react-dom/test-utils';
-import { WorkflowJobTemplatesAPI } from '@api';
+import { WorkflowJobTemplatesAPI, OrganizationsAPI } from '@api';
 import { mountWithContexts } from '@testUtils/enzymeHelpers';
 import { createMemoryHistory } from 'history';
 import WorkflowJobTemplateEdit from './WorkflowJobTemplateEdit';
 
-jest.mock('@api');
+jest.mock('@api/models/WorkflowJobTemplates');
+jest.mock('@api/models/Organizations');
+
 const mockTemplate = {
   id: 6,
   name: 'Foo',
@@ -53,6 +55,7 @@ describe('<WorkflowJobTemplateEdit/>', () => {
 
   afterEach(async () => {
     wrapper.unmount();
+    jest.clearAllMocks();
   });
 
   test('renders successfully', () => {
@@ -65,8 +68,8 @@ describe('<WorkflowJobTemplateEdit/>', () => {
         id: 6,
         name: 'Alex',
         description: 'Apollo and Athena',
-        inventory: { id: 1, name: 'Inventory 1' },
-        organization: 2,
+        inventory: 1,
+        organization: 1,
         labels: [{ name: 'Label 2', id: 2 }, { name: 'Generated Label' }],
         scm_branch: 'master',
         limit: '5000',
@@ -78,8 +81,8 @@ describe('<WorkflowJobTemplateEdit/>', () => {
       id: 6,
       name: 'Alex',
       description: 'Apollo and Athena',
-      inventory: { id: 1, name: 'Inventory 1' },
-      organization: 2,
+      inventory: 1,
+      organization: 1,
       scm_branch: 'master',
       limit: '5000',
       variables: '---',
@@ -94,9 +97,9 @@ describe('<WorkflowJobTemplateEdit/>', () => {
     await expect(WorkflowJobTemplatesAPI.associateLabel).toBeCalledTimes(1);
   });
 
-  test('handleCancel navigates the user to the /templates', async () => {
-    await act(async () => {
-      await wrapper.find('WorkflowJobTemplateForm').invoke('handleCancel')();
+  test('handleCancel navigates the user to the /templates', () => {
+    act(() => {
+      wrapper.find('WorkflowJobTemplateForm').invoke('handleCancel')();
     });
     expect(history.location.pathname).toBe(
       '/templates/workflow_job_template/6/details'
@@ -105,25 +108,41 @@ describe('<WorkflowJobTemplateEdit/>', () => {
 
   test('throwing error renders FormSubmitError component', async () => {
     const error = new Error('oops');
-    WorkflowJobTemplatesAPI.update.mockImplementation(() =>
-      Promise.reject(error)
-    );
+    OrganizationsAPI.read.mockResolvedValue({ results: [{ id: 1 }] });
+    WorkflowJobTemplatesAPI.update.mockRejectedValue(error);
     await act(async () => {
-      wrapper.find('WorkflowJobTemplateForm').prop('handleSubmit')({
-        id: 6,
-        name: 'Alex',
-        description: 'Apollo and Athena',
-        inventory: { id: 1, name: 'Inventory 1' },
-        organization: 2,
-        scm_branch: 'master',
-        limit: '5000',
-        variables: '---',
-      });
+      wrapper.find('Button[aria-label="Save"]').simulate('click');
     });
-    wrapper.update();
     expect(WorkflowJobTemplatesAPI.update).toHaveBeenCalled();
+    wrapper.update();
     expect(wrapper.find('WorkflowJobTemplateForm').prop('submitError')).toEqual(
       error
     );
   });
+
+  test('throwing error prevents form submission', () => {
+    OrganizationsAPI.read.mockRejectedValue(new Error('An error occurred'));
+    WorkflowJobTemplatesAPI.update.mockResolvedValue();
+
+    act(() => {
+      wrapper = mountWithContexts(
+        <WorkflowJobTemplateEdit template={mockTemplate} />,
+        {
+          context: {
+            router: {
+              history,
+            },
+          },
+        }
+      );
+    });
+    wrapper.find('Button[aria-label="Save"]').simulate('click');
+
+    expect(wrapper.find('WorkflowJobTemplateForm').length).toBe(1);
+    expect(OrganizationsAPI.read).toBeCalled();
+    expect(WorkflowJobTemplatesAPI.update).not.toBeCalled();
+    expect(history.location.pathname).toBe(
+      '/templates/workflow_job_template/6/edit'
+    );
+  });
 });

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

@@ -97,11 +97,14 @@ function WorkflowJobTemplateForm({
     result: credTypeId,
   } = useRequest(
     useCallback(async () => {
-      const results = await CredentialTypesAPI.read({
-        namespace: `${webhookService}_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)
+      let results;
+      if (webhookService) {
+        results = await CredentialTypesAPI.read({
+          namespace: `${webhookService}_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])
   );
@@ -109,10 +112,10 @@ function WorkflowJobTemplateForm({
   useEffect(() => {
     loadCredentialType();
   }, [loadCredentialType]);
-  // TODO: Convert this function below to useRequest. 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
-  let initialWebhookKey = webhook_key;
+
+  // 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 {
@@ -124,7 +127,9 @@ function WorkflowJobTemplateForm({
     }
   };
 
+  let initialWebhookKey = webhook_key;
   const initialWebhookCredential = template?.summary_fields?.webhook_credential;
+
   const storeWebhookValues = (form, webhookServiceValue) => {
     if (
       webhookServiceValue === form.initialValues.webhook_service ||
@@ -135,12 +140,17 @@ function WorkflowJobTemplateForm({
         form.initialValues.webhook_credential
       );
       setWebhookCredential(initialWebhookCredential);
+
       form.setFieldValue('webhook_url', form.initialValues.webhook_url);
+
       form.setFieldValue('webhook_service', form.initialValues.webhook_service);
+      setWebHookService(form.initialValues.webhook_service);
+
       setWebHookKey(initialWebhookKey);
     } else {
       form.setFieldValue('webhook_credential', null);
       setWebhookCredential(null);
+
       form.setFieldValue(
         'webhook_url',
         `${urlOrigin}/api/v2/workflow_job_templates/${template.id}/${webhookServiceValue}/`
@@ -232,36 +242,37 @@ function WorkflowJobTemplateForm({
               {({ form }) => (
                 <OrganizationLookup
                   helperTextInvalid={form.errors.organization}
-                  onBlur={() => form.setFieldTouched('organization')}
                   onChange={value => {
                     form.setFieldValue('organization', value?.id || null);
                     setOrganization(value);
                   }}
                   value={organization}
-                  isValid={
-                    !(form.touched.organization || form.errors.organization)
-                  }
-                  touched={form.touched.organization}
-                  error={form.errors.organization}
+                  isValid={!form.errors.organization}
                 />
               )}
             </Field>
             <Field name="inventory">
               {({ form }) => (
-                <InventoryLookup
-                  value={inventory}
-                  tooltip={i18n._(
-                    t`Select an inventory for the workflow. This inventory is applied to all job template nodes that prompt for an inventory.`
-                  )}
-                  isValid={!(form.touched.inventory || form.errors.inventory)}
-                  helperTextInvalid={form.errors.inventory}
-                  onChange={value => {
-                    form.setFieldValue('inventory', value?.id || null);
-                    setInventory(value);
-                    form.setFieldValue('organizationId', value?.organization);
-                  }}
-                  error={form.errors.inventory}
-                />
+                <FormGroup
+                  label={i18n._(t`Inventory`)}
+                  fieldId="wfjt-inventory"
+                >
+                  <FieldTooltip
+                    content={i18n._(
+                      t`Select an inventory for the workflow. This inventory is applied to all job template nodes that prompt for an inventory.`
+                    )}
+                  />
+                  <InventoryLookup
+                    value={inventory}
+                    isValid={!form.errors.inventory}
+                    helperTextInvalid={form.errors.inventory}
+                    onChange={value => {
+                      form.setFieldValue('inventory', value?.id || null);
+                      setInventory(value);
+                      form.setFieldValue('organizationId', value?.organization);
+                    }}
+                  />
+                </FormGroup>
               )}
             </Field>
             <FormField
@@ -459,12 +470,7 @@ function WorkflowJobTemplateForm({
                         );
                         setWebhookCredential(value);
                       }}
-                      isValid={
-                        !(
-                          form.touched.webhook_credential ||
-                          form.errors.webhook_credential
-                        )
-                      }
+                      isValid={!form.errors.webhook_credential}
                       helperTextInvalid={form.errors.webhook_credential}
                       value={webhookCredential}
                     />
@@ -473,7 +479,7 @@ function WorkflowJobTemplateForm({
               )}
             </FormColumnLayout>
           )}
-          <FormSubmitError error={submitError} />
+          {submitError && <FormSubmitError error={submitError} />}
           <FormActionGroup
             onCancel={handleCancel}
             onSubmit={formik.handleSubmit}