reduce duplicate network request in JT form

This commit is contained in:
Keith J. Grant
2021-06-18 10:11:40 -07:00
committed by Shane McDonald
parent 2afa406b7f
commit 0be68fe84f
8 changed files with 44 additions and 22 deletions

View File

@@ -41,7 +41,7 @@ function ExecutionEnvironmentLookup({
const { const {
request: fetchProject, request: fetchProject,
error: fetchProjectError, error: fetchProjectError,
isLoading: fetchProjectLoading, isLoading: isProjectLoading,
result: project, result: project,
} = useRequest( } = useRequest(
useCallback(async () => { useCallback(async () => {
@@ -53,6 +53,7 @@ function ExecutionEnvironmentLookup({
}, [projectId]), }, [projectId]),
{ {
project: null, project: null,
isLoading: true,
} }
); );
@@ -72,6 +73,12 @@ function ExecutionEnvironmentLookup({
isLoading, isLoading,
} = useRequest( } = useRequest(
useCallback(async () => { useCallback(async () => {
if (isProjectLoading) {
return {
executionEnvironments: [],
count: 0,
};
}
const params = parseQueryString(QS_CONFIG, location.search); const params = parseQueryString(QS_CONFIG, location.search);
const globallyAvailableParams = globallyAvailable const globallyAvailableParams = globallyAvailable
? { or__organization__isnull: 'True' } ? { or__organization__isnull: 'True' }
@@ -105,7 +112,14 @@ function ExecutionEnvironmentLookup({
actionsResponse.data.actions?.GET || {} actionsResponse.data.actions?.GET || {}
).filter(key => actionsResponse.data.actions?.GET[key].filterable), ).filter(key => actionsResponse.data.actions?.GET[key].filterable),
}; };
}, [location, globallyAvailable, organizationId, projectId, project]), }, [
location,
globallyAvailable,
organizationId,
projectId,
project,
isProjectLoading,
]),
{ {
executionEnvironments: [], executionEnvironments: [],
count: 0, count: 0,
@@ -149,7 +163,7 @@ function ExecutionEnvironmentLookup({
fieldName={fieldName} fieldName={fieldName}
validate={validate} validate={validate}
qsConfig={QS_CONFIG} qsConfig={QS_CONFIG}
isLoading={isLoading || fetchProjectLoading} isLoading={isLoading || isProjectLoading}
isDisabled={isDisabled} isDisabled={isDisabled}
renderOptionsList={({ state, dispatch, canDelete }) => ( renderOptionsList={({ state, dispatch, canDelete }) => (
<OptionsList <OptionsList

View File

@@ -30,9 +30,9 @@ describe('ExecutionEnvironmentLookup', () => {
let wrapper; let wrapper;
beforeEach(() => { beforeEach(() => {
ExecutionEnvironmentsAPI.read.mockResolvedValue( ExecutionEnvironmentsAPI.read.mockResolvedValue({
mockedExecutionEnvironments data: mockedExecutionEnvironments,
); });
ProjectsAPI.readDetail.mockResolvedValue({ data: { organization: 39 } }); ProjectsAPI.readDetail.mockResolvedValue({ data: { organization: 39 } });
}); });
@@ -63,7 +63,7 @@ describe('ExecutionEnvironmentLookup', () => {
); );
}); });
wrapper.update(); wrapper.update();
expect(ExecutionEnvironmentsAPI.read).toHaveBeenCalledTimes(2); expect(ExecutionEnvironmentsAPI.read).toHaveBeenCalledTimes(1);
expect(wrapper.find('ExecutionEnvironmentLookup')).toHaveLength(1); expect(wrapper.find('ExecutionEnvironmentLookup')).toHaveLength(1);
expect( expect(
wrapper.find('FormGroup[label="Default Execution Environment"]').length wrapper.find('FormGroup[label="Default Execution Environment"]').length
@@ -84,7 +84,7 @@ describe('ExecutionEnvironmentLookup', () => {
</Formik> </Formik>
); );
}); });
expect(ExecutionEnvironmentsAPI.read).toHaveBeenCalledTimes(2); expect(ExecutionEnvironmentsAPI.read).toHaveBeenCalledTimes(1);
expect( expect(
wrapper.find('FormGroup[label="Default Execution Environment"]').length wrapper.find('FormGroup[label="Default Execution Environment"]').length
).toBe(0); ).toBe(0);

View File

@@ -109,8 +109,10 @@ describe('<JobTemplateAdd />', () => {
let wrapper; let wrapper;
await act(async () => { await act(async () => {
wrapper = mountWithContexts(<JobTemplateAdd />); wrapper = mountWithContexts(<JobTemplateAdd />);
await waitForElement(wrapper, 'EmptyStateBody', el => el.length === 0); // await waitForElement(wrapper, 'EmptyStateBody', el => el.length === 0);
}); });
wrapper.update();
expect(wrapper.find('input#template-description').text()).toBe( expect(wrapper.find('input#template-description').text()).toBe(
defaultProps.description defaultProps.description
); );

View File

@@ -18,11 +18,7 @@ function JobTemplateEdit({ template }) {
const detailsUrl = `/templates/${template.type}/${template.id}/details`; const detailsUrl = `/templates/${template.type}/${template.id}/details`;
const { const { request: fetchProject, error: fetchProjectError } = useRequest(
request: fetchProject,
error: fetchProjectError,
isLoading: projectLoading,
} = useRequest(
useCallback(async () => { useCallback(async () => {
await ProjectsAPI.readDetail(template.project); await ProjectsAPI.readDetail(template.project);
}, [template.project]) }, [template.project])
@@ -130,7 +126,7 @@ function JobTemplateEdit({ template }) {
if (!canEdit) { if (!canEdit) {
return <Redirect to={detailsUrl} />; return <Redirect to={detailsUrl} />;
} }
if (isLoading || projectLoading) { if (isLoading) {
return <ContentLoading />; return <ContentLoading />;
} }

View File

@@ -287,8 +287,8 @@ describe('<JobTemplateEdit />', () => {
wrapper = mountWithContexts( wrapper = mountWithContexts(
<JobTemplateEdit template={mockJobTemplate} /> <JobTemplateEdit template={mockJobTemplate} />
); );
await waitForElement(wrapper, 'EmptyStateBody', el => el.length === 0);
}); });
wrapper.update();
expect(wrapper.find('FormGroup[label="Host Config Key"]').length).toBe(1); expect(wrapper.find('FormGroup[label="Host Config Key"]').length).toBe(1);
expect( expect(
wrapper.find('FormGroup[label="Host Config Key"]').prop('isRequired') wrapper.find('FormGroup[label="Host Config Key"]').prop('isRequired')
@@ -301,8 +301,9 @@ describe('<JobTemplateEdit />', () => {
wrapper = mountWithContexts( wrapper = mountWithContexts(
<JobTemplateEdit template={mockJobTemplate} /> <JobTemplateEdit template={mockJobTemplate} />
); );
await waitForElement(wrapper, 'EmptyStateBody', el => el.length === 0);
}); });
wrapper.update();
const updatedTemplateData = { const updatedTemplateData = {
job_type: 'check', job_type: 'check',
name: 'new name', name: 'new name',

View File

@@ -56,7 +56,7 @@ function JobTemplateForm({
setFieldTouched, setFieldTouched,
submitError, submitError,
validateField, validateField,
isOverrideDisabledLookup, isOverrideDisabledLookup, // TODO: this is a confusing variable name
}) { }) {
const [contentError, setContentError] = useState(false); const [contentError, setContentError] = useState(false);
const [allowCallbacks, setAllowCallbacks] = useState( const [allowCallbacks, setAllowCallbacks] = useState(
@@ -123,7 +123,10 @@ function JobTemplateForm({
setFieldValue('instanceGroups', [...data.results]); setFieldValue('instanceGroups', [...data.results]);
} }
// eslint-disable-next-line react-hooks/exhaustive-deps // eslint-disable-next-line react-hooks/exhaustive-deps
}, [setFieldValue, template]) }, [setFieldValue, template]),
{
isLoading: true,
}
); );
useEffect(() => { useEffect(() => {

View File

@@ -185,7 +185,7 @@ describe('<JobTemplateForm />', () => {
test('should update form values on input changes', async () => { test('should update form values on input changes', async () => {
let wrapper; let wrapper;
await act(async () => { await act(() => {
wrapper = mountWithContexts( wrapper = mountWithContexts(
<JobTemplateForm <JobTemplateForm
template={mockData} template={mockData}
@@ -193,8 +193,9 @@ describe('<JobTemplateForm />', () => {
handleCancel={jest.fn()} handleCancel={jest.fn()}
/> />
); );
await waitForElement(wrapper, 'EmptyStateBody', el => el.length === 0);
}); });
wrapper.update();
await act(async () => { await act(async () => {
wrapper.find('input#template-name').simulate('change', { wrapper.find('input#template-name').simulate('change', {
target: { value: 'new foo', name: 'name' }, target: { value: 'new foo', name: 'name' },
@@ -308,6 +309,8 @@ describe('<JobTemplateForm />', () => {
} }
); );
}); });
wrapper.update();
act(() => { act(() => {
wrapper.find('Checkbox[aria-label="Enable Webhook"]').invoke('onChange')( wrapper.find('Checkbox[aria-label="Enable Webhook"]').invoke('onChange')(
true, true,
@@ -392,6 +395,8 @@ describe('<JobTemplateForm />', () => {
} }
); );
}); });
wrapper.update();
expect( expect(
wrapper.find('TextInputBase#template-webhook_key').prop('value') wrapper.find('TextInputBase#template-webhook_key').prop('value')
).toBe('A NEW WEBHOOK KEY WILL BE GENERATED ON SAVE.'); ).toBe('A NEW WEBHOOK KEY WILL BE GENERATED ON SAVE.');
@@ -399,6 +404,7 @@ describe('<JobTemplateForm />', () => {
wrapper.find('Button[aria-label="Update webhook key"]').prop('isDisabled') wrapper.find('Button[aria-label="Update webhook key"]').prop('isDisabled')
).toBe(true); ).toBe(true);
}); });
test('should call handleSubmit when Submit button is clicked', async () => { test('should call handleSubmit when Submit button is clicked', async () => {
const handleSubmit = jest.fn(); const handleSubmit = jest.fn();
let wrapper; let wrapper;

View File

@@ -18,7 +18,7 @@ import useIsMounted from './useIsMounted';
export default function useRequest(makeRequest, initialValue) { export default function useRequest(makeRequest, initialValue) {
const [result, setResult] = useState(initialValue); const [result, setResult] = useState(initialValue);
const [error, setError] = useState(null); const [error, setError] = useState(null);
const [isLoading, setIsLoading] = useState(false); const [isLoading, setIsLoading] = useState(initialValue?.isLoading || false);
const isMounted = useIsMounted(); const isMounted = useIsMounted();
return { return {