Merge pull request #8041 from mabashian/7680-inv-pending-delete

Adds support for pending deletion on inventory list

Reviewed-by: https://github.com/apps/softwarefactory-project-zuul
This commit is contained in:
softwarefactory-project-zuul[bot] 2020-09-29 20:07:17 +00:00 committed by GitHub
commit a9cfae70ff
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
14 changed files with 341 additions and 95 deletions

View File

@ -27,6 +27,7 @@ const initialValues = {
changes: false,
escalation: false,
extra_vars: '---',
module_name: 'shell',
};
describe('<AdHocDetailsStep />', () => {

View File

@ -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 }) {
<>
<Tooltip content={helperText.tooltip} position="top">
<Button
isDisabled={isDisabled}
aria-label={i18n._(t`Copy`)}
variant="plain"
onClick={copyItemToAPI}
@ -50,11 +58,17 @@ function CopyButton({ i18n, copyItem, onLoading, onDoneLoading, helperText }) {
CopyButton.propTypes = {
copyItem: PropTypes.func.isRequired,
onLoading: PropTypes.func.isRequired,
onDoneLoading: PropTypes.func.isRequired,
onCopyStart: PropTypes.func.isRequired,
onCopyFinish: PropTypes.func.isRequired,
helperText: PropTypes.shape({
tooltip: PropTypes.string.isRequired,
errorMessage: PropTypes.string.isRequired,
}).isRequired,
isDisabled: PropTypes.bool,
};
CopyButton.defaultProps = {
isDisabled: false,
};
export default withI18n()(CopyButton);

View File

@ -8,8 +8,8 @@ describe('<CopyButton/>', () => {
test('shold mount properly', () => {
const wrapper = mountWithContexts(
<CopyButton
onLoading={() => {}}
onDoneLoading={() => {}}
onCopyStart={() => {}}
onCopyFinish={() => {}}
copyItem={() => {}}
helperText={{
tooltip: `Copy Template`,
@ -22,8 +22,8 @@ describe('<CopyButton/>', () => {
test('should render proper tooltip', () => {
const wrapper = mountWithContexts(
<CopyButton
onLoading={() => {}}
onDoneLoading={() => {}}
onCopyStart={() => {}}
onCopyFinish={() => {}}
copyItem={() => {}}
helperText={{
tooltip: `Copy Template`,

View File

@ -64,9 +64,9 @@ class ErrorDetail extends Component {
<CardBody>
{Array.isArray(message) ? (
<ul>
{message.map(m => (
<li key={m}>{m}</li>
))}
{message.map(m =>
typeof m === 'string' ? <li key={m}>{m}</li> : null
)}
</ul>
) : (
message

View File

@ -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({
<br />
</span>
))}
{warningMessage && (
<WarningMessage variant="warning" isInline title={warningMessage} />
)}
</AlertModal>
)}
</>
@ -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);

View File

@ -7,6 +7,7 @@ exports[`<ToolbarDeleteButton /> should render button 1`] = `
itemsToDelete={Array []}
onDelete={[Function]}
pluralizedItemName="Items"
warningMessage={null}
>
<Tooltip
content="Select a row to delete"

View File

@ -48,6 +48,14 @@ function CredentialListItem({
await fetchCredentials();
}, [credential.id, credential.name, fetchCredentials]);
const handleCopyStart = useCallback(() => {
setIsDisabled(true);
}, []);
const handleCopyFinish = useCallback(() => {
setIsDisabled(false);
}, []);
return (
<DataListItem
key={credential.id}
@ -95,8 +103,8 @@ function CredentialListItem({
{credential.summary_fields.user_capabilities.copy && (
<CopyButton
isDisabled={isDisabled}
onLoading={() => setIsDisabled(true)}
onDoneLoading={() => setIsDisabled(false)}
onCopyStart={handleCopyStart}
onCopyFinish={handleCopyFinish}
copyItem={copyCredential}
helperText={{
tooltip: i18n._(t`Copy Credential`),

View File

@ -3,6 +3,7 @@ import { Link, useHistory } from 'react-router-dom';
import { withI18n } from '@lingui/react';
import { t } from '@lingui/macro';
import { Button, Chip } from '@patternfly/react-core';
import AlertModal from '../../../components/AlertModal';
import { CardBody, CardActionsRow } from '../../../components/Card';
import {
DetailList,
@ -11,11 +12,12 @@ import {
} from '../../../components/DetailList';
import { VariablesDetail } from '../../../components/CodeMirrorInput';
import DeleteButton from '../../../components/DeleteButton';
import ErrorDetail from '../../../components/ErrorDetail';
import ContentError from '../../../components/ContentError';
import ContentLoading from '../../../components/ContentLoading';
import ChipGroup from '../../../components/ChipGroup';
import { InventoriesAPI } from '../../../api';
import useRequest from '../../../util/useRequest';
import useRequest, { useDismissableError } from '../../../util/useRequest';
import { Inventory } from '../../../types';
function InventoryDetail({ inventory, i18n }) {
@ -24,7 +26,7 @@ function InventoryDetail({ inventory, i18n }) {
const {
result: instanceGroups,
isLoading,
error,
error: instanceGroupsError,
request: fetchInstanceGroups,
} = useRequest(
useCallback(async () => {
@ -38,10 +40,14 @@ function InventoryDetail({ inventory, i18n }) {
fetchInstanceGroups();
}, [fetchInstanceGroups]);
const deleteInventory = async () => {
await InventoriesAPI.destroy(inventory.id);
history.push(`/inventories`);
};
const { request: deleteInventory, error: deleteError } = useRequest(
useCallback(async () => {
await InventoriesAPI.destroy(inventory.id);
history.push(`/inventories`);
}, [inventory.id, history])
);
const { error, dismissError } = useDismissableError(deleteError);
const {
organization,
@ -52,8 +58,8 @@ function InventoryDetail({ inventory, i18n }) {
return <ContentLoading />;
}
if (error) {
return <ContentError error={error} />;
if (instanceGroupsError) {
return <ContentError error={instanceGroupsError} />;
}
return (
@ -123,6 +129,18 @@ function InventoryDetail({ inventory, i18n }) {
</DeleteButton>
)}
</CardActionsRow>
{/* Update delete modal to show dependencies https://github.com/ansible/awx/issues/5546 */}
{error && (
<AlertModal
isOpen={error}
variant="error"
title={i18n._(t`Error!`)}
onClose={dismissError}
>
{i18n._(t`Failed to delete inventory.`)}
<ErrorDetail error={error} />
</AlertModal>
)}
</CardBody>
);
}

View File

@ -3,7 +3,6 @@ import { useLocation, useRouteMatch } from 'react-router-dom';
import { withI18n } from '@lingui/react';
import { t } from '@lingui/macro';
import { Card, PageSection } from '@patternfly/react-core';
import { InventoriesAPI } from '../../../api';
import useRequest, { useDeleteItems } from '../../../util/useRequest';
import AlertModal from '../../../components/AlertModal';
@ -73,20 +72,26 @@ function InventoryList({ i18n }) {
const fetchInventoriesById = useCallback(
async ids => {
const params = parseQueryString(QS_CONFIG, location.search);
const params = { ...parseQueryString(QS_CONFIG, location.search) };
params.id__in = ids.join(',');
const { data } = await InventoriesAPI.read(params);
return data.results;
},
[location.search] // eslint-disable-line react-hooks/exhaustive-deps
);
const inventories = useWsInventories(results, fetchInventoriesById);
const inventories = useWsInventories(
results,
fetchInventories,
fetchInventoriesById,
QS_CONFIG
);
const isAllSelected =
selected.length === inventories.length && selected.length > 0;
const {
isLoading: isDeleteLoading,
deleteItems: deleteTeams,
deleteItems: deleteInventories,
deletionError,
clearDeletionError,
} = useDeleteItems(
@ -94,14 +99,12 @@ function InventoryList({ i18n }) {
return Promise.all(selected.map(team => InventoriesAPI.destroy(team.id)));
}, [selected]),
{
qsConfig: QS_CONFIG,
allItemsSelected: isAllSelected,
fetchItems: fetchInventories,
}
);
const handleInventoryDelete = async () => {
await deleteTeams();
await deleteInventories();
setSelected([]);
};
@ -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));
}
}
};
@ -187,6 +192,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 }
)}
/>,
]}
/>

View File

@ -8,6 +8,7 @@ import {
DataListItem,
DataListItemCells,
DataListItemRow,
Label,
Tooltip,
} from '@patternfly/react-core';
import { PencilAltIcon } from '@patternfly/react-icons';
@ -51,6 +52,14 @@ function InventoryListItem({
await fetchInventories();
}, [inventory.id, inventory.name, fetchInventories]);
const handleCopyStart = useCallback(() => {
setIsDisabled(true);
}, []);
const handleCopyFinish = useCallback(() => {
setIsDisabled(false);
}, []);
const labelId = `check-action-${inventory.id}`;
let syncStatus = 'disabled';
@ -69,6 +78,7 @@ function InventoryListItem({
<DataListItemRow>
<DataListCheck
id={`select-inventory-${inventory.id}`}
isDisabled={inventory.pending_deletion}
checked={isSelected}
onChange={onSelect}
aria-labelledby={labelId}
@ -79,52 +89,63 @@ function InventoryListItem({
<SyncStatusIndicator status={syncStatus} />
</DataListCell>,
<DataListCell key="name">
<Link to={`${detailUrl}`}>
{inventory.pending_deletion ? (
<b>{inventory.name}</b>
</Link>
) : (
<Link to={`${detailUrl}`}>
<b>{inventory.name}</b>
</Link>
)}
</DataListCell>,
<DataListCell key="kind">
{inventory.kind === 'smart'
? i18n._(t`Smart Inventory`)
: i18n._(t`Inventory`)}
</DataListCell>,
inventory.pending_deletion && (
<DataListCell alignRight isFilled={false} key="pending-delete">
<Label color="red">{i18n._(t`Pending delete`)}</Label>
</DataListCell>
),
]}
/>
<DataListAction
aria-label="actions"
aria-labelledby={labelId}
id={labelId}
>
{inventory.summary_fields.user_capabilities.edit ? (
<Tooltip content={i18n._(t`Edit Inventory`)} position="top">
<Button
{!inventory.pending_deletion && (
<DataListAction
aria-label="actions"
aria-labelledby={labelId}
id={labelId}
>
{inventory.summary_fields.user_capabilities.edit ? (
<Tooltip content={i18n._(t`Edit Inventory`)} position="top">
<Button
isDisabled={isDisabled}
aria-label={i18n._(t`Edit Inventory`)}
variant="plain"
component={Link}
to={`/inventories/${
inventory.kind === 'smart' ? 'smart_inventory' : 'inventory'
}/${inventory.id}/edit`}
>
<PencilAltIcon />
</Button>
</Tooltip>
) : (
''
)}
{inventory.summary_fields.user_capabilities.copy && (
<CopyButton
copyItem={copyInventory}
isDisabled={isDisabled}
aria-label={i18n._(t`Edit Inventory`)}
variant="plain"
component={Link}
to={`/inventories/${
inventory.kind === 'smart' ? 'smart_inventory' : 'inventory'
}/${inventory.id}/edit`}
>
<PencilAltIcon />
</Button>
</Tooltip>
) : (
''
)}
{inventory.summary_fields.user_capabilities.copy && (
<CopyButton
copyItem={copyInventory}
isDisabled={isDisabled}
onLoading={() => setIsDisabled(true)}
onDoneLoading={() => setIsDisabled(false)}
helperText={{
tooltip: i18n._(t`Copy Inventory`),
errorMessage: i18n._(t`Failed to copy inventory.`),
}}
/>
)}
</DataListAction>
onCopyStart={handleCopyStart}
onCopyFinish={handleCopyFinish}
helperText={{
tooltip: i18n._(t`Copy Inventory`),
errorMessage: i18n._(t`Failed to copy inventory.`),
}}
/>
)}
</DataListAction>
)}
</DataListItemRow>
</DataListItem>
);

View File

@ -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,59 @@ export default function useWsInventories(
return;
}
if (!['pending', 'waiting', 'running'].includes(lastMessage.status)) {
enqueueId(lastMessage.inventory_id);
return;
}
const params = parseQueryString(qsConfig, location.search);
const inventory = inventories[index];
const updatedInventory = {
...inventory,
isSourceSyncRunning: true,
};
if (
lastMessage.group_name === 'inventories' &&
lastMessage.status === 'deleted' &&
inventories.length === 1 &&
params.page > 1
) {
// We've deleted the last inventory on this page so we'll
// try to navigate back to the previous page
const newParams = encodeNonDefaultQueryString(
qsConfig,
replaceParams(params, {
page: params.page - 1,
})
);
history.push(`${location.pathname}?${newParams}`);
return;
}
if (
lastMessage.group_name === 'inventories' &&
lastMessage.status === 'deleted'
) {
fetchInventories();
return;
}
if (
!['pending', 'waiting', 'running', 'pending_deletion'].includes(
lastMessage.status
)
) {
enqueueId(lastMessage.inventory_id);
return;
}
if (
lastMessage.group_name === 'inventories' &&
lastMessage.status === 'pending_deletion'
) {
updatedInventory.pending_deletion = true;
}
if (lastMessage.group_name !== 'inventories') {
updatedInventory.isSourceSyncRunning = true;
}
setInventories([
...inventories.slice(0, index),
updatedInventory,

View File

@ -16,11 +16,25 @@ jest.mock('../../../util/useThrottle', () => ({
function TestInner() {
return <div />;
}
function Test({ inventories, fetch }) {
const syncedJobs = useWsInventories(inventories, fetch);
return <TestInner inventories={syncedJobs} />;
function Test({
inventories,
fetchInventories,
fetchInventoriesById,
qsConfig,
}) {
const syncedInventories = useWsInventories(
inventories,
fetchInventories,
fetchInventoriesById,
qsConfig
);
return <TestInner inventories={syncedInventories} />;
}
const QS_CONFIG = {
defaultParams: {},
};
describe('useWsInventories hook', () => {
let debug;
let wrapper;
@ -31,14 +45,16 @@ describe('useWsInventories hook', () => {
afterEach(() => {
global.console.debug = debug;
WS.clean();
});
test('should return inventories list', () => {
const inventories = [{ id: 1 }];
wrapper = mountWithContexts(<Test inventories={inventories} />);
wrapper = mountWithContexts(
<Test inventories={inventories} qsConfig={QS_CONFIG} />
);
expect(wrapper.find('TestInner').prop('inventories')).toEqual(inventories);
WS.clean();
});
test('should establish websocket connection', async () => {
@ -47,7 +63,9 @@ describe('useWsInventories hook', () => {
const inventories = [{ id: 1 }];
await act(async () => {
wrapper = await mountWithContexts(<Test inventories={inventories} />);
wrapper = await mountWithContexts(
<Test inventories={inventories} qsConfig={QS_CONFIG} />
);
});
await mockServer.connected;
@ -61,7 +79,6 @@ describe('useWsInventories hook', () => {
},
})
);
WS.clean();
});
test('should update inventory sync status', async () => {
@ -70,7 +87,9 @@ describe('useWsInventories hook', () => {
const inventories = [{ id: 1 }];
await act(async () => {
wrapper = await mountWithContexts(<Test inventories={inventories} />);
wrapper = await mountWithContexts(
<Test inventories={inventories} qsConfig={QS_CONFIG} />
);
});
await mockServer.connected;
@ -98,17 +117,22 @@ describe('useWsInventories hook', () => {
expect(
wrapper.find('TestInner').prop('inventories')[0].isSourceSyncRunning
).toEqual(true);
WS.clean();
});
test('should fetch fresh inventory after sync runs', async () => {
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(
<Test inventories={inventories} fetch={fetch} />
<Test
inventories={inventories}
fetchInventories={fetchInventories}
fetchInventoriesById={fetchInventoriesById}
qsConfig={QS_CONFIG}
/>
);
});
@ -123,7 +147,75 @@ describe('useWsInventories hook', () => {
);
});
expect(fetch).toHaveBeenCalledWith([1]);
WS.clean();
expect(fetchInventoriesById).toHaveBeenCalledWith([1]);
});
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(
<Test inventories={inventories} qsConfig={QS_CONFIG} />
);
});
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);
});
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(
<Test
inventories={inventories}
fetchInventories={fetchInventories}
fetchInventoriesById={fetchInventoriesById}
qsConfig={QS_CONFIG}
/>
);
});
await mockServer.connected;
await act(async () => {
mockServer.send(
JSON.stringify({
inventory_id: 1,
group_name: 'inventories',
status: 'deleted',
})
);
});
expect(fetchInventories).toHaveBeenCalled();
});
});

View File

@ -79,6 +79,14 @@ function ProjectListItem({
);
};
const handleCopyStart = useCallback(() => {
setIsDisabled(true);
}, []);
const handleCopyFinish = useCallback(() => {
setIsDisabled(false);
}, []);
const labelId = `check-action-${project.id}`;
return (
<DataListItem
@ -182,8 +190,8 @@ function ProjectListItem({
<CopyButton
copyItem={copyProject}
isDisabled={isDisabled}
onLoading={() => setIsDisabled(true)}
onDoneLoading={() => setIsDisabled(false)}
onCopyStart={handleCopyStart}
onCopyFinish={handleCopyFinish}
helperText={{
tooltip: i18n._(t`Copy Project`),
errorMessage: i18n._(t`Failed to copy project.`),

View File

@ -60,6 +60,14 @@ function TemplateListItem({
await fetchTemplates();
}, [fetchTemplates, template.id, template.name, template.type]);
const handleCopyStart = useCallback(() => {
setIsDisabled(true);
}, []);
const handleCopyFinish = useCallback(() => {
setIsDisabled(false);
}, []);
const missingResourceIcon =
template.type === 'job_template' &&
(!template.summary_fields.project ||
@ -157,8 +165,8 @@ function TemplateListItem({
errorMessage: i18n._(t`Failed to copy template.`),
}}
isDisabled={isDisabled}
onLoading={() => setIsDisabled(true)}
onDoneLoading={() => setIsDisabled(false)}
onCopyStart={handleCopyStart}
onCopyFinish={handleCopyFinish}
copyItem={copyTemplate}
/>
)}