From 0be68fe84fba18b7ee0cf6488fcad0f7cb94c599 Mon Sep 17 00:00:00 2001 From: "Keith J. Grant" Date: Fri, 18 Jun 2021 10:11:40 -0700 Subject: [PATCH] reduce duplicate network request in JT form --- .../Lookup/ExecutionEnvironmentLookup.jsx | 20 ++++++++++++++++--- .../ExecutionEnvironmentLookup.test.jsx | 10 +++++----- .../JobTemplateAdd/JobTemplateAdd.test.jsx | 4 +++- .../JobTemplateEdit/JobTemplateEdit.jsx | 8 ++------ .../JobTemplateEdit/JobTemplateEdit.test.jsx | 5 +++-- .../Template/shared/JobTemplateForm.jsx | 7 +++++-- .../Template/shared/JobTemplateForm.test.jsx | 10 ++++++++-- awx/ui_next/src/util/useRequest.js | 2 +- 8 files changed, 44 insertions(+), 22 deletions(-) diff --git a/awx/ui_next/src/components/Lookup/ExecutionEnvironmentLookup.jsx b/awx/ui_next/src/components/Lookup/ExecutionEnvironmentLookup.jsx index 2ca14dae42..88af22c807 100644 --- a/awx/ui_next/src/components/Lookup/ExecutionEnvironmentLookup.jsx +++ b/awx/ui_next/src/components/Lookup/ExecutionEnvironmentLookup.jsx @@ -41,7 +41,7 @@ function ExecutionEnvironmentLookup({ const { request: fetchProject, error: fetchProjectError, - isLoading: fetchProjectLoading, + isLoading: isProjectLoading, result: project, } = useRequest( useCallback(async () => { @@ -53,6 +53,7 @@ function ExecutionEnvironmentLookup({ }, [projectId]), { project: null, + isLoading: true, } ); @@ -72,6 +73,12 @@ function ExecutionEnvironmentLookup({ isLoading, } = useRequest( useCallback(async () => { + if (isProjectLoading) { + return { + executionEnvironments: [], + count: 0, + }; + } const params = parseQueryString(QS_CONFIG, location.search); const globallyAvailableParams = globallyAvailable ? { or__organization__isnull: 'True' } @@ -105,7 +112,14 @@ function ExecutionEnvironmentLookup({ actionsResponse.data.actions?.GET || {} ).filter(key => actionsResponse.data.actions?.GET[key].filterable), }; - }, [location, globallyAvailable, organizationId, projectId, project]), + }, [ + location, + globallyAvailable, + organizationId, + projectId, + project, + isProjectLoading, + ]), { executionEnvironments: [], count: 0, @@ -149,7 +163,7 @@ function ExecutionEnvironmentLookup({ fieldName={fieldName} validate={validate} qsConfig={QS_CONFIG} - isLoading={isLoading || fetchProjectLoading} + isLoading={isLoading || isProjectLoading} isDisabled={isDisabled} renderOptionsList={({ state, dispatch, canDelete }) => ( { let wrapper; beforeEach(() => { - ExecutionEnvironmentsAPI.read.mockResolvedValue( - mockedExecutionEnvironments - ); + ExecutionEnvironmentsAPI.read.mockResolvedValue({ + data: mockedExecutionEnvironments, + }); ProjectsAPI.readDetail.mockResolvedValue({ data: { organization: 39 } }); }); @@ -63,7 +63,7 @@ describe('ExecutionEnvironmentLookup', () => { ); }); wrapper.update(); - expect(ExecutionEnvironmentsAPI.read).toHaveBeenCalledTimes(2); + expect(ExecutionEnvironmentsAPI.read).toHaveBeenCalledTimes(1); expect(wrapper.find('ExecutionEnvironmentLookup')).toHaveLength(1); expect( wrapper.find('FormGroup[label="Default Execution Environment"]').length @@ -84,7 +84,7 @@ describe('ExecutionEnvironmentLookup', () => { ); }); - expect(ExecutionEnvironmentsAPI.read).toHaveBeenCalledTimes(2); + expect(ExecutionEnvironmentsAPI.read).toHaveBeenCalledTimes(1); expect( wrapper.find('FormGroup[label="Default Execution Environment"]').length ).toBe(0); diff --git a/awx/ui_next/src/screens/Template/JobTemplateAdd/JobTemplateAdd.test.jsx b/awx/ui_next/src/screens/Template/JobTemplateAdd/JobTemplateAdd.test.jsx index 517194a8c3..81fd95a5ff 100644 --- a/awx/ui_next/src/screens/Template/JobTemplateAdd/JobTemplateAdd.test.jsx +++ b/awx/ui_next/src/screens/Template/JobTemplateAdd/JobTemplateAdd.test.jsx @@ -109,8 +109,10 @@ describe('', () => { let wrapper; await act(async () => { wrapper = mountWithContexts(); - 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( defaultProps.description ); diff --git a/awx/ui_next/src/screens/Template/JobTemplateEdit/JobTemplateEdit.jsx b/awx/ui_next/src/screens/Template/JobTemplateEdit/JobTemplateEdit.jsx index 0882a1519d..d4e076fe87 100644 --- a/awx/ui_next/src/screens/Template/JobTemplateEdit/JobTemplateEdit.jsx +++ b/awx/ui_next/src/screens/Template/JobTemplateEdit/JobTemplateEdit.jsx @@ -18,11 +18,7 @@ function JobTemplateEdit({ template }) { const detailsUrl = `/templates/${template.type}/${template.id}/details`; - const { - request: fetchProject, - error: fetchProjectError, - isLoading: projectLoading, - } = useRequest( + const { request: fetchProject, error: fetchProjectError } = useRequest( useCallback(async () => { await ProjectsAPI.readDetail(template.project); }, [template.project]) @@ -130,7 +126,7 @@ function JobTemplateEdit({ template }) { if (!canEdit) { return ; } - if (isLoading || projectLoading) { + if (isLoading) { return ; } diff --git a/awx/ui_next/src/screens/Template/JobTemplateEdit/JobTemplateEdit.test.jsx b/awx/ui_next/src/screens/Template/JobTemplateEdit/JobTemplateEdit.test.jsx index 7eb653e225..fd7683d902 100644 --- a/awx/ui_next/src/screens/Template/JobTemplateEdit/JobTemplateEdit.test.jsx +++ b/awx/ui_next/src/screens/Template/JobTemplateEdit/JobTemplateEdit.test.jsx @@ -287,8 +287,8 @@ describe('', () => { wrapper = mountWithContexts( ); - 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"]').prop('isRequired') @@ -301,8 +301,9 @@ describe('', () => { wrapper = mountWithContexts( ); - await waitForElement(wrapper, 'EmptyStateBody', el => el.length === 0); }); + wrapper.update(); + const updatedTemplateData = { job_type: 'check', name: 'new name', diff --git a/awx/ui_next/src/screens/Template/shared/JobTemplateForm.jsx b/awx/ui_next/src/screens/Template/shared/JobTemplateForm.jsx index 856dc6516d..cdf92d0240 100644 --- a/awx/ui_next/src/screens/Template/shared/JobTemplateForm.jsx +++ b/awx/ui_next/src/screens/Template/shared/JobTemplateForm.jsx @@ -56,7 +56,7 @@ function JobTemplateForm({ setFieldTouched, submitError, validateField, - isOverrideDisabledLookup, + isOverrideDisabledLookup, // TODO: this is a confusing variable name }) { const [contentError, setContentError] = useState(false); const [allowCallbacks, setAllowCallbacks] = useState( @@ -123,7 +123,10 @@ function JobTemplateForm({ setFieldValue('instanceGroups', [...data.results]); } // eslint-disable-next-line react-hooks/exhaustive-deps - }, [setFieldValue, template]) + }, [setFieldValue, template]), + { + isLoading: true, + } ); useEffect(() => { diff --git a/awx/ui_next/src/screens/Template/shared/JobTemplateForm.test.jsx b/awx/ui_next/src/screens/Template/shared/JobTemplateForm.test.jsx index 81d80ddd1d..341c433b4f 100644 --- a/awx/ui_next/src/screens/Template/shared/JobTemplateForm.test.jsx +++ b/awx/ui_next/src/screens/Template/shared/JobTemplateForm.test.jsx @@ -185,7 +185,7 @@ describe('', () => { test('should update form values on input changes', async () => { let wrapper; - await act(async () => { + await act(() => { wrapper = mountWithContexts( ', () => { handleCancel={jest.fn()} /> ); - await waitForElement(wrapper, 'EmptyStateBody', el => el.length === 0); }); + wrapper.update(); + await act(async () => { wrapper.find('input#template-name').simulate('change', { target: { value: 'new foo', name: 'name' }, @@ -308,6 +309,8 @@ describe('', () => { } ); }); + wrapper.update(); + act(() => { wrapper.find('Checkbox[aria-label="Enable Webhook"]').invoke('onChange')( true, @@ -392,6 +395,8 @@ describe('', () => { } ); }); + wrapper.update(); + expect( wrapper.find('TextInputBase#template-webhook_key').prop('value') ).toBe('A NEW WEBHOOK KEY WILL BE GENERATED ON SAVE.'); @@ -399,6 +404,7 @@ describe('', () => { wrapper.find('Button[aria-label="Update webhook key"]').prop('isDisabled') ).toBe(true); }); + test('should call handleSubmit when Submit button is clicked', async () => { const handleSubmit = jest.fn(); let wrapper; diff --git a/awx/ui_next/src/util/useRequest.js b/awx/ui_next/src/util/useRequest.js index 54ffb9d6dd..22809f5b78 100644 --- a/awx/ui_next/src/util/useRequest.js +++ b/awx/ui_next/src/util/useRequest.js @@ -18,7 +18,7 @@ import useIsMounted from './useIsMounted'; export default function useRequest(makeRequest, initialValue) { const [result, setResult] = useState(initialValue); const [error, setError] = useState(null); - const [isLoading, setIsLoading] = useState(false); + const [isLoading, setIsLoading] = useState(initialValue?.isLoading || false); const isMounted = useIsMounted(); return {