refactor: [M3-8777] - Refactor CreateCluster component to use react-hook-form #11581

hasyed-akamai · 2025-01-29T10:07:58Z

Description 📝

This PR refactors the CreateCluster component to use react-hook-form for form state management. It also removes the MultipleIPInput components for cleaner code.

Changes 🔄

Used React Hook Form for better state management

Target release date 🗓️

N/A

Verification steps

Check if the CreateCluster component's functionality is unaffected by the changes

Author Checklists

As an Author, to speed up the review process, I considered 🤔

👀 Doing a self review
❔ Our contribution guidelines
🤏 Splitting feature into small PRs
➕ Adding a changeset
🧪 Providing/improving test coverage
🔐 Removing all sensitive information from the code and PR description
🚩 Using a feature flag to protect the release
👣 Providing comprehensive reproduction steps
📑 Providing or updating our documentation
🕛 Scheduling a pair reviewing session
📱 Providing mobile support
♿ Providing accessibility support

I have read and considered all applicable items listed above.

As an Author, before moving this PR from Draft to Open, I confirmed ✅

All unit tests are passing
TypeScript compilation succeeded without errors
Code passes all linting rules

…to-use-react-hook-form

packages/manager/src/features/Kubernetes/CreateCluster/ControlPlaneACLPane.tsx

packages/manager/src/features/Kubernetes/CreateCluster/CreateCluster.tsx

packages/manager/src/features/Kubernetes/CreateCluster/ControlPlaneACLPane.tsx

github-actions · 2025-02-12T11:02:17Z

Coverage Report: ❌
Base Coverage: 80.07%
Current Coverage: 80.03%

hana-akamai · 2025-02-12T20:08:23Z

packages/manager/cypress/e2e/core/linodes/plan-selection.spec.ts

-      cy.findAllByRole('alert').should('have.length', 1);
-      cy.get(notices.limitedAvailability).should('be.visible');
+      cy.findAllByRole('alert').should('have.length', 0);


Is the alert and limited availability notice the same thing?

hana-akamai · 2025-02-12T20:09:40Z

packages/manager/cypress/e2e/core/linodes/plan-selection.spec.ts

-        cy.get('[data-qa-plan-row="gpu-2 Ada"]').should(
-          'have.attr',
-          'disabled'
-        );
+        cy.get('[data-qa-plan-row="gpu-2 Ada"]').should('be.visible');


What's the context for this change? It seems like this PR is a refactor, so I'm a bit confused as to why some test cases are changing

packages/manager/src/features/Kubernetes/CreateCluster/ControlPlaneACLPane.tsx

linode-gh-bot · 2025-02-13T01:00:53Z

Cloud Manager UI test results

🔺 2 failing tests on test run #17 ↗︎

❌ Failing	✅ Passing	↪️ Skipped	🕐 Duration
`2 Failing`	`498 Passing`	`2 Skipped`	`114m 31s`

Details

Failing Tests
	Spec	Test
❌	`smoke-community-stackscripts.spec.ts`	Community Stackscripts integration tests » pagination works with infinite scrolling
❌	`smoke-community-stackscripts.spec.ts`	Community Stackscripts integration tests » deploys a new linode as expected

Troubleshooting

Use this command to re-run the failing tests:

yarn cy:run -s "cypress/e2e/core/stackscripts/smoke-community-stackscripts.spec.ts"

bnussman-akamai · 2025-02-18T20:00:20Z

packages/manager/src/features/Kubernetes/CreateCluster/ControlPlaneACLPane.tsx

+  React.useEffect(() => {
+    if (enableControlPlaneACL) {
+      if (
+        (!ipv4Addresses || ipv4Addresses.length === 0) &&
+        !ipv4Fields.length
+      ) {
+        appendIPv4('');
+      }
+      if (
+        (!ipv6Addresses || ipv6Addresses.length === 0) &&
+        !ipv6Fields.length
+      ) {
+        appendIPv6('');
+      }
+    }
+  }, [
+    enableControlPlaneACL,
+    ipv4Addresses,
+    ipv6Addresses,
+    ipv4Fields,
+    ipv6Fields,
+    appendIPv4,
+    appendIPv6,
+  ]);


Instead of doing this here using a useEffect, can we just do this in the initialValues function of useForm?

packages/manager/src/features/Kubernetes/CreateCluster/ControlPlaneACLPane.tsx

packages/manager/src/features/Kubernetes/CreateCluster/CreateCluster.tsx

bnussman-akamai · 2025-02-18T20:08:40Z

packages/manager/src/features/Kubernetes/CreateCluster/CreateCluster.tsx

  const [version, setVersion] = React.useState<string | undefined>();
  const [errors, setErrors] = React.useState<APIError[] | undefined>();
-  const [submitting, setSubmitting] = React.useState<boolean>(false);
  const [hasAgreed, setAgreed] = React.useState<boolean>(false);


If we are going to use react-hook-form, it should ideally manage all of the form's state. All of of these states should be removed

bnussman-akamai · 2025-02-18T20:15:16Z

packages/manager/src/features/Kubernetes/CreateCluster/CreateCluster.tsx

+  React.useEffect(() => {
+    if (versions.length > 0) {
+      setVersion(getLatestVersion(versions).value);
+    }


Rather than using a useEffect, we should probably use an async defaultValues function for this form's useForm that sets version to the latest version.

It would look something very roughly like this:

const { control, getValues, handleSubmit, setValue, watch, } = useForm<CreateKubeClusterPayload>({ async defaultValues() { const versions = await queryClient.ensureQueryData(kubernetesQueries.versions) return { k8s_version: getLatestVersion(versions) }; }, });

bnussman-akamai · 2025-02-18T20:16:03Z

packages/manager/src/features/Kubernetes/CreateCluster/CreateCluster.tsx

-      scrollErrorIntoViewV2(formContainerRef);
-      return;
-    }
+  const createCluster = (formData: any) => {


Suggested change

const createCluster = (formData: any) => {

const createCluster = (formData: CreateKubeClusterPayload) => {

bnussman-akamai · 2025-02-18T20:17:05Z

packages/manager/src/features/Kubernetes/CreateCluster/CreateCluster.tsx

@@ -301,208 +286,306 @@ export const CreateCluster = () => {
    isSelectedRegionEligibleForPlan,
  } = plansNoticesUtils({
    regionsData,
-    selectedRegionID: selectedRegion?.id,
+    selectedRegionID: '',


Do we need to pass the selected region ID?

bnussman-akamai · 2025-02-18T20:18:17Z

packages/manager/src/features/Kubernetes/CreateCluster/CreateCluster.tsx

-                handleClusterTypeSelection={handleClusterTypeSelection}
-                selectedTier={selectedTier}
-              />
-            </>
-          )}
-          <Divider sx={{ marginTop: 4 }} />
-          <StyledFieldWithDocsStack>
-            <Stack>
-              <RegionSelect
-                currentCapability={
-                  isLkeEnterpriseLAFeatureEnabled &&
-                  selectedTier === 'enterprise'
-                    ? 'Kubernetes Enterprise'
-                    : 'Kubernetes'
-                }
-                textFieldProps={{
-                  helperText: <RegionHelperText mb={2} />,
-                  helperTextPosition: 'top',
-                }}
-                tooltipText={
-                  isLkeEnterpriseLAFeatureEnabled &&
-                  selectedTier === 'enterprise'
-                    ? 'Only regions that support Kubernetes Enterprise are listed.'
-                    : undefined
-                }
-                disableClearable
-                errorText={errorMap.region}
-                onChange={(e, region) => setSelectedRegion(region)}
-                regions={regionsData}
-                value={selectedRegion?.id}
-              />
-            </Stack>
-            <StyledDocsLinkContainer
-              sx={(theme) => ({ marginTop: theme.spacing(2) })}
-            >
-              <DocsLink
-                href="https://www.linode.com/pricing"
-                label={DOCS_LINK_LABEL_DC_PRICING}
-              />
-            </StyledDocsLinkContainer>
-          </StyledFieldWithDocsStack>
-          <Divider sx={{ marginTop: 4 }} />
-          <Autocomplete
-            onChange={(_, selected) => {
-              setVersion(selected?.value);
-            }}
-            disableClearable={!!version}
-            errorText={errorMap.k8s_version}
-            label="Kubernetes Version"
-            loading={isLoadingVersions}
-            options={versions}
-            placeholder={' '}
-            value={versions.find((v) => v.value === version) ?? null}
-          />
-          {showAPL && (
-            <>
-              <Divider sx={{ marginTop: 4 }} />
-              <StyledFieldWithDocsStack>
-                <Stack>
-                  <ApplicationPlatform
-                    setAPL={setApl_enabled}
-                    setHighAvailability={setHighAvailability}
-                  />
-                </Stack>
-              </StyledFieldWithDocsStack>
-            </>
-          )}
-          <Divider sx={{ marginTop: showAPL ? 1 : 4 }} />
-          {showHighAvailability && selectedTier !== 'enterprise' && (
-            <Box data-testid="ha-control-plane">
-              <HAControlPlane
-                highAvailabilityPrice={
-                  isErrorKubernetesTypes || !highAvailabilityPrice
-                    ? UNKNOWN_PRICE
-                    : highAvailabilityPrice
-                }
-                isAPLEnabled={apl_enabled}
-                isErrorKubernetesTypes={isErrorKubernetesTypes}
-                isLoadingKubernetesTypes={isLoadingKubernetesTypes}
-                selectedRegionId={selectedRegion?.id}
-                setHighAvailability={setHighAvailability}
-              />
-            </Box>
-          )}
-          {showControlPlaneACL && (
-            <>
-              {selectedTier !== 'enterprise' && <Divider />}
-              <ControlPlaneACLPane
-                handleIPv4Change={(newIpV4Addr: ExtendedIP[]) => {
-                  setIPv4Addr(newIpV4Addr);
-                }}
-                handleIPv6Change={(newIpV6Addr: ExtendedIP[]) => {
-                  setIPv6Addr(newIpV6Addr);
-                }}
-                enableControlPlaneACL={controlPlaneACL}
-                errorText={errorMap.control_plane}
-                ipV4Addr={ipV4Addr}
-                ipV6Addr={ipV6Addr}
-                setControlPlaneACL={setControlPlaneACL}
+    <form onSubmit={onSubmit}>


This could be cleaned up to be something like

return ( <form onSubmit={handleSubmit(createCluster)}>

…to-use-react-hook-form

initial commit

e65871b

hasyed-akamai self-assigned this Jan 29, 2025

hasyed-akamai added Work in Progress Do Not Merge labels Jan 29, 2025

hasyed-akamai added 13 commits January 30, 2025 12:42

populating IpV4 and ipV6

5f3f86f

ACL Enable and IPv4 and IPv6 are added correctly

9b81d82

fix unit test and add IPv4 and IPv6 input for default case

7388c0c

cleanup

c5b792d

Fix Cypress Test Part-1

d96d617

Fix e2e Test Part-2

e863e1e

Remove Loading Bug

83be6c2

Fix k8_version initial value

dfa1223

Add Validation to the IPv4 and IPv6

f6e7795

Set Validation errors and k8_version

f902c94

Add Error Scrollable Behaviour

5cdebc0

Added changeset: Optimize CreateCluster component: Use React Hook Form

e841372

Merge branch 'develop' into M3-8777-refactor-CreateCluster-component-…

8768c89

…to-use-react-hook-form

hasyed-akamai marked this pull request as ready for review February 6, 2025 12:14

hasyed-akamai requested review from a team as code owners February 6, 2025 12:14

hasyed-akamai requested review from jdamore-linode, hana-akamai and harsh-akamai and removed request for a team February 6, 2025 12:14

hasyed-akamai added Ready for Review Tech Debt and removed Work in Progress Do Not Merge labels Feb 6, 2025

harsh-akamai reviewed Feb 11, 2025

View reviewed changes

hasyed-akamai added 2 commits February 12, 2025 11:29

removed unused props and added updateFor prop

795d6ea

Fixed issue of Multiple call of ControlPlaneACLPane component

c1f15bb

hasyed-akamai requested a review from harsh-akamai February 12, 2025 11:18

hana-akamai reviewed Feb 12, 2025

View reviewed changes

hana-akamai requested a review from bnussman-akamai February 12, 2025 20:28

disabled remove button for the first input

7fb587b

bnussman-akamai reviewed Feb 18, 2025

View reviewed changes

Merge branch 'develop' into M3-8777-refactor-CreateCluster-component-…

31a5987

…to-use-react-hook-form

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor: [M3-8777] - Refactor CreateCluster component to use react-hook-form #11581

refactor: [M3-8777] - Refactor CreateCluster component to use react-hook-form #11581

hasyed-akamai commented Jan 29, 2025 •

edited

Loading

github-actions bot commented Feb 12, 2025 •

edited

Loading

hana-akamai Feb 12, 2025

hana-akamai Feb 12, 2025

linode-gh-bot commented Feb 13, 2025

bnussman-akamai Feb 18, 2025

bnussman-akamai Feb 18, 2025

bnussman-akamai Feb 18, 2025

bnussman-akamai Feb 18, 2025

bnussman-akamai Feb 18, 2025

bnussman-akamai Feb 18, 2025

	const createCluster = (formData: any) => {
	const createCluster = (formData: CreateKubeClusterPayload) => {

refactor: [M3-8777] - Refactor CreateCluster component to use react-hook-form #11581

Are you sure you want to change the base?

refactor: [M3-8777] - Refactor CreateCluster component to use react-hook-form #11581

Conversation

hasyed-akamai commented Jan 29, 2025 • edited Loading

Description 📝

Changes 🔄

Target release date 🗓️

Verification steps

As an Author, to speed up the review process, I considered 🤔

As an Author, before moving this PR from Draft to Open, I confirmed ✅

github-actions bot commented Feb 12, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

linode-gh-bot commented Feb 13, 2025

Cloud Manager UI test results

🔺 2 failing tests on test run #17 ↗︎

Details

Troubleshooting

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hasyed-akamai commented Jan 29, 2025 •

edited

Loading

github-actions bot commented Feb 12, 2025 •

edited

Loading