Skip to content
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.

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

upcoming: [M3-7726] - Assign Linodes to Placement Group Drawer #10140

Merged
merged 22 commits into from
Feb 9, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
18 changes: 8 additions & 10 deletions packages/api-v4/src/placement-groups/placement-groups.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
import {
assignVMsToPlacementGroupSchema,
createPlacementGroupSchema,
unassignVMsFromPlacementGroupSchema,
renamePlacementGroupSchema,
} from '@linode/validation';
import { API_ROOT } from '../constants';
Expand All @@ -15,10 +13,10 @@ import Request, {
} from '../request';
import type { Filter, Params, ResourcePage as Page } from '../types';
import type {
AssignVMsToPlacementGroupPayload,
AssignLinodesToPlacementGroupPayload,
CreatePlacementGroupPayload,
PlacementGroup,
UnassignVMsFromPlacementGroupPayload,
UnassignLinodesFromPlacementGroupPayload,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've updated utils to read "linode(s)" instead of "VM(s)" for consistency since VM is more of of product name and linode is more clear in code

RenamePlacementGroupPayload,
} from './types';

Expand Down Expand Up @@ -109,9 +107,9 @@ export const deletePlacementGroup = (placementGroupId: number) =>
*
* @note While this accepts an array of Linode ids (future proofing), only one Linode id is supported at this time.
*/
export const assignVMsToPlacementGroup = (
export const assignLinodesToPlacementGroup = (
placementGroupId: number,
linodeIds: AssignVMsToPlacementGroupPayload
payload: AssignLinodesToPlacementGroupPayload
) =>
Request<PlacementGroup>(
setURL(
Expand All @@ -120,7 +118,7 @@ export const assignVMsToPlacementGroup = (
)}/assign`
),
setMethod('POST'),
setData(linodeIds, assignVMsToPlacementGroupSchema)
setData(payload)
);

/**
Expand All @@ -133,9 +131,9 @@ export const assignVMsToPlacementGroup = (
*
* @note While this accepts an array of Linode ids (future proofing), only one Linode id is supported at this time.
*/
export const unassignVMsFromPlacementGroup = (
export const unassignLinodesFromPlacementGroup = (
placementGroupId: number,
linodeIds: UnassignVMsFromPlacementGroupPayload
payload: UnassignLinodesFromPlacementGroupPayload
) =>
Request<PlacementGroup>(
setURL(
Expand All @@ -144,5 +142,5 @@ export const unassignVMsFromPlacementGroup = (
)}/unassign`
),
setMethod('POST'),
setData(linodeIds, unassignVMsFromPlacementGroupSchema)
setData(payload)
);
8 changes: 6 additions & 2 deletions packages/api-v4/src/placement-groups/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,5 +27,9 @@ export type RenamePlacementGroupPayload = Pick<PlacementGroup, 'label'>;
/**
* Since the API expects an array of ONE linode id, we'll use a tuple here.
*/
export type AssignVMsToPlacementGroupPayload = [number];
export type UnassignVMsFromPlacementGroupPayload = [number];
export type AssignLinodesToPlacementGroupPayload = {
linodes: [number];
};
export type UnassignLinodesFromPlacementGroupPayload = {
linodes: [number];
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@linode/manager": Upcoming Features
---

Add AssignLinodesToPlacementGroup drawer ([#10140](https://github.com/linode/manager/pull/10140))
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import Close from '@mui/icons-material/Close';
import * as React from 'react';

import { Box } from 'src/components/Box';
import { IconButton } from 'src/components/IconButton';

import {
Expand All @@ -13,6 +14,8 @@ import {
StyledScrollBox,
} from './RemovableSelectionsList.style';

import type { SxProps, Theme } from '@mui/material';

export type RemovableItem = {
id: number;
label: string;
Expand All @@ -29,7 +32,11 @@ export interface RemovableSelectionsListProps {
/**
* The descriptive text to display above the list
*/
headerText: string;
headerText: JSX.Element | string;
/**
* The id of the list component
*/
id?: string;
/**
* If false, hide the remove button
*/
Expand Down Expand Up @@ -60,6 +67,10 @@ export interface RemovableSelectionsListProps {
* The data to display in the list
*/
selectionData: RemovableItem[];
/**
* Additional styles to apply to the component
*/
sx?: SxProps<Theme>;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Improving the RemovableSelectionList to make it a tad more flexible

}

export const RemovableSelectionsList = (
Expand All @@ -68,13 +79,15 @@ export const RemovableSelectionsList = (
const {
LabelComponent,
headerText,
id,
isRemovable = true,
maxHeight = 427,
maxWidth = 416,
noDataText,
onRemove,
preferredDataLabel,
selectionData,
sx,
} = props;

// used to determine when to display a box-shadow to indicate scrollability
Expand All @@ -92,11 +105,12 @@ export const RemovableSelectionsList = (
};

return (
<>
<Box data-testid={id} sx={sx}>
<SelectedOptionsHeader>{headerText}</SelectedOptionsHeader>
{selectionData.length > 0 ? (
<StyledBoxShadowWrapper
displayShadow={listHeight > maxHeight}
id={id}
maxWidth={maxWidth}
>
<StyledScrollBox maxHeight={maxHeight} maxWidth={maxWidth}>
Expand Down Expand Up @@ -132,10 +146,10 @@ export const RemovableSelectionsList = (
</StyledScrollBox>
</StyledBoxShadowWrapper>
) : (
<StyledNoAssignedLinodesBox maxWidth={maxWidth}>
<StyledNoAssignedLinodesBox id={id} maxWidth={maxWidth}>
<StyledLabel>{noDataText}</StyledLabel>
</StyledNoAssignedLinodesBox>
)}
</>
</Box>
);
};
15 changes: 13 additions & 2 deletions packages/manager/src/factories/linodes.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import { placementGroupFactory } from './placementGroups';
import { RegionalNetworkUtilization } from '@linode/api-v4/lib/account';
import {
CreateLinodeRequest,
Expand All @@ -15,6 +14,8 @@ import {
} from '@linode/api-v4/lib/linodes/types';
import * as Factory from 'factory.ts';

import { placementGroupFactory } from './placementGroups';

export const linodeAlertsFactory = Factory.Sync.makeFactory<LinodeAlerts>({
cpu: 10,
io: 10000,
Expand Down Expand Up @@ -261,7 +262,17 @@ export const linodeFactory = Factory.Sync.makeFactory<Linode>({
ipv4: ['50.116.6.212', '192.168.203.1'],
ipv6: '2600:3c00::f03c:92ff:fee2:6c40/64',
label: Factory.each((i) => `linode-${i}`),
placement_groups: [placementGroupFactory.build()],
placement_groups: [
placementGroupFactory.build({
affinity_type: 'anti_affinity',
capacity: 10,
compliant: true,
id: 1,
label: 'test',
linode_ids: [1],
region: 'us-east',
}),
],
region: 'us-east',
specs: linodeSpecsFactory.build(),
status: 'running',
Expand Down
14 changes: 3 additions & 11 deletions packages/manager/src/factories/placementGroups.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,21 +8,13 @@ import type {
} from '@linode/api-v4';

export const placementGroupFactory = Factory.Sync.makeFactory<PlacementGroup>({
affinity_type: Factory.each(() => pickRandom(['affinity', 'anti_affinity'])),
affinity_type: 'anti_affinity',
capacity: 10,
compliant: Factory.each(() => pickRandom([true, false])),
id: Factory.each((id) => id),
label: Factory.each((id) => `pg-${id}`),
linode_ids: Factory.each(() => [
0,
pickRandom([1, 2, 3]),
pickRandom([4, 5, 6]),
pickRandom([7, 8, 9]),
43,
]),
region: Factory.each(() =>
pickRandom(['us-east', 'us-southeast', 'ca-central'])
),
linode_ids: [0, 1, 2, 3, 5, 6, 7, 8, 43],
region: 'us-east',
});

export const createPlacementGroupPayloadFactory = Factory.Sync.makeFactory<CreatePlacementGroupPayload>(
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,140 @@
import { fireEvent } from '@testing-library/react';
import * as React from 'react';

import {
linodeFactory,
placementGroupFactory,
regionFactory,
} from 'src/factories';
import { renderWithTheme } from 'src/utilities/testHelpers';

import { PlacementGroupsAssignLinodesDrawer } from './PlacementGroupsAssignLinodesDrawer';

const queryMocks = vi.hoisted(() => ({
useAllLinodesQuery: vi.fn().mockReturnValue({}),
useAssignLinodesToPlacementGroup: vi.fn().mockReturnValue({}),
useRegionsQuery: vi.fn().mockReturnValue({}),
useUnpaginatedPlacementGroupsQuery: vi.fn().mockReturnValue({}),
}));

vi.mock('src/queries/linodes/linodes', async () => {
const actual = await vi.importActual('src/queries/linodes/linodes');
return {
...actual,
useAllLinodesQuery: queryMocks.useAllLinodesQuery,
};
});

vi.mock('src/queries/placementGroups', async () => {
const actual = await vi.importActual('src/queries/placementGroups');
return {
...actual,
useUnpaginatedPlacementGroupsQuery:
queryMocks.useUnpaginatedPlacementGroupsQuery,
};
});

vi.mock('src/queries/regions', async () => {
const actual = await vi.importActual('src/queries/regions');
return {
...actual,
useRegionsQuery: queryMocks.useRegionsQuery,
};
});

vi.mock('src/queries/placementGroups', async () => {
const actual = await vi.importActual('src/queries/placementGroups');
return {
...actual,
useAssignLinodesToPlacementGroup:
queryMocks.useAssignLinodesToPlacementGroup,
};
});

describe('PlacementGroupsAssignLinodesDrawer', () => {
it('should render the error state', () => {
queryMocks.useAllLinodesQuery.mockReturnValue({
error: [{ reason: 'Not found' }],
});

const { getByText } = renderWithTheme(
<PlacementGroupsAssignLinodesDrawer
numberOfPlacementGroupsCreated={9}
onClose={vi.fn()}
open={true}
selectedPlacementGroup={placementGroupFactory.build()}
/>
);

expect(
getByText(
'There was a problem retrieving your placement group. Please try again'
)
).toBeInTheDocument();
});

it('should render the drawer components', () => {
queryMocks.useAllLinodesQuery.mockReturnValue({
data: [
linodeFactory.build({ id: 1, label: 'Linode-1', region: 'us-east' }),
linodeFactory.build({ id: 2, label: 'Linode-2', region: 'us-east' }),
linodeFactory.build({ id: 11, label: 'Linode-11', region: 'us-east' }),
],
});
queryMocks.useRegionsQuery.mockReturnValue(regionFactory.buildList(5));
queryMocks.useUnpaginatedPlacementGroupsQuery.mockReturnValue({
data: placementGroupFactory.build(),
});
queryMocks.useAssignLinodesToPlacementGroup.mockReturnValue(
placementGroupFactory.build({
linode_ids: [1, 2, 0, 1, 2, 3, 5, 6, 7, 8, 43, 11],
})
);

const {
getByPlaceholderText,
getByRole,
getByTestId,
getByText,
} = renderWithTheme(
<PlacementGroupsAssignLinodesDrawer
selectedPlacementGroup={placementGroupFactory.build({
affinity_type: 'anti_affinity',
label: 'PG-1',
region: 'us-east',
})}
onClose={vi.fn()}
open={true}
/>
);

const linodesSelect = getByPlaceholderText('Select a Linode');
const addLinodeButton = getByRole('button', { name: 'Add Linode' });
const removableLinodesList = getByTestId('pg-linode-removable-list');

expect(linodesSelect).toBeInTheDocument();
expect(addLinodeButton).toHaveAttribute('aria-disabled', 'true');
expect(removableLinodesList).toHaveTextContent(
'No Linodes have been assigned.'
);

fireEvent.focus(linodesSelect);
fireEvent.change(linodesSelect, { target: { value: 'Linode-11' } });
const optionElement = getByText('Linode-11');
fireEvent.click(optionElement);

expect(addLinodeButton).not.toHaveAttribute('aria-disabled', 'true');

fireEvent.click(getByRole('button', { name: 'Add Linode' }));

expect(addLinodeButton).toHaveAttribute('aria-disabled', 'true');
expect(removableLinodesList).toHaveTextContent('Linode-11');

const removeButton = getByRole('button', { name: 'remove Linode-11' });
fireEvent.click(removeButton);

expect(removableLinodesList).toHaveTextContent(
'No Linodes have been assigned.'
);
});
});
Loading
Loading