Fix project lookup autocomplete behavior

* Refactor credential lookup to use useRequest hook
* Update unit tests
This commit is contained in:
Marliana Lara
2020-05-20 12:29:02 -04:00
parent e509bbfbb3
commit ea5e35910f
9 changed files with 141 additions and 99 deletions

View File

@@ -1,4 +1,4 @@
import React, { useEffect, useState, useRef } from 'react'; import React, { useCallback, useEffect } from 'react';
import { bool, func, node, number, string, oneOfType } from 'prop-types'; import { bool, func, node, number, string, oneOfType } from 'prop-types';
import { withRouter } from 'react-router-dom'; import { withRouter } from 'react-router-dom';
import { withI18n } from '@lingui/react'; import { withI18n } from '@lingui/react';
@@ -10,6 +10,7 @@ import { getQSConfig, parseQueryString, mergeParams } from '../../util/qs';
import { FieldTooltip } from '../FormField'; import { FieldTooltip } from '../FormField';
import Lookup from './Lookup'; import Lookup from './Lookup';
import OptionsList from '../OptionsList'; import OptionsList from '../OptionsList';
import useRequest from '../../util/useRequest';
import LookupErrorMessage from './shared/LookupErrorMessage'; import LookupErrorMessage from './shared/LookupErrorMessage';
const QS_CONFIG = getQSConfig('credentials', { const QS_CONFIG = getQSConfig('credentials', {
@@ -32,20 +33,12 @@ function CredentialLookup({
i18n, i18n,
tooltip, tooltip,
}) { }) {
const [credentials, setCredentials] = useState([]); const {
const [count, setCount] = useState(0); result: { count, credentials },
const [error, setError] = useState(null); error,
const isMounted = useRef(null); request: fetchCredentials,
} = useRequest(
useEffect(() => { useCallback(async () => {
isMounted.current = true;
return () => {
isMounted.current = false;
};
}, []);
useEffect(() => {
(async () => {
const params = parseQueryString(QS_CONFIG, history.location.search); const params = parseQueryString(QS_CONFIG, history.location.search);
const typeIdParams = credentialTypeId const typeIdParams = credentialTypeId
? { credential_type: credentialTypeId } ? { credential_type: credentialTypeId }
@@ -54,21 +47,23 @@ function CredentialLookup({
? { credential_type__kind: credentialTypeKind } ? { credential_type__kind: credentialTypeKind }
: {}; : {};
try { const { data } = await CredentialsAPI.read(
const { data } = await CredentialsAPI.read( mergeParams(params, { ...typeIdParams, ...typeKindParams })
mergeParams(params, { ...typeIdParams, ...typeKindParams }) );
); return {
if (isMounted.current) { count: data.count,
setCredentials(data.results); credentials: data.results,
setCount(data.count); };
} }, [credentialTypeId, credentialTypeKind, history.location.search]),
} catch (err) { {
if (isMounted.current) { count: 0,
setError(err); credentials: [],
} }
} );
})();
}, [credentialTypeId, credentialTypeKind, history.location.search]); useEffect(() => {
fetchCredentials();
}, [fetchCredentials]);
// TODO: replace credential type search with REST-based grabbing of cred types // TODO: replace credential type search with REST-based grabbing of cred types

View File

@@ -21,6 +21,7 @@ const QS_CONFIG = getQSConfig('project', {
function ProjectLookup({ function ProjectLookup({
helperTextInvalid, helperTextInvalid,
autocomplete,
i18n, i18n,
isValid, isValid,
onChange, onChange,
@@ -38,14 +39,14 @@ function ProjectLookup({
useCallback(async () => { useCallback(async () => {
const params = parseQueryString(QS_CONFIG, history.location.search); const params = parseQueryString(QS_CONFIG, history.location.search);
const { data } = await ProjectsAPI.read(params); const { data } = await ProjectsAPI.read(params);
if (data.count === 1) { if (data.count === 1 && autocomplete) {
onChange(data.results[0]); autocomplete(data.results[0]);
} }
return { return {
count: data.count, count: data.count,
projects: data.results, projects: data.results,
}; };
}, [onChange, history.location.search]), }, [history.location.search, autocomplete]),
{ {
count: 0, count: 0,
projects: [], projects: [],
@@ -131,22 +132,24 @@ function ProjectLookup({
} }
ProjectLookup.propTypes = { ProjectLookup.propTypes = {
value: Project, autocomplete: func,
helperTextInvalid: node, helperTextInvalid: node,
isValid: bool, isValid: bool,
onBlur: func, onBlur: func,
onChange: func.isRequired, onChange: func.isRequired,
required: bool, required: bool,
tooltip: string, tooltip: string,
value: Project,
}; };
ProjectLookup.defaultProps = { ProjectLookup.defaultProps = {
autocomplete: () => {},
helperTextInvalid: '', helperTextInvalid: '',
isValid: true, isValid: true,
onBlur: () => {},
required: false, required: false,
tooltip: '', tooltip: '',
value: null, value: null,
onBlur: () => {},
}; };
export { ProjectLookup as _ProjectLookup }; export { ProjectLookup as _ProjectLookup };

View File

@@ -15,12 +15,14 @@ describe('<ProjectLookup />', () => {
count: 1, count: 1,
}, },
}); });
const onChange = jest.fn(); const autocomplete = jest.fn();
await act(async () => { await act(async () => {
mountWithContexts(<ProjectLookup onChange={onChange} />); mountWithContexts(
<ProjectLookup autocomplete={autocomplete} onChange={() => {}} />
);
}); });
await sleep(0); await sleep(0);
expect(onChange).toHaveBeenCalledWith({ id: 1 }); expect(autocomplete).toHaveBeenCalledWith({ id: 1 });
}); });
test('should not auto-select project when multiple available', async () => { test('should not auto-select project when multiple available', async () => {
@@ -30,11 +32,13 @@ describe('<ProjectLookup />', () => {
count: 2, count: 2,
}, },
}); });
const onChange = jest.fn(); const autocomplete = jest.fn();
await act(async () => { await act(async () => {
mountWithContexts(<ProjectLookup onChange={onChange} />); mountWithContexts(
<ProjectLookup autocomplete={autocomplete} onChange={() => {}} />
);
}); });
await sleep(0); await sleep(0);
expect(onChange).not.toHaveBeenCalled(); expect(autocomplete).not.toHaveBeenCalled();
}); });
}); });

View File

@@ -144,7 +144,7 @@ const InventorySourceForm = ({
overwrite: source?.overwrite || false, overwrite: source?.overwrite || false,
overwrite_vars: source?.overwrite_vars || false, overwrite_vars: source?.overwrite_vars || false,
source: source?.source || '', source: source?.source || '',
source_path: source?.source_path || '', source_path: source?.source_path === '' ? '/ (project root)' : '',
source_project: source?.summary_fields?.source_project || null, source_project: source?.summary_fields?.source_project || null,
source_vars: source?.source_vars || '---\n', source_vars: source?.source_vars || '---\n',
update_cache_timeout: source?.update_cache_timeout || 0, update_cache_timeout: source?.update_cache_timeout || 0,

View File

@@ -37,10 +37,10 @@ const SCMSubForm = ({ i18n }) => {
); );
useEffect(() => { useEffect(() => {
if (projectField.value?.id) { if (projectMeta.initialValue) {
fetchSourcePath(projectField.value.id); fetchSourcePath(projectMeta.initialValue.id);
} }
}, [fetchSourcePath, projectField.value]); }, [fetchSourcePath, projectMeta.initialValue]);
const handleProjectUpdate = useCallback( const handleProjectUpdate = useCallback(
value => { value => {
@@ -51,6 +51,16 @@ const SCMSubForm = ({ i18n }) => {
[] // eslint-disable-line react-hooks/exhaustive-deps [] // eslint-disable-line react-hooks/exhaustive-deps
); );
const handleProjectAutocomplete = useCallback(
val => {
projectHelpers.setValue(val);
if (!projectMeta.initialValue) {
fetchSourcePath(val.id);
}
},
[] // eslint-disable-line react-hooks/exhaustive-deps
);
return ( return (
<> <>
<CredentialLookup <CredentialLookup
@@ -62,6 +72,7 @@ const SCMSubForm = ({ i18n }) => {
}} }}
/> />
<ProjectLookup <ProjectLookup
autocomplete={handleProjectAutocomplete}
value={projectField.value} value={projectField.value}
isValid={!projectMeta.touched || !projectMeta.error} isValid={!projectMeta.touched || !projectMeta.error}
helperTextInvalid={projectMeta.error} helperTextInvalid={projectMeta.error}

View File

@@ -6,9 +6,25 @@ import {
waitForElement, waitForElement,
} from '../../../../testUtils/enzymeHelpers'; } from '../../../../testUtils/enzymeHelpers';
import JobTemplateAdd from './JobTemplateAdd'; import JobTemplateAdd from './JobTemplateAdd';
import { JobTemplatesAPI, LabelsAPI } from '../../../api'; import {
CredentialsAPI,
CredentialTypesAPI,
JobTemplatesAPI,
LabelsAPI,
ProjectsAPI,
} from '../../../api';
jest.mock('../../../api'); jest.mock('../../../api');
CredentialsAPI.read.mockResolvedValue({
data: {
results: [],
count: 0,
},
});
CredentialTypesAPI.loadAllTypes.mockResolvedValue([]);
ProjectsAPI.readPlaybooks.mockResolvedValue({
data: [],
});
const jobTemplateData = { const jobTemplateData = {
allow_callbacks: false, allow_callbacks: false,

View File

@@ -6,7 +6,13 @@ import {
mountWithContexts, mountWithContexts,
waitForElement, waitForElement,
} from '../../../../testUtils/enzymeHelpers'; } from '../../../../testUtils/enzymeHelpers';
import { JobTemplatesAPI, LabelsAPI, ProjectsAPI } from '../../../api'; import {
CredentialsAPI,
CredentialTypesAPI,
JobTemplatesAPI,
LabelsAPI,
ProjectsAPI,
} from '../../../api';
import JobTemplateEdit from './JobTemplateEdit'; import JobTemplateEdit from './JobTemplateEdit';
jest.mock('../../../api'); jest.mock('../../../api');
@@ -60,7 +66,7 @@ const mockJobTemplate = {
{ id: 2, kind: 'ssh', name: 'Bar' }, { id: 2, kind: 'ssh', name: 'Bar' },
], ],
project: { project: {
id: 15, id: 3,
name: 'Boo', name: 'Boo',
}, },
}, },
@@ -176,6 +182,13 @@ ProjectsAPI.readPlaybooks.mockResolvedValue({
data: mockRelatedProjectPlaybooks, data: mockRelatedProjectPlaybooks,
}); });
LabelsAPI.read.mockResolvedValue({ data: { results: [] } }); LabelsAPI.read.mockResolvedValue({ data: { results: [] } });
CredentialsAPI.read.mockResolvedValue({
data: {
results: [],
count: 0,
},
});
CredentialTypesAPI.loadAllTypes.mockResolvedValue([]);
describe('<JobTemplateEdit />', () => { describe('<JobTemplateEdit />', () => {
beforeEach(() => { beforeEach(() => {
@@ -251,7 +264,7 @@ describe('<JobTemplateEdit />', () => {
const expected = { const expected = {
...mockJobTemplate, ...mockJobTemplate,
project: mockJobTemplate.project.id, project: mockJobTemplate.project,
...updatedTemplateData, ...updatedTemplateData,
}; };
delete expected.summary_fields; delete expected.summary_fields;

View File

@@ -46,27 +46,25 @@ const { origin } = document.location;
function JobTemplateForm({ function JobTemplateForm({
template, template,
validateField,
handleCancel, handleCancel,
handleSubmit, handleSubmit,
setFieldValue, setFieldValue,
submitError, submitError,
i18n, i18n,
}) { }) {
const { values: formikValues } = useFormikContext();
const [contentError, setContentError] = useState(false); const [contentError, setContentError] = useState(false);
const [project, setProject] = useState(template?.summary_fields?.project);
const [inventory, setInventory] = useState( const [inventory, setInventory] = useState(
template?.summary_fields?.inventory template?.summary_fields?.inventory
); );
const [allowCallbacks, setAllowCallbacks] = useState( const [allowCallbacks, setAllowCallbacks] = useState(
Boolean(template?.host_config_key) Boolean(template?.host_config_key)
); );
const [enableWebhooks, setEnableWebhooks] = useState( const [enableWebhooks, setEnableWebhooks] = useState(
Boolean(template.webhook_service) Boolean(template.webhook_service)
); );
const { values: formikValues } = useFormikContext();
const [jobTypeField, jobTypeMeta, jobTypeHelpers] = useField({ const [jobTypeField, jobTypeMeta, jobTypeHelpers] = useField({
name: 'job_type', name: 'job_type',
validate: required(null, i18n), validate: required(null, i18n),
@@ -74,16 +72,13 @@ function JobTemplateForm({
const [, inventoryMeta, inventoryHelpers] = useField('inventory'); const [, inventoryMeta, inventoryHelpers] = useField('inventory');
const [projectField, projectMeta, projectHelpers] = useField({ const [projectField, projectMeta, projectHelpers] = useField({
name: 'project', name: 'project',
validate: () => handleProjectValidation(), validate: project => handleProjectValidation(project),
}); });
const [scmField, , scmHelpers] = useField('scm_branch'); const [scmField, , scmHelpers] = useField('scm_branch');
const [playbookField, playbookMeta, playbookHelpers] = useField({ const [playbookField, playbookMeta, playbookHelpers] = useField({
name: 'playbook', name: 'playbook',
validate: required(i18n._(t`Select a value for this field`), i18n), validate: required(i18n._(t`Select a value for this field`), i18n),
}); });
const [credentialField, , credentialHelpers] = useField('credentials'); const [credentialField, , credentialHelpers] = useField('credentials');
const [labelsField, , labelsHelpers] = useField('labels'); const [labelsField, , labelsHelpers] = useField('labels');
const [limitField, limitMeta] = useField('limit'); const [limitField, limitMeta] = useField('limit');
@@ -101,13 +96,10 @@ function JobTemplateForm({
contentLoading: hasProjectLoading, contentLoading: hasProjectLoading,
} = useRequest( } = useRequest(
useCallback(async () => { useCallback(async () => {
let projectData;
if (template?.project) { if (template?.project) {
projectData = await ProjectsAPI.readDetail(template?.project); await ProjectsAPI.readDetail(template?.project);
validateField('project');
setProject(projectData.data);
} }
}, [template, validateField]) }, [template])
); );
const { const {
@@ -133,26 +125,28 @@ function JobTemplateForm({
loadRelatedInstanceGroups(); loadRelatedInstanceGroups();
}, [loadRelatedInstanceGroups]); }, [loadRelatedInstanceGroups]);
const handleProjectValidation = () => { const handleProjectValidation = project => {
if (!project && projectMeta.touched) { if (!project && projectMeta.touched) {
return i18n._(t`Select a value for this field`); return i18n._(t`Select a value for this field`);
} }
if (project && project.status === 'never updated') { if (project?.value?.status === 'never updated') {
return i18n._(t`This project needs to be updated`); return i18n._(t`This project needs to be updated`);
} }
return undefined; return undefined;
}; };
const handleProjectUpdate = useCallback( const handleProjectUpdate = useCallback(
newProject => { value => {
if (project?.id !== newProject?.id) { playbookHelpers.setValue(0);
// Clear the selected playbook value when a different project is selected or
// when the project is deselected.
playbookHelpers.setValue(0);
}
setProject(newProject);
projectHelpers.setValue(newProject);
scmHelpers.setValue(''); scmHelpers.setValue('');
projectHelpers.setValue(value);
},
[] // eslint-disable-line react-hooks/exhaustive-deps
);
const handleProjectAutocomplete = useCallback(
val => {
projectHelpers.setValue(val);
}, },
[] // eslint-disable-line react-hooks/exhaustive-deps [] // eslint-disable-line react-hooks/exhaustive-deps
); );
@@ -189,8 +183,12 @@ function JobTemplateForm({
return <ContentLoading />; return <ContentLoading />;
} }
if (instanceGroupError || projectContentError) { if (contentError || instanceGroupError || projectContentError) {
return <ContentError error={contentError} />; return (
<ContentError
error={contentError || instanceGroupError || projectContentError}
/>
);
} }
return ( return (
@@ -261,18 +259,18 @@ function JobTemplateForm({
</div> </div>
)} )}
</FieldWithPrompt> </FieldWithPrompt>
<ProjectLookup <ProjectLookup
value={project} value={projectField.value}
onBlur={() => projectHelpers.setTouched()} onBlur={() => projectHelpers.setTouched()}
tooltip={i18n._(t`Select the project containing the playbook tooltip={i18n._(t`Select the project containing the playbook
you want this job to execute.`)} you want this job to execute.`)}
isValid={!projectMeta.touched || !projectMeta.error} isValid={!projectMeta.touched || !projectMeta.error}
helperTextInvalid={projectMeta.error} helperTextInvalid={projectMeta.error}
onChange={handleProjectUpdate} onChange={handleProjectUpdate}
autocomplete={handleProjectAutocomplete}
required required
/> />
{project?.allow_override && ( {projectField.value?.allow_override && (
<FieldWithPrompt <FieldWithPrompt
fieldId="template-scm-branch" fieldId="template-scm-branch"
label={i18n._(t`Source Control Branch`)} label={i18n._(t`Source Control Branch`)}
@@ -299,7 +297,7 @@ function JobTemplateForm({
content={i18n._(t`Select the playbook to be executed by this job.`)} content={i18n._(t`Select the playbook to be executed by this job.`)}
/> />
<PlaybookSelect <PlaybookSelect
projectId={project?.id || projectField.value?.id} projectId={projectField.value?.id}
isValid={!playbookMeta.touched || !playbookMeta.error} isValid={!playbookMeta.touched || !playbookMeta.error}
field={playbookField} field={playbookField}
onBlur={() => playbookHelpers.setTouched()} onBlur={() => playbookHelpers.setTouched()}
@@ -609,6 +607,8 @@ const FormikApp = withFormik({
} = template; } = template;
return { return {
allow_callbacks: template.allow_callbacks || false,
allow_simultaneous: template.allow_simultaneous || false,
ask_credential_on_launch: template.ask_credential_on_launch || false, ask_credential_on_launch: template.ask_credential_on_launch || false,
ask_diff_mode_on_launch: template.ask_diff_mode_on_launch || false, ask_diff_mode_on_launch: template.ask_diff_mode_on_launch || false,
ask_inventory_on_launch: template.ask_inventory_on_launch || false, ask_inventory_on_launch: template.ask_inventory_on_launch || false,
@@ -619,31 +619,29 @@ const FormikApp = withFormik({
ask_tags_on_launch: template.ask_tags_on_launch || false, ask_tags_on_launch: template.ask_tags_on_launch || false,
ask_variables_on_launch: template.ask_variables_on_launch || false, ask_variables_on_launch: template.ask_variables_on_launch || false,
ask_verbosity_on_launch: template.ask_verbosity_on_launch || false, ask_verbosity_on_launch: template.ask_verbosity_on_launch || false,
name: template.name || '',
description: template.description || '',
job_type: template.job_type || 'run',
inventory: template.inventory || null,
project: template.project || null,
scm_branch: template.scm_branch || '',
playbook: template.playbook || '',
labels: summary_fields.labels.results || [],
forks: template.forks || 0,
limit: template.limit || '',
verbosity: template.verbosity || '0',
job_slice_count: template.job_slice_count || 1,
timeout: template.timeout || 0,
diff_mode: template.diff_mode || false,
job_tags: template.job_tags || '',
skip_tags: template.skip_tags || '',
become_enabled: template.become_enabled || false, become_enabled: template.become_enabled || false,
allow_callbacks: template.allow_callbacks || false, credentials: summary_fields.credentials || [],
allow_simultaneous: template.allow_simultaneous || false, description: template.description || '',
use_fact_cache: template.use_fact_cache || false, diff_mode: template.diff_mode || false,
extra_vars: template.extra_vars || '---\n',
forks: template.forks || 0,
host_config_key: template.host_config_key || '', host_config_key: template.host_config_key || '',
initialInstanceGroups: [], initialInstanceGroups: [],
instanceGroups: [], instanceGroups: [],
credentials: summary_fields.credentials || [], inventory: template.inventory || null,
extra_vars: template.extra_vars || '---\n', job_slice_count: template.job_slice_count || 1,
job_tags: template.job_tags || '',
job_type: template.job_type || 'run',
labels: summary_fields.labels.results || [],
limit: template.limit || '',
name: template.name || '',
playbook: template.playbook || '',
project: summary_fields?.project || null,
scm_branch: template.scm_branch || '',
skip_tags: template.skip_tags || '',
timeout: template.timeout || 0,
use_fact_cache: template.use_fact_cache || false,
verbosity: template.verbosity || '0',
webhook_service: template.webhook_service || '', webhook_service: template.webhook_service || '',
webhook_url: template?.related?.webhook_receiver webhook_url: template?.related?.webhook_receiver
? `${origin}${template.related.webhook_receiver}` ? `${origin}${template.related.webhook_receiver}`

View File

@@ -13,6 +13,7 @@ import {
JobTemplatesAPI, JobTemplatesAPI,
ProjectsAPI, ProjectsAPI,
CredentialsAPI, CredentialsAPI,
CredentialTypesAPI,
} from '../../../api'; } from '../../../api';
jest.mock('../../../api'); jest.mock('../../../api');
@@ -99,6 +100,7 @@ describe('<JobTemplateForm />', () => {
LabelsAPI.read.mockReturnValue({ LabelsAPI.read.mockReturnValue({
data: mockData.summary_fields.labels, data: mockData.summary_fields.labels,
}); });
CredentialTypesAPI.loadAllTypes.mockResolvedValue([]);
CredentialsAPI.read.mockReturnValue({ CredentialsAPI.read.mockReturnValue({
data: { results: mockCredentials }, data: { results: mockCredentials },
}); });