From b055aad64197af5ffbf778c13d6c2c54499a77f3 Mon Sep 17 00:00:00 2001 From: Alex Corey Date: Mon, 2 Mar 2020 14:48:44 -0500 Subject: [PATCH] Adds error handling test to add and edit form. Updates Form component --- .../src/api/models/WorkflowJobTemplates.js | 2 +- .../components/Lookup/CredentialLookup.jsx | 1 + awx/ui_next/src/screens/Template/Template.jsx | 54 ++++++----- .../src/screens/Template/Templates.jsx | 5 +- .../screens/Template/WorkflowJobTemplate.jsx | 10 -- .../WorkflowJobTemplateAdd.jsx | 2 +- .../WorkflowJobTemplateAdd.test.jsx | 92 ++++++++++++------- .../WorkflowJobTemplateDetail.jsx | 2 +- .../WorkflowJobTemplateEdit.jsx | 4 +- .../WorkflowJobTemplateEdit.test.jsx | 56 ++++++++--- .../shared/WorkflowJobTemplateForm.jsx | 58 ++++++------ .../shared/WorkflowJobTemplateForm.test.jsx | 60 ++++++++---- 12 files changed, 216 insertions(+), 130 deletions(-) diff --git a/awx/ui_next/src/api/models/WorkflowJobTemplates.js b/awx/ui_next/src/api/models/WorkflowJobTemplates.js index eec62cedfc..a71fe68cbc 100644 --- a/awx/ui_next/src/api/models/WorkflowJobTemplates.js +++ b/awx/ui_next/src/api/models/WorkflowJobTemplates.js @@ -48,7 +48,7 @@ class WorkflowJobTemplates extends Base { readScheduleList(id, params) { return this.http.get(`${this.baseUrl}${id}/schedules/`, { - params + params, }); } } diff --git a/awx/ui_next/src/components/Lookup/CredentialLookup.jsx b/awx/ui_next/src/components/Lookup/CredentialLookup.jsx index 3f3221adeb..37f8a2e3bb 100644 --- a/awx/ui_next/src/components/Lookup/CredentialLookup.jsx +++ b/awx/ui_next/src/components/Lookup/CredentialLookup.jsx @@ -100,6 +100,7 @@ function CredentialLookup({ }, ]} readOnly={!canDelete} + name="credential" selectItem={item => dispatch({ type: 'SELECT_ITEM', item })} deselectItem={item => dispatch({ type: 'DESELECT_ITEM', item })} /> diff --git a/awx/ui_next/src/screens/Template/Template.jsx b/awx/ui_next/src/screens/Template/Template.jsx index 6fe466482a..61f63e70e4 100644 --- a/awx/ui_next/src/screens/Template/Template.jsx +++ b/awx/ui_next/src/screens/Template/Template.jsx @@ -221,36 +221,34 @@ class Template extends Component { )} - {template?.id && ( - - - - )} - {template && ( + {template && ( + ( + + )} + /> + )} } + key="not-found" + path="*" + render={() => + !hasContentLoading && ( + + {match.params.id && ( + + {i18n._(`View Template Details`)} + + )} + + ) + } /> - )} - - !hasContentLoading && ( - - {match.params.id && ( - - {i18n._(`View Template Details`)} - - )} - - ) - } - /> - - + + + ); } } diff --git a/awx/ui_next/src/screens/Template/Templates.jsx b/awx/ui_next/src/screens/Template/Templates.jsx index 97b86ebb84..f6572a54ab 100644 --- a/awx/ui_next/src/screens/Template/Templates.jsx +++ b/awx/ui_next/src/screens/Template/Templates.jsx @@ -21,7 +21,7 @@ class Templates extends Component { '/templates': i18n._(t`Templates`), '/templates/job_template/add': i18n._(t`Create New Job Template`), '/templates/workflow_job_template/add': i18n._( - t`Create New Workflow Job Template` + t`Create New Workflow Template` ), }, }; @@ -35,6 +35,9 @@ class Templates extends Component { const breadcrumbConfig = { '/templates': i18n._(t`Templates`), '/templates/job_template/add': i18n._(t`Create New Job Template`), + '/templates/workflow_job_template/add': i18n._( + t`Create New Workflow Template` + ), [`/templates/${template.type}/${template.id}`]: `${template.name}`, [`/templates/${template.type}/${template.id}/details`]: i18n._( t`Details` diff --git a/awx/ui_next/src/screens/Template/WorkflowJobTemplate.jsx b/awx/ui_next/src/screens/Template/WorkflowJobTemplate.jsx index 35e0336d36..6c63e16ab0 100644 --- a/awx/ui_next/src/screens/Template/WorkflowJobTemplate.jsx +++ b/awx/ui_next/src/screens/Template/WorkflowJobTemplate.jsx @@ -186,16 +186,6 @@ class WorkflowJobTemplate extends Component { )} /> )} - {template?.id && ( - - - - )} - {template?.id && ( ', () => { let wrapper; let history; beforeEach(async () => { WorkflowJobTemplatesAPI.create.mockResolvedValue({ data: { id: 1 } }); + OrganizationsAPI.read.mockResolvedValue({ results: [{ id: 1 }] }); + LabelsAPI.read.mockResolvedValue({ + data: { + results: [ + { name: 'Label 1', id: 1 }, + { name: 'Label 2', id: 2 }, + { name: 'Label 3', id: 3 }, + ], + }, + }); + await act(async () => { history = createMemoryHistory({ initialEntries: ['/templates/workflow_job_template/add'], }); - wrapper = mountWithContexts( - } - />, - { - context: { - router: { - history, - route: { - location: history.location, + await act(async () => { + wrapper = await mountWithContexts( + } + />, + { + context: { + router: { + history, + route: { + location: history.location, + }, }, }, - }, - } - ); + } + ); + }); }); }); afterEach(async () => { wrapper.unmount(); + jest.clearAllMocks(); }); test('initially renders successfully', async () => { @@ -67,28 +84,39 @@ describe('', () => { test('throwing error renders FormSubmitError component', async () => { const error = new Error('oops'); - WorkflowJobTemplatesAPI.create.mockImplementation(() => - Promise.reject(error) - ); + WorkflowJobTemplatesAPI.create.mockRejectedValue(error); await act(async () => { - await 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('WorkflowJobTemplateForm').invoke('handleSubmit')({ + name: 'Foo', }); }); - expect(wrapper.find('ContentError').length).toBe(0); - expect(wrapper.length).toBe(1); + expect(WorkflowJobTemplatesAPI.create).toHaveBeenCalled(); wrapper.update(); - expect(WorkflowJobTemplatesAPI.create).toBeCalled(); - expect(wrapper.find('ContentError').length).toBe(1); expect(wrapper.find('WorkflowJobTemplateForm').prop('submitError')).toEqual( error ); }); + + test('throwing error prevents navigation away from form', async () => { + OrganizationsAPI.read.mockRejectedValue( + new Error({ + response: { + config: { + method: 'get', + }, + data: 'An error occurred', + }, + }) + ); + WorkflowJobTemplatesAPI.update.mockResolvedValue(); + + await act(async () => { + await wrapper.find('Button[aria-label="Save"]').simulate('click'); + }); + expect(wrapper.find('WorkflowJobTemplateForm').length).toBe(1); + expect(OrganizationsAPI.read).toBeCalled(); + expect(history.location.pathname).toBe( + '/templates/workflow_job_template/add' + ); + }); }); diff --git a/awx/ui_next/src/screens/Template/WorkflowJobTemplateDetail/WorkflowJobTemplateDetail.jsx b/awx/ui_next/src/screens/Template/WorkflowJobTemplateDetail/WorkflowJobTemplateDetail.jsx index f39035286a..d528013def 100644 --- a/awx/ui_next/src/screens/Template/WorkflowJobTemplateDetail/WorkflowJobTemplateDetail.jsx +++ b/awx/ui_next/src/screens/Template/WorkflowJobTemplateDetail/WorkflowJobTemplateDetail.jsx @@ -58,7 +58,7 @@ function WorkflowJobTemplateDetail({ template, i18n, webhook_key }) { )} {template.webhook_service && ( - {i18n._(t`- Webhooks`)} + {i18n._(t`- Enable Webhook`)} )} diff --git a/awx/ui_next/src/screens/Template/WorkflowJobTemplateEdit/WorkflowJobTemplateEdit.jsx b/awx/ui_next/src/screens/Template/WorkflowJobTemplateEdit/WorkflowJobTemplateEdit.jsx index 6c9356e70d..b958ff9ff4 100644 --- a/awx/ui_next/src/screens/Template/WorkflowJobTemplateEdit/WorkflowJobTemplateEdit.jsx +++ b/awx/ui_next/src/screens/Template/WorkflowJobTemplateEdit/WorkflowJobTemplateEdit.jsx @@ -13,7 +13,9 @@ function WorkflowJobTemplateEdit({ template, webhook_key }) { const handleSubmit = async values => { const { labels, ...remainingValues } = values; try { - await submitLabels(labels, values.organization, template.organization); + await Promise.all( + await submitLabels(labels, values.organization, template.organization) + ); await WorkflowJobTemplatesAPI.update(template.id, remainingValues); history.push(`/templates/workflow_job_template/${template.id}/details`); } catch (err) { 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 f38fe2134b..648f5caa4c 100644 --- a/awx/ui_next/src/screens/Template/WorkflowJobTemplateEdit/WorkflowJobTemplateEdit.test.jsx +++ b/awx/ui_next/src/screens/Template/WorkflowJobTemplateEdit/WorkflowJobTemplateEdit.test.jsx @@ -1,13 +1,15 @@ import React from 'react'; import { Route } from 'react-router-dom'; import { act } from 'react-dom/test-utils'; -import { WorkflowJobTemplatesAPI, OrganizationsAPI } from '@api'; +import { WorkflowJobTemplatesAPI, OrganizationsAPI, LabelsAPI } from '@api'; import { mountWithContexts } from '@testUtils/enzymeHelpers'; import { createMemoryHistory } from 'history'; import WorkflowJobTemplateEdit from './WorkflowJobTemplateEdit'; jest.mock('@api/models/WorkflowJobTemplates'); +jest.mock('@api/models/Labels'); jest.mock('@api/models/Organizations'); +jest.mock('@api/models/Inventories'); const mockTemplate = { id: 6, @@ -27,8 +29,14 @@ const mockTemplate = { describe('', () => { let wrapper; let history; - beforeEach(async () => { + LabelsAPI.read.mockResolvedValue({ + data: { + results: [{ name: 'Label 1', id: 1 }, { name: 'Label 2', id: 2 }], + }, + }); + OrganizationsAPI.read.mockResolvedValue({ results: [{ id: 1 }] }); + await act(async () => { history = createMemoryHistory({ initialEntries: ['/templates/workflow_job_template/6/edit'], @@ -93,7 +101,6 @@ describe('', () => { id: 1, }); wrapper.update(); - await expect(WorkflowJobTemplatesAPI.associateLabel).toBeCalledTimes(1); }); @@ -108,7 +115,6 @@ describe('', () => { test('throwing error renders FormSubmitError component', async () => { const error = new Error('oops'); - OrganizationsAPI.read.mockResolvedValue({ results: [{ id: 1 }] }); WorkflowJobTemplatesAPI.update.mockRejectedValue(error); await act(async () => { wrapper.find('Button[aria-label="Save"]').simulate('click'); @@ -120,13 +126,26 @@ describe('', () => { ); }); - test('throwing error prevents form submission', () => { - OrganizationsAPI.read.mockRejectedValue(new Error('An error occurred')); - WorkflowJobTemplatesAPI.update.mockResolvedValue(); + test('throwing error prevents form submission', async () => { + const templateWithoutOrg = { + id: 6, + name: 'Foo', + description: 'Foo description', + summary_fields: { + inventory: { id: 1, name: 'Inventory 1' }, + labels: { + results: [{ name: 'Label 1', id: 1 }, { name: 'Label 2', id: 2 }], + }, + }, + scm_branch: 'devel', + limit: '5000', + variables: '---', + }; - act(() => { - wrapper = mountWithContexts( - , + let newWrapper; + await act(async () => { + newWrapper = await mountWithContexts( + , { context: { router: { @@ -136,9 +155,22 @@ describe('', () => { } ); }); - wrapper.find('Button[aria-label="Save"]').simulate('click'); + OrganizationsAPI.read.mockRejectedValue( + new Error({ + response: { + config: { + method: 'get', + }, + data: 'An error occurred', + }, + }) + ); + WorkflowJobTemplatesAPI.update.mockResolvedValue(); - expect(wrapper.find('WorkflowJobTemplateForm').length).toBe(1); + await act(async () => { + await newWrapper.find('Button[aria-label="Save"]').simulate('click'); + }); + expect(newWrapper.find('WorkflowJobTemplateForm').length).toBe(1); expect(OrganizationsAPI.read).toBeCalled(); expect(WorkflowJobTemplatesAPI.update).not.toBeCalled(); expect(history.location.pathname).toBe( diff --git a/awx/ui_next/src/screens/Template/shared/WorkflowJobTemplateForm.jsx b/awx/ui_next/src/screens/Template/shared/WorkflowJobTemplateForm.jsx index 7ebea06f9a..323010ce34 100644 --- a/awx/ui_next/src/screens/Template/shared/WorkflowJobTemplateForm.jsx +++ b/awx/ui_next/src/screens/Template/shared/WorkflowJobTemplateForm.jsx @@ -49,12 +49,15 @@ function WorkflowJobTemplateForm({ 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 [inventory, setInventory] = useState( template?.summary_fields?.inventory || null ); @@ -141,8 +144,11 @@ function WorkflowJobTemplateForm({ ); setWebhookCredential(initialWebhookCredential); - form.setFieldValue('webhook_url', form.initialValues.webhook_url); - + setWebhookUrl( + template?.related?.webhook_receiver + ? `${urlOrigin}${template.related.webhook_receiver}` + : '' + ); form.setFieldValue('webhook_service', form.initialValues.webhook_service); setWebHookService(form.initialValues.webhook_service); @@ -151,8 +157,7 @@ function WorkflowJobTemplateForm({ form.setFieldValue('webhook_credential', null); setWebhookCredential(null); - form.setFieldValue( - 'webhook_url', + setWebhookUrl( `${urlOrigin}/api/v2/workflow_job_templates/${template.id}/${webhookServiceValue}/` ); @@ -171,7 +176,7 @@ function WorkflowJobTemplateForm({ initialWebhookKey = webhookKey; form.setFieldValue('webhook_credential', null); form.setFieldValue('webhook_service', ''); - form.setFieldValue('webhook_url', ''); + setWebhookUrl(''); setWebHookService(''); setWebHookKey(''); } else { @@ -203,11 +208,8 @@ function WorkflowJobTemplateForm({ labels: template.summary_fields?.labels?.results || [], extra_vars: template.variables || '---', limit: template.limit || '', - scmBranch: template.scm_branch || '', + scm_branch: template.scm_branch || '', allow_simultaneous: template.allow_simultaneous || false, - webhook_url: - template?.related?.webhook_receiver && - `${urlOrigin}${template?.related?.webhook_receiver}`, webhook_credential: template?.summary_fields?.webhook_credential?.id || null, webhook_service: template.webhook_service || '', @@ -290,8 +292,8 @@ function WorkflowJobTemplateForm({ tooltip={i18n._( t`Select a branch for the workflow. This branch is applied to all job template nodes that prompt for a branch.` )} - id="wfjt-scmBranch" - name="scmBranch" + id="wfjt-scm_branch" + name="scm_branch" /> @@ -333,17 +335,13 @@ function WorkflowJobTemplateForm({ isInline label={i18n._(t`Options`)} > - + {({ form }) => ( - {i18n._(t`Webhooks`)} + {i18n._(t`Enable Webhook`)}   {!wfjtAddMatch && ( <> - + > + + + {({ form }) => ( ', () => { let wrapper; @@ -38,12 +42,31 @@ describe('', () => { }, }; - beforeEach(() => { + beforeEach(async () => { + WorkflowJobTemplatesAPI.updateWebhookKey.mockResolvedValue({ + data: { webhook_key: 'sdafdghjkl2345678ionbvcxz' }, + }); + LabelsAPI.read.mockResolvedValue({ + data: { + results: [ + { name: 'Label 1', id: 1 }, + { name: 'Label 2', id: 2 }, + { name: 'Label 3', id: 3 }, + ], + }, + }); + OrganizationsAPI.read.mockResolvedValue({ + results: [{ id: 1 }, { id: 2 }], + }); + InventoriesAPI.read.mockResolvedValue({ + results: [{ id: 1, name: 'Foo' }, { id: 2, name: 'Bar' }], + }); + history = createMemoryHistory({ initialEntries: ['/templates/workflow_job_template/6/edit'], }); - act(() => { - wrapper = mountWithContexts( + await act(async () => { + wrapper = await mountWithContexts( ( @@ -86,7 +109,7 @@ describe('', () => { 'Field[name="organization"]', 'Field[name="inventory"]', 'FormField[name="limit"]', - 'FormField[name="scmBranch"]', + 'FormField[name="scm_branch"]', 'Field[name="labels"]', 'VariablesField', ]; @@ -108,8 +131,8 @@ describe('', () => { }, { element: 'wfjt-limit', value: { value: 1234567890, name: 'limit' } }, { - element: 'wfjt-scmBranch', - value: { value: 'new branch', name: 'scmBranch' }, + element: 'wfjt-scm_branch', + value: { value: 'new branch', name: 'scm_branch' }, }, ]; const changeInputs = async ({ element, value }) => { @@ -122,7 +145,7 @@ describe('', () => { inputsToChange.map(input => changeInputs(input)); wrapper.find('LabelSelect').invoke('onChange')([ - { name: 'new label', id: 5 }, + { name: 'Label 3', id: 3 }, { name: 'Label 1', id: 1 }, { name: 'Label 2', id: 2 }, ]); @@ -148,13 +171,16 @@ describe('', () => { test('webhooks and enable concurrent jobs functions properly', async () => { act(() => { - wrapper.find('Checkbox[aria-label="Webhooks"]').invoke('onChange')(true, { - currentTarget: { value: true, type: 'change', checked: true }, - }); + wrapper.find('Checkbox[aria-label="Enable Webhook"]').invoke('onChange')( + true, + { + currentTarget: { value: true, type: 'change', checked: true }, + } + ); }); wrapper.update(); expect( - wrapper.find('Checkbox[aria-label="Webhooks"]').prop('isChecked') + wrapper.find('Checkbox[aria-label="Enable Webhook"]').prop('isChecked') ).toBe(true); expect( @@ -171,7 +197,7 @@ describe('', () => { ); expect(WorkflowJobTemplatesAPI.updateWebhookKey).toBeCalledWith('6'); expect( - wrapper.find('TextInputBase[name="webhook_url"]').prop('value') + wrapper.find('TextInputBase[aria-label="Webhook URL"]').prop('value') ).toContain('/api/v2/workflow_job_templates/57/gitlab/'); wrapper.update();