From 4b566e938807e2e14c40c8e2185df3c155fc9f54 Mon Sep 17 00:00:00 2001 From: mabashian Date: Fri, 28 Aug 2020 14:25:37 -0400 Subject: [PATCH 1/8] Adds support for pending deletion on inventory list --- .../PaginatedDataList/ToolbarDeleteButton.jsx | 14 ++- .../ToolbarDeleteButton.test.jsx.snap | 1 + .../Inventory/InventoryList/InventoryList.jsx | 25 +++-- .../InventoryList/InventoryListItem.jsx | 85 ++++++++++------- .../InventoryList/useWsInventories.js | 52 ++++++++-- .../InventoryList/useWsInventories.test.jsx | 95 +++++++++++++++++-- 6 files changed, 213 insertions(+), 59 deletions(-) diff --git a/awx/ui_next/src/components/PaginatedDataList/ToolbarDeleteButton.jsx b/awx/ui_next/src/components/PaginatedDataList/ToolbarDeleteButton.jsx index 25a280d549..744d3e26fd 100644 --- a/awx/ui_next/src/components/PaginatedDataList/ToolbarDeleteButton.jsx +++ b/awx/ui_next/src/components/PaginatedDataList/ToolbarDeleteButton.jsx @@ -2,18 +2,24 @@ import React, { useContext, useEffect, useState } from 'react'; import { func, bool, + node, number, string, arrayOf, shape, checkPropTypes, } from 'prop-types'; -import { Button, DropdownItem, Tooltip } from '@patternfly/react-core'; +import styled from 'styled-components'; +import { Alert, Button, DropdownItem, Tooltip } from '@patternfly/react-core'; import { withI18n } from '@lingui/react'; import { t } from '@lingui/macro'; import AlertModal from '../AlertModal'; import { KebabifiedContext } from '../../contexts/Kebabified'; +const WarningMessage = styled(Alert)` + margin-top: 10px; +`; + const requireNameOrUsername = props => { const { name, username } = props; if (!name && !username) { @@ -64,6 +70,7 @@ function ToolbarDeleteButton({ pluralizedItemName, errorMessage, onDelete, + warningMessage, i18n, }) { const { isKebabified, onKebabModalChange } = useContext(KebabifiedContext); @@ -171,6 +178,9 @@ function ToolbarDeleteButton({
))} + {warningMessage && ( + + )} )} @@ -182,11 +192,13 @@ ToolbarDeleteButton.propTypes = { itemsToDelete: arrayOf(ItemToDelete).isRequired, pluralizedItemName: string, errorMessage: string, + warningMessage: node, }; ToolbarDeleteButton.defaultProps = { pluralizedItemName: 'Items', errorMessage: '', + warningMessage: null, }; export default withI18n()(ToolbarDeleteButton); diff --git a/awx/ui_next/src/components/PaginatedDataList/__snapshots__/ToolbarDeleteButton.test.jsx.snap b/awx/ui_next/src/components/PaginatedDataList/__snapshots__/ToolbarDeleteButton.test.jsx.snap index 9d60823722..61265726ed 100644 --- a/awx/ui_next/src/components/PaginatedDataList/__snapshots__/ToolbarDeleteButton.test.jsx.snap +++ b/awx/ui_next/src/components/PaginatedDataList/__snapshots__/ToolbarDeleteButton.test.jsx.snap @@ -7,6 +7,7 @@ exports[` should render button 1`] = ` itemsToDelete={Array []} onDelete={[Function]} pluralizedItemName="Items" + warningMessage={null} > 0; @@ -94,9 +99,7 @@ function InventoryList({ i18n }) { return Promise.all(selected.map(team => InventoriesAPI.destroy(team.id))); }, [selected]), { - qsConfig: QS_CONFIG, allItemsSelected: isAllSelected, - fetchItems: fetchInventories, } ); @@ -113,10 +116,12 @@ function InventoryList({ i18n }) { }; const handleSelect = row => { - if (selected.some(s => s.id === row.id)) { - setSelected(selected.filter(s => s.id !== row.id)); - } else { - setSelected(selected.concat(row)); + if (!row.pending_deletion) { + if (selected.some(s => s.id === row.id)) { + setSelected(selected.filter(s => s.id !== row.id)); + } else { + setSelected(selected.concat(row)); + } } }; @@ -183,6 +188,10 @@ function InventoryList({ i18n }) { onDelete={handleInventoryDelete} itemsToDelete={selected} pluralizedItemName={i18n._(t`Inventories`)} + warningMessage={i18n._( + '{numItemsToDelete, plural, one {The inventory will be in a pending status until the final delete is processed.} other {The inventories will be in a pending status until the final delete is processed.}}', + { numItemsToDelete: selected.length } + )} />, ]} /> diff --git a/awx/ui_next/src/screens/Inventory/InventoryList/InventoryListItem.jsx b/awx/ui_next/src/screens/Inventory/InventoryList/InventoryListItem.jsx index 9070656bba..519fd9cbd6 100644 --- a/awx/ui_next/src/screens/Inventory/InventoryList/InventoryListItem.jsx +++ b/awx/ui_next/src/screens/Inventory/InventoryList/InventoryListItem.jsx @@ -8,6 +8,7 @@ import { DataListItem, DataListItemCells, DataListItemRow, + Label, Tooltip, } from '@patternfly/react-core'; import { PencilAltIcon } from '@patternfly/react-icons'; @@ -69,6 +70,7 @@ function InventoryListItem({ , - + {inventory.pending_deletion ? ( {inventory.name} - + ) : ( + + {inventory.name} + + )} , {inventory.kind === 'smart' ? i18n._(t`Smart Inventory`) : i18n._(t`Inventory`)} , + inventory.pending_deletion && ( + + + + ), ]} /> - - {inventory.summary_fields.user_capabilities.edit ? ( - - + + ) : ( + '' + )} + {inventory.summary_fields.user_capabilities.copy && ( + - - - - ) : ( - '' - )} - {inventory.summary_fields.user_capabilities.copy && ( - setIsDisabled(true)} - onDoneLoading={() => setIsDisabled(false)} - helperText={{ - tooltip: i18n._(t`Copy Inventory`), - errorMessage: i18n._(t`Failed to copy inventory.`), - }} - /> - )} - + onLoading={() => setIsDisabled(true)} + onDoneLoading={() => setIsDisabled(false)} + helperText={{ + tooltip: i18n._(t`Copy Inventory`), + errorMessage: i18n._(t`Failed to copy inventory.`), + }} + /> + )} + + )} ); diff --git a/awx/ui_next/src/screens/Inventory/InventoryList/useWsInventories.js b/awx/ui_next/src/screens/Inventory/InventoryList/useWsInventories.js index 60fae9e90e..6156d80692 100644 --- a/awx/ui_next/src/screens/Inventory/InventoryList/useWsInventories.js +++ b/awx/ui_next/src/screens/Inventory/InventoryList/useWsInventories.js @@ -1,11 +1,21 @@ import { useState, useEffect } from 'react'; +import { useLocation, useHistory } from 'react-router-dom'; +import { + parseQueryString, + replaceParams, + encodeNonDefaultQueryString, +} from '../../../util/qs'; import useWebsocket from '../../../util/useWebsocket'; import useThrottle from '../../../util/useThrottle'; export default function useWsInventories( initialInventories, - fetchInventoriesById + fetchInventories, + fetchInventoriesById, + qsConfig ) { + const location = useLocation(); + const history = useHistory(); const [inventories, setInventories] = useState(initialInventories); const [inventoriesToFetch, setInventoriesToFetch] = useState([]); const throttledInventoriesToFetch = useThrottle(inventoriesToFetch, 5000); @@ -53,7 +63,8 @@ export default function useWsInventories( function processWsMessage() { if ( !lastMessage?.inventory_id || - lastMessage.type !== 'inventory_update' + (lastMessage.type !== 'inventory_update' && + lastMessage.group_name !== 'inventories') ) { return; } @@ -64,16 +75,41 @@ export default function useWsInventories( return; } - if (!['pending', 'waiting', 'running'].includes(lastMessage.status)) { - enqueueId(lastMessage.inventory_id); - return; - } - const inventory = inventories[index]; const updatedInventory = { ...inventory, - isSourceSyncRunning: true, }; + + if (lastMessage.group_name === 'inventories') { + if (lastMessage.status === 'pending_deletion') { + updatedInventory.pending_deletion = true; + } else if (lastMessage.status === 'deleted') { + if (inventories.length === 1) { + const params = parseQueryString(qsConfig, location.search); + if (params.page > 1) { + const newParams = encodeNonDefaultQueryString( + qsConfig, + replaceParams(params, { + page: params.page - 1, + }) + ); + history.push(`${location.pathname}?${newParams}`); + } + } else { + fetchInventories(); + } + } else { + return; + } + } else { + if (!['pending', 'waiting', 'running'].includes(lastMessage.status)) { + enqueueId(lastMessage.inventory_id); + return; + } + + updatedInventory.isSourceSyncRunning = true; + } + setInventories([ ...inventories.slice(0, index), updatedInventory, diff --git a/awx/ui_next/src/screens/Inventory/InventoryList/useWsInventories.test.jsx b/awx/ui_next/src/screens/Inventory/InventoryList/useWsInventories.test.jsx index 196166add6..f7640522d2 100644 --- a/awx/ui_next/src/screens/Inventory/InventoryList/useWsInventories.test.jsx +++ b/awx/ui_next/src/screens/Inventory/InventoryList/useWsInventories.test.jsx @@ -16,9 +16,19 @@ jest.mock('../../../util/useThrottle', () => ({ function TestInner() { return
; } -function Test({ inventories, fetch }) { - const syncedJobs = useWsInventories(inventories, fetch); - return ; +function Test({ + inventories, + fetchInventories, + fetchInventoriesById, + qsConfig, +}) { + const syncedInventories = useWsInventories( + inventories, + fetchInventories, + fetchInventoriesById, + qsConfig + ); + return ; } describe('useWsInventories hook', () => { @@ -105,10 +115,15 @@ describe('useWsInventories hook', () => { global.document.cookie = 'csrftoken=abc123'; const mockServer = new WS('wss://localhost/websocket/'); const inventories = [{ id: 1 }]; - const fetch = jest.fn(() => []); + const fetchInventories = jest.fn(() => []); + const fetchInventoriesById = jest.fn(() => []); await act(async () => { wrapper = await mountWithContexts( - + ); }); @@ -123,7 +138,75 @@ describe('useWsInventories hook', () => { ); }); - expect(fetch).toHaveBeenCalledWith([1]); + expect(fetchInventoriesById).toHaveBeenCalledWith([1]); + WS.clean(); + }); + + test('should update inventory pending_deletion', async () => { + global.document.cookie = 'csrftoken=abc123'; + const mockServer = new WS('wss://localhost/websocket/'); + + const inventories = [{ id: 1, pending_deletion: false }]; + await act(async () => { + wrapper = await mountWithContexts(); + }); + + await mockServer.connected; + await expect(mockServer).toReceiveMessage( + JSON.stringify({ + xrftoken: 'abc123', + groups: { + inventories: ['status_changed'], + jobs: ['status_changed'], + control: ['limit_reached_1'], + }, + }) + ); + act(() => { + mockServer.send( + JSON.stringify({ + inventory_id: 1, + group_name: 'inventories', + status: 'pending_deletion', + }) + ); + }); + wrapper.update(); + + expect( + wrapper.find('TestInner').prop('inventories')[0].pending_deletion + ).toEqual(true); + WS.clean(); + }); + + test('should refetch inventories after an inventory is deleted', async () => { + global.document.cookie = 'csrftoken=abc123'; + const mockServer = new WS('wss://localhost/websocket/'); + const inventories = [{ id: 1 }, { id: 2 }]; + const fetchInventories = jest.fn(() => []); + const fetchInventoriesById = jest.fn(() => []); + await act(async () => { + wrapper = await mountWithContexts( + + ); + }); + + await mockServer.connected; + await act(async () => { + mockServer.send( + JSON.stringify({ + inventory_id: 1, + group_name: 'inventories', + status: 'deleted', + }) + ); + }); + + expect(fetchInventories).toHaveBeenCalled(); WS.clean(); }); }); From d84615f64bd34d4c7b8ba514cb31ea52e3bde53f Mon Sep 17 00:00:00 2001 From: mabashian Date: Thu, 10 Sep 2020 16:21:24 -0400 Subject: [PATCH 2/8] Rename onLoading/onDoneLoading props to onCopyStart and onCopyFinish. Wrap the functions being passed in as those props in useCallback to keep them hooks safe. --- .../src/components/CopyButton/CopyButton.jsx | 26 ++++++++++++++----- .../components/CopyButton/CopyButton.test.jsx | 8 +++--- .../CredentialList/CredentialListItem.jsx | 12 +++++++-- .../InventoryList/InventoryListItem.jsx | 12 +++++++-- .../Project/ProjectList/ProjectListItem.jsx | 12 +++++++-- .../TemplateList/TemplateListItem.jsx | 12 +++++++-- 6 files changed, 64 insertions(+), 18 deletions(-) diff --git a/awx/ui_next/src/components/CopyButton/CopyButton.jsx b/awx/ui_next/src/components/CopyButton/CopyButton.jsx index dd1e91a7aa..f8eeda7626 100644 --- a/awx/ui_next/src/components/CopyButton/CopyButton.jsx +++ b/awx/ui_next/src/components/CopyButton/CopyButton.jsx @@ -9,17 +9,24 @@ import useRequest, { useDismissableError } from '../../util/useRequest'; import AlertModal from '../AlertModal'; import ErrorDetail from '../ErrorDetail'; -function CopyButton({ i18n, copyItem, onLoading, onDoneLoading, helperText }) { +function CopyButton({ + i18n, + copyItem, + isDisabled, + onCopyStart, + onCopyFinish, + helperText, +}) { const { isLoading, error: copyError, request: copyItemToAPI } = useRequest( copyItem ); useEffect(() => { if (isLoading) { - return onLoading(); + return onCopyStart(); } - return onDoneLoading(); - }, [isLoading, onLoading, onDoneLoading]); + return onCopyFinish(); + }, [isLoading, onCopyStart, onCopyFinish]); const { error, dismissError } = useDismissableError(copyError); @@ -27,6 +34,7 @@ function CopyButton({ i18n, copyItem, onLoading, onDoneLoading, helperText }) { <>