upcoming: [M3-8364] - Use React Hook Form instead of Formik for Create Bucket #10699

jaalah-akamai · 2024-07-22T17:27:12Z

Description 📝

Switching from Formik to React Hook Form before updating the drawer with the new UI / API requirements.

Changes 🔄

Using isFeatureEnabledV2 instead of deprecated isFeatureEnabled
Unique label validation is happening in the schema now
Reset of values is now happening onExited

Target release date 🗓️

N/A

Preview 📷

N/A

How to test 🧪

Note

Ensure you test both legacy and multi-cluster (gen1) by toggling flag

Verification steps

Create a new bucket and ensure the creation process still works
Try creating a bucket with a label that's already used and ensure the validation works
- You should see an error with helper text under label input field
The bucket creation form should be more responsive, with real-time validation feedback
Form fields should validate input as the user types, not just on submission
The form should prevent submission if there are any validation errors

As an Author I have considered 🤔

Check all that apply

…e Bucket

github-actions · 2024-07-22T17:54:52Z

Coverage Report: ✅
Base Coverage: 82.46%
Current Coverage: 82.46%

jaalah-akamai · 2024-07-23T21:26:31Z

packages/manager/src/features/ObjectStorage/BucketLanding/ClusterSelect.tsx

@@ -11,7 +11,7 @@ interface Props {
  onBlur: (e: any) => void;
  onChange: (value: string) => void;
  required?: boolean;
-  selectedCluster: string;
+  selectedCluster: string | undefined;


The RegionSelect for which this is a wrapper has the value as string | undefined which I believe to be correct.

jaalah-akamai · 2024-07-23T21:29:41Z

@bnussman I would love a review for the React Hook refactor / @cpathipa for overall sanity check that functionality still works for OBJ.

cpathipa

Create Bucket functionality is working as expected for Gen1 and legacy.

Able to create a new bucket for Gen1 and legacy.
Validation is working as expected for label that's already used.
Confirming on error with helper text under label input field
- Confirming form prevents submission if there are any validation errors.

Note: Since we are using V2 flag, we should enable the flag in LD for Gen1 in respective environment when this is shipped to production.

packages/manager/src/features/ObjectStorage/BucketLanding/CreateBucketDrawer.tsx

bnussman-akamai

On an account without Object Storage enabled, I wasn't able to enable Object Storage

Screen.Recording.2024-07-25.at.9.10.11.AM.mov

bnussman-akamai · 2024-07-25T13:04:19Z

packages/manager/src/features/ObjectStorage/BucketLanding/CreateBucketDrawer.tsx

+    setError,
+    watch,
+  } = useForm<CreateObjectStorageBucketPayload>({
+    context: { buckets: bucketsData?.buckets ?? [] },


Neat. Did not know about context

jaalah-akamai · 2024-07-25T20:28:59Z

I need to fix the OBJ test since I'm now disabling Create Bucket unless they provide a label and a region, and the tests aren't accounting for that.

@hana-linode The clone-linode.spec.ts is looking for the old region format, surprised this wasn't caught in the PR where we changed this? 🤔

clone-linode.spec.ts.mp4

carrillo-erik · 2024-07-25T20:32:03Z

I still need to test this with the multi-cluster feature flag turned on. The legacy experience worked as expected and did not see any visual regressions. Nothing in the code stood out on first inspection.

AzureLatte

Looks good!

hana-akamai · 2024-07-26T14:34:42Z

The clone-linode.spec.ts is looking for the old region format, surprised this wasn't caught in the PR where we changed this? 🤔

@jaalah tests were not updated since we're still going to display the old format for now since Gecko LA/GA got pushed 2 months (new format is feature flagged under Gecko GA). The tests passed on my PR 🤔

carrillo-erik · 2024-07-26T15:25:14Z

The clone-linode.spec.ts is looking for the old region format, surprised this wasn't caught in the PR where we changed this? 🤔

@jaalah tests were not updated since we're still going to display the old format for now since Gecko LA/GA got pushed 2 months (new format is feature flagged under Gecko GA). The tests passed on my PR 🤔

@jaalah-akamai @hana-linode
I experienced the region label issue in the clone-linode.spec.ts when I tried to merge M3-8142. In order to have the tests pass without updating the tests, I had to comment out the gecko2 feature flag from the useFlags.ts file.

  return {
    ...flags,
    ...mockFlags,
    // gecko2: {
    //   enabled: true,
    //   ga: true,
    // },
  };

jaalah-akamai · 2024-07-26T16:13:12Z

@carrillo-erik Yea this issue is stemming from #10702. We are looking into a patch to fix this without having to revert the work. Until then if you need to get your tests passing, you can disable the gecko2 flag in your mock test. Something like this should work

mockAppendFeatureFlags({
   gecko2: makeFeatureFlagData(false),
}).as('getFeatureFlags');

packages/manager/cypress/e2e/core/objectStorage/enable-object-storage.spec.ts

bnussman-akamai · 2024-07-26T16:29:23Z

packages/validation/src/buckets.schema.ts

+        .test(
+          'unique-label',
+          'A bucket with this label already exists in your selected region',
+          (value, context) => {
+            const { cluster, region } = context.parent;
+            const buckets = context.options.context?.buckets;
+
+            if (!Array.isArray(buckets)) {
+              // If buckets is not an array, assume the label is unique
+              return true;
+            }
+
+            return !buckets.some(
+              (bucket) =>
+                bucket.label === value &&
+                (bucket.cluster === cluster || bucket.region === region)
+            );
+          }
+        ),


In my opinion, I think we should just remove this logic and rely on API errors for uniqueness, but this seems fine!

Hmm, I haven't looked too deeply into this yet, but it seems that no API error is caught in the case of a duplicate. Instead, it silently reverts the optimistic UI rendering of the bucket or something similar. I'll investigate this further and try to address it in my next PR. For now, I think the validation is useful. 👍

Ohh, got it! Thanks for the investigation 🙏

bnussman-akamai · 2024-07-26T16:31:35Z

packages/manager/src/features/ObjectStorage/BucketLanding/OMC_CreateBucketDrawer.tsx

+              !watchLabel ||
+              !watchRegion ||


From previous cafe discussion, UX decided that we should to keep form submit buttons enabled globally so that the user can submit at any time and receive validation errors.

Not a huge deal here because this is a small form, but this will diverge from our pattern.

Ok no problem, I might have forgotten we talked about this 👍

carrillo-erik · 2024-07-26T16:43:25Z

Tested and verified all workflows related to these changes. In addition, all tests are now ✅.

jaalah-akamai · 2024-07-27T01:13:05Z

This appears to be a flaky e2e test looking at detail/PR-10699/9/pipeline.

upcoming: [M3-8301] - Use React Hook Form instead of Formik for Creat…

6fc0760

…e Bucket

jaalah-akamai added Ready for Review Object Storage Gen2 labels Jul 22, 2024

jaalah-akamai requested a review from cpathipa July 22, 2024 17:27

jaalah-akamai self-assigned this Jul 22, 2024

jaalah-akamai requested a review from a team as a code owner July 22, 2024 17:27

jaalah-akamai requested review from carrillo-erik and AzureLatte and removed request for a team July 22, 2024 17:27

Add changesets

dbea22f

jaalah-akamai changed the title ~~upcoming: [M3-8301] - Use React Hook Form instead of Formik for Create Bucket~~ upcoming: [M3-8364] - Use React Hook Form instead of Formik for Create Bucket Jul 22, 2024

jaalah-akamai marked this pull request as draft July 23, 2024 02:21

jaalah-akamai added Work in Progress and removed Ready for Review labels Jul 23, 2024

jaalah added 5 commits July 22, 2024 22:22

More isFeatureEnabled changes

72df763

Saving...

725877a

Remove cluster

d4e94d1

More changes

8b6d6e2

Cleanup

7835e63

jaalah-akamai commented Jul 23, 2024

View reviewed changes

jaalah-akamai added Ready for Review and removed Work in Progress labels Jul 23, 2024

jaalah-akamai requested a review from bnussman-akamai July 23, 2024 21:28

jaalah-akamai marked this pull request as ready for review July 23, 2024 21:28

cpathipa approved these changes Jul 24, 2024

View reviewed changes

packages/manager/src/features/ObjectStorage/BucketLanding/CreateBucketDrawer.tsx Outdated Show resolved Hide resolved

Review updates @cpathipa

95732de

jaalah-akamai added the Add'tl Approval Needed Waiting on another approval! label Jul 24, 2024

bnussman-akamai reviewed Jul 25, 2024

View reviewed changes

jaalah added 2 commits July 25, 2024 13:31

Fix issue with enabling object storage

bbb8983

Merge branch 'develop' of github.com:linode/manager into M3-8301-v2

43f6d91

Fix obj e2e spec

0b3ecda

jaalah-akamai requested a review from a team as a code owner July 25, 2024 21:46

jaalah-akamai requested review from cliu-akamai and removed request for a team July 25, 2024 21:46

jaalah-akamai added SDET Approval Needed and removed Add'tl Approval Needed Waiting on another approval! labels Jul 25, 2024

AzureLatte approved these changes Jul 26, 2024

View reviewed changes

Fix create bucket submit

f379949

bnussman-akamai approved these changes Jul 26, 2024

View reviewed changes

carrillo-erik approved these changes Jul 26, 2024

View reviewed changes

jaalah added 2 commits July 26, 2024 16:09

Final review updates

8ba1b9c

Update e2e with safer check

754066a

jaalah-akamai merged commit 62b5b7b into linode:develop Jul 27, 2024
18 of 19 checks passed

jaalah-akamai deleted the M3-8301-v2 branch July 27, 2024 01:39

This was referenced Aug 1, 2024

Release v1.125.0 - release → staging #10741

Merged

Release v1.125.0 - staging → master #10748

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

upcoming: [M3-8364] - Use React Hook Form instead of Formik for Create Bucket #10699

upcoming: [M3-8364] - Use React Hook Form instead of Formik for Create Bucket #10699

jaalah-akamai commented Jul 22, 2024 •

edited

Loading

github-actions bot commented Jul 22, 2024 •

edited

Loading

jaalah-akamai Jul 23, 2024

jaalah-akamai commented Jul 23, 2024

cpathipa left a comment •

edited

Loading

bnussman-akamai left a comment

bnussman-akamai Jul 25, 2024

jaalah-akamai commented Jul 25, 2024

carrillo-erik commented Jul 25, 2024

AzureLatte left a comment

hana-akamai commented Jul 26, 2024

carrillo-erik commented Jul 26, 2024

jaalah-akamai commented Jul 26, 2024

bnussman-akamai Jul 26, 2024

jaalah-akamai Jul 26, 2024

bnussman-akamai Jul 26, 2024

bnussman-akamai Jul 26, 2024

jaalah-akamai Jul 26, 2024

carrillo-erik commented Jul 26, 2024

jaalah-akamai commented Jul 27, 2024

upcoming: [M3-8364] - Use React Hook Form instead of Formik for Create Bucket #10699

upcoming: [M3-8364] - Use React Hook Form instead of Formik for Create Bucket #10699

Conversation

jaalah-akamai commented Jul 22, 2024 • edited Loading

Description 📝

Changes 🔄

Target release date 🗓️

Preview 📷

How to test 🧪

Verification steps

As an Author I have considered 🤔

github-actions bot commented Jul 22, 2024 • edited Loading

jaalah-akamai Jul 23, 2024

Choose a reason for hiding this comment

jaalah-akamai commented Jul 23, 2024

cpathipa left a comment • edited Loading

Choose a reason for hiding this comment

bnussman-akamai left a comment

Choose a reason for hiding this comment

bnussman-akamai Jul 25, 2024

Choose a reason for hiding this comment

jaalah-akamai commented Jul 25, 2024

carrillo-erik commented Jul 25, 2024

AzureLatte left a comment

Choose a reason for hiding this comment

hana-akamai commented Jul 26, 2024

carrillo-erik commented Jul 26, 2024

jaalah-akamai commented Jul 26, 2024

bnussman-akamai Jul 26, 2024

Choose a reason for hiding this comment

jaalah-akamai Jul 26, 2024

Choose a reason for hiding this comment

bnussman-akamai Jul 26, 2024

Choose a reason for hiding this comment

bnussman-akamai Jul 26, 2024

Choose a reason for hiding this comment

jaalah-akamai Jul 26, 2024

Choose a reason for hiding this comment

carrillo-erik commented Jul 26, 2024

jaalah-akamai commented Jul 27, 2024

jaalah-akamai commented Jul 22, 2024 •

edited

Loading

github-actions bot commented Jul 22, 2024 •

edited

Loading

cpathipa left a comment •

edited

Loading