From 1ea924aa131c3104773545fb16673497e5889100 Mon Sep 17 00:00:00 2001 From: Alex Corey Date: Thu, 30 Apr 2020 17:25:14 -0400 Subject: [PATCH] improves isLoading state and removes unnecessary RBAC --- .../src/api/models/InventorySources.js | 9 +- .../src/api/models/InventoryUpdates.js | 9 +- .../InventorySourceListItem.jsx | 18 ++-- .../InventorySourceListItem.test.jsx | 1 + .../InventorySourceSyncButton.jsx | 57 +++---------- .../InventorySourceSyncButton.test.jsx | 83 ++----------------- 6 files changed, 32 insertions(+), 145 deletions(-) diff --git a/awx/ui_next/src/api/models/InventorySources.js b/awx/ui_next/src/api/models/InventorySources.js index 716fa61a76..d525f992f0 100644 --- a/awx/ui_next/src/api/models/InventorySources.js +++ b/awx/ui_next/src/api/models/InventorySources.js @@ -6,15 +6,10 @@ class InventorySources extends LaunchUpdateMixin(Base) { super(http); this.baseUrl = '/api/v2/inventory_sources/'; - this.allowSyncStart = this.allowSyncStart.bind(this); - this.startSyncSource = this.startSyncSource.bind(this); + this.createSyncStart = this.createSyncStart.bind(this); } - allowSyncStart(sourceId) { - return this.http.get(`${this.baseUrl}${sourceId}/update/`); - } - - startSyncSource(sourceId, extraVars) { + createSyncStart(sourceId, extraVars) { return this.http.post(`${this.baseUrl}${sourceId}/update/`, { extra_vars: extraVars, }); diff --git a/awx/ui_next/src/api/models/InventoryUpdates.js b/awx/ui_next/src/api/models/InventoryUpdates.js index c860b6267b..1700c7b26b 100644 --- a/awx/ui_next/src/api/models/InventoryUpdates.js +++ b/awx/ui_next/src/api/models/InventoryUpdates.js @@ -5,15 +5,10 @@ class InventoryUpdates extends LaunchUpdateMixin(Base) { constructor(http) { super(http); this.baseUrl = '/api/v2/inventory_updates/'; - this.allowSyncCancel = this.allowSyncCancel.bind(this); - this.cancelSyncSource = this.cancelSyncSource.bind(this); + this.createSyncCancel = this.createSyncCancel.bind(this); } - allowSyncCancel(sourceId) { - return this.http.get(`${this.baseUrl}${sourceId}/cancel/`); - } - - cancelSyncSource(sourceId) { + createSyncCancel(sourceId) { return this.http.post(`${this.baseUrl}${sourceId}/cancel/`); } } diff --git a/awx/ui_next/src/screens/Inventory/InventorySources/InventorySourceListItem.jsx b/awx/ui_next/src/screens/Inventory/InventorySources/InventorySourceListItem.jsx index 455698144e..4437daf67d 100644 --- a/awx/ui_next/src/screens/Inventory/InventorySources/InventorySourceListItem.jsx +++ b/awx/ui_next/src/screens/Inventory/InventorySources/InventorySourceListItem.jsx @@ -25,10 +25,7 @@ function InventorySourceListItem({ detailUrl, label, }) { - const [isCancelSyncLoading, setIsCancelSyncLoading] = useState(false); - const [isStartSyncLoading, setIsStartSyncLoading] = useState(false); - - const isDisabled = isCancelSyncLoading || isStartSyncLoading; + const [isSyncLoading, setIsSyncLoading] = useState(false); const generateLastJobTooltip = job => { return ( @@ -53,7 +50,7 @@ function InventorySourceListItem({ {source.summary_fields.user_capabilities.start && ( - setIsCancelSyncLoading(isLoading) - } - onStartSyncLoading={isLoading => - setIsStartSyncLoading(isLoading) - } + onSyncLoading={isLoading => { + setIsSyncLoading(isLoading); + }} source={source} /> )} @@ -113,7 +107,7 @@ function InventorySourceListItem({ aria-label={i18n._(t`Edit Source`)} variant="plain" component={Link} - isDisabled={isDisabled} + isDisabled={isSyncLoading} to={`${detailUrl}/edit`} > diff --git a/awx/ui_next/src/screens/Inventory/InventorySources/InventorySourceListItem.test.jsx b/awx/ui_next/src/screens/Inventory/InventorySources/InventorySourceListItem.test.jsx index 531ebdffa4..86b4a48d2e 100644 --- a/awx/ui_next/src/screens/Inventory/InventorySources/InventorySourceListItem.test.jsx +++ b/awx/ui_next/src/screens/Inventory/InventorySources/InventorySourceListItem.test.jsx @@ -106,6 +106,7 @@ describe('', () => { ); expect(wrapper.find('StatusIcon').length).toBe(0); }); + test('should not render sync buttons', async () => { const onSelect = jest.fn(); wrapper = mountWithContexts( diff --git a/awx/ui_next/src/screens/Inventory/InventorySources/InventorySourceSyncButton.jsx b/awx/ui_next/src/screens/Inventory/InventorySources/InventorySourceSyncButton.jsx index 47f6cf293b..801f8c170a 100644 --- a/awx/ui_next/src/screens/Inventory/InventorySources/InventorySourceSyncButton.jsx +++ b/awx/ui_next/src/screens/Inventory/InventorySources/InventorySourceSyncButton.jsx @@ -9,12 +9,7 @@ import AlertModal from '@components/AlertModal/AlertModal'; import ErrorDetail from '@components/ErrorDetail/ErrorDetail'; import { InventoryUpdatesAPI, InventorySourcesAPI } from '@api'; -function InventorySourceSyncButton({ - onCancelSyncLoading, - onStartSyncLoading, - source, - i18n, -}) { +function InventorySourceSyncButton({ onSyncLoading, source, i18n }) { const [updateStatus, setUpdateStatus] = useState(source.status); const { @@ -23,25 +18,14 @@ function InventorySourceSyncButton({ request: startSyncProcess, } = useRequest( useCallback(async () => { - let syncStatus; - const { - data: { can_update }, - } = await InventorySourcesAPI.allowSyncStart(source.id); - if (can_update) { - syncStatus = await InventorySourcesAPI.startSyncSource(source.id); - } else { - throw new Error( - i18n._( - t`You do not have permission to start this inventory source sync` - ) - ); - } + data: { status }, + } = await InventorySourcesAPI.createSyncStart(source.id); - setUpdateStatus(syncStatus.data.status); + setUpdateStatus(status); - return syncStatus.data.status; - }, [source.id, i18n]), + return status; + }, [source.id]), {} ); @@ -58,29 +42,15 @@ function InventorySourceSyncButton({ }, }, } = await InventorySourcesAPI.readDetail(source.id); - const { - data: { can_cancel }, - } = await InventoryUpdatesAPI.allowSyncCancel(id); - if (can_cancel) { - await InventoryUpdatesAPI.cancelSyncSource(id); - setUpdateStatus(null); - } else { - throw new Error( - i18n._( - t`You do not have permission to cancel this inventory source sync` - ) - ); - } - }, [source.id, i18n]) + + await InventoryUpdatesAPI.createSyncCancel(id); + setUpdateStatus(null); + }, [source.id]) ); - useEffect(() => onStartSyncLoading(startSyncLoading), [ - onStartSyncLoading, + useEffect(() => onSyncLoading(startSyncLoading || cancelSyncLoading), [ + onSyncLoading, startSyncLoading, - ]); - - useEffect(() => onCancelSyncLoading(cancelSyncLoading), [ - onCancelSyncLoading, cancelSyncLoading, ]); @@ -131,8 +101,7 @@ function InventorySourceSyncButton({ } InventorySourceSyncButton.propTypes = { - onCancelSyncLoading: PropTypes.func.isRequired, - onStartSyncLoading: PropTypes.func.isRequired, + onSyncLoading: PropTypes.func.isRequired, source: PropTypes.shape({}), }; diff --git a/awx/ui_next/src/screens/Inventory/InventorySources/InventorySourceSyncButton.test.jsx b/awx/ui_next/src/screens/Inventory/InventorySources/InventorySourceSyncButton.test.jsx index c6fdcc6f51..f31a300089 100644 --- a/awx/ui_next/src/screens/Inventory/InventorySources/InventorySourceSyncButton.test.jsx +++ b/awx/ui_next/src/screens/Inventory/InventorySources/InventorySourceSyncButton.test.jsx @@ -8,8 +8,7 @@ jest.mock('@api/models/InventoryUpdates'); jest.mock('@api/models/InventorySources'); const source = { id: 1, name: 'Foo', source: 'Source Bar' }; -const onCancelSyncLoading = jest.fn(); -const onStartSyncLoading = jest.fn(); +const onSyncLoading = jest.fn(); describe('', () => { let wrapper; @@ -17,8 +16,7 @@ describe('', () => { wrapper = mountWithContexts( ); }); @@ -41,26 +39,21 @@ describe('', () => { wrapper = mountWithContexts( ); expect(wrapper.find('MinusCircleIcon').length).toBe(1); }); test('should start sync properly', async () => { - InventorySourcesAPI.allowSyncStart.mockResolvedValue({ - data: { can_update: true }, - }); - InventorySourcesAPI.startSyncSource.mockResolvedValue({ + InventorySourcesAPI.createSyncStart.mockResolvedValue({ data: { status: 'pending' }, }); await act(async () => wrapper.find('Button[aria-label="Start sync source"]').simulate('click') ); - expect(InventorySourcesAPI.allowSyncStart).toBeCalledWith(1); - expect(InventorySourcesAPI.startSyncSource).toBeCalledWith(1); + expect(InventorySourcesAPI.createSyncStart).toBeCalledWith(1); wrapper.update(); expect(wrapper.find('Button[aria-label="Cancel sync source"]').length).toBe( 1 @@ -70,18 +63,14 @@ describe('', () => { InventorySourcesAPI.readDetail.mockResolvedValue({ data: { summary_fields: { current_update: { id: 120 } } }, }); - InventoryUpdatesAPI.allowSyncCancel.mockResolvedValue({ - data: { can_cancel: true }, - }); - InventoryUpdatesAPI.cancelSyncSource.mockResolvedValue({ + InventoryUpdatesAPI.createSyncCancel.mockResolvedValue({ data: { status: '' }, }); wrapper = mountWithContexts( ); expect(wrapper.find('Button[aria-label="Cancel sync source"]').length).toBe( @@ -93,8 +82,7 @@ describe('', () => { ); expect(InventorySourcesAPI.readDetail).toBeCalledWith(1); - expect(InventoryUpdatesAPI.allowSyncCancel).toBeCalledWith(120); - expect(InventoryUpdatesAPI.cancelSyncSource).toBeCalledWith(120); + expect(InventoryUpdatesAPI.createSyncCancel).toBeCalledWith(120); wrapper.update(); @@ -102,59 +90,4 @@ describe('', () => { 1 ); }); - test('Should prevent user from starting sync', async () => { - InventorySourcesAPI.allowSyncStart.mockResolvedValue({ - data: { can_update: false }, - }); - InventorySourcesAPI.startSyncSource.mockResolvedValue({ - data: { status: 'pending' }, - }); - - await act(async () => - wrapper.find('Button[aria-label="Start sync source"]').simulate('click') - ); - expect(InventorySourcesAPI.allowSyncStart).toBeCalledWith(1); - expect(InventorySourcesAPI.startSyncSource).not.toBeCalledWith(); - wrapper.update(); - expect(wrapper.find('AlertModal').length).toBe(1); - expect(wrapper.find('Button[aria-label="Start sync source"]').length).toBe( - 1 - ); - }); - test('should prevent user from canceling sync', async () => { - InventorySourcesAPI.readDetail.mockResolvedValue({ - data: { summary_fields: { current_update: { id: 120 } } }, - }); - InventoryUpdatesAPI.allowSyncCancel.mockResolvedValue({ - data: { can_cancel: false }, - }); - InventoryUpdatesAPI.cancelSyncSource.mockResolvedValue({ - data: { status: '' }, - }); - - wrapper = mountWithContexts( - - ); - expect(wrapper.find('Button[aria-label="Cancel sync source"]').length).toBe( - 1 - ); - - await act(async () => - wrapper.find('Button[aria-label="Cancel sync source"]').simulate('click') - ); - - expect(InventorySourcesAPI.readDetail).toBeCalledWith(1); - expect(InventoryUpdatesAPI.allowSyncCancel).toBeCalledWith(120); - expect(InventoryUpdatesAPI.cancelSyncSource).not.toBeCalledWith(120); - - wrapper.update(); - expect(wrapper.find('AlertModal').length).toBe(1); - expect(wrapper.find('Button[aria-label="Cancel sync source"]').length).toBe( - 1 - ); - }); });