From 42137a08f06e3e03f20caa03abe656f18685707b Mon Sep 17 00:00:00 2001 From: Alban Bailly <130582365+abailly-akamai@users.noreply.github.com> Date: Fri, 16 Feb 2024 11:24:28 -0500 Subject: [PATCH] upcoming: [M3-7618] - Delete Placement Group Modal (#10162) * Initial commit: save work * Wrap up comment and add test * Cleanup * Error handling * Cleanup and more error handling * Add linode list * Add unassign logic * error handling * Test * Restore initial mock data * Cleanup * Test and story for changes in removable selection list * Added changeset: Add Delete Placement Group Modal * Invalidate related linode when removed from PG * Feedback --- ...r-10162-upcoming-features-1708036002117.md | 5 + .../RemovableSelectionsList.stories.tsx | 46 +++++ .../RemovableSelectionsList.test.tsx | 12 ++ .../RemovableSelectionsList.tsx | 40 +++-- .../TypeToConfirmDialog.tsx | 3 + .../PlacementGroupsDeleteModal.test.tsx | 125 ++++++++++++++ .../PlacementGroupsDeleteModal.tsx | 163 ++++++++++++++++++ .../PlacementGroupsDetail.tsx | 2 +- .../PlacementGroupsLanding.tsx | 9 +- .../PlacementGroupsRow.tsx | 2 +- .../PlacementGroupsUnassignModal.tsx | 6 +- .../src/features/PlacementGroups/index.tsx | 5 + packages/manager/src/mocks/serverHandlers.ts | 2 +- .../manager/src/queries/placementGroups.ts | 19 +- 14 files changed, 398 insertions(+), 41 deletions(-) create mode 100644 packages/manager/.changeset/pr-10162-upcoming-features-1708036002117.md create mode 100644 packages/manager/src/features/PlacementGroups/PlacementGroupsDeleteModal.test.tsx create mode 100644 packages/manager/src/features/PlacementGroups/PlacementGroupsDeleteModal.tsx diff --git a/packages/manager/.changeset/pr-10162-upcoming-features-1708036002117.md b/packages/manager/.changeset/pr-10162-upcoming-features-1708036002117.md new file mode 100644 index 00000000000..858fe9b9d8b --- /dev/null +++ b/packages/manager/.changeset/pr-10162-upcoming-features-1708036002117.md @@ -0,0 +1,5 @@ +--- +"@linode/manager": Upcoming Features +--- + +Add Delete Placement Group Modal ([#10162](https://github.com/linode/manager/pull/10162)) diff --git a/packages/manager/src/components/RemovableSelectionsList/RemovableSelectionsList.stories.tsx b/packages/manager/src/components/RemovableSelectionsList/RemovableSelectionsList.stories.tsx index 81d5ee88869..300f3e4486b 100644 --- a/packages/manager/src/components/RemovableSelectionsList/RemovableSelectionsList.stories.tsx +++ b/packages/manager/src/components/RemovableSelectionsList/RemovableSelectionsList.stories.tsx @@ -110,6 +110,52 @@ export const CustomHeightAndWidth: Story = { ), }; +/** + * Example of a RemovableSelectionsList with no data to remove + */ +export const WithReadableRemoveCTA: Story = { + render: () => { + const SpecifiedLabelWrapper = () => { + const [data, setData] = React.useState(diffLabelListItems); + + const handleRemove = (item: RemovableItem) => { + setData([...data].filter((data) => data.id !== item.id)); + }; + + const resetList = () => { + setData([...diffLabelListItems]); + }; + + return ( + <> + ( + + )} + headerText="Linodes to remove" + noDataText="No Linodes available" + onRemove={handleRemove} + selectionData={data} + /> + + + ); + }; + + return ; + }, +}; + /** * Example of a RemovableSelectionsList with no data to remove */ diff --git a/packages/manager/src/components/RemovableSelectionsList/RemovableSelectionsList.test.tsx b/packages/manager/src/components/RemovableSelectionsList/RemovableSelectionsList.test.tsx index 9eae742e28a..89bbf72f9d4 100644 --- a/packages/manager/src/components/RemovableSelectionsList/RemovableSelectionsList.test.tsx +++ b/packages/manager/src/components/RemovableSelectionsList/RemovableSelectionsList.test.tsx @@ -3,6 +3,7 @@ import * as React from 'react'; import { renderWithTheme } from 'src/utilities/testHelpers'; +import { Button } from '../Button/Button'; import { RemovableSelectionsList } from './RemovableSelectionsList'; const defaultList = Array.from({ length: 5 }, (_, index) => { @@ -89,4 +90,15 @@ describe('Removable Selections List', () => { const removeButton = screen.queryByLabelText(`remove my-linode-1`); expect(removeButton).not.toBeInTheDocument(); }); + + it('should render the remove button as text when removeButtonText is declared', () => { + const { queryAllByText } = renderWithTheme( + } + isRemovable + /> + ); + expect(queryAllByText('Remove Linode')).toHaveLength(5); + }); }); diff --git a/packages/manager/src/components/RemovableSelectionsList/RemovableSelectionsList.tsx b/packages/manager/src/components/RemovableSelectionsList/RemovableSelectionsList.tsx index ec09036a215..9bcf7c8ad12 100644 --- a/packages/manager/src/components/RemovableSelectionsList/RemovableSelectionsList.tsx +++ b/packages/manager/src/components/RemovableSelectionsList/RemovableSelectionsList.tsx @@ -15,6 +15,7 @@ import { } from './RemovableSelectionsList.style'; import type { SxProps, Theme } from '@mui/material'; +import type { ButtonProps } from 'src/components/Button/Button'; export type RemovableItem = { id: number; @@ -29,6 +30,11 @@ export interface RemovableSelectionsListProps { * The custom label component */ LabelComponent?: React.ComponentType<{ selection: RemovableItem }>; + /** + * Overrides the render of the X Button + * Has no effect if isRemovable is false + */ + RemoveButton?: (props: ButtonProps) => JSX.Element; /** * The descriptive text to display above the list */ @@ -78,6 +84,7 @@ export const RemovableSelectionsList = ( ) => { const { LabelComponent, + RemoveButton, headerText, id, isRemovable = true, @@ -115,9 +122,9 @@ export const RemovableSelectionsList = ( > {selectionData.map((selection) => ( @@ -130,20 +137,23 @@ export const RemovableSelectionsList = ( selection.label )} - {isRemovable && ( - handleOnClick(selection)} - size="medium" - > - - - )} + {isRemovable && + (RemoveButton ? ( + handleOnClick(selection)} /> + ) : ( + handleOnClick(selection)} + size="medium" + > + + + ))} ))} diff --git a/packages/manager/src/components/TypeToConfirmDialog/TypeToConfirmDialog.tsx b/packages/manager/src/components/TypeToConfirmDialog/TypeToConfirmDialog.tsx index bd6a89ab5c7..883a2d5cc48 100644 --- a/packages/manager/src/components/TypeToConfirmDialog/TypeToConfirmDialog.tsx +++ b/packages/manager/src/components/TypeToConfirmDialog/TypeToConfirmDialog.tsx @@ -25,6 +25,7 @@ interface EntityInfo { | 'Linode' | 'Load Balancer' | 'NodeBalancer' + | 'Placement Group' | 'Subnet' | 'VPC' | 'Volume'; @@ -49,6 +50,7 @@ export const TypeToConfirmDialog = (props: CombinedProps) => { children, entity, errors, + inputProps, label, loading, onClick, @@ -120,6 +122,7 @@ export const TypeToConfirmDialog = (props: CombinedProps) => { data-testid={'dialog-confirm-text-input'} expand hideInstructions={entity.subType === 'CloseAccount'} + inputProps={inputProps} label={label} placeholder={entity.subType === 'CloseAccount' ? 'Username' : ''} textFieldStyle={textFieldStyle} diff --git a/packages/manager/src/features/PlacementGroups/PlacementGroupsDeleteModal.test.tsx b/packages/manager/src/features/PlacementGroups/PlacementGroupsDeleteModal.test.tsx new file mode 100644 index 00000000000..929e4f21f55 --- /dev/null +++ b/packages/manager/src/features/PlacementGroups/PlacementGroupsDeleteModal.test.tsx @@ -0,0 +1,125 @@ +import { fireEvent } from '@testing-library/react'; +import * as React from 'react'; + +import { linodeFactory, placementGroupFactory } from 'src/factories'; +import { renderWithTheme } from 'src/utilities/testHelpers'; + +import { PlacementGroupsDeleteModal } from './PlacementGroupsDeleteModal'; + +const queryMocks = vi.hoisted(() => ({ + useAllLinodesQuery: vi.fn().mockReturnValue({}), + useDeletePlacementGroup: vi.fn().mockReturnValue({ + mutateAsync: vi.fn().mockResolvedValue({}), + reset: vi.fn(), + }), + useParams: vi.fn().mockReturnValue({}), + usePlacementGroupQuery: vi.fn().mockReturnValue({}), +})); + +vi.mock('react-router-dom', async () => { + const actual = await vi.importActual('react-router-dom'); + return { + ...actual, + useParams: queryMocks.useParams, + }; +}); + +vi.mock('src/queries/placementGroups', async () => { + const actual = await vi.importActual('src/queries/placementGroups'); + return { + ...actual, + useDeletePlacementGroup: queryMocks.useDeletePlacementGroup, + usePlacementGroupQuery: queryMocks.usePlacementGroupQuery, + }; +}); + +vi.mock('src/queries/linodes/linodes', async () => { + const actual = await vi.importActual('src/queries/linodes/linodes'); + return { + ...actual, + useAllLinodesQuery: queryMocks.useAllLinodesQuery, + }; +}); + +const props = { + onClose: vi.fn(), + open: true, +}; + +describe('PlacementGroupsDeleteModal', () => { + beforeAll(() => { + queryMocks.useParams.mockReturnValue({ + id: '1', + }); + queryMocks.useAllLinodesQuery.mockReturnValue({ + data: [ + linodeFactory.build({ + id: 1, + label: 'test-linode', + }), + ], + }); + }); + + it('should render the right form elements', () => { + queryMocks.usePlacementGroupQuery.mockReturnValue({ + data: placementGroupFactory.build({ + affinity_type: 'anti_affinity', + id: 1, + label: 'PG-to-delete', + linode_ids: [1], + }), + }); + + const { getByRole, getByTestId, getByText } = renderWithTheme( + + ); + + expect( + getByRole('heading', { + name: 'Delete Placement Group PG-to-delete (Anti-affinity)', + }) + ).toBeInTheDocument(); + expect( + getByText( + 'Linodes assigned to Placement Group PG-to-delete (Anti-affinity)' + ) + ).toBeInTheDocument(); + expect(getByTestId('assigned-linodes')).toContainElement( + getByText('test-linode') + ); + expect(getByTestId('textfield-input')).toBeDisabled(); + expect(getByRole('button', { name: 'Close' })).toBeInTheDocument(); + expect(getByRole('button', { name: 'Cancel' })).toBeInTheDocument(); + expect(getByRole('button', { name: 'Delete' })).toBeDisabled(); + }); + + it("should be enabled when there's no assigned linodes", () => { + queryMocks.usePlacementGroupQuery.mockReturnValue({ + data: placementGroupFactory.build({ + affinity_type: 'anti_affinity', + id: 1, + label: 'PG-to-delete', + linode_ids: [], + }), + }); + const { getByRole, getByTestId, getByText } = renderWithTheme( + + ); + + expect(getByText('No Linodes assigned to this Placement Group.')); + + const textField = getByTestId('textfield-input'); + const deleteButton = getByRole('button', { name: 'Delete' }); + + expect(textField).toBeEnabled(); + expect(deleteButton).toBeDisabled(); + + fireEvent.change(textField, { target: { value: 'PG-to-delete' } }); + + expect(deleteButton).toBeEnabled(); + fireEvent.click(deleteButton); + + expect(queryMocks.useDeletePlacementGroup).toHaveBeenCalled(); + }); +}); diff --git a/packages/manager/src/features/PlacementGroups/PlacementGroupsDeleteModal.tsx b/packages/manager/src/features/PlacementGroups/PlacementGroupsDeleteModal.tsx new file mode 100644 index 00000000000..af7c99be0f4 --- /dev/null +++ b/packages/manager/src/features/PlacementGroups/PlacementGroupsDeleteModal.tsx @@ -0,0 +1,163 @@ +import { AFFINITY_TYPES } from '@linode/api-v4'; +import * as React from 'react'; +import { useParams } from 'react-router-dom'; + +import { Button } from 'src/components/Button/Button'; +import { List } from 'src/components/List'; +import { ListItem } from 'src/components/ListItem'; +import { Notice } from 'src/components/Notice/Notice'; +import { RemovableSelectionsList } from 'src/components/RemovableSelectionsList/RemovableSelectionsList'; +import { TypeToConfirmDialog } from 'src/components/TypeToConfirmDialog/TypeToConfirmDialog'; +import { Typography } from 'src/components/Typography'; +import { usePlacementGroupData } from 'src/hooks/usePlacementGroupsData'; +import { + useDeletePlacementGroup, + usePlacementGroupQuery, + useUnassignLinodesFromPlacementGroup, +} from 'src/queries/placementGroups'; + +import type { + Linode, + UnassignLinodesFromPlacementGroupPayload, +} from '@linode/api-v4'; + +interface Props { + onClose: () => void; + open: boolean; +} + +export const PlacementGroupsDeleteModal = (props: Props) => { + const { onClose, open } = props; + const { id } = useParams<{ id: string }>(); + const { data: selectedPlacementGroup } = usePlacementGroupQuery(+id); + const { + assignedLinodes, + isLoading: placementGroupDataLoading, + linodesCount: assignedLinodesCount, + } = usePlacementGroupData({ + placementGroup: selectedPlacementGroup, + }); + const { + error: deletePlacementError, + isLoading: deletePlacementLoading, + mutateAsync: deletePlacementGroup, + reset: resetDeletePlacementGroup, + } = useDeletePlacementGroup(selectedPlacementGroup?.id ?? -1); + const { + error: unassignLinodeError, + isLoading: unassignLinodeLoading, + mutateAsync: unassignLinodes, + reset: resetUnassignLinodes, + } = useUnassignLinodesFromPlacementGroup(selectedPlacementGroup?.id ?? -1); + + const error = deletePlacementError || unassignLinodeError; + + React.useEffect(() => { + if (open) { + resetDeletePlacementGroup(); + resetUnassignLinodes(); + } + }, [open, resetUnassignLinodes, resetDeletePlacementGroup]); + + const handleUnassignLinode = async (linode: Linode) => { + const payload: UnassignLinodesFromPlacementGroupPayload = { + linodes: [linode.id], + }; + + await unassignLinodes(payload); + }; + + const onDelete = async () => { + await deletePlacementGroup(); + onClose(); + }; + + const placementGroupLabel = selectedPlacementGroup + ? `Placement Group ${selectedPlacementGroup?.label} (${ + AFFINITY_TYPES[selectedPlacementGroup.affinity_type] + })` + : 'Placement Group'; + + const isDisabled = !selectedPlacementGroup || assignedLinodesCount > 0; + + return ( + + {error && ( + + )} + + + + Warning: + + ({ + '& > li': { + display: 'list-item', + fontSize: '0.875rem', + pb: 0, + pl: 0, + }, + listStyle: 'disc', + ml: theme.spacing(2), + mt: theme.spacing(), + })} + > + + Deleting a placement group is permanent and cannot be undone. + + + You need to unassign all Linodes before deleting a placement group. + + + + ( + + )} + headerText={`Linodes assigned to ${placementGroupLabel}`} + id="assigned-linodes" + maxWidth={540} + noDataText="No Linodes assigned to this Placement Group." + onRemove={handleUnassignLinode} + selectionData={assignedLinodes ?? []} + sx={{ mb: 3, mt: 1 }} + /> + + ); +}; diff --git a/packages/manager/src/features/PlacementGroups/PlacementGroupsDetail/PlacementGroupsDetail.tsx b/packages/manager/src/features/PlacementGroups/PlacementGroupsDetail/PlacementGroupsDetail.tsx index 8cddb981a5d..ff2e103a72b 100644 --- a/packages/manager/src/features/PlacementGroups/PlacementGroupsDetail/PlacementGroupsDetail.tsx +++ b/packages/manager/src/features/PlacementGroups/PlacementGroupsDetail/PlacementGroupsDetail.tsx @@ -26,7 +26,7 @@ export const PlacementGroupsDetail = () => { const flags = useFlags(); const { id, tab } = useParams<{ id: string; tab?: string }>(); const history = useHistory(); - const placementGroupId = Number(id); + const placementGroupId = +id; const { data: placementGroup, diff --git a/packages/manager/src/features/PlacementGroups/PlacementGroupsLanding/PlacementGroupsLanding.tsx b/packages/manager/src/features/PlacementGroups/PlacementGroupsLanding/PlacementGroupsLanding.tsx index df3e582ed2e..9728225396e 100644 --- a/packages/manager/src/features/PlacementGroups/PlacementGroupsLanding/PlacementGroupsLanding.tsx +++ b/packages/manager/src/features/PlacementGroups/PlacementGroupsLanding/PlacementGroupsLanding.tsx @@ -22,6 +22,7 @@ import { usePlacementGroupsQuery } from 'src/queries/placementGroups'; import { getAPIErrorOrDefault } from 'src/utilities/errorUtils'; import { PlacementGroupsCreateDrawer } from '../PlacementGroupsCreateDrawer'; +import { PlacementGroupsDeleteModal } from '../PlacementGroupsDeleteModal'; import { PlacementGroupsRenameDrawer } from '../PlacementGroupsRenameDrawer'; import { PlacementGroupsLandingEmptyState } from './PlacementGroupsLandingEmptyState'; import { PlacementGroupsRow } from './PlacementGroupsRow'; @@ -75,15 +76,16 @@ export const PlacementGroupsLanding = React.memo(() => { const handleDeletePlacementGroup = (placementGroup: PlacementGroup) => { setSelectedPlacementGroup(placementGroup); + history.replace(`/placement-groups/delete/${placementGroup.id}`); }; const onClosePlacementGroupDrawer = () => { history.replace('/placement-groups'); - setSelectedPlacementGroup(undefined); }; const isPlacementGroupCreateDrawerOpen = location.pathname.endsWith('create'); const isPlacementGroupRenameDrawerOpen = location.pathname.includes('rename'); + const isPlacementGroupDeleteModalOpen = location.pathname.includes('delete'); if (isLoading) { return ; @@ -206,7 +208,10 @@ export const PlacementGroupsLanding = React.memo(() => { open={isPlacementGroupRenameDrawerOpen} selectedPlacementGroup={selectedPlacementGroup} /> - {/* TODO VM_Placement: add delete dialog */} + ); }); diff --git a/packages/manager/src/features/PlacementGroups/PlacementGroupsLanding/PlacementGroupsRow.tsx b/packages/manager/src/features/PlacementGroups/PlacementGroupsLanding/PlacementGroupsRow.tsx index d8bc510db5a..55d778aae5f 100644 --- a/packages/manager/src/features/PlacementGroups/PlacementGroupsLanding/PlacementGroupsRow.tsx +++ b/packages/manager/src/features/PlacementGroups/PlacementGroupsLanding/PlacementGroupsRow.tsx @@ -1,9 +1,9 @@ import { AFFINITY_TYPES } from '@linode/api-v4'; import React from 'react'; -import { Link } from 'react-router-dom'; import { Hidden } from 'src/components/Hidden'; import { InlineMenuAction } from 'src/components/InlineMenuAction/InlineMenuAction'; +import { Link } from 'src/components/Link'; import { List } from 'src/components/List'; import { ListItem } from 'src/components/ListItem'; import { TableCell } from 'src/components/TableCell'; diff --git a/packages/manager/src/features/PlacementGroups/PlacementGroupsUnassignModal.tsx b/packages/manager/src/features/PlacementGroups/PlacementGroupsUnassignModal.tsx index 9f6093f7cc5..3d1ac69bb1c 100644 --- a/packages/manager/src/features/PlacementGroups/PlacementGroupsUnassignModal.tsx +++ b/packages/manager/src/features/PlacementGroups/PlacementGroupsUnassignModal.tsx @@ -25,11 +25,11 @@ export const PlacementGroupsUnassignModal = (props: Props) => { error, isLoading, mutateAsync: unassignLinodes, - } = useUnassignLinodesFromPlacementGroup(Number(placementGroupId) ?? -1); - const { data: selectedLinode } = useLinodeQuery(Number(linodeId) ?? -1); + } = useUnassignLinodesFromPlacementGroup(+placementGroupId ?? -1); + const { data: selectedLinode } = useLinodeQuery(+linodeId ?? -1); const payload: UnassignLinodesFromPlacementGroupPayload = { - linodes: [Number(linodeId) ?? -1], + linodes: [+linodeId ?? -1], }; const onUnassign = async () => { diff --git a/packages/manager/src/features/PlacementGroups/index.tsx b/packages/manager/src/features/PlacementGroups/index.tsx index 5a94d394c1c..b8e2e1f3b2b 100644 --- a/packages/manager/src/features/PlacementGroups/index.tsx +++ b/packages/manager/src/features/PlacementGroups/index.tsx @@ -36,6 +36,11 @@ export const PlacementGroups = () => { exact path={`${path}/rename/:id`} /> + { - if (req.params.placementGroupId === 'undefined') { + if (req.params.placementGroupId === '-1') { return res(ctx.status(404)); } diff --git a/packages/manager/src/queries/placementGroups.ts b/packages/manager/src/queries/placementGroups.ts index 3f2e1f26108..a1952724458 100644 --- a/packages/manager/src/queries/placementGroups.ts +++ b/packages/manager/src/queries/placementGroups.ts @@ -17,7 +17,6 @@ import { useMutation, useQuery, useQueryClient } from 'react-query'; import { getAll } from 'src/utilities/getAll'; -import { queryKey as LINODES_QUERY_KEY } from './linodes/linodes'; import { queryKey as PROFILE_QUERY_KEY } from './profile'; import type { @@ -119,23 +118,7 @@ export const useAssignLinodesToPlacementGroup = (placementGroupId: number) => { >({ mutationFn: (data) => assignLinodesToPlacementGroup(placementGroupId, data), onSuccess: (updatedPlacementGroup) => { - // Invalidate placement group linodes - queryClient.invalidateQueries([ - queryKey, - 'placement-group', - placementGroupId, - 'linodes', - ]); - - // Invalidate linode placement group data - queryClient.invalidateQueries([ - LINODES_QUERY_KEY, - 'linode', - updatedPlacementGroup.linode_ids[0], - 'placement_groups', - ]); - - // Set the updated placement group + queryClient.invalidateQueries([queryKey, 'paginated']); queryClient.setQueryData( [queryKey, 'placement-group', placementGroupId], updatedPlacementGroup