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

feat: [M3-6743] - Create Subnet Drawer #9652

Merged
merged 16 commits into from
Sep 13, 2023
Merged
Show file tree
Hide file tree
Changes from 12 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
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@linode/manager": Upcoming Features
---

Subnet Create Drawer ([#9652](https://github.com/linode/manager/pull/9652))
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ describe('MultipleSubnetInput', () => {
expect(props.onChange).toHaveBeenCalledWith([
...props.subnets,
{
ip: { ipv4: '10.0.0.0/24', ipv4Error: '', availIPv4s: 256 },
ip: { ipv4: '10.0.4.0/24', ipv4Error: '', availIPv4s: 256 },
label: '',
labelError: '',
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,4 +44,49 @@ describe('SubnetNode', () => {
const availIps = screen.queryByText('Available IP Addresses:');
expect(availIps).not.toBeInTheDocument();
});

it('should show a label and ip textfield inputs at minimum', () => {
renderWithTheme(
<SubnetNode
disabled={false}
onChange={() => {}}
subnet={{ ip: { ipv4: '' }, label: '' }}
/>
);

const label = screen.getByText('Subnet label');
expect(label).toBeInTheDocument();
const ipAddress = screen.getByText('Subnet IP Address Range');
expect(ipAddress).toBeInTheDocument();
});

it('should show a removeable button if isRemovable is true and subnet idx exists and is > 0', () => {
coliu-akamai marked this conversation as resolved.
Show resolved Hide resolved
renderWithTheme(
<SubnetNode
disabled={false}
idx={1}
isRemovable={true}
onChange={() => {}}
subnet={{ ip: { ipv4: '' }, label: '' }}
/>
);

const removeableButton = screen.getByTestId('delete-subnet-1');
coliu-akamai marked this conversation as resolved.
Show resolved Hide resolved
expect(removeableButton).toBeInTheDocument();
});

it('should not show a removeable button for a subnet with idx 0', () => {
coliu-akamai marked this conversation as resolved.
Show resolved Hide resolved
renderWithTheme(
<SubnetNode
disabled={false}
idx={0}
isRemovable={true}
onChange={() => {}}
subnet={{ ip: { ipv4: '' }, label: '' }}
/>
);

const removeableButton = screen.queryByTestId('delete-subnet-0');
coliu-akamai marked this conversation as resolved.
Show resolved Hide resolved
expect(removeableButton).not.toBeInTheDocument();
});
});
14 changes: 10 additions & 4 deletions packages/manager/src/features/VPCs/VPCCreate/SubnetNode.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,11 @@ import * as React from 'react';
import { Button } from 'src/components/Button/Button';
import { FormHelperText } from 'src/components/FormHelperText';
import { TextField } from 'src/components/TextField';
import { RESERVED_IP_NUMBER } from 'src/utilities/subnets';
import { SubnetFieldState } from 'src/utilities/subnets';
import { calculateAvailableIPv4s } from 'src/utilities/subnets';
import {
calculateAvailableIPv4s,
SubnetFieldState,
RESERVED_IP_NUMBER,
} from 'src/utilities/subnets';

interface Props {
disabled?: boolean;
Expand Down Expand Up @@ -53,13 +55,17 @@ export const SubnetNode = (props: Props) => {
return (
<Grid key={idx} sx={{ maxWidth: 460 }}>
<Grid container direction="row" spacing={2}>
<Grid xs={isRemovable ? 11 : 12}>
<Grid
xs={isRemovable ? 11 : 12}
sx={{ ...(!isRemovable && { width: '100%' }) }}
>
<TextField
disabled={disabled}
errorText={subnet.labelError}
inputId={`subnet-label-${idx}`}
label="Subnet label"
onChange={onLabelChange}
placeholder="Enter a subnet label"
value={subnet.label}
/>
</Grid>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,6 @@ describe('VPC create page', () => {
const { getAllByTestId } = renderWithTheme(<VPCCreate />);
const subnetIP = getAllByTestId('textfield-input');
expect(subnetIP[3]).toBeInTheDocument();
expect(subnetIP[3]).toHaveValue('10.0.0.0/24');
expect(subnetIP[3]).toHaveValue('10.0.4.0/24');
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
import * as React from 'react';

import { renderWithTheme } from 'src/utilities/testHelpers';

import { SubnetCreateDrawer } from './SubnetCreateDrawer';
import { fireEvent } from '@testing-library/react';

const props = {
onClose: jest.fn(),
open: true,
vpcId: 1,
};

describe('Create Subnet Drawer', () => {
it('should render title, label, ipv4 input, ipv4 availability, and action buttons', () => {
const { getByTestId, getByText } = renderWithTheme(
<SubnetCreateDrawer {...props} />
);

const drawerTitle = getByText('Create Subnet');
expect(drawerTitle).toBeVisible();

const label = getByText('Subnet label');
expect(label).toBeVisible();
expect(label).toBeEnabled();

const ipv4Input = getByText('Subnet IP Address Range');
expect(ipv4Input).toBeVisible();
expect(ipv4Input).toBeEnabled();

const saveButton = getByTestId('create-subnet-button');
expect(saveButton).toBeVisible();

const cancelBtn = getByText(/Cancel/);
expect(cancelBtn).toBeEnabled();
expect(cancelBtn).toBeVisible();
});

it('should close the drawer if the close cancel button is clicked', () => {
const { getByText } = renderWithTheme(<SubnetCreateDrawer {...props} />);

const cancelBtn = getByText(/Cancel/);
expect(cancelBtn).toBeEnabled();
expect(cancelBtn).toBeVisible();

fireEvent.click(cancelBtn);
expect(props.onClose).toHaveBeenCalled();
});
});
118 changes: 118 additions & 0 deletions packages/manager/src/features/VPCs/VPCDetail/SubnetCreateDrawer.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,118 @@
import { createSubnetSchema } from '@linode/validation';
import { useFormik } from 'formik';
import * as React from 'react';

import { ActionsPanel } from 'src/components/ActionsPanel/ActionsPanel';
import { Drawer } from 'src/components/Drawer';
import { Notice } from 'src/components/Notice/Notice';
import { useGrants, useProfile } from 'src/queries/profile';
import { useCreateSubnetMutation } from 'src/queries/vpcs';
import { getErrorMap } from 'src/utilities/errorUtils';
import {
DEFAULT_SUBNET_IPV4_VALUE,
SubnetFieldState,
} from 'src/utilities/subnets';

import { SubnetNode } from '../VPCCreate/SubnetNode';

interface Props {
onClose: () => void;
open: boolean;
vpcId: number;
}

export const SubnetCreateDrawer = (props: Props) => {
const { onClose, open, vpcId } = props;

const { data: profile } = useProfile();
const { data: grants } = useGrants();

const userCannotAddSubnet = profile?.restricted && !grants?.global.add_vpcs;

const [errorMap, setErrorMap] = React.useState<
Record<string, string | undefined>
>({});

const {
isLoading,
mutateAsync: createSubnet,
reset,
} = useCreateSubnetMutation(vpcId);

const onCreateSubnet = async () => {
try {
await createSubnet({ label: values.label, ipv4: values.ip.ipv4 });
onClose();
} catch (errors) {
const newErrors = getErrorMap(['label', 'ipv4'], errors);
setErrorMap(newErrors);
setValues({
label: values.label,
labelError: newErrors.label,
ip: {
...values.ip,
ipv4Error: newErrors.ipv4,
},
Copy link
Contributor

Choose a reason for hiding this comment

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

When we get a general error (would end up being stored on errorMap.none since it doesn't belong to a particular field -- you can try blocking network requests to *subnet* to replicate this), it's persisting between drawer openings and closings currently.

Might have to do something like setErrorMap({}) in the useEffect(), although I'm not sure why resetForm() isn't taking care of this aspect

Copy link
Contributor Author

Choose a reason for hiding this comment

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

thanks for catching, just pushed! Maybe it could be because I've a separate useState hook for the error map due to the SubnetNode, and Formik can't reset that with resetForm? Not too sure though, just a guess 💭

});
}
};

const { dirty, handleSubmit, resetForm, setValues, values } = useFormik({
enableReinitialize: true,
initialValues: {
// TODO VPC - add IPv6 when that is supported
label: '',
ip: {
ipv4: DEFAULT_SUBNET_IPV4_VALUE,
availIPv4s: 256,
},
} as SubnetFieldState,
onSubmit: onCreateSubnet,
validateOnBlur: false,
validateOnChange: false,
validationSchema: createSubnetSchema,
});
Comment on lines +47 to +74
Copy link
Contributor Author

@coliu-akamai coliu-akamai Sep 8, 2023

Choose a reason for hiding this comment

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

I have a version where I don't use the SubnetNode on one of my branches -- the code for error handling is a bit cleaner (+ we can keep the formik in the same shape as the CreateSubnetPayload) in that case in exchange for a more cluttered form (but both versions should have the same functionality, if SubnetNode is as flexible as I'd originally intended it to be way back during VPCCreate).


React.useEffect(() => {
if (open) {
resetForm();
reset();
}
}, [open]);

return (
<Drawer onClose={onClose} open={open} title={'Create Subnet'}>
{errorMap.none && <Notice text={errorMap.none} variant="error" />}
{userCannotAddSubnet && (
<Notice
text={
"You don't have permissions to create a new Subnet. Please contact an account administrator for details."
}
important
spacingTop={16}
variant="error"
/>
)}
<form onSubmit={handleSubmit}>
<SubnetNode
disabled={userCannotAddSubnet}
onChange={(subnetState) => {
setValues(subnetState);
}}
subnet={values}
/>
<ActionsPanel
primaryButtonProps={{
'data-testid': 'create-subnet-button',
disabled: !dirty || userCannotAddSubnet,
label: 'Create subnet',
loading: isLoading,
type: 'submit',
onClick: onCreateSubnet,
}}
secondaryButtonProps={{ label: 'Cancel', onClick: onClose }}
/>
</form>
</Drawer>
);
};
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ describe('VPC Subnets table', () => {
getByText('Linodes');
getByText(subnet.linodes.length);

const actionMenuButton = getAllByRole('button')[3];
const actionMenuButton = getAllByRole('button')[4];
fireEvent.click(actionMenuButton);

getByText('Assign Linode');
Expand All @@ -73,7 +73,7 @@ describe('VPC Subnets table', () => {

await waitForElementToBeRemoved(getByTestId(loadingTestId));

const expandTableButton = getAllByRole('button')[2];
const expandTableButton = getAllByRole('button')[3];
fireEvent.click(expandTableButton);
getByText('No Linodes');
});
Expand All @@ -91,7 +91,7 @@ describe('VPC Subnets table', () => {

await waitForElementToBeRemoved(getByTestId(loadingTestId));

const expandTableButton = getAllByRole('button')[2];
const expandTableButton = getAllByRole('button')[3];
fireEvent.click(expandTableButton);

getByText('Linode Label');
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
import { styled } from '@mui/material/styles';
import { styled, useTheme } from '@mui/material/styles';
import * as React from 'react';

import { Box } from 'src/components/Box';
import { Button } from 'src/components/Button/Button';
import { CircleProgress } from 'src/components/CircleProgress/CircleProgress';
import {
CollapsibleTable,
Expand All @@ -22,6 +24,7 @@ import { useOrder } from 'src/hooks/useOrder';
import { usePagination } from 'src/hooks/usePagination';
import { useSubnetsQuery } from 'src/queries/vpcs';

import { SubnetCreateDrawer } from './SubnetCreateDrawer';
import { SubnetLinodeRow, SubnetLinodeTableRowHead } from './SubnetLinodeRow';

interface Props {
Expand All @@ -31,7 +34,11 @@ interface Props {
const preferenceKey = 'vpc-subnets';

export const VPCSubnetsTable = ({ vpcId }: Props) => {
const theme = useTheme();
const [subnetsFilterText, setSubnetsFilterText] = React.useState('');
const [subnetCreateDrawerOpen, setSubnetCreateDrawerOpen] = React.useState(
false
);

const pagination = usePagination(1, preferenceKey);

Expand Down Expand Up @@ -170,14 +177,35 @@ export const VPCSubnetsTable = ({ vpcId }: Props) => {

return (
<>
<DebouncedSearchTextField
debounceTime={250}
hideLabel
isSearching={false}
label="Filter Subnets by label or id"
onSearch={handleSearch}
placeholder="Filter Subnets by label or id"
sx={{ paddingBottom: 2 }}
<Box
display="flex"
justifyContent="space-between"
paddingBottom={theme.spacing(2)}
>
<DebouncedSearchTextField
debounceTime={250}
hideLabel
isSearching={false}
label="Filter Subnets by label or id"
onSearch={handleSearch}
placeholder="Filter Subnets by label or id"
sx={{
[theme.breakpoints.up('sm')]: {
width: '416px',
},
}}
/>
<Button
buttonType="primary"
onClick={() => setSubnetCreateDrawerOpen(true)}
>
Create Subnet
</Button>
</Box>
<SubnetCreateDrawer
onClose={() => setSubnetCreateDrawerOpen(false)}
open={subnetCreateDrawerOpen}
vpcId={vpcId}
/>
<CollapsibleTable
TableItems={getTableItems()}
Expand Down
4 changes: 4 additions & 0 deletions packages/manager/src/mocks/serverHandlers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -454,6 +454,10 @@ const vpc = [
const vpc = vpcFactory.build({ ...(req.body as any) });
return res(ctx.json(vpc));
}),
rest.post('*/vpcs/:vpcId/subnets', (req, res, ctx) => {
const subnet = subnetFactory.build({ ...(req.body as any) });
return res(ctx.json(subnet));
}),
];

const nanodeType = linodeTypeFactory.build({ id: 'g6-nanode-1' });
Expand Down
2 changes: 1 addition & 1 deletion packages/manager/src/utilities/subnets.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { determineIPType, vpcsValidateIP } from '@linode/validation';

export const DEFAULT_SUBNET_IPV4_VALUE = '10.0.0.0/24';
export const DEFAULT_SUBNET_IPV4_VALUE = '10.0.4.0/24';
Copy link
Contributor Author

Choose a reason for hiding this comment

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

UI mocks updated to use 10.0.4.0/24 as default subnet ip value, so changed it here (+ had to update a lot of related tests)

export const RESERVED_IP_NUMBER = 4;

// VPC: TODO - added ipv6 related fields here, but they will not be used until VPCs support ipv6
Expand Down
Loading