Adds pageSection to jt add form and fixes other PR issues

-Fixes spelling error on WFJTDetail
-Adds page section to JT Add Form to fix styling issue
-Adds spacing between functions
-Fixes form submission error by allowing state to handle the lookups while formik
only handles their ids.
This commit is contained in:
Alex Corey
2020-02-25 17:23:22 -05:00
parent e90ee5113d
commit fc89b627d7
9 changed files with 77 additions and 31 deletions

View File

@@ -1,6 +1,6 @@
import React, { useState } from 'react'; import React, { useState } from 'react';
import { useHistory } from 'react-router-dom'; import { useHistory } from 'react-router-dom';
import { Card } from '@patternfly/react-core'; import { Card, PageSection } from '@patternfly/react-core';
import { CardBody } from '@components/Card'; import { CardBody } from '@components/Card';
import JobTemplateForm from '../shared/JobTemplateForm'; import JobTemplateForm from '../shared/JobTemplateForm';
import { JobTemplatesAPI } from '@api'; import { JobTemplatesAPI } from '@api';
@@ -61,15 +61,17 @@ function JobTemplateAdd() {
} }
return ( return (
<Card> <PageSection>
<CardBody> <Card>
<JobTemplateForm <CardBody>
handleCancel={handleCancel} <JobTemplateForm
handleSubmit={handleSubmit} handleCancel={handleCancel}
submitError={formSubmitError} handleSubmit={handleSubmit}
/> submitError={formSubmitError}
</CardBody> />
</Card> </CardBody>
</Card>
</PageSection>
); );
} }

View File

@@ -8,8 +8,9 @@ import { WorkflowJobTemplatesAPI } from '@api';
import WorkflowJobTemplateForm from '../shared/WorkflowJobTemplateForm'; import WorkflowJobTemplateForm from '../shared/WorkflowJobTemplateForm';
function WorkflowJobTemplateAdd() { function WorkflowJobTemplateAdd() {
const [formSubmitError, setFormSubmitError] = useState();
const history = useHistory(); const history = useHistory();
const [formSubmitError, setFormSubmitError] = useState();
const handleSubmit = async values => { const handleSubmit = async values => {
const { labels, organizationId, ...remainingValues } = values; const { labels, organizationId, ...remainingValues } = values;
try { try {

View File

@@ -39,9 +39,11 @@ describe('<WorkflowJobTemplateAdd/>', () => {
afterEach(async () => { afterEach(async () => {
wrapper.unmount(); wrapper.unmount();
}); });
test('initially renders successfully', async () => { test('initially renders successfully', async () => {
expect(wrapper.length).toBe(1); expect(wrapper.length).toBe(1);
}); });
test('calls workflowJobTemplatesAPI with correct information on submit', async () => { test('calls workflowJobTemplatesAPI with correct information on submit', async () => {
await act(async () => { await act(async () => {
await wrapper.find('WorkflowJobTemplateForm').invoke('handleSubmit')({ await wrapper.find('WorkflowJobTemplateForm').invoke('handleSubmit')({
@@ -55,12 +57,14 @@ describe('<WorkflowJobTemplateAdd/>', () => {
}); });
expect(WorkflowJobTemplatesAPI.associateLabel).toHaveBeenCalledTimes(2); expect(WorkflowJobTemplatesAPI.associateLabel).toHaveBeenCalledTimes(2);
}); });
test('handleCancel navigates the user to the /templates', async () => { test('handleCancel navigates the user to the /templates', async () => {
await act(async () => { await act(async () => {
await wrapper.find('WorkflowJobTemplateForm').invoke('handleCancel')(); await wrapper.find('WorkflowJobTemplateForm').invoke('handleCancel')();
}); });
expect(history.location.pathname).toBe('/templates'); expect(history.location.pathname).toBe('/templates');
}); });
test('throwing error renders FormSubmitError component', async () => { test('throwing error renders FormSubmitError component', async () => {
const error = new Error('oops'); const error = new Error('oops');
WorkflowJobTemplatesAPI.create.mockImplementation(() => WorkflowJobTemplatesAPI.create.mockImplementation(() =>

View File

@@ -39,12 +39,15 @@ function WorkflowJobTemplateDetail({ template, i18n, webhook_key }) {
related, related,
webhook_credential, webhook_credential,
} = template; } = template;
const urlOrigin = window.location.origin; const urlOrigin = window.location.origin;
const history = useHistory(); const history = useHistory();
const [deletionError, setDeletionError] = useState(null); const [deletionError, setDeletionError] = useState(null);
const [hasContentLoading, setHasContentLoading] = useState(false); const [hasContentLoading, setHasContentLoading] = useState(false);
const renderOptionsField = const renderOptionsField =
template.allow_simultaneous || template.webhook_servicee; template.allow_simultaneous || template.webhook_service;
const renderOptions = ( const renderOptions = (
<TextList component={TextListVariants.ul}> <TextList component={TextListVariants.ul}>
@@ -75,6 +78,7 @@ function WorkflowJobTemplateDetail({ template, i18n, webhook_key }) {
} }
setHasContentLoading(false); setHasContentLoading(false);
}; };
const inventoryValue = (kind, inventoryId) => { const inventoryValue = (kind, inventoryId) => {
const inventorykind = kind === 'smart' ? 'smart_inventory' : 'inventory'; const inventorykind = kind === 'smart' ? 'smart_inventory' : 'inventory';
@@ -91,6 +95,7 @@ function WorkflowJobTemplateDetail({ template, i18n, webhook_key }) {
</Link> </Link>
); );
}; };
const canLaunch = summary_fields?.user_capabilities?.start; const canLaunch = summary_fields?.user_capabilities?.start;
const recentPlaybookJobs = summary_fields.recent_jobs.map(job => ({ const recentPlaybookJobs = summary_fields.recent_jobs.map(job => ({
...job, ...job,

View File

@@ -40,6 +40,7 @@ describe('<WorkflowJobTemplateDetail/>', () => {
}, },
webhook_service: 'Github', webhook_service: 'Github',
}; };
beforeEach(async () => { beforeEach(async () => {
history = createMemoryHistory({ history = createMemoryHistory({
initialEntries: ['/templates/workflow_job_template/1/details'], initialEntries: ['/templates/workflow_job_template/1/details'],
@@ -75,12 +76,15 @@ describe('<WorkflowJobTemplateDetail/>', () => {
); );
}); });
}); });
afterEach(() => { afterEach(() => {
wrapper.unmount(); wrapper.unmount();
}); });
test('renders successfully', () => { test('renders successfully', () => {
expect(wrapper.find(WorkflowJobTemplateDetail).length).toBe(1); expect(wrapper.find(WorkflowJobTemplateDetail).length).toBe(1);
}); });
test('expect detail fields to render properly', () => { test('expect detail fields to render properly', () => {
const renderedValues = [ const renderedValues = [
{ {
@@ -147,6 +151,7 @@ describe('<WorkflowJobTemplateDetail/>', () => {
renderedValues.map(value => assertValue(value)); renderedValues.map(value => assertValue(value));
}); });
test('link out resource have the correct url', () => { test('link out resource have the correct url', () => {
const inventory = wrapper.find('Detail[label="Inventory"]').find('Link'); const inventory = wrapper.find('Detail[label="Inventory"]').find('Link');
const organization = wrapper const organization = wrapper

View File

@@ -7,8 +7,8 @@ import { WorkflowJobTemplatesAPI, OrganizationsAPI } from '@api';
import { WorkflowJobTemplateForm } from '../shared'; import { WorkflowJobTemplateForm } from '../shared';
function WorkflowJobTemplateEdit({ template, webhook_key }) { function WorkflowJobTemplateEdit({ template, webhook_key }) {
const [formSubmitError, setFormSubmitError] = useState();
const history = useHistory(); const history = useHistory();
const [formSubmitError, setFormSubmitError] = useState();
const handleSubmit = async values => { const handleSubmit = async values => {
const { labels, ...remainingValues } = values; const { labels, ...remainingValues } = values;

View File

@@ -25,6 +25,7 @@ const mockTemplate = {
describe('<WorkflowJobTemplateEdit/>', () => { describe('<WorkflowJobTemplateEdit/>', () => {
let wrapper; let wrapper;
let history; let history;
beforeEach(async () => { beforeEach(async () => {
await act(async () => { await act(async () => {
history = createMemoryHistory({ history = createMemoryHistory({
@@ -49,12 +50,15 @@ describe('<WorkflowJobTemplateEdit/>', () => {
); );
}); });
}); });
afterEach(async () => { afterEach(async () => {
wrapper.unmount(); wrapper.unmount();
}); });
test('renders successfully', () => { test('renders successfully', () => {
expect(wrapper.find('WorkflowJobTemplateEdit').length).toBe(1); expect(wrapper.find('WorkflowJobTemplateEdit').length).toBe(1);
}); });
test('api is called to properly to save the updated template.', async () => { test('api is called to properly to save the updated template.', async () => {
await act(async () => { await act(async () => {
await wrapper.find('WorkflowJobTemplateForm').invoke('handleSubmit')({ await wrapper.find('WorkflowJobTemplateForm').invoke('handleSubmit')({
@@ -69,6 +73,7 @@ describe('<WorkflowJobTemplateEdit/>', () => {
variables: '---', variables: '---',
}); });
}); });
expect(WorkflowJobTemplatesAPI.update).toHaveBeenCalledWith(6, { expect(WorkflowJobTemplatesAPI.update).toHaveBeenCalledWith(6, {
id: 6, id: 6,
name: 'Alex', name: 'Alex',
@@ -88,12 +93,14 @@ describe('<WorkflowJobTemplateEdit/>', () => {
await expect(WorkflowJobTemplatesAPI.associateLabel).toBeCalledTimes(1); await expect(WorkflowJobTemplatesAPI.associateLabel).toBeCalledTimes(1);
}); });
test('handleCancel navigates the user to the /templates', async () => { test('handleCancel navigates the user to the /templates', async () => {
await act(async () => { await act(async () => {
await wrapper.find('WorkflowJobTemplateForm').invoke('handleCancel')(); await wrapper.find('WorkflowJobTemplateForm').invoke('handleCancel')();
}); });
expect(history.location.pathname).toBe('/templates'); expect(history.location.pathname).toBe('/templates');
}); });
test('throwing error renders FormSubmitError component', async () => { test('throwing error renders FormSubmitError component', async () => {
const error = new Error('oops'); const error = new Error('oops');
WorkflowJobTemplatesAPI.update.mockImplementation(() => WorkflowJobTemplatesAPI.update.mockImplementation(() =>

View File

@@ -46,14 +46,24 @@ function WorkflowJobTemplateForm({
webhook_key, webhook_key,
submitError, submitError,
}) { }) {
const urlOrigin = window.location.origin;
const { id } = useParams(); const { id } = useParams();
const wfjtAddMatch = useRouteMatch('/templates/workflow_job_template/add'); const wfjtAddMatch = useRouteMatch('/templates/workflow_job_template/add');
const urlOrigin = window.location.origin;
const [contentError, setContentError] = useState(null); const [contentError, setContentError] = useState(null);
const [webhookKey, setWebHookKey] = useState(webhook_key);
const [credTypeId, setCredentialTypeId] = useState(); const [credTypeId, setCredentialTypeId] = useState();
const [inventory, setInventory] = useState(
template?.summary_fields?.inventory || null
);
const [organization, setOrganization] = useState(
template?.summary_fields?.organization || null
);
const [webhookCredential, setWebhookCredential] = useState(
template?.summary_fields?.webhook_credential || null
);
const [webhookKey, setWebHookKey] = useState(webhook_key);
const [webhookService, setWebHookService] = useState( const [webhookService, setWebHookService] = useState(
template.webhook_service || '' template.webhook_service || ''
); );
@@ -159,16 +169,16 @@ function WorkflowJobTemplateForm({
return ( return (
<Formik <Formik
onSubmit={values => { onSubmit={values => {
values.webhook_credential = values?.webhook_credential?.id; if (values.webhook_service === '') {
values.organization = values?.organization?.id; values.webhook_credential = '';
values.inventory = values?.inventory?.id; }
return handleSubmit(values); return handleSubmit(values);
}} }}
initialValues={{ initialValues={{
name: template.name || '', name: template.name || '',
description: template.description || '', description: template.description || '',
inventory: template?.summary_fields?.inventory || null, inventory: template?.summary_fields?.inventory?.id || null,
organization: template?.summary_fields?.organization || null, organization: template?.summary_fields?.organization?.id || null,
labels: template.summary_fields?.labels?.results || [], labels: template.summary_fields?.labels?.results || [],
extra_vars: template.variables || '---', extra_vars: template.variables || '---',
limit: template.limit || '', limit: template.limit || '',
@@ -178,7 +188,7 @@ function WorkflowJobTemplateForm({
template?.related?.webhook_receiver && template?.related?.webhook_receiver &&
`${urlOrigin}${template?.related?.webhook_receiver}`, `${urlOrigin}${template?.related?.webhook_receiver}`,
webhook_credential: webhook_credential:
template?.summary_fields?.webhook_credential || null, template?.summary_fields?.webhook_credential?.id || null,
webhook_service: template.webhook_service || '', webhook_service: template.webhook_service || '',
ask_limit_on_launch: template.ask_limit_on_launch || false, ask_limit_on_launch: template.ask_limit_on_launch || false,
ask_inventory_on_launch: template.ask_inventory_on_launch || false, ask_inventory_on_launch: template.ask_inventory_on_launch || false,
@@ -208,7 +218,7 @@ function WorkflowJobTemplateForm({
label={i18n._(t`Organization`)} label={i18n._(t`Organization`)}
name="organization" name="organization"
> >
{({ form, field }) => ( {({ form }) => (
<OrganizationLookup <OrganizationLookup
helperTextInvalid={form.errors.organization} helperTextInvalid={form.errors.organization}
isValid={ isValid={
@@ -216,25 +226,27 @@ function WorkflowJobTemplateForm({
} }
onBlur={() => form.setFieldTouched('organization')} onBlur={() => form.setFieldTouched('organization')}
onChange={value => { onChange={value => {
form.setFieldValue('organization', value); form.setFieldValue('organization', value.id);
setOrganization(value);
}} }}
value={field.value} value={organization}
touched={form.touched.organization} touched={form.touched.organization}
error={form.errors.organization} error={form.errors.organization}
/> />
)} )}
</Field> </Field>
<Field name="inventory"> <Field name="inventory">
{({ form, field }) => ( {({ form }) => (
<InventoryLookup <InventoryLookup
value={field.value} value={inventory}
tooltip={i18n._( tooltip={i18n._(
t`Select an inventory for the workflow. This inventory is applied to all job template nodes that prompt for an inventory.` 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} isValid={!form.touched.inventory || !form.errors.inventory}
helperTextInvalid={form.errors.inventory} helperTextInvalid={form.errors.inventory}
onChange={value => { onChange={value => {
form.setFieldValue('inventory', value); form.setFieldValue('inventory', value.id);
setInventory(value);
form.setFieldValue('organizationId', value.organization); form.setFieldValue('organizationId', value.organization);
}} }}
error={form.errors.inventory} error={form.errors.inventory}
@@ -426,7 +438,7 @@ function WorkflowJobTemplateForm({
)} )}
{credTypeId && ( {credTypeId && (
<Field name="webhook_credential"> <Field name="webhook_credential">
{({ form, field }) => ( {({ form }) => (
<FormGroup <FormGroup
fieldId="webhook_credential" fieldId="webhook_credential"
id="webhook_credential" id="webhook_credential"
@@ -439,9 +451,10 @@ function WorkflowJobTemplateForm({
)} )}
credentialTypeId={credTypeId || null} credentialTypeId={credTypeId || null}
onChange={value => { onChange={value => {
form.setFieldValue('webhook_credential', value); form.setFieldValue('webhook_credential', value.id);
setWebhookCredential(value);
}} }}
value={field.value} value={webhookCredential}
/> />
</FormGroup> </FormGroup>
)} )}

View File

@@ -9,9 +9,11 @@ import WorkflowJobTemplateForm from './WorkflowJobTemplateForm';
import { WorkflowJobTemplatesAPI } from '../../../api'; import { WorkflowJobTemplatesAPI } from '../../../api';
jest.mock('@api/models/WorkflowJobTemplates'); jest.mock('@api/models/WorkflowJobTemplates');
WorkflowJobTemplatesAPI.updateWebhookKey.mockResolvedValue({ WorkflowJobTemplatesAPI.updateWebhookKey.mockResolvedValue({
data: { webhook_key: 'sdafdghjkl2345678ionbvcxz' }, data: { webhook_key: 'sdafdghjkl2345678ionbvcxz' },
}); });
describe('<WorkflowJobTemplateForm/>', () => { describe('<WorkflowJobTemplateForm/>', () => {
let wrapper; let wrapper;
let history; let history;
@@ -35,6 +37,7 @@ describe('<WorkflowJobTemplateForm/>', () => {
webhook_receiver: '/api/v2/workflow_job_templates/57/gitlab/', webhook_receiver: '/api/v2/workflow_job_templates/57/gitlab/',
}, },
}; };
beforeEach(() => { beforeEach(() => {
history = createMemoryHistory({ history = createMemoryHistory({
initialEntries: ['/templates/workflow_job_template/6/edit'], initialEntries: ['/templates/workflow_job_template/6/edit'],
@@ -66,13 +69,16 @@ describe('<WorkflowJobTemplateForm/>', () => {
); );
}); });
}); });
afterEach(() => { afterEach(() => {
wrapper.unmount(); wrapper.unmount();
jest.clearAllMocks(); jest.clearAllMocks();
}); });
test('renders successfully', () => { test('renders successfully', () => {
expect(wrapper.length).toBe(1); expect(wrapper.length).toBe(1);
}); });
test('all the fields render successfully', () => { test('all the fields render successfully', () => {
const fields = [ const fields = [
'FormField[name="name"]', 'FormField[name="name"]',
@@ -89,6 +95,7 @@ describe('<WorkflowJobTemplateForm/>', () => {
}; };
fields.map((field, index) => assertField(field, index)); fields.map((field, index) => assertField(field, index));
}); });
test('changing inputs should update values', async () => { test('changing inputs should update values', async () => {
const inputsToChange = [ const inputsToChange = [
{ {
@@ -193,6 +200,7 @@ describe('<WorkflowJobTemplateForm/>', () => {
expect(wrapper.find('AnsibleSelect').prop('value')).toBe('gitlab'); expect(wrapper.find('AnsibleSelect').prop('value')).toBe('gitlab');
}); });
test('handleSubmit is called on submit button click', async () => { test('handleSubmit is called on submit button click', async () => {
act(() => { act(() => {
wrapper.find('Formik').prop('onSubmit')({}); wrapper.find('Formik').prop('onSubmit')({});
@@ -201,6 +209,7 @@ describe('<WorkflowJobTemplateForm/>', () => {
sleep(0); sleep(0);
expect(handleSubmit).toBeCalled(); expect(handleSubmit).toBeCalled();
}); });
test('handleCancel is called on cancel button click', async () => { test('handleCancel is called on cancel button click', async () => {
act(() => { act(() => {
wrapper.find('button[aria-label="Cancel"]').simulate('click'); wrapper.find('button[aria-label="Cancel"]').simulate('click');