Skip to content

Remove bulk delete from collections table #3776

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
dvail merged 1 commit into from
Nov 11, 2022
Merged

Remove bulk delete from collections table #3776

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
118 changes: 22 additions & 96 deletions ui/apps/platform/src/Containers/Collections/CollectionsTable.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,28 +4,21 @@ import {
Bullseye,
Button,
ButtonVariant,
Dropdown,
DropdownItem,
DropdownToggle,
Pagination,
SearchInput,
Text,
Toolbar,
ToolbarContent,
ToolbarItem,
ToolbarItemVariant,
Truncate,
} from '@patternfly/react-core';
import { CaretDownIcon, SearchIcon } from '@patternfly/react-icons';
import { SearchIcon } from '@patternfly/react-icons';
import { TableComposable, TableVariant, Tbody, Td, Th, Thead, Tr } from '@patternfly/react-table';
import debounce from 'lodash/debounce';
import pluralize from 'pluralize';

import ConfirmationModal from 'Components/PatternFly/ConfirmationModal';
import EmptyStateTemplate from 'Components/PatternFly/EmptyStateTemplate';
import LinkShim from 'Components/PatternFly/LinkShim';
import useSelectToggle from 'hooks/patternfly/useSelectToggle';
import useTableSelection from 'hooks/useTableSelection';
import { UseURLPaginationResult } from 'hooks/useURLPagination';
import { GetSortParams } from 'hooks/useURLSort';
import { CollectionResponse } from 'services/CollectionsService';
Expand All @@ -39,7 +32,7 @@ export type CollectionsTableProps = {
searchFilter: SearchFilter;
setSearchFilter: (searchFilter: SearchFilter) => void;
getSortParams: GetSortParams;
onCollectionDelete: (ids: string[]) => Promise<void>;
onCollectionDelete: (collection: CollectionResponse) => Promise<void>;
hasWriteAccess: boolean;
};

Expand All @@ -57,11 +50,8 @@ function CollectionsTable({
}: CollectionsTableProps) {
const history = useHistory();
const { page, perPage, setPage, setPerPage } = pagination;
const { isOpen, onToggle, closeSelect } = useSelectToggle();
const { selected, allRowsSelected, hasSelections, onSelect, onSelectAll, getSelectedIds } =
useTableSelection(collections);
const [isDeleting, setIsDeleting] = useState(false);
const [deletingIds, setDeletingIds] = useState<string[]>([]);
const [collectionToDelete, setCollectionToDelete] = useState<CollectionResponse | null>(null);
const hasCollections = collections.length > 0;

function getEnabledSortParams(field: string) {
Expand Down Expand Up @@ -91,29 +81,18 @@ function CollectionsTable({
[setSearchFilter]
);

function onConfirmDeleteCollection() {
function onConfirmDeleteCollection(collection: CollectionResponse) {
Copy link
Copy Markdown
Contributor Author

@dvail dvail Nov 10, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

By passing the collection here instead of reading from state, we can take advantage of the fact that the type has already been narrow to be non-null.

setIsDeleting(true);
onCollectionDelete(deletingIds).finally(() => {
setDeletingIds([]);
onCollectionDelete(collection).finally(() => {
setCollectionToDelete(null);
setIsDeleting(false);
});
}

function onCancelDeleteCollection() {
setDeletingIds([]);
setCollectionToDelete(null);
}

const unusedSelectedCollectionIds = collections
.filter((c) => getSelectedIds().includes(c.id) && !c.inUse)
.map((c) => c.id);

// A map to keep track of row index within the table to the collection id
// for checkbox selection after the table has been sorted.
const rowIdToIndex = {};
collections.forEach(({ id }, idx) => {
rowIdToIndex[id] = idx;
});

// Currently, it is not expected that the value of `searchFilter.Collection` will
// be an array even though it would valid. This is a safeguard for future code
// changes that might change this assumption.
Expand All @@ -133,41 +112,6 @@ function CollectionsTable({
onChange={onSearchInputChange}
/>
</ToolbarItem>
{hasWriteAccess && (
<>
<ToolbarItem variant={ToolbarItemVariant.separator} />
<ToolbarItem className="pf-u-flex-grow-1">
<Dropdown
onSelect={closeSelect}
toggle={
<DropdownToggle
isDisabled={!hasSelections}
isPrimary
onToggle={onToggle}
toggleIndicator={CaretDownIcon}
>
Bulk actions
</DropdownToggle>
}
isOpen={isOpen}
dropdownItems={[
<DropdownItem
key="Delete collection"
component="button"
isDisabled={unusedSelectedCollectionIds.length === 0}
onClick={() => {
setDeletingIds(unusedSelectedCollectionIds);
}}
>
{unusedSelectedCollectionIds.length > 0
? `Delete collections (${unusedSelectedCollectionIds.length})`
: 'Cannot delete (in use)'}
</DropdownItem>,
]}
/>
</ToolbarItem>
</>
)}
<ToolbarItem variant="pagination" alignment={{ default: 'alignRight' }}>
<Pagination
isCompact
Expand All @@ -188,14 +132,6 @@ function CollectionsTable({
<TableComposable variant={TableVariant.compact}>
<Thead>
<Tr>
{hasWriteAccess && (
<Th
select={{
onSelect: onSelectAll,
isSelected: allRowsSelected,
}}
/>
)}
<Th modifier="wrap" width={25} sort={getEnabledSortParams('name')}>
Collection
</Th>
Expand Down Expand Up @@ -227,8 +163,8 @@ function CollectionsTable({
</Td>
</Tr>
)}
{collections.map(({ id, name, description, inUse }) => {
const rowIndex = rowIdToIndex[id];
{collections.map((collection) => {
const { id, name, description, inUse } = collection;
const actionItems = [
{
title: 'Edit collection',
Expand All @@ -243,24 +179,13 @@ function CollectionsTable({
},
{
title: inUse ? 'Cannot delete (in use)' : 'Delete collection',
onClick: () => setDeletingIds([id]),
onClick: () => setCollectionToDelete(collection),
isDisabled: inUse,
},
];

return (
<Tr key={id}>
{hasWriteAccess && (
<Td
title={inUse ? 'Collection is in use' : ''}
select={{
disable: inUse,
rowIndex,
onSelect,
isSelected: selected[rowIndex],
}}
/>
)}
<Td dataLabel="Collection">
<Button
variant={ButtonVariant.link}
Expand All @@ -281,17 +206,18 @@ function CollectionsTable({
})}
</Tbody>
</TableComposable>
<ConfirmationModal
ariaLabel="Confirm delete"
confirmText="Delete"
isLoading={isDeleting}
isOpen={deletingIds.length !== 0}
onConfirm={onConfirmDeleteCollection}
onCancel={onCancelDeleteCollection}
>
Are you sure you want to delete {deletingIds.length}&nbsp;
{pluralize('collection', deletingIds.length)}?
</ConfirmationModal>
{collectionToDelete && (
<ConfirmationModal
ariaLabel="Confirm delete"
confirmText="Delete"
isLoading={isDeleting}
isOpen
onConfirm={() => onConfirmDeleteCollection(collectionToDelete)}
onCancel={onCancelDeleteCollection}
>
Are you sure you want to delete &lsquo;{collectionToDelete.name}&rsquo;?
</ConfirmationModal>
)}
</>
);
}
Expand Down
54 changes: 17 additions & 37 deletions ui/apps/platform/src/Containers/Collections/CollectionsTablePage.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,17 +14,20 @@ import {
AlertActionCloseButton,
AlertGroup,
} from '@patternfly/react-core';
import pluralize from 'pluralize';

import PageTitle from 'Components/PageTitle';
import LinkShim from 'Components/PatternFly/LinkShim';
import { collectionsBasePath } from 'routePaths';
import useRestQuery from 'Containers/Dashboard/hooks/useRestQuery';
import { deleteCollection, getCollectionCount, listCollections } from 'services/CollectionsService';
import {
CollectionResponse,
deleteCollection,
getCollectionCount,
listCollections,
} from 'services/CollectionsService';
import useURLSearch from 'hooks/useURLSearch';
import useURLPagination from 'hooks/useURLPagination';
import useURLSort from 'hooks/useURLSort';
import { Empty } from 'services/types';
import useToasts, { Toast } from 'hooks/patternfly/useToasts';
import CollectionsTable from './CollectionsTable';

Expand Down Expand Up @@ -66,41 +69,18 @@ function CollectionsTablePage({ hasWriteAccessForCollections }: CollectionsTable
const isLoading = !isDataAvailable && (listLoading || countLoading);
const loadError = listError || countError;

/**
* Deletes an array of collections by ids. Will alert individually for any deletion
* requests that fail.
*/
function onCollectionDelete(ids: string[]) {
const promises: Promise<Empty>[] = [];
ids.forEach((id) => {
const deletionPromise = deleteCollection(id).request.catch((err) => {
addToast(`Could not delete collection ${id}`, 'danger', err.message);
return Promise.reject(err);
});
promises.push(deletionPromise);
});

return Promise.allSettled(promises).then((promiseResults) => {
const totalDeleted = promiseResults.filter((res) => res.status === 'fulfilled').length;
const collectionText = pluralize('collection', ids.length);
function onCollectionDelete({ id, name }: CollectionResponse) {
const { request } = deleteCollection(id);

if (totalDeleted > 0 && totalDeleted === ids.length) {
// All collections deleted successfully
addToast(
`Successfully deleted ${totalDeleted} selected ${collectionText}`,
'success'
);
// Some, but not all, deletion requests failed
} else if (totalDeleted > 0) {
addToast(
`Deleted ${totalDeleted} of ${ids.length} selected ${collectionText}`,
'warning'
);
}

listRefetch();
countRefetch();
});
return request
.then(() => {
addToast(`Successfully deleted '${name}'`, 'success');
listRefetch();
countRefetch();
})
.catch((err) => {
addToast(`Could not delete collection '${name}'`, 'danger', err.message);
});
}

let pageContent = (
Expand Down