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

refactor: [M3-8877] - Refactor VPC Create to use react-hook-form #11357

Merged
merged 17 commits into from
Dec 11, 2024
Merged
Show file tree
Hide file tree
Changes from 15 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": Tech Stories
---

Refactor VPC Create to use `react-hook-form` instead of `formik` ([#11357](https://github.com/linode/manager/pull/11357))
18 changes: 11 additions & 7 deletions packages/manager/cypress/e2e/core/vpc/vpc-create.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,9 @@ describe('VPC create flow', () => {
subnets: mockSubnets,
});

const ipValidationErrorMessage = 'The IPv4 range must be in CIDR format';
const ipValidationErrorMessage1 =
'A subnet must have either IPv4 or IPv6, or both, but not neither.';
Copy link
Contributor Author

@coliu-akamai coliu-akamai Dec 9, 2024

Choose a reason for hiding this comment

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

This error now surfaces bc of how the VPC's validation schema is set up. However, wondering if it's better to match the original erroring behavior, especially since we don't support ipv6 in VPCs currently - ?

update: adding a temporary IPv4 required message (+ todo comments to change back when IPv6 is supported) to hide mention of IPv6, since we don't show any IPv6 field/IPv6 related stuff when creating a VPC (felt like it could be confusing that we mention ipv6s)

const ipValidationErrorMessage2 = 'The IPv4 range must be in CIDR format.';
const vpcCreationErrorMessage = 'An unknown error has occurred.';
const totalSubnetUniqueLinodes = getUniqueLinodesFromSubnets(mockSubnets);

Expand Down Expand Up @@ -111,7 +113,7 @@ describe('VPC create flow', () => {
.should('be.enabled')
.click();

cy.findByText(ipValidationErrorMessage).should('be.visible');
cy.findByText(ipValidationErrorMessage1).should('be.visible');

// Enter a random non-IP address string to further test client side validation.
cy.findByText('Subnet IP Address Range')
Expand All @@ -126,7 +128,7 @@ describe('VPC create flow', () => {
.should('be.enabled')
.click();

cy.findByText(ipValidationErrorMessage).should('be.visible');
cy.findByText(ipValidationErrorMessage2).should('be.visible');

// Enter a valid IP address with an invalid network prefix to further test client side validation.
cy.findByText('Subnet IP Address Range')
Expand All @@ -141,7 +143,7 @@ describe('VPC create flow', () => {
.should('be.enabled')
.click();

cy.findByText(ipValidationErrorMessage).should('be.visible');
cy.findByText(ipValidationErrorMessage2).should('be.visible');

// Replace invalid IP address range with valid range.
cy.findByText('Subnet IP Address Range')
Expand Down Expand Up @@ -180,10 +182,12 @@ describe('VPC create flow', () => {
getSubnetNodeSection(1)
.should('be.visible')
.within(() => {
cy.findByText('Label is required').should('be.visible');
cy.findByText('Label must be between 1 and 64 characters.').should(
'be.visible'
);

// Delete subnet.
cy.findByLabelText('Remove Subnet')
cy.findByLabelText('Remove Subnet 1')
.should('be.visible')
.should('be.enabled')
.click();
Expand Down Expand Up @@ -300,7 +304,7 @@ describe('VPC create flow', () => {
getSubnetNodeSection(0)
.should('be.visible')
.within(() => {
cy.findByLabelText('Remove Subnet')
cy.findByLabelText('Remove Subnet 0')
.should('be.visible')
.should('be.enabled')
.click();
Expand Down
Original file line number Diff line number Diff line change
@@ -1,24 +1,22 @@
import * as React from 'react';

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

import { SubnetContent } from './SubnetContent';

const props = {
disabled: false,
onChangeField: vi.fn(),
subnets: [
{
ip: { ipv4: '', ipv4Error: '' },
label: '',
labelError: '',
},
],
};

describe('Subnet form content', () => {
it('renders the subnet content correctly', () => {
const { getByText } = renderWithTheme(<SubnetContent {...props} />);
const { getByText } = renderWithThemeAndHookFormContext({
component: <SubnetContent />,
useFormOptions: {
defaultValues: {
description: '',
label: '',
region: '',
subnets: [{ ipv4: '', label: '' }],
},
},
});

getByText('Subnets');
getByText('Subnet Label');
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { Notice } from '@linode/ui';
import * as React from 'react';
import { useFormContext } from 'react-hook-form';
import { useLocation } from 'react-router-dom';

import { Link } from 'src/components/Link';
Expand All @@ -13,28 +14,28 @@ import {
StyledHeaderTypography,
} from './VPCCreateForm.styles';

import type { APIError } from '@linode/api-v4';
import type { CreateVPCPayload } from '@linode/api-v4';
import type { LinodeCreateType } from 'src/features/Linodes/LinodeCreate/types';
import type { LinodeCreateQueryParams } from 'src/features/Linodes/types';
import type { SubnetFieldState } from 'src/utilities/subnets';

interface Props {
disabled?: boolean;
isDrawer?: boolean;
onChangeField: (field: string, value: SubnetFieldState[]) => void;
subnetErrors?: APIError[];
subnets: SubnetFieldState[];
}

export const SubnetContent = (props: Props) => {
const { disabled, isDrawer, onChangeField, subnetErrors, subnets } = props;
const { disabled, isDrawer } = props;

const location = useLocation();
const isFromLinodeCreate = location.pathname.includes('/linodes/create');
const queryParams = getQueryParamsFromQueryString<LinodeCreateQueryParams>(
location.search
);

const {
formState: { errors },
} = useFormContext<CreateVPCPayload>();

return (
<>
<StyledHeaderTypography isDrawer={isDrawer} variant="h2">
Expand All @@ -59,22 +60,21 @@ export const SubnetContent = (props: Props) => {
</Link>
.
</StyledBodyTypography>
{subnetErrors
? subnetErrors.map((apiError: APIError) => (
<Notice
key={apiError.reason}
spacingBottom={8}
text={apiError.reason}
variant="error"
/>
))
: null}
<MultipleSubnetInput
disabled={disabled}
isDrawer={isDrawer}
onChange={(subnets) => onChangeField('subnets', subnets)}
subnets={subnets}
/>
{errors.root?.subnetLabel && (
<Notice
spacingBottom={8}
text={errors.root.subnetLabel.message}
variant="error"
/>
)}
{errors.root?.subnetIPv4 && (
<Notice
spacingBottom={8}
text={errors.root.subnetIPv4.message}
variant="error"
/>
)}
<MultipleSubnetInput disabled={disabled} isDrawer={isDrawer} />
</>
);
};
Original file line number Diff line number Diff line change
@@ -1,19 +1,26 @@
import * as React from 'react';

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

import { VPCTopSectionContent } from './VPCTopSectionContent';

const props = {
errors: {},
onChangeField: vi.fn(),
regions: [],
values: { description: '', label: '', region: '', subnets: [] },
};

describe('VPC Top Section form content', () => {
it('renders the vpc top section form content correctly', () => {
const { getByText } = renderWithTheme(<VPCTopSectionContent {...props} />);
const { getByText } = renderWithThemeAndHookFormContext({
component: <VPCTopSectionContent {...props} />,
useFormOptions: {
defaultValues: {
description: '',
label: '',
region: '',
subnets: [],
},
},
});

getByText('Region');
getByText('VPC Label');
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { TextField } from '@linode/ui';
import * as React from 'react';
import { Controller, useFormContext } from 'react-hook-form';
import { useLocation } from 'react-router-dom';

import { Link } from 'src/components/Link';
Expand All @@ -10,29 +11,27 @@ import { getQueryParamsFromQueryString } from 'src/utilities/queryParams';
import { VPC_CREATE_FORM_VPC_HELPER_TEXT } from '../../constants';
import { StyledBodyTypography } from './VPCCreateForm.styles';

import type { CreateVPCPayload } from '@linode/api-v4';
import type { Region } from '@linode/api-v4';
import type { FormikErrors } from 'formik';
import type { LinodeCreateType } from 'src/features/Linodes/LinodeCreate/types';
import type { LinodeCreateQueryParams } from 'src/features/Linodes/types';
import type { CreateVPCFieldState } from 'src/hooks/useCreateVPC';

interface Props {
disabled?: boolean;
errors: FormikErrors<CreateVPCFieldState>;
isDrawer?: boolean;
onChangeField: (field: string, value: string) => void;
regions: Region[];
values: CreateVPCFieldState;
}

export const VPCTopSectionContent = (props: Props) => {
const { disabled, errors, isDrawer, onChangeField, regions, values } = props;
const { disabled, isDrawer, regions } = props;
const location = useLocation();
const isFromLinodeCreate = location.pathname.includes('/linodes/create');
const queryParams = getQueryParamsFromQueryString<LinodeCreateQueryParams>(
location.search
);

const { control } = useFormContext<CreateVPCPayload>();

return (
<>
<StyledBodyTypography isDrawer={isDrawer} variant="body1">
Expand All @@ -53,36 +52,53 @@ export const VPCTopSectionContent = (props: Props) => {
</Link>
.
</StyledBodyTypography>
<RegionSelect
aria-label="Choose a region"
currentCapability="VPCs"
disabled={isDrawer ? true : disabled}
errorText={errors.region}
onChange={(e, region) => onChangeField('region', region?.id ?? '')}
regions={regions}
value={values.region}
<Controller
render={({ field, fieldState }) => (
<RegionSelect
aria-label="Choose a region"
currentCapability="VPCs"
disabled={isDrawer ? true : disabled}
errorText={fieldState.error?.message}
onBlur={field.onBlur}
onChange={(_, region) => field.onChange(region?.id ?? '')}
regions={regions}
value={field.value}
/>
)}
control={control}
name="region"
/>
<TextField
onChange={(e: React.ChangeEvent<HTMLInputElement>) =>
onChangeField('label', e.target.value)
}
aria-label="Enter a label"
disabled={disabled}
errorText={errors.label}
label="VPC Label"
value={values.label}
<Controller
render={({ field, fieldState }) => (
<TextField
aria-label="Enter a label"
disabled={disabled}
errorText={fieldState.error?.message}
label="VPC Label"
onBlur={field.onBlur}
onChange={field.onChange}
value={field.value}
/>
)}
control={control}
name="label"
/>
<TextField
onChange={(e: React.ChangeEvent<HTMLInputElement>) =>
onChangeField('description', e.target.value)
}
disabled={disabled}
errorText={errors.description}
label="Description"
maxRows={1}
multiline
optional
value={values.description}
<Controller
render={({ field, fieldState }) => (
<TextField
disabled={disabled}
errorText={fieldState.error?.message}
label="Description"
maxRows={1}
multiline
onBlur={field.onBlur}
onChange={field.onChange}
optional
value={field.value}
/>
)}
control={control}
name="description"
/>
</>
);
Expand Down
Loading
Loading