From b5be29400458a3d061b218513f9db8b7d17dd4be Mon Sep 17 00:00:00 2001 From: Connie Liu Date: Tue, 5 Nov 2024 17:24:21 -0500 Subject: [PATCH 01/17] begin refactoring VPC create hook --- packages/manager/src/hooks/useCreateVPCv2.ts | 110 +++++++++++++++++++ 1 file changed, 110 insertions(+) create mode 100644 packages/manager/src/hooks/useCreateVPCv2.ts diff --git a/packages/manager/src/hooks/useCreateVPCv2.ts b/packages/manager/src/hooks/useCreateVPCv2.ts new file mode 100644 index 00000000000..d0dd7d297d1 --- /dev/null +++ b/packages/manager/src/hooks/useCreateVPCv2.ts @@ -0,0 +1,110 @@ +import { yupResolver } from '@hookform/resolvers/yup'; +import { createVPCSchema } from '@linode/validation'; +import * as React from 'react'; +import { useForm } from 'react-hook-form'; +import { useHistory, useLocation } from 'react-router-dom'; + +import { useGrants, useProfile } from 'src/queries/profile/profile'; +import { useRegionsQuery } from 'src/queries/regions/regions'; +import { useCreateVPCMutation } from 'src/queries/vpcs/vpcs'; +import { sendLinodeCreateFormStepEvent } from 'src/utilities/analytics/formEventAnalytics'; +import { getQueryParamsFromQueryString } from 'src/utilities/queryParams'; +import { DEFAULT_SUBNET_IPV4_VALUE } from 'src/utilities/subnets'; + +import type { + APIError, + CreateSubnetPayload, + CreateVPCPayload, + VPC, +} from '@linode/api-v4'; +import type { LinodeCreateType } from 'src/features/Linodes/LinodeCreate/types'; + +// Custom hook to consolidate shared logic between VPCCreate.tsx and VPCCreateDrawer.tsx + +// DON'T FORGET TO PUT BACK IN SENDING THE ANALYTICAL EVENT GIRL + +export interface UseCreateVPCInputs { + handleSelectVPC?: (vpc: VPC) => void; + onDrawerClose?: () => void; + pushToVPCPage?: boolean; + selectedRegion?: string; +} + +export const useCreateVPC = (inputs: UseCreateVPCInputs) => { + const { + handleSelectVPC, + onDrawerClose, + pushToVPCPage, + selectedRegion, + } = inputs; + + const history = useHistory(); + const { data: profile } = useProfile(); + const { data: grants } = useGrants(); + const userCannotAddVPC = profile?.restricted && !grants?.global.add_vpcs; + + const location = useLocation(); + const isFromLinodeCreate = location.pathname.includes('/linodes/create'); + const queryParams = getQueryParamsFromQueryString(location.search); + + const { data: regions } = useRegionsQuery(); + const regionsData = regions ?? []; + + const { + isPending: isLoadingCreateVPC, + mutateAsync: createVPC, + } = useCreateVPCMutation(); + + const onCreateVPC = async (values: CreateVPCPayload) => { + try { + const vpc = await createVPC(values); + if (pushToVPCPage) { + history.push(`/vpcs/${vpc.id}`); + } else { + if (handleSelectVPC && onDrawerClose) { + handleSelectVPC(vpc); + onDrawerClose(); + } + } + + // Fire analytics form submit upon successful VPC creation from Linode Create flow. + if (isFromLinodeCreate) { + sendLinodeCreateFormStepEvent({ + createType: (queryParams.type as LinodeCreateType) ?? 'OS', + headerName: 'Create VPC', + interaction: 'click', + label: 'Create VPC', + }); + } + } catch (errors) { + for (const error of errors) { + form.setError(error?.field ?? 'root', { message: error.reason }); + } + } + }; + + const form = useForm({ + defaultValues: { + description: '', + label: '', + region: selectedRegion ?? '', + subnets: [ + { + ipv4: DEFAULT_SUBNET_IPV4_VALUE, + label: '', + }, + ], + }, + + mode: 'onBlur', + resolver: yupResolver(createVPCSchema), + }); + + return { + form, + isLoadingCreateVPC, + onCreateVPC, + regionsData, + userCannotAddVPC, + }; +}; From c1c94c5f74922c7a80099ea3a286eca79540c794 Mon Sep 17 00:00:00 2001 From: Connie Liu Date: Wed, 6 Nov 2024 17:30:06 -0500 Subject: [PATCH 02/17] save --- .../FormComponents/SubnetContent2.tsx | 86 +++++++++++++ .../FormComponents/VPCTopSectionContent2.tsx | 104 +++++++++++++++ .../VPCs/VPCCreate/MultipleSubnetInput2.tsx | 88 +++++++++++++ .../features/VPCs/VPCCreate/SubnetNode2.tsx | 120 ++++++++++++++++++ .../src/features/VPCs/VPCCreate/VPCCreate.tsx | 35 +++-- packages/manager/src/hooks/useCreateVPCv2.ts | 8 +- 6 files changed, 415 insertions(+), 26 deletions(-) create mode 100644 packages/manager/src/features/VPCs/VPCCreate/FormComponents/SubnetContent2.tsx create mode 100644 packages/manager/src/features/VPCs/VPCCreate/FormComponents/VPCTopSectionContent2.tsx create mode 100644 packages/manager/src/features/VPCs/VPCCreate/MultipleSubnetInput2.tsx create mode 100644 packages/manager/src/features/VPCs/VPCCreate/SubnetNode2.tsx diff --git a/packages/manager/src/features/VPCs/VPCCreate/FormComponents/SubnetContent2.tsx b/packages/manager/src/features/VPCs/VPCCreate/FormComponents/SubnetContent2.tsx new file mode 100644 index 00000000000..4c99cdf6ccc --- /dev/null +++ b/packages/manager/src/features/VPCs/VPCCreate/FormComponents/SubnetContent2.tsx @@ -0,0 +1,86 @@ +import * as React from 'react'; +import { useLocation } from 'react-router-dom'; +import { useFormContext } from 'react-hook-form'; + +import { Link } from 'src/components/Link'; +import { Notice } from 'src/components/Notice/Notice'; +import { sendLinodeCreateFormInputEvent } from 'src/utilities/analytics/formEventAnalytics'; +import { getQueryParamsFromQueryString } from 'src/utilities/queryParams'; + +import { VPC_CREATE_FORM_SUBNET_HELPER_TEXT } from '../../constants'; +import { MultipleSubnetInput } from '../MultipleSubnetInput'; +import { + StyledBodyTypography, + StyledHeaderTypography, +} from './VPCCreateForm.styles'; + +import type { APIError, 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 location = useLocation(); + const isFromLinodeCreate = location.pathname.includes('/linodes/create'); + const queryParams = getQueryParamsFromQueryString( + location.search + ); + + const { + formState: { errors }, + setValue, + } = useFormContext(); + + return ( + <> + + Subnets + + + {VPC_CREATE_FORM_SUBNET_HELPER_TEXT}{' '} + + isFromLinodeCreate && + sendLinodeCreateFormInputEvent({ + createType: (queryParams.type as LinodeCreateType) ?? 'OS', + headerName: 'Create VPC', + interaction: 'click', + label: 'Learn more', + subheaderName: 'Subnets', + }) + } + to="https://techdocs.akamai.com/cloud-computing/docs/manage-vpc-subnets" + > + Learn more + + . + + {subnetErrors + ? subnetErrors.map((apiError: APIError) => ( + + )) + : null} + onChangeField('subnets', subnets)} + subnets={subnets} + /> + + ); +}; diff --git a/packages/manager/src/features/VPCs/VPCCreate/FormComponents/VPCTopSectionContent2.tsx b/packages/manager/src/features/VPCs/VPCCreate/FormComponents/VPCTopSectionContent2.tsx new file mode 100644 index 00000000000..76ce180ba3f --- /dev/null +++ b/packages/manager/src/features/VPCs/VPCCreate/FormComponents/VPCTopSectionContent2.tsx @@ -0,0 +1,104 @@ +import * as React from 'react'; +import { Controller, useFormContext } from 'react-hook-form'; +import { useLocation } from 'react-router-dom'; + +import { Link } from 'src/components/Link'; +import { RegionSelect } from 'src/components/RegionSelect/RegionSelect'; +import { TextField } from 'src/components/TextField'; +import { sendLinodeCreateFormInputEvent } from 'src/utilities/analytics/formEventAnalytics'; +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 { LinodeCreateType } from 'src/features/Linodes/LinodeCreate/types'; +import type { LinodeCreateQueryParams } from 'src/features/Linodes/types'; + +interface Props { + disabled?: boolean; + isDrawer?: boolean; + regions: Region[]; +} + +export const VPCTopSectionContent = (props: Props) => { + const { disabled, isDrawer, regions } = props; + const location = useLocation(); + const isFromLinodeCreate = location.pathname.includes('/linodes/create'); + const queryParams = getQueryParamsFromQueryString( + location.search + ); + + const { control } = useFormContext(); + + return ( + <> + + {VPC_CREATE_FORM_VPC_HELPER_TEXT}{' '} + + isFromLinodeCreate && + sendLinodeCreateFormInputEvent({ + createType: (queryParams.type as LinodeCreateType) ?? 'OS', + headerName: 'Create VPC', + interaction: 'click', + label: 'Learn more', + }) + } + to="https://techdocs.akamai.com/cloud-computing/docs/vpc" + > + Learn more + + . + + ( + + )} + control={control} + name="region" + /> + ( + + )} + control={control} + name="label" + /> + ( + + )} + control={control} + name="description" + /> + + ); +}; diff --git a/packages/manager/src/features/VPCs/VPCCreate/MultipleSubnetInput2.tsx b/packages/manager/src/features/VPCs/VPCCreate/MultipleSubnetInput2.tsx new file mode 100644 index 00000000000..7c5dc9d88e5 --- /dev/null +++ b/packages/manager/src/features/VPCs/VPCCreate/MultipleSubnetInput2.tsx @@ -0,0 +1,88 @@ +import Grid from '@mui/material/Unstable_Grid2'; +import * as React from 'react'; + +import { Button } from 'src/components/Button/Button'; +import { Divider } from 'src/components/Divider'; +import { + DEFAULT_SUBNET_IPV4_VALUE, + getRecommendedSubnetIPv4, +} from 'src/utilities/subnets'; + +import { SubnetNode } from './SubnetNode'; + +import type { SubnetFieldState } from 'src/utilities/subnets'; + +interface Props { + disabled?: boolean; + isDrawer?: boolean; + onChange: (subnets: SubnetFieldState[]) => void; + subnets: SubnetFieldState[]; +} + +export const MultipleSubnetInput = (props: Props) => { + const { disabled, isDrawer, onChange, subnets } = props; + + const [lastRecommendedIPv4, setLastRecommendedIPv4] = React.useState( + DEFAULT_SUBNET_IPV4_VALUE + ); + + const addSubnet = () => { + const recommendedIPv4 = getRecommendedSubnetIPv4( + lastRecommendedIPv4, + subnets.map((subnet) => subnet.ip.ipv4 ?? '') + ); + setLastRecommendedIPv4(recommendedIPv4); + onChange([ + ...subnets, + { + ip: { availIPv4s: 256, ipv4: recommendedIPv4, ipv4Error: '' }, + label: '', + labelError: '', + }, + ]); + }; + + const handleSubnetChange = ( + subnet: SubnetFieldState, + subnetIdx: number, + removable: boolean + ) => { + const newSubnets = [...subnets]; + if (removable) { + newSubnets.splice(subnetIdx, 1); + } else { + newSubnets[subnetIdx] = subnet; + } + onChange(newSubnets); + }; + + return ( + + {subnets.map((subnet, subnetIdx) => ( + + {subnetIdx !== 0 && ( + ({ marginTop: theme.spacing(2.5) })} /> + )} + + handleSubnetChange(subnet, subnetIdx ?? 0, !!removable) + } + disabled={disabled} + idx={subnetIdx} + isCreateVPCDrawer={isDrawer} + isRemovable={true} + subnet={subnet} + /> + + ))} + + + ); +}; diff --git a/packages/manager/src/features/VPCs/VPCCreate/SubnetNode2.tsx b/packages/manager/src/features/VPCs/VPCCreate/SubnetNode2.tsx new file mode 100644 index 00000000000..032a8a6393d --- /dev/null +++ b/packages/manager/src/features/VPCs/VPCCreate/SubnetNode2.tsx @@ -0,0 +1,120 @@ +import { FormHelperText } from '@linode/ui'; +import Close from '@mui/icons-material/Close'; +import Stack from '@mui/material/Stack'; +import { styled } from '@mui/material/styles'; +import Grid from '@mui/material/Unstable_Grid2'; +import * as React from 'react'; + +import { Button } from 'src/components/Button/Button'; +import { TextField } from 'src/components/TextField'; +import { + RESERVED_IP_NUMBER, + calculateAvailableIPv4sRFC1918, +} from 'src/utilities/subnets'; + +import type { SubnetFieldState } from 'src/utilities/subnets'; + +interface Props { + disabled?: boolean; + // extra props enable SubnetNode to be an independent component or be part of MultipleSubnetInput + // potential refactor - isRemoveable, and subnetIdx & remove in onChange prop + idx: number; + isCreateVPCDrawer?: boolean; + onChange: ( + subnet: SubnetFieldState, + subnetIdx?: number, + remove?: boolean + ) => void; + subnet: SubnetFieldState; +} + +// @TODO VPC: currently only supports IPv4, must update when/if IPv6 is also supported +export const SubnetNode = (props: Props) => { + const { disabled, idx, isCreateVPCDrawer, onChange, subnet } = props; + + const onLabelChange = (e: React.ChangeEvent) => { + const newSubnet = { + ...subnet, + label: e.target.value, + labelError: '', + }; + onChange(newSubnet, idx); + }; + + const onIpv4Change = (e: React.ChangeEvent) => { + const availIPs = calculateAvailableIPv4sRFC1918(e.target.value); + const newSubnet = { + ...subnet, + ip: { availIPv4s: availIPs, ipv4: e.target.value }, + }; + onChange(newSubnet, idx); + }; + + const removeSubnet = () => { + onChange(subnet, idx); + }; + + const showRemoveButton = !(isCreateVPCDrawer && idx === 0); + + return ( + + + + + + + {subnet.ip.availIPv4s && ( + + Number of Available IP Addresses:{' '} + {subnet.ip.availIPv4s > 4 + ? (subnet.ip.availIPv4s - RESERVED_IP_NUMBER).toLocaleString() + : 0} + + )} + + + {showRemoveButton && ( + + + + + + )} + + + ); +}; + +const StyledButton = styled(Button, { label: 'StyledButton' })(({ theme }) => ({ + '& :hover, & :focus': { + backgroundColor: theme.color.grey2, + }, + '& > span': { + padding: 2, + }, + color: theme.textColors.tableHeader, + marginTop: theme.spacing(6), + minHeight: 'auto', + minWidth: 'auto', + padding: 0, +})); diff --git a/packages/manager/src/features/VPCs/VPCCreate/VPCCreate.tsx b/packages/manager/src/features/VPCs/VPCCreate/VPCCreate.tsx index 161425ee5c4..4426759195c 100644 --- a/packages/manager/src/features/VPCs/VPCCreate/VPCCreate.tsx +++ b/packages/manager/src/features/VPCs/VPCCreate/VPCCreate.tsx @@ -3,34 +3,35 @@ import { styled } from '@mui/material/styles'; import Grid from '@mui/material/Unstable_Grid2'; import { createLazyRoute } from '@tanstack/react-router'; import * as React from 'react'; +import { FormProvider } from 'react-hook-form'; import { ActionsPanel } from 'src/components/ActionsPanel/ActionsPanel'; import { DocumentTitleSegment } from 'src/components/DocumentTitle'; import { LandingHeader } from 'src/components/LandingHeader'; import { VPC_GETTING_STARTED_LINK } from 'src/features/VPCs/constants'; -import { SubnetContent } from 'src/features/VPCs/VPCCreate/FormComponents/SubnetContent'; -import { useCreateVPC } from 'src/hooks/useCreateVPC'; +import { SubnetContent } from 'src/features/VPCs/VPCCreate/FormComponents/SubnetContent2'; +import { useCreateVPC } from 'src/hooks/useCreateVPCv2'; import { CannotCreateVPCNotice } from './FormComponents/CannotCreateVPCNotice'; import { StyledHeaderTypography } from './FormComponents/VPCCreateForm.styles'; -import { VPCTopSectionContent } from './FormComponents/VPCTopSectionContent'; +import { VPCTopSectionContent } from './FormComponents/VPCTopSectionContent2'; const VPCCreate = () => { const { - formik, - generalAPIError, - generalSubnetErrorsFromAPI, + form, isLoadingCreateVPC, - onChangeField, onCreateVPC, regionsData, userCannotAddVPC, } = useCreateVPC({ pushToVPCPage: true }); - const { errors, handleSubmit, setFieldValue, values } = formik; + const { + formState: { errors }, + handleSubmit, + } = form; return ( - <> + { /> {userCannotAddVPC && CannotCreateVPCNotice} - {generalAPIError ? ( - + {errors.root?.message ? ( + ) : null} -
+ VPC ({ marginTop: theme.spacing(2.5) })}> - + /> */} { disabled: userCannotAddVPC, label: 'Create VPC', loading: isLoadingCreateVPC, - onClick: onCreateVPC, }} />
- + ); }; diff --git a/packages/manager/src/hooks/useCreateVPCv2.ts b/packages/manager/src/hooks/useCreateVPCv2.ts index d0dd7d297d1..fb6c2a3077e 100644 --- a/packages/manager/src/hooks/useCreateVPCv2.ts +++ b/packages/manager/src/hooks/useCreateVPCv2.ts @@ -1,6 +1,5 @@ import { yupResolver } from '@hookform/resolvers/yup'; import { createVPCSchema } from '@linode/validation'; -import * as React from 'react'; import { useForm } from 'react-hook-form'; import { useHistory, useLocation } from 'react-router-dom'; @@ -11,12 +10,7 @@ import { sendLinodeCreateFormStepEvent } from 'src/utilities/analytics/formEvent import { getQueryParamsFromQueryString } from 'src/utilities/queryParams'; import { DEFAULT_SUBNET_IPV4_VALUE } from 'src/utilities/subnets'; -import type { - APIError, - CreateSubnetPayload, - CreateVPCPayload, - VPC, -} from '@linode/api-v4'; +import type { CreateVPCPayload, VPC } from '@linode/api-v4'; import type { LinodeCreateType } from 'src/features/Linodes/LinodeCreate/types'; // Custom hook to consolidate shared logic between VPCCreate.tsx and VPCCreateDrawer.tsx From a1965c28e439f1d3dae81d07837f425d2d9f315f Mon Sep 17 00:00:00 2001 From: Connie Liu Date: Thu, 7 Nov 2024 13:04:42 -0500 Subject: [PATCH 03/17] save progress --- .../FormComponents/SubnetContent.tsx | 38 +++--- .../FormComponents/SubnetContent2.tsx | 86 ------------- .../FormComponents/VPCTopSectionContent.tsx | 83 +++++++----- .../FormComponents/VPCTopSectionContent2.tsx | 104 --------------- .../VPCs/VPCCreate/MultipleSubnetInput.tsx | 80 +++++------- .../VPCs/VPCCreate/MultipleSubnetInput2.tsx | 88 ------------- .../features/VPCs/VPCCreate/SubnetNode.tsx | 112 +++++++--------- .../features/VPCs/VPCCreate/SubnetNode2.tsx | 120 ------------------ .../src/features/VPCs/VPCCreate/VPCCreate.tsx | 25 ++-- .../VPCs/VPCCreateDrawer/VPCCreateDrawer.tsx | 4 +- packages/manager/src/hooks/useCreateVPCv2.ts | 23 +++- 11 files changed, 179 insertions(+), 584 deletions(-) delete mode 100644 packages/manager/src/features/VPCs/VPCCreate/FormComponents/SubnetContent2.tsx delete mode 100644 packages/manager/src/features/VPCs/VPCCreate/FormComponents/VPCTopSectionContent2.tsx delete mode 100644 packages/manager/src/features/VPCs/VPCCreate/MultipleSubnetInput2.tsx delete mode 100644 packages/manager/src/features/VPCs/VPCCreate/SubnetNode2.tsx diff --git a/packages/manager/src/features/VPCs/VPCCreate/FormComponents/SubnetContent.tsx b/packages/manager/src/features/VPCs/VPCCreate/FormComponents/SubnetContent.tsx index 0e514bec10c..6469b3b42be 100644 --- a/packages/manager/src/features/VPCs/VPCCreate/FormComponents/SubnetContent.tsx +++ b/packages/manager/src/features/VPCs/VPCCreate/FormComponents/SubnetContent.tsx @@ -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'; @@ -13,21 +14,17 @@ 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'); @@ -35,6 +32,10 @@ export const SubnetContent = (props: Props) => { location.search ); + const { + formState: { errors }, + } = useFormContext(); + return ( <> @@ -59,22 +60,15 @@ export const SubnetContent = (props: Props) => { . - {subnetErrors - ? subnetErrors.map((apiError: APIError) => ( - - )) - : null} - onChangeField('subnets', subnets)} - subnets={subnets} - /> + {/* TODO: figure out general subnet errors */} + {errors.subnets?.root?.message && ( + + )} + ); }; diff --git a/packages/manager/src/features/VPCs/VPCCreate/FormComponents/SubnetContent2.tsx b/packages/manager/src/features/VPCs/VPCCreate/FormComponents/SubnetContent2.tsx deleted file mode 100644 index 4c99cdf6ccc..00000000000 --- a/packages/manager/src/features/VPCs/VPCCreate/FormComponents/SubnetContent2.tsx +++ /dev/null @@ -1,86 +0,0 @@ -import * as React from 'react'; -import { useLocation } from 'react-router-dom'; -import { useFormContext } from 'react-hook-form'; - -import { Link } from 'src/components/Link'; -import { Notice } from 'src/components/Notice/Notice'; -import { sendLinodeCreateFormInputEvent } from 'src/utilities/analytics/formEventAnalytics'; -import { getQueryParamsFromQueryString } from 'src/utilities/queryParams'; - -import { VPC_CREATE_FORM_SUBNET_HELPER_TEXT } from '../../constants'; -import { MultipleSubnetInput } from '../MultipleSubnetInput'; -import { - StyledBodyTypography, - StyledHeaderTypography, -} from './VPCCreateForm.styles'; - -import type { APIError, 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 location = useLocation(); - const isFromLinodeCreate = location.pathname.includes('/linodes/create'); - const queryParams = getQueryParamsFromQueryString( - location.search - ); - - const { - formState: { errors }, - setValue, - } = useFormContext(); - - return ( - <> - - Subnets - - - {VPC_CREATE_FORM_SUBNET_HELPER_TEXT}{' '} - - isFromLinodeCreate && - sendLinodeCreateFormInputEvent({ - createType: (queryParams.type as LinodeCreateType) ?? 'OS', - headerName: 'Create VPC', - interaction: 'click', - label: 'Learn more', - subheaderName: 'Subnets', - }) - } - to="https://techdocs.akamai.com/cloud-computing/docs/manage-vpc-subnets" - > - Learn more - - . - - {subnetErrors - ? subnetErrors.map((apiError: APIError) => ( - - )) - : null} - onChangeField('subnets', subnets)} - subnets={subnets} - /> - - ); -}; diff --git a/packages/manager/src/features/VPCs/VPCCreate/FormComponents/VPCTopSectionContent.tsx b/packages/manager/src/features/VPCs/VPCCreate/FormComponents/VPCTopSectionContent.tsx index 0befdae844f..6c3231af139 100644 --- a/packages/manager/src/features/VPCs/VPCCreate/FormComponents/VPCTopSectionContent.tsx +++ b/packages/manager/src/features/VPCs/VPCCreate/FormComponents/VPCTopSectionContent.tsx @@ -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'; @@ -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; 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( location.search ); + const { control } = useFormContext(); + return ( <> @@ -53,36 +52,52 @@ export const VPCTopSectionContent = (props: Props) => { . - onChangeField('region', region?.id ?? '')} - regions={regions} - value={values.region} + ( + field.onChange(region?.id ?? '')} + regions={regions} + value={field.value} + /> + )} + control={control} + name="region" /> - ) => - onChangeField('label', e.target.value) - } - aria-label="Enter a label" - disabled={disabled} - errorText={errors.label} - label="VPC Label" - value={values.label} + ( + + )} + control={control} + name="label" /> - ) => - onChangeField('description', e.target.value) - } - disabled={disabled} - errorText={errors.description} - label="Description" - maxRows={1} - multiline - optional - value={values.description} + ( + + )} + control={control} + name="description" /> ); diff --git a/packages/manager/src/features/VPCs/VPCCreate/FormComponents/VPCTopSectionContent2.tsx b/packages/manager/src/features/VPCs/VPCCreate/FormComponents/VPCTopSectionContent2.tsx deleted file mode 100644 index 76ce180ba3f..00000000000 --- a/packages/manager/src/features/VPCs/VPCCreate/FormComponents/VPCTopSectionContent2.tsx +++ /dev/null @@ -1,104 +0,0 @@ -import * as React from 'react'; -import { Controller, useFormContext } from 'react-hook-form'; -import { useLocation } from 'react-router-dom'; - -import { Link } from 'src/components/Link'; -import { RegionSelect } from 'src/components/RegionSelect/RegionSelect'; -import { TextField } from 'src/components/TextField'; -import { sendLinodeCreateFormInputEvent } from 'src/utilities/analytics/formEventAnalytics'; -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 { LinodeCreateType } from 'src/features/Linodes/LinodeCreate/types'; -import type { LinodeCreateQueryParams } from 'src/features/Linodes/types'; - -interface Props { - disabled?: boolean; - isDrawer?: boolean; - regions: Region[]; -} - -export const VPCTopSectionContent = (props: Props) => { - const { disabled, isDrawer, regions } = props; - const location = useLocation(); - const isFromLinodeCreate = location.pathname.includes('/linodes/create'); - const queryParams = getQueryParamsFromQueryString( - location.search - ); - - const { control } = useFormContext(); - - return ( - <> - - {VPC_CREATE_FORM_VPC_HELPER_TEXT}{' '} - - isFromLinodeCreate && - sendLinodeCreateFormInputEvent({ - createType: (queryParams.type as LinodeCreateType) ?? 'OS', - headerName: 'Create VPC', - interaction: 'click', - label: 'Learn more', - }) - } - to="https://techdocs.akamai.com/cloud-computing/docs/vpc" - > - Learn more - - . - - ( - - )} - control={control} - name="region" - /> - ( - - )} - control={control} - name="label" - /> - ( - - )} - control={control} - name="description" - /> - - ); -}; diff --git a/packages/manager/src/features/VPCs/VPCCreate/MultipleSubnetInput.tsx b/packages/manager/src/features/VPCs/VPCCreate/MultipleSubnetInput.tsx index d7a690f0119..9a42b960d53 100644 --- a/packages/manager/src/features/VPCs/VPCCreate/MultipleSubnetInput.tsx +++ b/packages/manager/src/features/VPCs/VPCCreate/MultipleSubnetInput.tsx @@ -1,6 +1,7 @@ import { Button, Divider } from '@linode/ui'; import Grid from '@mui/material/Unstable_Grid2'; import * as React from 'react'; +import { useFieldArray, useFormContext } from 'react-hook-form'; import { DEFAULT_SUBNET_IPV4_VALUE, @@ -9,78 +10,63 @@ import { import { SubnetNode } from './SubnetNode'; -import type { SubnetFieldState } from 'src/utilities/subnets'; +import type { CreateVPCPayload } from '@linode/api-v4'; interface Props { disabled?: boolean; isDrawer?: boolean; - onChange: (subnets: SubnetFieldState[]) => void; - subnets: SubnetFieldState[]; } export const MultipleSubnetInput = (props: Props) => { - const { disabled, isDrawer, onChange, subnets } = props; + const { disabled, isDrawer } = props; + + const { control } = useFormContext(); + + const { append, fields, remove } = useFieldArray({ + control, + name: 'subnets', + }); const [lastRecommendedIPv4, setLastRecommendedIPv4] = React.useState( DEFAULT_SUBNET_IPV4_VALUE ); - const addSubnet = () => { + const handleAddSubnet = () => { const recommendedIPv4 = getRecommendedSubnetIPv4( lastRecommendedIPv4, - subnets.map((subnet) => subnet.ip.ipv4 ?? '') + fields.map((subnet) => subnet.ipv4 ?? '') ); setLastRecommendedIPv4(recommendedIPv4); - onChange([ - ...subnets, - { - ip: { availIPv4s: 256, ipv4: recommendedIPv4, ipv4Error: '' }, - label: '', - labelError: '', - }, - ]); - }; - - const handleSubnetChange = ( - subnet: SubnetFieldState, - subnetIdx: number, - removable: boolean - ) => { - const newSubnets = [...subnets]; - if (removable) { - newSubnets.splice(subnetIdx, 1); - } else { - newSubnets[subnetIdx] = subnet; - } - onChange(newSubnets); + append({ + ipv4: recommendedIPv4, + label: '', + }); }; return ( - {subnets.map((subnet, subnetIdx) => ( - - {subnetIdx !== 0 && ( - ({ marginTop: theme.spacing(2.5) })} /> - )} - - handleSubnetChange(subnet, subnetIdx ?? 0, !!removable) - } - disabled={disabled} - idx={subnetIdx} - isCreateVPCDrawer={isDrawer} - isRemovable={true} - subnet={subnet} - /> - - ))} + {fields.map((subnet, subnetIdx) => { + return ( + + {subnetIdx !== 0 && ( + ({ marginTop: theme.spacing(2.5) })} /> + )} + + + ); + })} ); diff --git a/packages/manager/src/features/VPCs/VPCCreate/MultipleSubnetInput2.tsx b/packages/manager/src/features/VPCs/VPCCreate/MultipleSubnetInput2.tsx deleted file mode 100644 index 7c5dc9d88e5..00000000000 --- a/packages/manager/src/features/VPCs/VPCCreate/MultipleSubnetInput2.tsx +++ /dev/null @@ -1,88 +0,0 @@ -import Grid from '@mui/material/Unstable_Grid2'; -import * as React from 'react'; - -import { Button } from 'src/components/Button/Button'; -import { Divider } from 'src/components/Divider'; -import { - DEFAULT_SUBNET_IPV4_VALUE, - getRecommendedSubnetIPv4, -} from 'src/utilities/subnets'; - -import { SubnetNode } from './SubnetNode'; - -import type { SubnetFieldState } from 'src/utilities/subnets'; - -interface Props { - disabled?: boolean; - isDrawer?: boolean; - onChange: (subnets: SubnetFieldState[]) => void; - subnets: SubnetFieldState[]; -} - -export const MultipleSubnetInput = (props: Props) => { - const { disabled, isDrawer, onChange, subnets } = props; - - const [lastRecommendedIPv4, setLastRecommendedIPv4] = React.useState( - DEFAULT_SUBNET_IPV4_VALUE - ); - - const addSubnet = () => { - const recommendedIPv4 = getRecommendedSubnetIPv4( - lastRecommendedIPv4, - subnets.map((subnet) => subnet.ip.ipv4 ?? '') - ); - setLastRecommendedIPv4(recommendedIPv4); - onChange([ - ...subnets, - { - ip: { availIPv4s: 256, ipv4: recommendedIPv4, ipv4Error: '' }, - label: '', - labelError: '', - }, - ]); - }; - - const handleSubnetChange = ( - subnet: SubnetFieldState, - subnetIdx: number, - removable: boolean - ) => { - const newSubnets = [...subnets]; - if (removable) { - newSubnets.splice(subnetIdx, 1); - } else { - newSubnets[subnetIdx] = subnet; - } - onChange(newSubnets); - }; - - return ( - - {subnets.map((subnet, subnetIdx) => ( - - {subnetIdx !== 0 && ( - ({ marginTop: theme.spacing(2.5) })} /> - )} - - handleSubnetChange(subnet, subnetIdx ?? 0, !!removable) - } - disabled={disabled} - idx={subnetIdx} - isCreateVPCDrawer={isDrawer} - isRemovable={true} - subnet={subnet} - /> - - ))} - - - ); -}; diff --git a/packages/manager/src/features/VPCs/VPCCreate/SubnetNode.tsx b/packages/manager/src/features/VPCs/VPCCreate/SubnetNode.tsx index fde1a471489..2416f626621 100644 --- a/packages/manager/src/features/VPCs/VPCCreate/SubnetNode.tsx +++ b/packages/manager/src/features/VPCs/VPCCreate/SubnetNode.tsx @@ -1,67 +1,36 @@ -import { Button, FormHelperText, Stack, TextField } from '@linode/ui'; +import { Button, FormHelperText, TextField } from '@linode/ui'; import Close from '@mui/icons-material/Close'; +import Stack from '@mui/material/Stack'; import { styled } from '@mui/material/styles'; import Grid from '@mui/material/Unstable_Grid2'; import * as React from 'react'; +import { Controller, useFormContext } from 'react-hook-form'; import { RESERVED_IP_NUMBER, calculateAvailableIPv4sRFC1918, } from 'src/utilities/subnets'; -import type { SubnetFieldState } from 'src/utilities/subnets'; +import type { CreateVPCPayload } from '@linode/api-v4'; interface Props { disabled?: boolean; - // extra props enable SubnetNode to be an independent component or be part of MultipleSubnetInput - // potential refactor - isRemoveable, and subnetIdx & remove in onChange prop - idx?: number; + idx: number; isCreateVPCDrawer?: boolean; - isRemovable?: boolean; - onChange: ( - subnet: SubnetFieldState, - subnetIdx?: number, - remove?: boolean - ) => void; - subnet: SubnetFieldState; + remove: (index?: number | number[]) => void; } // @TODO VPC: currently only supports IPv4, must update when/if IPv6 is also supported export const SubnetNode = (props: Props) => { - const { - disabled, - idx, - isCreateVPCDrawer, - isRemovable, - onChange, - subnet, - } = props; + const { disabled, idx, isCreateVPCDrawer, remove } = props; - const onLabelChange = (e: React.ChangeEvent) => { - const newSubnet = { - ...subnet, - label: e.target.value, - labelError: '', - }; - onChange(newSubnet, idx); - }; + const { control, watch } = useFormContext(); - const onIpv4Change = (e: React.ChangeEvent) => { - const availIPs = calculateAvailableIPv4sRFC1918(e.target.value); - const newSubnet = { - ...subnet, - ip: { availIPv4s: availIPs, ipv4: e.target.value }, - }; - onChange(newSubnet, idx); - }; + const subnet = watch(`subnets.${idx}`); - const removeSubnet = () => { - onChange(subnet, idx, isRemovable); - }; + const numberOfAvailIPs = calculateAvailableIPv4sRFC1918(subnet.ipv4 ?? ''); - const showRemoveButton = isCreateVPCDrawer - ? idx !== 0 && isRemovable - : isRemovable; + const showRemoveButton = !(isCreateVPCDrawer && idx === 0); return ( @@ -71,30 +40,44 @@ export const SubnetNode = (props: Props) => { xs={showRemoveButton ? 11 : 12} > - ( + + )} + control={control} + name={`subnets.${idx}.label`} /> - ( + + )} + control={control} + name={`subnets.${idx}.ipv4`} /> - {subnet.ip.availIPv4s && ( + {numberOfAvailIPs && ( Number of Available IP Addresses:{' '} - {subnet.ip.availIPv4s > 4 - ? (subnet.ip.availIPv4s - RESERVED_IP_NUMBER).toLocaleString() + {numberOfAvailIPs > 4 + ? (numberOfAvailIPs - RESERVED_IP_NUMBER).toLocaleString() : 0} )} @@ -102,7 +85,10 @@ export const SubnetNode = (props: Props) => { {showRemoveButton && ( - + remove(idx)} + > diff --git a/packages/manager/src/features/VPCs/VPCCreate/SubnetNode2.tsx b/packages/manager/src/features/VPCs/VPCCreate/SubnetNode2.tsx deleted file mode 100644 index 032a8a6393d..00000000000 --- a/packages/manager/src/features/VPCs/VPCCreate/SubnetNode2.tsx +++ /dev/null @@ -1,120 +0,0 @@ -import { FormHelperText } from '@linode/ui'; -import Close from '@mui/icons-material/Close'; -import Stack from '@mui/material/Stack'; -import { styled } from '@mui/material/styles'; -import Grid from '@mui/material/Unstable_Grid2'; -import * as React from 'react'; - -import { Button } from 'src/components/Button/Button'; -import { TextField } from 'src/components/TextField'; -import { - RESERVED_IP_NUMBER, - calculateAvailableIPv4sRFC1918, -} from 'src/utilities/subnets'; - -import type { SubnetFieldState } from 'src/utilities/subnets'; - -interface Props { - disabled?: boolean; - // extra props enable SubnetNode to be an independent component or be part of MultipleSubnetInput - // potential refactor - isRemoveable, and subnetIdx & remove in onChange prop - idx: number; - isCreateVPCDrawer?: boolean; - onChange: ( - subnet: SubnetFieldState, - subnetIdx?: number, - remove?: boolean - ) => void; - subnet: SubnetFieldState; -} - -// @TODO VPC: currently only supports IPv4, must update when/if IPv6 is also supported -export const SubnetNode = (props: Props) => { - const { disabled, idx, isCreateVPCDrawer, onChange, subnet } = props; - - const onLabelChange = (e: React.ChangeEvent) => { - const newSubnet = { - ...subnet, - label: e.target.value, - labelError: '', - }; - onChange(newSubnet, idx); - }; - - const onIpv4Change = (e: React.ChangeEvent) => { - const availIPs = calculateAvailableIPv4sRFC1918(e.target.value); - const newSubnet = { - ...subnet, - ip: { availIPv4s: availIPs, ipv4: e.target.value }, - }; - onChange(newSubnet, idx); - }; - - const removeSubnet = () => { - onChange(subnet, idx); - }; - - const showRemoveButton = !(isCreateVPCDrawer && idx === 0); - - return ( - - - - - - - {subnet.ip.availIPv4s && ( - - Number of Available IP Addresses:{' '} - {subnet.ip.availIPv4s > 4 - ? (subnet.ip.availIPv4s - RESERVED_IP_NUMBER).toLocaleString() - : 0} - - )} - - - {showRemoveButton && ( - - - - - - )} - - - ); -}; - -const StyledButton = styled(Button, { label: 'StyledButton' })(({ theme }) => ({ - '& :hover, & :focus': { - backgroundColor: theme.color.grey2, - }, - '& > span': { - padding: 2, - }, - color: theme.textColors.tableHeader, - marginTop: theme.spacing(6), - minHeight: 'auto', - minWidth: 'auto', - padding: 0, -})); diff --git a/packages/manager/src/features/VPCs/VPCCreate/VPCCreate.tsx b/packages/manager/src/features/VPCs/VPCCreate/VPCCreate.tsx index 4426759195c..14826c8f814 100644 --- a/packages/manager/src/features/VPCs/VPCCreate/VPCCreate.tsx +++ b/packages/manager/src/features/VPCs/VPCCreate/VPCCreate.tsx @@ -9,21 +9,23 @@ import { ActionsPanel } from 'src/components/ActionsPanel/ActionsPanel'; import { DocumentTitleSegment } from 'src/components/DocumentTitle'; import { LandingHeader } from 'src/components/LandingHeader'; import { VPC_GETTING_STARTED_LINK } from 'src/features/VPCs/constants'; -import { SubnetContent } from 'src/features/VPCs/VPCCreate/FormComponents/SubnetContent2'; +import { SubnetContent } from 'src/features/VPCs/VPCCreate/FormComponents/SubnetContent'; import { useCreateVPC } from 'src/hooks/useCreateVPCv2'; import { CannotCreateVPCNotice } from './FormComponents/CannotCreateVPCNotice'; import { StyledHeaderTypography } from './FormComponents/VPCCreateForm.styles'; -import { VPCTopSectionContent } from './FormComponents/VPCTopSectionContent2'; +import { VPCTopSectionContent } from './FormComponents/VPCTopSectionContent'; const VPCCreate = () => { + const formContainerRef = React.useRef(null); + const { form, isLoadingCreateVPC, onCreateVPC, regionsData, userCannotAddVPC, - } = useCreateVPC({ pushToVPCPage: true }); + } = useCreateVPC({ formContainerRef, pushToVPCPage: true }); const { formState: { errors }, @@ -49,10 +51,10 @@ const VPCCreate = () => { /> {userCannotAddVPC && CannotCreateVPCNotice} - {errors.root?.message ? ( - - ) : null} -
+ + {errors.root?.message ? ( + + ) : null} VPC { /> ({ marginTop: theme.spacing(2.5) })}> - {/* */} + { disabled: userCannotAddVPC, label: 'Create VPC', loading: isLoadingCreateVPC, + onClick: handleSubmit(onCreateVPC), + type: 'submit', }} /> diff --git a/packages/manager/src/features/VPCs/VPCCreateDrawer/VPCCreateDrawer.tsx b/packages/manager/src/features/VPCs/VPCCreateDrawer/VPCCreateDrawer.tsx index d477be0dadb..bf91b0c1a34 100644 --- a/packages/manager/src/features/VPCs/VPCCreateDrawer/VPCCreateDrawer.tsx +++ b/packages/manager/src/features/VPCs/VPCCreateDrawer/VPCCreateDrawer.tsx @@ -60,7 +60,7 @@ export const VPCCreateDrawer = (props: Props) => { {generalAPIError ? ( ) : null} -
+ {/* { }} style={{ marginTop: theme.spacing(3) }} /> - + */}
); diff --git a/packages/manager/src/hooks/useCreateVPCv2.ts b/packages/manager/src/hooks/useCreateVPCv2.ts index fb6c2a3077e..f3cbcd63086 100644 --- a/packages/manager/src/hooks/useCreateVPCv2.ts +++ b/packages/manager/src/hooks/useCreateVPCv2.ts @@ -1,5 +1,7 @@ import { yupResolver } from '@hookform/resolvers/yup'; +import { isEmpty } from '@linode/api-v4'; import { createVPCSchema } from '@linode/validation'; +import * as React from 'react'; import { useForm } from 'react-hook-form'; import { useHistory, useLocation } from 'react-router-dom'; @@ -8,16 +10,15 @@ import { useRegionsQuery } from 'src/queries/regions/regions'; import { useCreateVPCMutation } from 'src/queries/vpcs/vpcs'; import { sendLinodeCreateFormStepEvent } from 'src/utilities/analytics/formEventAnalytics'; import { getQueryParamsFromQueryString } from 'src/utilities/queryParams'; +import { scrollErrorIntoViewV2 } from 'src/utilities/scrollErrorIntoViewV2'; import { DEFAULT_SUBNET_IPV4_VALUE } from 'src/utilities/subnets'; import type { CreateVPCPayload, VPC } from '@linode/api-v4'; import type { LinodeCreateType } from 'src/features/Linodes/LinodeCreate/types'; // Custom hook to consolidate shared logic between VPCCreate.tsx and VPCCreateDrawer.tsx - -// DON'T FORGET TO PUT BACK IN SENDING THE ANALYTICAL EVENT GIRL - export interface UseCreateVPCInputs { + formContainerRef: React.RefObject; handleSelectVPC?: (vpc: VPC) => void; onDrawerClose?: () => void; pushToVPCPage?: boolean; @@ -26,6 +27,7 @@ export interface UseCreateVPCInputs { export const useCreateVPC = (inputs: UseCreateVPCInputs) => { const { + formContainerRef, handleSelectVPC, onDrawerClose, pushToVPCPage, @@ -72,7 +74,11 @@ export const useCreateVPC = (inputs: UseCreateVPCInputs) => { } } catch (errors) { for (const error of errors) { - form.setError(error?.field ?? 'root', { message: error.reason }); + if (error?.field === 'subnets.label') { + form.setError('subnets', { message: error.reason }); + } else { + form.setError(error?.field ?? 'root', { message: error.reason }); + } } } }; @@ -89,11 +95,18 @@ export const useCreateVPC = (inputs: UseCreateVPCInputs) => { }, ], }, - mode: 'onBlur', resolver: yupResolver(createVPCSchema), }); + const { errors } = form.formState; + + React.useEffect(() => { + if (!isEmpty(errors)) { + scrollErrorIntoViewV2(formContainerRef); + } + }, [errors]); + return { form, isLoadingCreateVPC, From 86d0aacc0ba83e2bdbe93c06da2abff7d2e63163 Mon Sep 17 00:00:00 2001 From: Connie Liu Date: Mon, 2 Dec 2024 12:47:02 -0500 Subject: [PATCH 04/17] update tests for vpc components --- .../cypress/e2e/core/vpc/vpc-create.spec.ts | 14 +- .../FormComponents/SubnetContent.test.tsx | 26 ++- .../VPCTopSectionContent.test.tsx | 17 +- .../VPCCreate/MultipleSubnetInput.test.tsx | 141 ++++++++------ .../VPCs/VPCCreate/SubnetNode.test.tsx | 135 +++++++------ .../VPCs/VPCCreate/VPCCreate.test.tsx | 48 +---- .../src/features/VPCs/VPCCreate/VPCCreate.tsx | 2 +- .../VPCCreateDrawer/VPCCreateDrawer.test.tsx | 2 +- .../VPCs/VPCCreateDrawer/VPCCreateDrawer.tsx | 144 +++++++------- packages/manager/src/hooks/useCreateVPC.ts | 182 +++--------------- packages/manager/src/hooks/useCreateVPCv2.ts | 117 ----------- packages/validation/src/vpcs.schema.ts | 4 +- 12 files changed, 311 insertions(+), 521 deletions(-) delete mode 100644 packages/manager/src/hooks/useCreateVPCv2.ts diff --git a/packages/manager/cypress/e2e/core/vpc/vpc-create.spec.ts b/packages/manager/cypress/e2e/core/vpc/vpc-create.spec.ts index 86601275d26..1159a7e3363 100644 --- a/packages/manager/cypress/e2e/core/vpc/vpc-create.spec.ts +++ b/packages/manager/cypress/e2e/core/vpc/vpc-create.spec.ts @@ -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.'; + const ipValidationErrorMessage2 = 'The IPv4 range must be in CIDR format.'; const vpcCreationErrorMessage = 'An unknown error has occurred.'; const totalSubnetUniqueLinodes = getUniqueLinodesFromSubnets(mockSubnets); @@ -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') @@ -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') @@ -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') @@ -180,7 +182,9 @@ 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') diff --git a/packages/manager/src/features/VPCs/VPCCreate/FormComponents/SubnetContent.test.tsx b/packages/manager/src/features/VPCs/VPCCreate/FormComponents/SubnetContent.test.tsx index 2fcb98fee30..f24a05f98ce 100644 --- a/packages/manager/src/features/VPCs/VPCCreate/FormComponents/SubnetContent.test.tsx +++ b/packages/manager/src/features/VPCs/VPCCreate/FormComponents/SubnetContent.test.tsx @@ -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(); + const { getByText } = renderWithThemeAndHookFormContext({ + component: , + useFormOptions: { + defaultValues: { + description: '', + label: '', + region: '', + subnets: [{ ipv4: '', label: '' }], + }, + }, + }); getByText('Subnets'); getByText('Subnet Label'); diff --git a/packages/manager/src/features/VPCs/VPCCreate/FormComponents/VPCTopSectionContent.test.tsx b/packages/manager/src/features/VPCs/VPCCreate/FormComponents/VPCTopSectionContent.test.tsx index 996d429a640..9de5fd147fa 100644 --- a/packages/manager/src/features/VPCs/VPCCreate/FormComponents/VPCTopSectionContent.test.tsx +++ b/packages/manager/src/features/VPCs/VPCCreate/FormComponents/VPCTopSectionContent.test.tsx @@ -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(); + const { getByText } = renderWithThemeAndHookFormContext({ + component: , + useFormOptions: { + defaultValues: { + description: '', + label: '', + region: '', + subnets: [], + }, + }, + }); getByText('Region'); getByText('VPC Label'); diff --git a/packages/manager/src/features/VPCs/VPCCreate/MultipleSubnetInput.test.tsx b/packages/manager/src/features/VPCs/VPCCreate/MultipleSubnetInput.test.tsx index 0babd9bf0f7..9eac79771e0 100644 --- a/packages/manager/src/features/VPCs/VPCCreate/MultipleSubnetInput.test.tsx +++ b/packages/manager/src/features/VPCs/VPCCreate/MultipleSubnetInput.test.tsx @@ -1,82 +1,115 @@ -import { fireEvent } from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; import * as React from 'react'; -import { renderWithTheme } from 'src/utilities/testHelpers'; +import { renderWithThemeAndHookFormContext } from 'src/utilities/testHelpers'; import { MultipleSubnetInput } from './MultipleSubnetInput'; const props = { - onChange: vi.fn(), - subnets: [ - { - ip: { ipv4: '', ipv4Error: '' }, - label: 'subnet 1', - labelError: '', - }, - { - ip: { ipv4: '', ipv4Error: '' }, - label: 'subnet 2', - labelError: '', - }, - { - ip: { ipv4: '', ipv4Error: '' }, - label: 'subnet 3', - labelError: '', - }, - ], + disabled: false, + isDrawer: false, +}; + +const formOptions = { + defaultValues: { + description: '', + label: '', + region: '', + subnets: [ + { + ipv4: '', + label: 'subnet 0', + }, + { + ipv4: '', + label: 'subnet 1', + }, + { + ipv4: '', + label: 'subnet 2', + }, + ], + }, }; describe('MultipleSubnetInput', () => { it('should render a subnet node for each of the given subnets', () => { - const { getAllByText, getByDisplayValue } = renderWithTheme( - - ); + const { + getAllByText, + getByDisplayValue, + } = renderWithThemeAndHookFormContext({ + component: , + useFormOptions: formOptions, + }); expect(getAllByText('Subnet Label')).toHaveLength(3); expect(getAllByText('Subnet IP Address Range')).toHaveLength(3); + getByDisplayValue('subnet 0'); getByDisplayValue('subnet 1'); getByDisplayValue('subnet 2'); - getByDisplayValue('subnet 3'); }); - it('should add a subnet to the array when the Add Subnet button is clicked', () => { - const { getByText } = renderWithTheme(); - const addButton = getByText('Add another Subnet'); - fireEvent.click(addButton); - expect(props.onChange).toHaveBeenCalledWith([ - ...props.subnets, - { - ip: { availIPv4s: 256, ipv4: '10.0.1.0/24', ipv4Error: '' }, - label: '', - labelError: '', + it('should display "Add a Subnet" if there are no subnets', () => { + const { getByText } = renderWithThemeAndHookFormContext({ + component: , + useFormOptions: { + defaultValues: { + description: '', + label: '', + region: '', + subnets: [], + }, }, - ]); + }); + + getByText('Add a Subnet'); }); - it('all inputs should have a close button (X)', () => { - const { queryAllByTestId } = renderWithTheme( - + it('should add a subnet to the array when the Add Subnet button is clicked', async () => { + const { getByDisplayValue, getByText } = renderWithThemeAndHookFormContext({ + component: , + useFormOptions: formOptions, + }); + const addButton = getByText('Add another Subnet'); + await userEvent.click(addButton); + + expect(getByDisplayValue('10.0.1.0/24')).toBeVisible(); + }); + + it('all subnets should have a delete button (X) if not in a drawer', () => { + const { queryAllByTestId } = renderWithThemeAndHookFormContext({ + component: , + useFormOptions: formOptions, + }); + expect(queryAllByTestId(/delete-subnet/)).toHaveLength( + formOptions.defaultValues.subnets.length ); + }); + + it('the first does not have a delete button if in a drawer', () => { + const { + queryAllByTestId, + queryByTestId, + } = renderWithThemeAndHookFormContext({ + component: , + useFormOptions: formOptions, + }); expect(queryAllByTestId(/delete-subnet/)).toHaveLength( - props.subnets.length + formOptions.defaultValues.subnets.length - 1 ); + expect(queryByTestId('delete-subnet-0')).toBeNull(); }); - it('should remove an element from the array based on its index when the X is clicked', () => { - const { getByTestId } = renderWithTheme(); + it('should remove an element from the array based on its index when the X is clicked', async () => { + const { + getByTestId, + queryByDisplayValue, + } = renderWithThemeAndHookFormContext({ + component: , + useFormOptions: formOptions, + }); const closeButton = getByTestId('delete-subnet-1'); - fireEvent.click(closeButton); - expect(props.onChange).toHaveBeenCalledWith([ - { - ip: { ipv4: '', ipv4Error: '' }, - label: 'subnet 1', - labelError: '', - }, - { - ip: { ipv4: '', ipv4Error: '' }, - label: 'subnet 3', - labelError: '', - }, - ]); + await userEvent.click(closeButton); + expect(queryByDisplayValue('subnet-1')).toBeNull(); }); }); diff --git a/packages/manager/src/features/VPCs/VPCCreate/SubnetNode.test.tsx b/packages/manager/src/features/VPCs/VPCCreate/SubnetNode.test.tsx index 538b2beab9b..338a924f924 100644 --- a/packages/manager/src/features/VPCs/VPCCreate/SubnetNode.test.tsx +++ b/packages/manager/src/features/VPCs/VPCCreate/SubnetNode.test.tsx @@ -1,77 +1,98 @@ -import { screen } from '@testing-library/react'; -import userEvent from '@testing-library/user-event'; import * as React from 'react'; -import { renderWithTheme } from 'src/utilities/testHelpers'; +import { renderWithThemeAndHookFormContext } from 'src/utilities/testHelpers'; import { SubnetNode } from './SubnetNode'; +const props = { + idx: 0, + isCreateVPCDrawer: false, + remove: vi.fn(), +}; + +const formOptions = { + defaultValues: { + description: '', + label: '', + region: '', + subnets: [ + { + ipv4: '10.0.0.0', + label: 'subnet 0', + }, + ], + }, +}; + describe('SubnetNode', () => { - // state that keeps track of available IPv4s has been moved out of this component, - // due to React key issues vs maintaining state, so this test will now fail - it.skip('should calculate the correct subnet mask', async () => { - renderWithTheme( - {}} - subnet={{ ip: { ipv4: '' }, label: '' }} - /> - ); - const subnetAddress = screen.getAllByTestId('textfield-input'); - expect(subnetAddress[1]).toBeInTheDocument(); - await userEvent.type(subnetAddress[1], '192.0.0.0/24', { delay: 1 }); + it('should show the correct subnet mask', async () => { + const { getByDisplayValue, getByText } = renderWithThemeAndHookFormContext({ + component: , + useFormOptions: { + defaultValues: { + ...formOptions.defaultValues, + subnets: [{ ipv4: '10.0.0.0/24', label: 'subnet 0' }], + }, + }, + }); - expect(subnetAddress[1]).toHaveValue('192.0.0.0/24'); - const availIps = screen.getByText('Number of Available IP Addresses: 252'); - expect(availIps).toBeInTheDocument(); + getByDisplayValue('10.0.0.0/24'); + getByText('Number of Available IP Addresses: 252'); }); it('should not show a subnet mask for an ip without a mask', async () => { - renderWithTheme( - {}} - subnet={{ ip: { ipv4: '' }, label: '' }} - /> - ); - const subnetAddress = screen.getAllByTestId('textfield-input'); - expect(subnetAddress[1]).toBeInTheDocument(); - await userEvent.type(subnetAddress[1], '192.0.0.0', { delay: 1 }); + const { + getByDisplayValue, + queryByText, + } = renderWithThemeAndHookFormContext({ + component: , + useFormOptions: formOptions, + }); - expect(subnetAddress[1]).toHaveValue('192.0.0.0'); - const availIps = screen.queryByText('Number of Available IP Addresses:'); - expect(availIps).not.toBeInTheDocument(); + getByDisplayValue('10.0.0.0'); + expect(queryByText('Number of Available IP Addresses:')).toBeNull(); }); it('should show a label and ip textfield inputs at minimum', () => { - renderWithTheme( - {}} - subnet={{ ip: { ipv4: '' }, label: '' }} - /> - ); + const { getByText } = renderWithThemeAndHookFormContext({ + component: , + useFormOptions: { + defaultValues: { + ...formOptions.defaultValues, + subnets: [{ ipv4: '10.0.0.0/24', label: 'subnet 0' }], + }, + }, + }); + + getByText('Subnet Label'); + getByText('Subnet IP Address Range'); + }); + + it('should show a removable button if not a drawer', () => { + const { getByTestId } = renderWithThemeAndHookFormContext({ + component: , + useFormOptions: { + defaultValues: { + ...formOptions.defaultValues, + subnets: [{ ipv4: '10.0.0.0/24', label: 'subnet 0' }], + }, + }, + }); - const label = screen.getByText('Subnet Label'); - expect(label).toBeInTheDocument(); - const ipAddress = screen.getByText('Subnet IP Address Range'); - expect(ipAddress).toBeInTheDocument(); + expect(getByTestId('delete-subnet-0')).toBeInTheDocument(); }); - it('should show a removable button if isRemovable is true', () => { - renderWithTheme( - {}} - subnet={{ ip: { ipv4: '' }, label: '' }} - /> - ); + it('should not show a removable button if a drawer for the first subnet', () => { + const { queryByTestId } = renderWithThemeAndHookFormContext({ + component: , + useFormOptions: { + defaultValues: { + ...formOptions.defaultValues, + subnets: [{ ipv4: '10.0.0.0/24', label: 'subnet 0' }], + }, + }, + }); - const removableButton = screen.getByTestId('delete-subnet-1'); - expect(removableButton).toBeInTheDocument(); + expect(queryByTestId('delete-subnet-0')).toBeNull(); }); }); diff --git a/packages/manager/src/features/VPCs/VPCCreate/VPCCreate.test.tsx b/packages/manager/src/features/VPCs/VPCCreate/VPCCreate.test.tsx index b67ce7af200..d810571b71d 100644 --- a/packages/manager/src/features/VPCs/VPCCreate/VPCCreate.test.tsx +++ b/packages/manager/src/features/VPCs/VPCCreate/VPCCreate.test.tsx @@ -1,7 +1,6 @@ import { screen } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; import * as React from 'react'; -import { act } from 'react-dom/test-utils'; import { renderWithTheme } from 'src/utilities/testHelpers'; @@ -29,37 +28,11 @@ describe('VPC create page', () => { getAllByText('Create VPC'); }); - // test fails due to new default value for subnet ip addresses - if we remove default text and just - // have a placeholder, we can put this test back in - it.skip('should require vpc labels and region and ignore subnets that are blank', async () => { - const { - getAllByTestId, - getByText, - getAllByText, - queryByText, - } = renderWithTheme(); - const createVPCButton = getByText('Create VPC'); - const subnetIP = getAllByTestId('textfield-input'); - expect(createVPCButton).toBeInTheDocument(); - expect(subnetIP[3]).toBeInTheDocument(); - await act(async () => { - await userEvent.click(createVPCButton); - }); - const regionError = getByText('Region is required'); - expect(regionError).toBeInTheDocument(); - const labelErrors = getAllByText('Label is required'); - expect(labelErrors).toHaveLength(1); - const badSubnetIP = queryByText('The IPv4 range must be in CIDR format'); - expect(badSubnetIP).not.toBeInTheDocument(); - }); - it('should add and delete subnets correctly', async () => { renderWithTheme(); const addSubnet = screen.getByText('Add another Subnet'); expect(addSubnet).toBeInTheDocument(); - await act(async () => { - await userEvent.click(addSubnet); - }); + await userEvent.click(addSubnet); const subnetLabels = screen.getAllByText('Subnet Label'); const subnetIps = screen.getAllByText('Subnet IP Address Range'); @@ -68,9 +41,7 @@ describe('VPC create page', () => { const deleteSubnet = screen.getByTestId('delete-subnet-1'); expect(deleteSubnet).toBeInTheDocument(); - await act(async () => { - await userEvent.click(deleteSubnet); - }); + await userEvent.click(deleteSubnet); const subnetLabelAfter = screen.getAllByText('Subnet Label'); const subnetIpsAfter = screen.getAllByText('Subnet IP Address Range'); @@ -80,24 +51,21 @@ describe('VPC create page', () => { it('should display that a subnet ip is invalid and require a subnet label if a user adds an invalid subnet ip', async () => { renderWithTheme(); - const subnetLabel = screen.getByText('Subnet Label'); - expect(subnetLabel).toBeInTheDocument(); const subnetIp = screen.getByText('Subnet IP Address Range'); expect(subnetIp).toBeInTheDocument(); const createVPCButton = screen.getByText('Create VPC'); expect(createVPCButton).toBeInTheDocument(); - await act(async () => { - await userEvent.type(subnetIp, 'bad'); - await userEvent.click(createVPCButton); - }); + await userEvent.type(subnetIp, 'bad'); + await userEvent.click(createVPCButton); const badSubnetIP = screen.getByText( - 'The IPv4 range must be in CIDR format' + 'The IPv4 range must be in CIDR format.' ); expect(badSubnetIP).toBeInTheDocument(); - const badLabels = screen.getAllByText('Label is required'); - expect(badLabels).toHaveLength(2); + expect( + screen.getAllByText('Label must be between 1 and 64 characters.') + ).toHaveLength(2); }); it('should have a default value for the subnet ip address', () => { diff --git a/packages/manager/src/features/VPCs/VPCCreate/VPCCreate.tsx b/packages/manager/src/features/VPCs/VPCCreate/VPCCreate.tsx index 14826c8f814..36053a42f0d 100644 --- a/packages/manager/src/features/VPCs/VPCCreate/VPCCreate.tsx +++ b/packages/manager/src/features/VPCs/VPCCreate/VPCCreate.tsx @@ -10,7 +10,7 @@ import { DocumentTitleSegment } from 'src/components/DocumentTitle'; import { LandingHeader } from 'src/components/LandingHeader'; import { VPC_GETTING_STARTED_LINK } from 'src/features/VPCs/constants'; import { SubnetContent } from 'src/features/VPCs/VPCCreate/FormComponents/SubnetContent'; -import { useCreateVPC } from 'src/hooks/useCreateVPCv2'; +import { useCreateVPC } from 'src/hooks/useCreateVPC'; import { CannotCreateVPCNotice } from './FormComponents/CannotCreateVPCNotice'; import { StyledHeaderTypography } from './FormComponents/VPCCreateForm.styles'; diff --git a/packages/manager/src/features/VPCs/VPCCreateDrawer/VPCCreateDrawer.test.tsx b/packages/manager/src/features/VPCs/VPCCreateDrawer/VPCCreateDrawer.test.tsx index 434decf1b5b..e639c38d2d4 100644 --- a/packages/manager/src/features/VPCs/VPCCreateDrawer/VPCCreateDrawer.test.tsx +++ b/packages/manager/src/features/VPCs/VPCCreateDrawer/VPCCreateDrawer.test.tsx @@ -11,7 +11,7 @@ const props = { open: true, }; -describe('VPC Create Drawer', () => { +describe.skip('VPC Create Drawer', () => { it('should render the vpc and subnet sections', () => { const { getAllByText } = renderWithTheme(); diff --git a/packages/manager/src/features/VPCs/VPCCreateDrawer/VPCCreateDrawer.tsx b/packages/manager/src/features/VPCs/VPCCreateDrawer/VPCCreateDrawer.tsx index bf91b0c1a34..99f28b3ada0 100644 --- a/packages/manager/src/features/VPCs/VPCCreateDrawer/VPCCreateDrawer.tsx +++ b/packages/manager/src/features/VPCs/VPCCreateDrawer/VPCCreateDrawer.tsx @@ -23,80 +23,80 @@ interface Props { } export const VPCCreateDrawer = (props: Props) => { - const theme = useTheme(); - const { onClose, onSuccess, open, selectedRegion } = props; + // const theme = useTheme(); + // const { onClose, onSuccess, open, selectedRegion } = props; - const { - formik, - generalAPIError, - generalSubnetErrorsFromAPI, - isLoadingCreateVPC, - onChangeField, - onCreateVPC, - regionsData, - setGeneralAPIError, - setGeneralSubnetErrorsFromAPI, - userCannotAddVPC, - } = useCreateVPC({ - handleSelectVPC: onSuccess, - onDrawerClose: onClose, - selectedRegion, - }); + // const { + // formik, + // generalAPIError, + // generalSubnetErrorsFromAPI, + // isLoadingCreateVPC, + // onChangeField, + // onCreateVPC, + // regionsData, + // setGeneralAPIError, + // setGeneralSubnetErrorsFromAPI, + // userCannotAddVPC, + // } = useCreateVPC({ + // handleSelectVPC: onSuccess, + // onDrawerClose: onClose, + // selectedRegion, + // }); - const { errors, handleSubmit, resetForm, setFieldValue, values } = formik; + // const { errors, handleSubmit, resetForm, setFieldValue, values } = formik; - React.useEffect(() => { - if (open) { - resetForm(); - setGeneralSubnetErrorsFromAPI([]); - setGeneralAPIError(undefined); - } - }, [open, resetForm, setGeneralAPIError, setGeneralSubnetErrorsFromAPI]); + // React.useEffect(() => { + // if (open) { + // resetForm(); + // setGeneralSubnetErrorsFromAPI([]); + // setGeneralAPIError(undefined); + // } + // }, [open, resetForm, setGeneralAPIError, setGeneralSubnetErrorsFromAPI]); - return ( - - {userCannotAddVPC && CannotCreateVPCNotice} - - {generalAPIError ? ( - - ) : null} - {/*
- - - - - { - onCreateVPC(); - }, - }} - secondaryButtonProps={{ - 'data-testid': 'cancel', - label: 'Cancel', - onClick: onClose, - }} - style={{ marginTop: theme.spacing(3) }} - /> - */} -
- - ); + // return ( + // + // {userCannotAddVPC && CannotCreateVPCNotice} + // + // {generalAPIError ? ( + // + // ) : null} + //
+ // + // + // + // + // { + // onCreateVPC(); + // }, + // }} + // secondaryButtonProps={{ + // 'data-testid': 'cancel', + // label: 'Cancel', + // onClick: onClose, + // }} + // style={{ marginTop: theme.spacing(3) }} + // /> + // + //
+ // + // ); }; diff --git a/packages/manager/src/hooks/useCreateVPC.ts b/packages/manager/src/hooks/useCreateVPC.ts index becb96d697c..f3cbcd63086 100644 --- a/packages/manager/src/hooks/useCreateVPC.ts +++ b/packages/manager/src/hooks/useCreateVPC.ts @@ -1,37 +1,24 @@ +import { yupResolver } from '@hookform/resolvers/yup'; +import { isEmpty } from '@linode/api-v4'; import { createVPCSchema } from '@linode/validation'; -import { useFormik } from 'formik'; import * as React from 'react'; +import { useForm } from 'react-hook-form'; import { useHistory, useLocation } from 'react-router-dom'; import { useGrants, useProfile } from 'src/queries/profile/profile'; import { useRegionsQuery } from 'src/queries/regions/regions'; import { useCreateVPCMutation } from 'src/queries/vpcs/vpcs'; import { sendLinodeCreateFormStepEvent } from 'src/utilities/analytics/formEventAnalytics'; -import { handleVPCAndSubnetErrors } from 'src/utilities/formikErrorUtils'; import { getQueryParamsFromQueryString } from 'src/utilities/queryParams'; -import { scrollErrorIntoView } from 'src/utilities/scrollErrorIntoView'; +import { scrollErrorIntoViewV2 } from 'src/utilities/scrollErrorIntoViewV2'; import { DEFAULT_SUBNET_IPV4_VALUE } from 'src/utilities/subnets'; -import type { - APIError, - CreateSubnetPayload, - CreateVPCPayload, - VPC, -} from '@linode/api-v4'; +import type { CreateVPCPayload, VPC } from '@linode/api-v4'; import type { LinodeCreateType } from 'src/features/Linodes/LinodeCreate/types'; -import type { SubnetError } from 'src/utilities/formikErrorUtils'; -import type { SubnetFieldState } from 'src/utilities/subnets'; // Custom hook to consolidate shared logic between VPCCreate.tsx and VPCCreateDrawer.tsx - -export interface CreateVPCFieldState { - description: string; - label: string; - region: string; - subnets: SubnetFieldState[]; -} - export interface UseCreateVPCInputs { + formContainerRef: React.RefObject; handleSelectVPC?: (vpc: VPC) => void; onDrawerClose?: () => void; pushToVPCPage?: boolean; @@ -40,6 +27,7 @@ export interface UseCreateVPCInputs { export const useCreateVPC = (inputs: UseCreateVPCInputs) => { const { + formContainerRef, handleSelectVPC, onDrawerClose, pushToVPCPage, @@ -58,85 +46,14 @@ export const useCreateVPC = (inputs: UseCreateVPCInputs) => { const { data: regions } = useRegionsQuery(); const regionsData = regions ?? []; - const [ - generalSubnetErrorsFromAPI, - setGeneralSubnetErrorsFromAPI, - ] = React.useState(); - const [generalAPIError, setGeneralAPIError] = React.useState< - string | undefined - >(); - const { isPending: isLoadingCreateVPC, mutateAsync: createVPC, } = useCreateVPCMutation(); - // When creating the subnet payloads, we also create a mapping of the indexes of the subnets that appear on - // the UI to the indexes of the subnets that the API will receive. This enables users to leave subnets blank - // on the UI and still have any errors returned by the API correspond to the correct subnet - const createSubnetsPayloadAndMapping = () => { - const subnetsPayload: CreateSubnetPayload[] = []; - const subnetIdxMapping: Record = {}; - let apiSubnetIdx = 0; - - for (let i = 0; i < formik.values.subnets.length; i++) { - const { ip, label } = formik.values.subnets[i]; - // if we are inside the VPCCreateDrawer, we force the first subnet to always be included in the payload, - // even if its fields are empty. This is for validation purposes - so that errors can be surfaced on the - // first subnet's label and ipv4 field if applicable. - if ((onDrawerClose && i === 0) || ip.ipv4 || label) { - subnetsPayload.push({ ipv4: ip.ipv4, label }); - subnetIdxMapping[i] = apiSubnetIdx; - apiSubnetIdx++; - } - } - - return { - subnetsPayload, - visualToAPISubnetMapping: subnetIdxMapping, - }; - }; - - const combineErrorsAndSubnets = ( - errors: Record, - visualToAPISubnetMapping: Record - ) => { - return formik.values.subnets.map((subnet, idx) => { - const apiSubnetIdx: number | undefined = visualToAPISubnetMapping[idx]; - // If the subnet has errors associated with it, include those errors in its state - if ((apiSubnetIdx || apiSubnetIdx === 0) && errors[apiSubnetIdx]) { - const errorData: SubnetError = errors[apiSubnetIdx]; - return { - ...subnet, - // @TODO VPC: IPv6 error handling - ip: { - ...subnet.ip, - ipv4Error: errorData.ipv4 ?? '', - }, - labelError: errorData.label ?? '', - }; - } else { - return subnet; - } - }); - }; - - const onCreateVPC = async () => { - formik.setSubmitting(true); - setGeneralAPIError(undefined); - - const { - subnetsPayload, - visualToAPISubnetMapping, - } = createSubnetsPayloadAndMapping(); - - const createVPCPayload: CreateVPCPayload = { - ...formik.values, - subnets: subnetsPayload, - }; - + const onCreateVPC = async (values: CreateVPCPayload) => { try { - const vpc = await createVPC(createVPCPayload); + const vpc = await createVPC(values); if (pushToVPCPage) { history.push(`/vpcs/${vpc.id}`); } else { @@ -156,86 +73,45 @@ export const useCreateVPC = (inputs: UseCreateVPCInputs) => { }); } } catch (errors) { - const generalSubnetErrors = errors.filter( - (error: APIError) => - // Both general and specific subnet errors include 'subnets' in their error field. - // General subnet errors come in as { field: subnets.some_field, ...}, whereas - // specific subnet errors come in as { field: subnets[some_index].some_field, ...}. So, - // to avoid specific subnet errors, we filter out errors with a field that includes '[' - error.field && - error.field.includes('subnets') && - !error.field.includes('[') - ); - - if (generalSubnetErrors) { - setGeneralSubnetErrorsFromAPI(generalSubnetErrors); + for (const error of errors) { + if (error?.field === 'subnets.label') { + form.setError('subnets', { message: error.reason }); + } else { + form.setError(error?.field ?? 'root', { message: error.reason }); + } } - const indivSubnetErrors = handleVPCAndSubnetErrors( - errors.filter( - // ignore general subnet errors: !(the logic of filtering for only general subnet errors) - (error: APIError) => - !error.field?.includes('subnets') || - !error.field || - error.field.includes('[') - ), - formik.setFieldError, - setGeneralAPIError - ); - - // must combine errors and subnet data to avoid indexing weirdness when deleting a subnet - const subnetsAndErrors = combineErrorsAndSubnets( - indivSubnetErrors, - visualToAPISubnetMapping - ); - formik.setFieldValue('subnets', subnetsAndErrors); - - scrollErrorIntoView(); } - - formik.setSubmitting(false); }; - const formik = useFormik({ - enableReinitialize: true, - initialValues: { + const form = useForm({ + defaultValues: { description: '', label: '', region: selectedRegion ?? '', subnets: [ { - ip: { - availIPv4s: 256, - ipv4: DEFAULT_SUBNET_IPV4_VALUE, - ipv4Error: '', - }, + ipv4: DEFAULT_SUBNET_IPV4_VALUE, label: '', - labelError: '', }, - ] as SubnetFieldState[], - } as CreateVPCFieldState, - onSubmit: onCreateVPC, - validateOnChange: false, - validationSchema: createVPCSchema, + ], + }, + mode: 'onBlur', + resolver: yupResolver(createVPCSchema), }); - // Helper method to set a field's value and clear existing errors - const onChangeField = (field: string, value: string) => { - formik.setFieldValue(field, value); - if (formik.errors[field as keyof CreateVPCFieldState]) { - formik.setFieldError(field, undefined); + const { errors } = form.formState; + + React.useEffect(() => { + if (!isEmpty(errors)) { + scrollErrorIntoViewV2(formContainerRef); } - }; + }, [errors]); return { - formik, - generalAPIError, - generalSubnetErrorsFromAPI, + form, isLoadingCreateVPC, - onChangeField, onCreateVPC, regionsData, - setGeneralAPIError, - setGeneralSubnetErrorsFromAPI, userCannotAddVPC, }; }; diff --git a/packages/manager/src/hooks/useCreateVPCv2.ts b/packages/manager/src/hooks/useCreateVPCv2.ts deleted file mode 100644 index f3cbcd63086..00000000000 --- a/packages/manager/src/hooks/useCreateVPCv2.ts +++ /dev/null @@ -1,117 +0,0 @@ -import { yupResolver } from '@hookform/resolvers/yup'; -import { isEmpty } from '@linode/api-v4'; -import { createVPCSchema } from '@linode/validation'; -import * as React from 'react'; -import { useForm } from 'react-hook-form'; -import { useHistory, useLocation } from 'react-router-dom'; - -import { useGrants, useProfile } from 'src/queries/profile/profile'; -import { useRegionsQuery } from 'src/queries/regions/regions'; -import { useCreateVPCMutation } from 'src/queries/vpcs/vpcs'; -import { sendLinodeCreateFormStepEvent } from 'src/utilities/analytics/formEventAnalytics'; -import { getQueryParamsFromQueryString } from 'src/utilities/queryParams'; -import { scrollErrorIntoViewV2 } from 'src/utilities/scrollErrorIntoViewV2'; -import { DEFAULT_SUBNET_IPV4_VALUE } from 'src/utilities/subnets'; - -import type { CreateVPCPayload, VPC } from '@linode/api-v4'; -import type { LinodeCreateType } from 'src/features/Linodes/LinodeCreate/types'; - -// Custom hook to consolidate shared logic between VPCCreate.tsx and VPCCreateDrawer.tsx -export interface UseCreateVPCInputs { - formContainerRef: React.RefObject; - handleSelectVPC?: (vpc: VPC) => void; - onDrawerClose?: () => void; - pushToVPCPage?: boolean; - selectedRegion?: string; -} - -export const useCreateVPC = (inputs: UseCreateVPCInputs) => { - const { - formContainerRef, - handleSelectVPC, - onDrawerClose, - pushToVPCPage, - selectedRegion, - } = inputs; - - const history = useHistory(); - const { data: profile } = useProfile(); - const { data: grants } = useGrants(); - const userCannotAddVPC = profile?.restricted && !grants?.global.add_vpcs; - - const location = useLocation(); - const isFromLinodeCreate = location.pathname.includes('/linodes/create'); - const queryParams = getQueryParamsFromQueryString(location.search); - - const { data: regions } = useRegionsQuery(); - const regionsData = regions ?? []; - - const { - isPending: isLoadingCreateVPC, - mutateAsync: createVPC, - } = useCreateVPCMutation(); - - const onCreateVPC = async (values: CreateVPCPayload) => { - try { - const vpc = await createVPC(values); - if (pushToVPCPage) { - history.push(`/vpcs/${vpc.id}`); - } else { - if (handleSelectVPC && onDrawerClose) { - handleSelectVPC(vpc); - onDrawerClose(); - } - } - - // Fire analytics form submit upon successful VPC creation from Linode Create flow. - if (isFromLinodeCreate) { - sendLinodeCreateFormStepEvent({ - createType: (queryParams.type as LinodeCreateType) ?? 'OS', - headerName: 'Create VPC', - interaction: 'click', - label: 'Create VPC', - }); - } - } catch (errors) { - for (const error of errors) { - if (error?.field === 'subnets.label') { - form.setError('subnets', { message: error.reason }); - } else { - form.setError(error?.field ?? 'root', { message: error.reason }); - } - } - } - }; - - const form = useForm({ - defaultValues: { - description: '', - label: '', - region: selectedRegion ?? '', - subnets: [ - { - ipv4: DEFAULT_SUBNET_IPV4_VALUE, - label: '', - }, - ], - }, - mode: 'onBlur', - resolver: yupResolver(createVPCSchema), - }); - - const { errors } = form.formState; - - React.useEffect(() => { - if (!isEmpty(errors)) { - scrollErrorIntoViewV2(formContainerRef); - } - }, [errors]); - - return { - form, - isLoadingCreateVPC, - onCreateVPC, - regionsData, - userCannotAddVPC, - }; -}; diff --git a/packages/validation/src/vpcs.schema.ts b/packages/validation/src/vpcs.schema.ts index 4484e08047f..5183cfb5d9c 100644 --- a/packages/validation/src/vpcs.schema.ts +++ b/packages/validation/src/vpcs.schema.ts @@ -130,7 +130,7 @@ export const createSubnetSchema = object().shape( then: (schema) => schema.required(IP_EITHER_BOTH_NOT_NEITHER).test({ name: 'IPv4 CIDR format', - message: 'The IPv4 range must be in CIDR format', + message: 'The IPv4 range must be in CIDR format.', test: (value) => vpcsValidateIP({ value, @@ -147,7 +147,7 @@ export const createSubnetSchema = object().shape( case 'string': return schema.notRequired().test({ name: 'IPv4 CIDR format', - message: 'The IPv4 range must be in CIDR format', + message: 'The IPv4 range must be in CIDR format.', test: (value) => vpcsValidateIP({ value, From 0ca008ed01f720d1b188f08192654250d203b89c Mon Sep 17 00:00:00 2001 From: Connie Liu Date: Tue, 3 Dec 2024 10:12:28 -0500 Subject: [PATCH 05/17] update drawer --- .../VPCCreateDrawer/VPCCreateDrawer.test.tsx | 45 ++++-- .../VPCs/VPCCreateDrawer/VPCCreateDrawer.tsx | 137 +++++++++--------- packages/manager/src/hooks/useCreateVPC.ts | 1 + 3 files changed, 99 insertions(+), 84 deletions(-) diff --git a/packages/manager/src/features/VPCs/VPCCreateDrawer/VPCCreateDrawer.test.tsx b/packages/manager/src/features/VPCs/VPCCreateDrawer/VPCCreateDrawer.test.tsx index e639c38d2d4..b18c26b67e3 100644 --- a/packages/manager/src/features/VPCs/VPCCreateDrawer/VPCCreateDrawer.test.tsx +++ b/packages/manager/src/features/VPCs/VPCCreateDrawer/VPCCreateDrawer.test.tsx @@ -1,7 +1,7 @@ -import { fireEvent, screen } from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; import * as React from 'react'; -import { renderWithTheme } from 'src/utilities/testHelpers'; +import { renderWithThemeAndHookFormContext } from 'src/utilities/testHelpers'; import { VPCCreateDrawer } from './VPCCreateDrawer'; @@ -11,11 +11,27 @@ const props = { open: true, }; -describe.skip('VPC Create Drawer', () => { +const formOptions = { + defaultValues: { + description: '', + label: '', + region: '', + subnets: [ + { + ipv4: '', + label: 'subnet 0', + }, + ], + }, +}; + +describe('VPC Create Drawer', () => { it('should render the vpc and subnet sections', () => { - const { getAllByText } = renderWithTheme(); + const { getAllByText } = renderWithThemeAndHookFormContext({ + component: , + useFormOptions: formOptions, + }); - getAllByText('Region'); getAllByText('VPC Label'); getAllByText('Region'); getAllByText('Description'); @@ -28,16 +44,21 @@ describe.skip('VPC Create Drawer', () => { }); it('should not be able to remove the first subnet', () => { - renderWithTheme(); + const { queryByTestId } = renderWithThemeAndHookFormContext({ + component: , + useFormOptions: formOptions, + }); - const deleteSubnet = screen.queryByTestId('delete-subnet-0'); - expect(deleteSubnet).not.toBeInTheDocument(); + expect(queryByTestId('delete-subnet-0')).toBeNull(); }); - it('should close the drawer', () => { - renderWithTheme(); - const cancelButton = screen.getByText('Cancel'); - fireEvent.click(cancelButton); + it('should close the drawer', async () => { + const { getByText } = renderWithThemeAndHookFormContext({ + component: , + useFormOptions: formOptions, + }); + const cancelButton = getByText('Cancel'); + await userEvent.click(cancelButton); expect(props.onClose).toHaveBeenCalled(); }); }); diff --git a/packages/manager/src/features/VPCs/VPCCreateDrawer/VPCCreateDrawer.tsx b/packages/manager/src/features/VPCs/VPCCreateDrawer/VPCCreateDrawer.tsx index 99f28b3ada0..2c355d077db 100644 --- a/packages/manager/src/features/VPCs/VPCCreateDrawer/VPCCreateDrawer.tsx +++ b/packages/manager/src/features/VPCs/VPCCreateDrawer/VPCCreateDrawer.tsx @@ -2,6 +2,7 @@ import { Box, Notice } from '@linode/ui'; import { useTheme } from '@mui/material/styles'; import Grid from '@mui/material/Unstable_Grid2'; import * as React from 'react'; +import { FormProvider } from 'react-hook-form'; import { ActionsPanel } from 'src/components/ActionsPanel/ActionsPanel'; import { Drawer } from 'src/components/Drawer'; @@ -23,80 +24,72 @@ interface Props { } export const VPCCreateDrawer = (props: Props) => { - // const theme = useTheme(); - // const { onClose, onSuccess, open, selectedRegion } = props; + const theme = useTheme(); + const formContainerRef = React.useRef(null); + const { onClose, onSuccess, open, selectedRegion } = props; - // const { - // formik, - // generalAPIError, - // generalSubnetErrorsFromAPI, - // isLoadingCreateVPC, - // onChangeField, - // onCreateVPC, - // regionsData, - // setGeneralAPIError, - // setGeneralSubnetErrorsFromAPI, - // userCannotAddVPC, - // } = useCreateVPC({ - // handleSelectVPC: onSuccess, - // onDrawerClose: onClose, - // selectedRegion, - // }); + const { + form, + isLoadingCreateVPC, + onCreateVPC, + regionsData, + userCannotAddVPC, + } = useCreateVPC({ + formContainerRef, + handleSelectVPC: onSuccess, + onDrawerClose: onClose, + selectedRegion, + }); - // const { errors, handleSubmit, resetForm, setFieldValue, values } = formik; + const { + formState: { errors }, + handleSubmit, + reset, + } = form; - // React.useEffect(() => { - // if (open) { - // resetForm(); - // setGeneralSubnetErrorsFromAPI([]); - // setGeneralAPIError(undefined); - // } - // }, [open, resetForm, setGeneralAPIError, setGeneralSubnetErrorsFromAPI]); + return ( + { + onClose(); + reset(); + }} + open={open} + title={'Create VPC'} + > + {userCannotAddVPC && CannotCreateVPCNotice} - // return ( - // - // {userCannotAddVPC && CannotCreateVPCNotice} - // - // {generalAPIError ? ( - // - // ) : null} - //
- // - // - // - // - // { - // onCreateVPC(); - // }, - // }} - // secondaryButtonProps={{ - // 'data-testid': 'cancel', - // label: 'Cancel', - // onClick: onClose, - // }} - // style={{ marginTop: theme.spacing(3) }} - // /> - // - //
- // - // ); + + +
+ {errors.root?.message ? ( + + ) : null} + + + + + + +
+ + + ); }; diff --git a/packages/manager/src/hooks/useCreateVPC.ts b/packages/manager/src/hooks/useCreateVPC.ts index f3cbcd63086..43be79864a3 100644 --- a/packages/manager/src/hooks/useCreateVPC.ts +++ b/packages/manager/src/hooks/useCreateVPC.ts @@ -60,6 +60,7 @@ export const useCreateVPC = (inputs: UseCreateVPCInputs) => { if (handleSelectVPC && onDrawerClose) { handleSelectVPC(vpc); onDrawerClose(); + form.reset(); } } From e06832c468c3255749c629caf506fc544fac077b Mon Sep 17 00:00:00 2001 From: Connie Liu Date: Tue, 3 Dec 2024 10:20:54 -0500 Subject: [PATCH 06/17] remove unnecessary types --- packages/manager/src/utilities/subnets.ts | 17 ----------------- 1 file changed, 17 deletions(-) diff --git a/packages/manager/src/utilities/subnets.ts b/packages/manager/src/utilities/subnets.ts index 31564f72f8f..93ff787506d 100644 --- a/packages/manager/src/utilities/subnets.ts +++ b/packages/manager/src/utilities/subnets.ts @@ -11,23 +11,6 @@ export const SUBNET_LINODE_CSV_HEADERS = [ { key: 'interfaceData.ip_ranges', label: 'IPv4 VPC Ranges' }, ]; -// @TODO VPC: added ipv6 related fields here, but they will not be used until VPCs support ipv6 -export interface SubnetIPState { - availIPv4s?: number; - ipv4?: string; - ipv4Error?: string; - ipv6?: string; - ipv6Error?: string; -} - -export interface SubnetFieldState { - ip: SubnetIPState; - label: string; - labelError?: string; -} - -export type SubnetIPType = 'ipv4' | 'ipv6'; - /** * Maps subnet mask length to number of theoretically available IPs. * - To get usable IPs, subtract 2 from the given number, as the first and last From 2291f7fbb42f410ad95e2330ec8189429cffd663 Mon Sep 17 00:00:00 2001 From: Connie Liu Date: Mon, 9 Dec 2024 11:19:25 -0500 Subject: [PATCH 07/17] fix vpc create drawer bug default values --- .../FormComponents/VPCTopSectionContent.tsx | 3 ++- .../VPCs/VPCCreateDrawer/VPCCreateDrawer.tsx | 1 - packages/manager/src/hooks/useCreateVPC.ts | 25 +++++++++++-------- 3 files changed, 16 insertions(+), 13 deletions(-) diff --git a/packages/manager/src/features/VPCs/VPCCreate/FormComponents/VPCTopSectionContent.tsx b/packages/manager/src/features/VPCs/VPCCreate/FormComponents/VPCTopSectionContent.tsx index 6c3231af139..5eae5db43f5 100644 --- a/packages/manager/src/features/VPCs/VPCCreate/FormComponents/VPCTopSectionContent.tsx +++ b/packages/manager/src/features/VPCs/VPCCreate/FormComponents/VPCTopSectionContent.tsx @@ -59,7 +59,8 @@ export const VPCTopSectionContent = (props: Props) => { currentCapability="VPCs" disabled={isDrawer ? true : disabled} errorText={fieldState.error?.message} - onChange={(e, region) => field.onChange(region?.id ?? '')} + onBlur={field.onBlur} + onChange={(_, region) => field.onChange(region?.id ?? '')} regions={regions} value={field.value} /> diff --git a/packages/manager/src/features/VPCs/VPCCreateDrawer/VPCCreateDrawer.tsx b/packages/manager/src/features/VPCs/VPCCreateDrawer/VPCCreateDrawer.tsx index 2c355d077db..31eb2f4a8ef 100644 --- a/packages/manager/src/features/VPCs/VPCCreateDrawer/VPCCreateDrawer.tsx +++ b/packages/manager/src/features/VPCs/VPCCreateDrawer/VPCCreateDrawer.tsx @@ -57,7 +57,6 @@ export const VPCCreateDrawer = (props: Props) => { title={'Create VPC'} > {userCannotAddVPC && CannotCreateVPCNotice} -
diff --git a/packages/manager/src/hooks/useCreateVPC.ts b/packages/manager/src/hooks/useCreateVPC.ts index 43be79864a3..a2be9f1ea05 100644 --- a/packages/manager/src/hooks/useCreateVPC.ts +++ b/packages/manager/src/hooks/useCreateVPC.ts @@ -84,20 +84,23 @@ export const useCreateVPC = (inputs: UseCreateVPCInputs) => { } }; + const defaultValues = { + description: '', + label: '', + region: selectedRegion ?? '', + subnets: [ + { + ipv4: DEFAULT_SUBNET_IPV4_VALUE, + label: '', + }, + ], + }; + const form = useForm({ - defaultValues: { - description: '', - label: '', - region: selectedRegion ?? '', - subnets: [ - { - ipv4: DEFAULT_SUBNET_IPV4_VALUE, - label: '', - }, - ], - }, + defaultValues, mode: 'onBlur', resolver: yupResolver(createVPCSchema), + values: { ...defaultValues }, }); const { errors } = form.formState; From bc20b430441bccaecd1e2d96e4eb981e73c16cd9 Mon Sep 17 00:00:00 2001 From: Connie Liu Date: Mon, 9 Dec 2024 12:41:04 -0500 Subject: [PATCH 08/17] general subnet errors? --- .../features/VPCs/VPCCreate/FormComponents/SubnetContent.tsx | 5 ++--- packages/manager/src/hooks/useCreateVPC.ts | 5 ++++- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/packages/manager/src/features/VPCs/VPCCreate/FormComponents/SubnetContent.tsx b/packages/manager/src/features/VPCs/VPCCreate/FormComponents/SubnetContent.tsx index 6469b3b42be..4b92f09d285 100644 --- a/packages/manager/src/features/VPCs/VPCCreate/FormComponents/SubnetContent.tsx +++ b/packages/manager/src/features/VPCs/VPCCreate/FormComponents/SubnetContent.tsx @@ -60,11 +60,10 @@ export const SubnetContent = (props: Props) => { . - {/* TODO: figure out general subnet errors */} - {errors.subnets?.root?.message && ( + {errors.subnets?.message && ( )} diff --git a/packages/manager/src/hooks/useCreateVPC.ts b/packages/manager/src/hooks/useCreateVPC.ts index a2be9f1ea05..bef4e510c45 100644 --- a/packages/manager/src/hooks/useCreateVPC.ts +++ b/packages/manager/src/hooks/useCreateVPC.ts @@ -75,7 +75,10 @@ export const useCreateVPC = (inputs: UseCreateVPCInputs) => { } } catch (errors) { for (const error of errors) { - if (error?.field === 'subnets.label') { + if ( + error?.field === 'subnets.label' || + error?.field === 'subnets.ipv4' + ) { form.setError('subnets', { message: error.reason }); } else { form.setError(error?.field ?? 'root', { message: error.reason }); From 90aeb37942e3666941f421120fd3657409266b5b Mon Sep 17 00:00:00 2001 From: Connie Liu Date: Mon, 9 Dec 2024 12:46:36 -0500 Subject: [PATCH 09/17] remove unnecessary util --- .../src/utilities/formikErrorUtils.test.ts | 95 ------------------- .../manager/src/utilities/formikErrorUtils.ts | 54 ----------- 2 files changed, 149 deletions(-) diff --git a/packages/manager/src/utilities/formikErrorUtils.test.ts b/packages/manager/src/utilities/formikErrorUtils.test.ts index 48adf78a959..11bc89e5d45 100644 --- a/packages/manager/src/utilities/formikErrorUtils.test.ts +++ b/packages/manager/src/utilities/formikErrorUtils.test.ts @@ -1,7 +1,6 @@ import { getFormikErrorsFromAPIErrors, handleAPIErrors, - handleVPCAndSubnetErrors, set, } from './formikErrorUtils'; @@ -34,100 +33,6 @@ describe('handleAPIErrors', () => { }); }); -const subnetMultipleErrorsPerField = [ - { - field: 'subnets[0].label', - reason: 'not expected error for label', - }, - { - field: 'subnets[0].label', - reason: 'expected error for label', - }, - { - field: 'subnets[3].label', - reason: 'not expected error for label', - }, - { - field: 'subnets[3].label', - reason: 'expected error for label', - }, - { - field: 'subnets[3].ipv4', - reason: 'not expected error for ipv4', - }, - { - field: 'subnets[3].ipv4', - reason: 'expected error for ipv4', - }, -]; - -const subnetErrors = [ - { - field: 'subnets[1].label', - reason: 'Label required', - }, - { - field: 'subnets[2].label', - reason: 'bad label', - }, - { - field: 'subnets[2].ipv4', - reason: 'cidr ipv4', - }, - { - field: 'subnets[4].ipv4', - reason: 'needs an ip', - }, - { - field: 'subnets[4].ipv6', - reason: 'needs an ipv6', - }, -]; - -describe('handleVpcAndConvertSubnetErrors', () => { - it('converts API errors for subnets into a map of subnet index to SubnetErrors', () => { - const errors = handleVPCAndSubnetErrors( - subnetErrors, - setFieldError, - setError - ); - expect(Object.keys(errors)).toHaveLength(3); - expect(Object.keys(errors)).toEqual(['1', '2', '4']); - expect(errors[1]).toEqual({ label: 'Label required' }); - expect(errors[2]).toEqual({ ipv4: 'cidr ipv4', label: 'bad label' }); - expect(errors[4]).toEqual({ ipv4: 'needs an ip', ipv6: 'needs an ipv6' }); - }); - - it('takes the last error to display if a subnet field has multiple errors associated with it', () => { - const errors = handleVPCAndSubnetErrors( - subnetMultipleErrorsPerField, - setFieldError, - setError - ); - expect(Object.keys(errors)).toHaveLength(2); - expect(errors[0]).toEqual({ label: 'expected error for label' }); - expect(errors[3]).toEqual({ - ipv4: 'expected error for ipv4', - label: 'expected error for label', - }); - }); - - it('passes errors without the subnet field to handleApiErrors', () => { - const errors = handleVPCAndSubnetErrors( - errorWithField, - setFieldError, - setError - ); - expect(Object.keys(errors)).toHaveLength(0); - expect(errors).toEqual({}); - expect(setFieldError).toHaveBeenCalledWith( - 'card_number', - errorWithField[0].reason - ); - expect(setError).not.toHaveBeenCalled(); - }); -}); - describe('getFormikErrorsFromAPIErrors', () => { it('should convert APIError[] to errors in the shape formik expects', () => { const testCases = [ diff --git a/packages/manager/src/utilities/formikErrorUtils.ts b/packages/manager/src/utilities/formikErrorUtils.ts index 290c75b9638..2ae6cbcb776 100644 --- a/packages/manager/src/utilities/formikErrorUtils.ts +++ b/packages/manager/src/utilities/formikErrorUtils.ts @@ -145,57 +145,3 @@ export const handleAPIErrors = ( } }); }; - -export interface SubnetError { - ipv4?: string; - ipv6?: string; - label?: string; -} - -/** - * Handles given API errors and converts any specific subnet related errors into a usable format; - * Returns a map of subnets' indexes to their @interface SubnetError - * Example: errors = [{ reason: 'error1', field: 'subnets[1].label' }, - * { reason: 'error2', field: 'subnets[1].ipv4' }, - * { reason: 'not a subnet error so will not appear in return obj', field: 'label'}, - * { reason: 'error3', field: 'subnets[4].ipv4' }] - * returns: { - * 1: { label: 'error1', ipv4: 'error2' }, - * 4: { ipv4: 'error3'} - * } - * - * @param errors the errors from the API - * @param setFieldError function to set non-subnet related field errors - * @param setError function to set (non-subnet related) general API errors - */ -export const handleVPCAndSubnetErrors = ( - errors: APIError[], - setFieldError: (field: string, message: string) => void, - setError?: (message: string) => void -) => { - const subnetErrors: Record = {}; - const nonSubnetErrors: APIError[] = []; - - errors.forEach((error) => { - if (error.field && error.field.includes('subnets[')) { - const [subnetIdx, field] = error.field.split('.'); - const idx = parseInt( - subnetIdx.substring(subnetIdx.indexOf('[') + 1, subnetIdx.indexOf(']')), - 10 - ); - - // if there already exists some previous error for the subnet at index idx, we - // just add the current error. Otherwise, we create a new entry for the subnet. - if (subnetErrors[idx]) { - subnetErrors[idx] = { ...subnetErrors[idx], [field]: error.reason }; - } else { - subnetErrors[idx] = { [field]: error.reason }; - } - } else { - nonSubnetErrors.push(error); - } - }); - - handleAPIErrors(nonSubnetErrors, setFieldError, setError); - return subnetErrors; -}; From 9a35ffc45bbfc957c136bf14a95a244683b9169f Mon Sep 17 00:00:00 2001 From: Connie Liu Date: Mon, 9 Dec 2024 14:27:43 -0500 Subject: [PATCH 10/17] switch back to scrollIntoError v1 :/ --- .../src/features/VPCs/VPCCreate/VPCCreate.tsx | 10 ++++------ .../VPCs/VPCCreateDrawer/VPCCreateDrawer.tsx | 4 +--- packages/manager/src/hooks/useCreateVPC.ts | 15 ++++++++------- 3 files changed, 13 insertions(+), 16 deletions(-) diff --git a/packages/manager/src/features/VPCs/VPCCreate/VPCCreate.tsx b/packages/manager/src/features/VPCs/VPCCreate/VPCCreate.tsx index 36053a42f0d..7dbbba8d96b 100644 --- a/packages/manager/src/features/VPCs/VPCCreate/VPCCreate.tsx +++ b/packages/manager/src/features/VPCs/VPCCreate/VPCCreate.tsx @@ -17,15 +17,13 @@ import { StyledHeaderTypography } from './FormComponents/VPCCreateForm.styles'; import { VPCTopSectionContent } from './FormComponents/VPCTopSectionContent'; const VPCCreate = () => { - const formContainerRef = React.useRef(null); - const { form, isLoadingCreateVPC, onCreateVPC, regionsData, userCannotAddVPC, - } = useCreateVPC({ formContainerRef, pushToVPCPage: true }); + } = useCreateVPC({ pushToVPCPage: true }); const { formState: { errors }, @@ -51,10 +49,10 @@ const VPCCreate = () => { /> {userCannotAddVPC && CannotCreateVPCNotice} - - {errors.root?.message ? ( + + {errors.root?.message && ( - ) : null} + )} VPC { const theme = useTheme(); - const formContainerRef = React.useRef(null); const { onClose, onSuccess, open, selectedRegion } = props; const { @@ -35,7 +34,6 @@ export const VPCCreateDrawer = (props: Props) => { regionsData, userCannotAddVPC, } = useCreateVPC({ - formContainerRef, handleSelectVPC: onSuccess, onDrawerClose: onClose, selectedRegion, @@ -59,7 +57,7 @@ export const VPCCreateDrawer = (props: Props) => { {userCannotAddVPC && CannotCreateVPCNotice} - + {errors.root?.message ? ( ) : null} diff --git a/packages/manager/src/hooks/useCreateVPC.ts b/packages/manager/src/hooks/useCreateVPC.ts index bef4e510c45..ba65e1107cc 100644 --- a/packages/manager/src/hooks/useCreateVPC.ts +++ b/packages/manager/src/hooks/useCreateVPC.ts @@ -10,7 +10,7 @@ import { useRegionsQuery } from 'src/queries/regions/regions'; import { useCreateVPCMutation } from 'src/queries/vpcs/vpcs'; import { sendLinodeCreateFormStepEvent } from 'src/utilities/analytics/formEventAnalytics'; import { getQueryParamsFromQueryString } from 'src/utilities/queryParams'; -import { scrollErrorIntoViewV2 } from 'src/utilities/scrollErrorIntoViewV2'; +import { scrollErrorIntoView } from 'src/utilities/scrollErrorIntoView'; import { DEFAULT_SUBNET_IPV4_VALUE } from 'src/utilities/subnets'; import type { CreateVPCPayload, VPC } from '@linode/api-v4'; @@ -18,7 +18,6 @@ import type { LinodeCreateType } from 'src/features/Linodes/LinodeCreate/types'; // Custom hook to consolidate shared logic between VPCCreate.tsx and VPCCreateDrawer.tsx export interface UseCreateVPCInputs { - formContainerRef: React.RefObject; handleSelectVPC?: (vpc: VPC) => void; onDrawerClose?: () => void; pushToVPCPage?: boolean; @@ -27,13 +26,14 @@ export interface UseCreateVPCInputs { export const useCreateVPC = (inputs: UseCreateVPCInputs) => { const { - formContainerRef, handleSelectVPC, onDrawerClose, pushToVPCPage, selectedRegion, } = inputs; + const previousSubmitCount = React.useRef(0); + const history = useHistory(); const { data: profile } = useProfile(); const { data: grants } = useGrants(); @@ -106,13 +106,14 @@ export const useCreateVPC = (inputs: UseCreateVPCInputs) => { values: { ...defaultValues }, }); - const { errors } = form.formState; + const { errors, submitCount } = form.formState; React.useEffect(() => { - if (!isEmpty(errors)) { - scrollErrorIntoViewV2(formContainerRef); + if (!isEmpty(errors) && submitCount > previousSubmitCount.current) { + scrollErrorIntoView(undefined, { behavior: 'smooth' }); } - }, [errors]); + previousSubmitCount.current = submitCount; + }, [errors, submitCount]); return { form, From b96c7ab0c78f2160d1b029635df8cf8956b17b6b Mon Sep 17 00:00:00 2001 From: Connie Liu Date: Mon, 9 Dec 2024 15:55:21 -0500 Subject: [PATCH 11/17] general subnet errors i think this works for real --- .../VPCs/VPCCreate/FormComponents/SubnetContent.tsx | 11 +++++++++-- packages/manager/src/hooks/useCreateVPC.ts | 9 ++++----- 2 files changed, 13 insertions(+), 7 deletions(-) diff --git a/packages/manager/src/features/VPCs/VPCCreate/FormComponents/SubnetContent.tsx b/packages/manager/src/features/VPCs/VPCCreate/FormComponents/SubnetContent.tsx index 4b92f09d285..33a250e5d7f 100644 --- a/packages/manager/src/features/VPCs/VPCCreate/FormComponents/SubnetContent.tsx +++ b/packages/manager/src/features/VPCs/VPCCreate/FormComponents/SubnetContent.tsx @@ -60,10 +60,17 @@ export const SubnetContent = (props: Props) => { . - {errors.subnets?.message && ( + {errors.root?.subnetLabel && ( + )} + {errors.root?.subnetIPv4 && ( + )} diff --git a/packages/manager/src/hooks/useCreateVPC.ts b/packages/manager/src/hooks/useCreateVPC.ts index ba65e1107cc..e8bddfd357c 100644 --- a/packages/manager/src/hooks/useCreateVPC.ts +++ b/packages/manager/src/hooks/useCreateVPC.ts @@ -75,11 +75,10 @@ export const useCreateVPC = (inputs: UseCreateVPCInputs) => { } } catch (errors) { for (const error of errors) { - if ( - error?.field === 'subnets.label' || - error?.field === 'subnets.ipv4' - ) { - form.setError('subnets', { message: error.reason }); + if (error?.field === 'subnets.label') { + form.setError('root.subnetLabel', { message: error.reason }); + } else if (error?.field === 'subnets.ipv4') { + form.setError('root.subnetIPv4', { message: error.reason }); } else { form.setError(error?.field ?? 'root', { message: error.reason }); } From b7694203bf0f5bdea7a23fa30df5ff3eb5e76d7f Mon Sep 17 00:00:00 2001 From: Connie Liu Date: Mon, 9 Dec 2024 16:16:00 -0500 Subject: [PATCH 12/17] Added changeset: Refactor VPC Create to use `react-hook-form` instead of `formik` --- .../.changeset/pr-11357-tech-stories-1733778960338.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 packages/manager/.changeset/pr-11357-tech-stories-1733778960338.md diff --git a/packages/manager/.changeset/pr-11357-tech-stories-1733778960338.md b/packages/manager/.changeset/pr-11357-tech-stories-1733778960338.md new file mode 100644 index 00000000000..f7d9d2977ee --- /dev/null +++ b/packages/manager/.changeset/pr-11357-tech-stories-1733778960338.md @@ -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)) From f1fb905da3550a345e2592ce039981098e8615c4 Mon Sep 17 00:00:00 2001 From: Connie Liu Date: Tue, 10 Dec 2024 15:16:19 -0500 Subject: [PATCH 13/17] feedback @bnussman-akamai, fix reset when using cancel button for VPCCreateDrawer --- .../features/VPCs/VPCCreate/SubnetNode.tsx | 96 +++++++++---------- .../VPCs/VPCCreateDrawer/VPCCreateDrawer.tsx | 16 ++-- 2 files changed, 55 insertions(+), 57 deletions(-) diff --git a/packages/manager/src/features/VPCs/VPCCreate/SubnetNode.tsx b/packages/manager/src/features/VPCs/VPCCreate/SubnetNode.tsx index 2416f626621..1326a83f383 100644 --- a/packages/manager/src/features/VPCs/VPCCreate/SubnetNode.tsx +++ b/packages/manager/src/features/VPCs/VPCCreate/SubnetNode.tsx @@ -1,10 +1,9 @@ -import { Button, FormHelperText, TextField } from '@linode/ui'; +import { Button, TextField } from '@linode/ui'; import Close from '@mui/icons-material/Close'; -import Stack from '@mui/material/Stack'; import { styled } from '@mui/material/styles'; import Grid from '@mui/material/Unstable_Grid2'; import * as React from 'react'; -import { Controller, useFormContext } from 'react-hook-form'; +import { Controller, useFormContext, useWatch } from 'react-hook-form'; import { RESERVED_IP_NUMBER, @@ -24,11 +23,21 @@ interface Props { export const SubnetNode = (props: Props) => { const { disabled, idx, isCreateVPCDrawer, remove } = props; - const { control, watch } = useFormContext(); + const { control } = useFormContext(); - const subnet = watch(`subnets.${idx}`); + const subnets = useWatch({ control, name: 'subnets' }); - const numberOfAvailIPs = calculateAvailableIPv4sRFC1918(subnet.ipv4 ?? ''); + const numberOfAvailIPs = calculateAvailableIPv4sRFC1918( + (subnets && subnets[idx].ipv4) ?? '' + ); + + const availableIPHelperText = numberOfAvailIPs + ? `Number of Available IP Addresses: ${ + numberOfAvailIPs > 4 + ? (numberOfAvailIPs - RESERVED_IP_NUMBER).toLocaleString() + : 0 + }` + : undefined; const showRemoveButton = !(isCreateVPCDrawer && idx === 0); @@ -39,49 +48,40 @@ export const SubnetNode = (props: Props) => { sx={{ ...(!showRemoveButton && { width: '100%' }), flexGrow: 1 }} xs={showRemoveButton ? 11 : 12} > - - ( - - )} - control={control} - name={`subnets.${idx}.label`} - /> - ( - - )} - control={control} - name={`subnets.${idx}.ipv4`} - /> - {numberOfAvailIPs && ( - - Number of Available IP Addresses:{' '} - {numberOfAvailIPs > 4 - ? (numberOfAvailIPs - RESERVED_IP_NUMBER).toLocaleString() - : 0} - + ( + + )} + control={control} + name={`subnets.${idx}.label`} + /> + ( + )} - + control={control} + name={`subnets.${idx}.ipv4`} + /> {showRemoveButton && ( diff --git a/packages/manager/src/features/VPCs/VPCCreateDrawer/VPCCreateDrawer.tsx b/packages/manager/src/features/VPCs/VPCCreateDrawer/VPCCreateDrawer.tsx index bad014dfa14..28d77e166ee 100644 --- a/packages/manager/src/features/VPCs/VPCCreateDrawer/VPCCreateDrawer.tsx +++ b/packages/manager/src/features/VPCs/VPCCreateDrawer/VPCCreateDrawer.tsx @@ -45,15 +45,13 @@ export const VPCCreateDrawer = (props: Props) => { reset, } = form; + const handleDrawerClose = () => { + onClose(); + reset(); + }; + return ( - { - onClose(); - reset(); - }} - open={open} - title={'Create VPC'} - > + {userCannotAddVPC && CannotCreateVPCNotice} @@ -80,7 +78,7 @@ export const VPCCreateDrawer = (props: Props) => { secondaryButtonProps={{ 'data-testid': 'cancel', label: 'Cancel', - onClick: onClose, + onClick: handleDrawerClose, }} style={{ marginTop: theme.spacing(3) }} /> From 85ce27b13f9ad9b58c64a7468ba2052356ff6da2 Mon Sep 17 00:00:00 2001 From: Connie Liu Date: Tue, 10 Dec 2024 15:27:08 -0500 Subject: [PATCH 14/17] cleanup use of useWatch and add some accessibility help to remove button --- .../manager/src/features/VPCs/VPCCreate/SubnetNode.tsx | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/packages/manager/src/features/VPCs/VPCCreate/SubnetNode.tsx b/packages/manager/src/features/VPCs/VPCCreate/SubnetNode.tsx index 1326a83f383..1b19eee28ec 100644 --- a/packages/manager/src/features/VPCs/VPCCreate/SubnetNode.tsx +++ b/packages/manager/src/features/VPCs/VPCCreate/SubnetNode.tsx @@ -25,11 +25,9 @@ export const SubnetNode = (props: Props) => { const { control } = useFormContext(); - const subnets = useWatch({ control, name: 'subnets' }); + const { ipv4, label } = useWatch({ control, name: `subnets.${idx}` }); - const numberOfAvailIPs = calculateAvailableIPv4sRFC1918( - (subnets && subnets[idx].ipv4) ?? '' - ); + const numberOfAvailIPs = calculateAvailableIPv4sRFC1918(ipv4 ?? ''); const availableIPHelperText = numberOfAvailIPs ? `Number of Available IP Addresses: ${ @@ -86,7 +84,7 @@ export const SubnetNode = (props: Props) => { {showRemoveButton && ( remove(idx)} > From af5dddd730176180a5e8c2f137e3de9103ecadac Mon Sep 17 00:00:00 2001 From: Connie Liu Date: Tue, 10 Dec 2024 15:35:31 -0500 Subject: [PATCH 15/17] update cypress test due to updated accessibility labels for subnet remove buttons --- packages/manager/cypress/e2e/core/vpc/vpc-create.spec.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/manager/cypress/e2e/core/vpc/vpc-create.spec.ts b/packages/manager/cypress/e2e/core/vpc/vpc-create.spec.ts index 1159a7e3363..404b8f20bb1 100644 --- a/packages/manager/cypress/e2e/core/vpc/vpc-create.spec.ts +++ b/packages/manager/cypress/e2e/core/vpc/vpc-create.spec.ts @@ -187,7 +187,7 @@ describe('VPC create flow', () => { ); // Delete subnet. - cy.findByLabelText('Remove Subnet') + cy.findByLabelText('Remove Subnet 1') .should('be.visible') .should('be.enabled') .click(); @@ -304,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(); From 3f200ef13c5c12a1f35ba3663bd9815c81b70a50 Mon Sep 17 00:00:00 2001 From: Connie Liu Date: Wed, 11 Dec 2024 10:03:22 -0500 Subject: [PATCH 16/17] hiding mention of IPv6 in validation for now since we don't support it --- packages/manager/cypress/e2e/core/vpc/vpc-create.spec.ts | 3 +-- packages/validation/src/vpcs.schema.ts | 6 +++++- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/packages/manager/cypress/e2e/core/vpc/vpc-create.spec.ts b/packages/manager/cypress/e2e/core/vpc/vpc-create.spec.ts index 404b8f20bb1..31c9522f730 100644 --- a/packages/manager/cypress/e2e/core/vpc/vpc-create.spec.ts +++ b/packages/manager/cypress/e2e/core/vpc/vpc-create.spec.ts @@ -71,8 +71,7 @@ describe('VPC create flow', () => { subnets: mockSubnets, }); - const ipValidationErrorMessage1 = - 'A subnet must have either IPv4 or IPv6, or both, but not neither.'; + const ipValidationErrorMessage1 = 'A subnet must have an IPv4 range.'; const ipValidationErrorMessage2 = 'The IPv4 range must be in CIDR format.'; const vpcCreationErrorMessage = 'An unknown error has occurred.'; const totalSubnetUniqueLinodes = getUniqueLinodesFromSubnets(mockSubnets); diff --git a/packages/validation/src/vpcs.schema.ts b/packages/validation/src/vpcs.schema.ts index 5183cfb5d9c..be92dd771e3 100644 --- a/packages/validation/src/vpcs.schema.ts +++ b/packages/validation/src/vpcs.schema.ts @@ -13,6 +13,8 @@ const labelTestDetails = { const IP_EITHER_BOTH_NOT_NEITHER = 'A subnet must have either IPv4 or IPv6, or both, but not neither.'; +// @TODO VPC - remove below constant when IPv6 is added +const TEMPORARY_IPV4_REQUIRED_MESSAGE = 'A subnet must have an IPv4 range.'; export const determineIPType = (ip: string) => { try { @@ -128,7 +130,9 @@ export const createSubnetSchema = object().shape( is: (value: unknown) => value === '' || value === null || value === undefined, then: (schema) => - schema.required(IP_EITHER_BOTH_NOT_NEITHER).test({ + // @TODO VPC - change required message back to IP_EITHER_BOTH_NOT_NEITHER when IPv6 is supported + // Since only IPv4 is currently supported, subnets must have an IPv4 + schema.required(TEMPORARY_IPV4_REQUIRED_MESSAGE).test({ name: 'IPv4 CIDR format', message: 'The IPv4 range must be in CIDR format.', test: (value) => From 988d5489f17b849ad2f13cfe900236b1da393f0c Mon Sep 17 00:00:00 2001 From: Connie Liu Date: Wed, 11 Dec 2024 10:07:01 -0500 Subject: [PATCH 17/17] validation changeset --- .../validation/.changeset/pr-11357-changed-1733929595429.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 packages/validation/.changeset/pr-11357-changed-1733929595429.md diff --git a/packages/validation/.changeset/pr-11357-changed-1733929595429.md b/packages/validation/.changeset/pr-11357-changed-1733929595429.md new file mode 100644 index 00000000000..66aec530c3f --- /dev/null +++ b/packages/validation/.changeset/pr-11357-changed-1733929595429.md @@ -0,0 +1,5 @@ +--- +"@linode/validation": Changed +--- + +Update VPC validation to temporarily hide mention of IPv6 in UI, fix punctuation ([#11357](https://github.com/linode/manager/pull/11357))