From b8369defd640ed9fdff13cf6f85ba42c185c9327 Mon Sep 17 00:00:00 2001 From: nixocio Date: Thu, 19 Nov 2020 13:13:33 -0500 Subject: [PATCH] Fix Inventory/Project rbac broken on JT form Fix Inventory/Project rbac broken on JT form. Also, update ProjectLookup to filter using `role_level: 'use_role'` as per old UI implementation. Also, update InventoryLookup to filter using `role_level: 'use_role'` as per old UI implementation. See: https://github.com/ansible/awx/issues/8194 --- .../AdHocCommands/AdHocCommands.jsx | 2 +- .../src/components/Lookup/InventoryLookup.jsx | 8 +- .../Lookup/InventoryLookup.test.jsx | 87 +++++++++++++++++++ .../src/components/Lookup/ProjectLookup.jsx | 8 +- .../components/Lookup/ProjectLookup.test.jsx | 46 ++++++++++ .../JobTemplateAdd/JobTemplateAdd.jsx | 1 + .../JobTemplateEdit/JobTemplateEdit.jsx | 37 ++++++-- .../Template/shared/JobTemplateForm.jsx | 6 ++ 8 files changed, 187 insertions(+), 8 deletions(-) create mode 100644 awx/ui_next/src/components/Lookup/InventoryLookup.test.jsx diff --git a/awx/ui_next/src/components/AdHocCommands/AdHocCommands.jsx b/awx/ui_next/src/components/AdHocCommands/AdHocCommands.jsx index 48fc566e2c..89387b9e8b 100644 --- a/awx/ui_next/src/components/AdHocCommands/AdHocCommands.jsx +++ b/awx/ui_next/src/components/AdHocCommands/AdHocCommands.jsx @@ -57,7 +57,7 @@ function AdHocCommands({ adHocItems, i18n, hasListItems }) { fetchData(); }, [fetchData]); const { - isloading: isLaunchLoading, + isLoading: isLaunchLoading, error: launchError, request: launchAdHocCommands, } = useRequest( diff --git a/awx/ui_next/src/components/Lookup/InventoryLookup.jsx b/awx/ui_next/src/components/Lookup/InventoryLookup.jsx index 9c42d521de..f55d669880 100644 --- a/awx/ui_next/src/components/Lookup/InventoryLookup.jsx +++ b/awx/ui_next/src/components/Lookup/InventoryLookup.jsx @@ -16,6 +16,7 @@ const QS_CONFIG = getQSConfig('inventory', { page: 1, page_size: 5, order_by: 'name', + role_level: 'use_role', }); function InventoryLookup({ @@ -29,6 +30,7 @@ function InventoryLookup({ fieldId, promptId, promptName, + isOverrideDisabled, }) { const { result: { @@ -57,8 +59,10 @@ function InventoryLookup({ searchableKeys: Object.keys( actionsResponse.data.actions?.GET || {} ).filter(key => actionsResponse.data.actions?.GET[key].filterable), - canEdit: Boolean(actionsResponse.data.actions.POST), + canEdit: + Boolean(actionsResponse.data.actions.POST) || isOverrideDisabled, }; + // eslint-disable-next-line react-hooks/exhaustive-deps }, [history.location]), { inventories: [], @@ -195,11 +199,13 @@ InventoryLookup.propTypes = { value: Inventory, onChange: func.isRequired, required: bool, + isOverrideDisabled: bool, }; InventoryLookup.defaultProps = { value: null, required: false, + isOverrideDisabled: false, }; export default withI18n()(withRouter(InventoryLookup)); diff --git a/awx/ui_next/src/components/Lookup/InventoryLookup.test.jsx b/awx/ui_next/src/components/Lookup/InventoryLookup.test.jsx new file mode 100644 index 0000000000..1c7d13f488 --- /dev/null +++ b/awx/ui_next/src/components/Lookup/InventoryLookup.test.jsx @@ -0,0 +1,87 @@ +import React from 'react'; +import { act } from 'react-dom/test-utils'; +import { mountWithContexts } from '../../../testUtils/enzymeHelpers'; +import InventoryLookup from './InventoryLookup'; +import { InventoriesAPI } from '../../api'; + +jest.mock('../../api'); + +const mockedInventories = { + data: { + count: 2, + results: [ + { id: 2, name: 'Bar' }, + { id: 3, name: 'Baz' }, + ], + }, +}; + +describe('InventoryLookup', () => { + let wrapper; + + beforeEach(() => { + InventoriesAPI.read.mockResolvedValue(mockedInventories); + }); + + afterEach(() => { + jest.clearAllMocks(); + wrapper.unmount(); + }); + + test('should render successfully and fetch data', async () => { + InventoriesAPI.readOptions.mockReturnValue({ + data: { + actions: { + GET: {}, + POST: {}, + }, + related_search_fields: [], + }, + }); + await act(async () => { + wrapper = mountWithContexts( {}} />); + }); + wrapper.update(); + expect(InventoriesAPI.read).toHaveBeenCalledTimes(1); + expect(wrapper.find('InventoryLookup')).toHaveLength(1); + expect(wrapper.find('Lookup').prop('isDisabled')).toBe(false); + }); + + test('inventory lookup should be enabled', async () => { + InventoriesAPI.readOptions.mockReturnValue({ + data: { + actions: { + GET: {}, + }, + related_search_fields: [], + }, + }); + await act(async () => { + wrapper = mountWithContexts( + {}} /> + ); + }); + wrapper.update(); + expect(InventoriesAPI.read).toHaveBeenCalledTimes(1); + expect(wrapper.find('InventoryLookup')).toHaveLength(1); + expect(wrapper.find('Lookup').prop('isDisabled')).toBe(false); + }); + + test('inventory lookup should be disabled', async () => { + InventoriesAPI.readOptions.mockReturnValue({ + data: { + actions: { + GET: {}, + }, + related_search_fields: [], + }, + }); + await act(async () => { + wrapper = mountWithContexts( {}} />); + }); + wrapper.update(); + expect(InventoriesAPI.read).toHaveBeenCalledTimes(1); + expect(wrapper.find('InventoryLookup')).toHaveLength(1); + expect(wrapper.find('Lookup').prop('isDisabled')).toBe(true); + }); +}); diff --git a/awx/ui_next/src/components/Lookup/ProjectLookup.jsx b/awx/ui_next/src/components/Lookup/ProjectLookup.jsx index dee802c2aa..a02ed40d84 100644 --- a/awx/ui_next/src/components/Lookup/ProjectLookup.jsx +++ b/awx/ui_next/src/components/Lookup/ProjectLookup.jsx @@ -18,6 +18,7 @@ const QS_CONFIG = getQSConfig('project', { page: 1, page_size: 5, order_by: 'name', + role_level: 'use_role', }); function ProjectLookup({ @@ -31,6 +32,7 @@ function ProjectLookup({ value, onBlur, history, + isOverrideDisabled, }) { const autoPopulateLookup = useAutoPopulateLookup(onChange); const { @@ -57,8 +59,10 @@ function ProjectLookup({ searchableKeys: Object.keys( actionsResponse.data.actions?.GET || {} ).filter(key => actionsResponse.data.actions?.GET[key].filterable), - canEdit: Boolean(actionsResponse.data.actions.POST), + canEdit: + Boolean(actionsResponse.data.actions.POST) || isOverrideDisabled, }; + // eslint-disable-next-line react-hooks/exhaustive-deps }, [autoPopulate, autoPopulateLookup, history.location.search]), { count: 0, @@ -160,6 +164,7 @@ ProjectLookup.propTypes = { required: bool, tooltip: string, value: Project, + isOverrideDisabled: bool, }; ProjectLookup.defaultProps = { @@ -170,6 +175,7 @@ ProjectLookup.defaultProps = { required: false, tooltip: '', value: null, + isOverrideDisabled: false, }; export { ProjectLookup as _ProjectLookup }; diff --git a/awx/ui_next/src/components/Lookup/ProjectLookup.test.jsx b/awx/ui_next/src/components/Lookup/ProjectLookup.test.jsx index 04ccad63fe..88060c2699 100644 --- a/awx/ui_next/src/components/Lookup/ProjectLookup.test.jsx +++ b/awx/ui_next/src/components/Lookup/ProjectLookup.test.jsx @@ -7,6 +7,10 @@ import ProjectLookup from './ProjectLookup'; jest.mock('../../api'); describe('', () => { + afterEach(() => { + jest.clearAllMocks(); + }); + test('should auto-select project when only one available and autoPopulate prop is true', async () => { ProjectsAPI.read.mockReturnValue({ data: { @@ -48,4 +52,46 @@ describe('', () => { }); expect(onChange).not.toHaveBeenCalled(); }); + + test('project lookup should be enabled', async () => { + let wrapper; + + ProjectsAPI.readOptions.mockReturnValue({ + data: { + actions: { + GET: {}, + }, + related_search_fields: [], + }, + }); + await act(async () => { + wrapper = mountWithContexts( + {}} /> + ); + }); + wrapper.update(); + expect(ProjectsAPI.read).toHaveBeenCalledTimes(1); + expect(wrapper.find('ProjectLookup')).toHaveLength(1); + expect(wrapper.find('Lookup').prop('isDisabled')).toBe(false); + }); + + test('project lookup should be disabled', async () => { + let wrapper; + + ProjectsAPI.readOptions.mockReturnValue({ + data: { + actions: { + GET: {}, + }, + related_search_fields: [], + }, + }); + await act(async () => { + wrapper = mountWithContexts( {}} />); + }); + wrapper.update(); + expect(ProjectsAPI.read).toHaveBeenCalledTimes(1); + expect(wrapper.find('ProjectLookup')).toHaveLength(1); + expect(wrapper.find('Lookup').prop('isDisabled')).toBe(true); + }); }); diff --git a/awx/ui_next/src/screens/Template/JobTemplateAdd/JobTemplateAdd.jsx b/awx/ui_next/src/screens/Template/JobTemplateAdd/JobTemplateAdd.jsx index e09a4b98f2..562987ba4b 100644 --- a/awx/ui_next/src/screens/Template/JobTemplateAdd/JobTemplateAdd.jsx +++ b/awx/ui_next/src/screens/Template/JobTemplateAdd/JobTemplateAdd.jsx @@ -83,6 +83,7 @@ function JobTemplateAdd() { handleCancel={handleCancel} handleSubmit={handleSubmit} submitError={formSubmitError} + isOverrideDisabledLookup /> diff --git a/awx/ui_next/src/screens/Template/JobTemplateEdit/JobTemplateEdit.jsx b/awx/ui_next/src/screens/Template/JobTemplateEdit/JobTemplateEdit.jsx index 5fbab04fa3..213900d40d 100644 --- a/awx/ui_next/src/screens/Template/JobTemplateEdit/JobTemplateEdit.jsx +++ b/awx/ui_next/src/screens/Template/JobTemplateEdit/JobTemplateEdit.jsx @@ -1,20 +1,45 @@ /* eslint react/no-unused-state: 0 */ -import React, { useState } from 'react'; +import React, { useState, useCallback, useEffect } from 'react'; import { Redirect, useHistory } from 'react-router-dom'; -import { CardBody } from '../../../components/Card'; -import { JobTemplatesAPI } from '../../../api'; + import { JobTemplate } from '../../../types'; +import { JobTemplatesAPI, ProjectsAPI } from '../../../api'; import { getAddedAndRemoved } from '../../../util/lists'; +import useRequest from '../../../util/useRequest'; import JobTemplateForm from '../shared/JobTemplateForm'; import ContentLoading from '../../../components/ContentLoading'; +import { CardBody } from '../../../components/Card'; function JobTemplateEdit({ template }) { const history = useHistory(); const [formSubmitError, setFormSubmitError] = useState(null); const [isLoading, setIsLoading] = useState(false); + const [isDisabled, setIsDisabled] = useState(false); const detailsUrl = `/templates/${template.type}/${template.id}/details`; + const { + request: fetchProject, + error: fetchProjectError, + isLoading: projectLoading, + } = useRequest( + useCallback(async () => { + await ProjectsAPI.readDetail(template.project); + }, [template.project]) + ); + + useEffect(() => { + fetchProject(); + }, [fetchProject]); + + useEffect(() => { + if (fetchProjectError) { + if (fetchProjectError.response.status === 403) { + setIsDisabled(true); + } + } + }, [fetchProjectError]); + const handleSubmit = async values => { const { labels, @@ -89,7 +114,7 @@ function JobTemplateEdit({ template }) { const associateCredentials = added.map(cred => JobTemplatesAPI.associateCredentials(template.id, cred.id) ); - const associatePromise = Promise.all(associateCredentials); + const associatePromise = await Promise.all(associateCredentials); return Promise.all([disassociatePromise, associatePromise]); }; @@ -100,9 +125,10 @@ function JobTemplateEdit({ template }) { if (!canEdit) { return ; } - if (isLoading) { + if (isLoading || projectLoading) { return ; } + return ( ); diff --git a/awx/ui_next/src/screens/Template/shared/JobTemplateForm.jsx b/awx/ui_next/src/screens/Template/shared/JobTemplateForm.jsx index 9b9dfb539e..e3bd18ec19 100644 --- a/awx/ui_next/src/screens/Template/shared/JobTemplateForm.jsx +++ b/awx/ui_next/src/screens/Template/shared/JobTemplateForm.jsx @@ -53,6 +53,7 @@ function JobTemplateForm({ setFieldValue, submitError, i18n, + isOverrideDisabledLookup, }) { const [contentError, setContentError] = useState(false); const [inventory, setInventory] = useState( @@ -254,6 +255,7 @@ function JobTemplateForm({ required={!askInventoryOnLaunchField.value} touched={inventoryMeta.touched} error={inventoryMeta.error} + isOverrideDisabled={isOverrideDisabledLookup} /> {projectField.value?.allow_override && (