tech-story: [M3-8939] - Get rid of `DrawerContent` #11338

abailly-akamai · 2024-11-27T16:50:33Z

Description 📝

Small PR as a follow up to #11273 (loading patterns in Drawer) to clean up remaining legacy Drawer patterns.

DrawerContent was meant to handle loading states and errors, which is not handled directly within Drawer 🧹

Changes 🔄

Delete DrawerContent
Achieve parity for a variety of components
Cleanup Kube ACL Drawer
fix useKubernetesKubeConfigQuery typo

Preview 📷

No visual changes/regressions expected as part fo this PR

How to test 🧪

Verification steps

Verify kubernetes summary drawers (/kubernetes/clusters/{id}/summary
- in Summary Panel, click on "view"
- in Summary Panel, click on "Control Plane ACL: {CTA}"
Verify managed monitor history drawer (/managed/monitors)
- use MSW if no data
- monitors table > "View Issue History"

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

abailly-akamai · 2024-11-27T18:07:42Z

packages/manager/src/components/Drawer.tsx

+              errorText={Array.isArray(error) ? error[0].reason : error}
+            />
+          )
+        ) : isFetching ? (


addition of error handling is the change here

abailly-akamai · 2024-11-27T18:08:21Z

packages/manager/src/components/Drawer.tsx

-  const { classes, cx } = useStyles();
-  const { children, isFetching, onClose, open, title, wide, ...rest } = props;
-  const titleID = convertForAria(title);
+export const Drawer = React.forwardRef<HTMLDivElement, DrawerProps>(


this change indentation but is a minor change

this should remove A LOT of warnings through the APP with mounted drawers

abailly-akamai · 2024-11-27T18:11:47Z

packages/manager/src/features/Kubernetes/KubernetesClusterDetail/KubeControlPaneACLDrawer.tsx

-    data: data,
-    error: isErrorKubernetesACL,
-    isLoading: isLoadingKubernetesACL,
-  } = useKubernetesControlPlaneACLQuery(clusterId, showControlPlaneACL);


We are already fetching this in the summary, so this query is moot. Error handling is also handled in the summary. We can just receive the acl Data as a prop and use the data for the drawer. This is a place where it does make sense to do so

abailly-akamai · 2024-11-27T18:12:50Z

packages/manager/src/features/Kubernetes/KubernetesClusterDetail/KubeControlPaneACLDrawer.tsx

                  />
                )}
                control={control}
-                name="acl.enabled"
+                name="acl.revision-id"


diff looks that way, but none of this was changed

abailly-akamai · 2024-11-27T18:13:07Z

packages/manager/src/queries/kubernetes.ts

@@ -196,15 +196,15 @@ export const useAllKubernetesClusterAPIEndpointsQuery = (id: number) => {
  });
 };

-export const useKubenetesKubeConfigQuery = (
+export const useKubernetesKubeConfigQuery = (


that typo bothered me!

bnussman-akamai · 2024-11-27T19:51:57Z

packages/manager/src/features/Kubernetes/KubernetesClusterDetail/KubeConfigDrawer.tsx

-  // refetchOnMount isnt good enough for this query because
-  // it is already mounted in the rendered Drawer
-  React.useEffect(() => {
-    if (open && !isLoading && !isFetching) {
-      refetch();
-    }
-  }, [open]);


Was this removal intentional?

Wondering if we should adopt this pattern here so we can actually use React Query's refetchOnMount

If I'm remembering correctly, we wanted the refetching of the bube config to intentionally be aggressive so that there would be less of a chance of the user seeing an old kube config in the case they re-generate it

it seemed like a bad pattern to me - i'll look around and test this some more.

less of a chance of the user seeing an old kube config in the case they re-generate it

is there a reason why this does not get invalidated properly?

I think this is cleaner? 06b5690

Also makes sure this issue wouldn't be happening in other places it is used

We do invalidate the kube config but, if I remember correctly, the kube config takes some time (a few minutes) on the backend to actually be regenerated

That works too!

github-actions · 2024-11-27T20:56:42Z

Coverage Report: ❌
Base Coverage: 86.95%
Current Coverage: 86.92%

linode-gh-bot · 2024-11-30T00:23:02Z

Cloud Manager UI test results

🎉 462 passing tests on test run #7 ↗︎

❌ Failing	✅ Passing	↪️ Skipped	🕐 Duration
`0 Failing`	`462 Passing`	`2 Skipped`	`105m 20s`

abailly-akamai · 2024-12-02T14:09:27Z

packages/manager/src/features/Databases/DatabaseDetail/DatabaseBackups/DatabaseBackups.test.tsx

      expect(backupItem).toBeVisible();
-    }
+    });


This a pattern we ought to employ instead of await in loops to avoid flakiness (esp with dates). Hopefully this fixes this one for good

abailly-akamai · 2024-12-02T14:10:45Z

packages/manager/src/queries/kubernetes.ts

  clusterId: number,
  enabled = false
 ) =>
  useQuery<string, APIError[]>({
    ...kubernetesQueries.cluster(clusterId)._ctx.kubeconfig,
    enabled,
-    refetchOnMount: true,
-    retry: true,


We should prob be careful with retry: true as it will fetch indefinitely and not return the error

coliu-akamai

thank you!

✅ confirmed lke drawers
✅ managed monitor history drawer

cypress · 2024-12-02T23:49:51Z

Cloud Manager E2E Run #6906

Run Properties: Passed #6906 • 2581eb55a2: tech-story: [M3-8939] - Get rid of `DrawerContent` (#11338)

Project	`Cloud Manager E2E`
Branch Review	`develop`
Run status	`Passed #6906`
Run duration	`29m 51s`
Commit	2581eb55a2: tech-story: [M3-8939] - Get rid of `DrawerContent` (#11338)
Committer	`Alban Bailly`
View all properties for this run ↗︎

Test results
Failures	`0`
Flaky	`5`
Pending	`2`
Skipped	`0`
Passing	`465`
View all changes introduced in this branch ↗︎

KubeConfigDrawer

f064d2a

abailly-akamai self-assigned this Nov 27, 2024

abailly-akamai added 2 commits November 27, 2024 12:22

KubeControlPlaneACLDrawer

05efcc0

HistoryDrawer

368c7ec

abailly-akamai added the Tech Debt label Nov 27, 2024

abailly-akamai commented Nov 27, 2024

View reviewed changes

abailly-akamai marked this pull request as ready for review November 27, 2024 18:13

abailly-akamai requested a review from a team as a code owner November 27, 2024 18:13

abailly-akamai requested review from bnussman-akamai and coliu-akamai and removed request for a team November 27, 2024 18:13

bnussman-akamai reviewed Nov 27, 2024

View reviewed changes

abailly-akamai added 2 commits November 27, 2024 15:21

feedback @bnussman-akamai

06b5690

feedback @bnussman-akamai

550bfd2

abailly-akamai added 2 commits November 28, 2024 10:42

Fix passing data

e9eb6a2

Update e2e lke update suite for better label

dbef87e

abailly-akamai requested a review from a team as a code owner November 28, 2024 17:12

abailly-akamai requested review from cliu-akamai and removed request for a team November 28, 2024 17:12

fix maybe flaky DB test

a9ebc5c

abailly-akamai requested a review from bnussman-akamai December 2, 2024 14:06

abailly-akamai commented Dec 2, 2024

View reviewed changes

bnussman-akamai approved these changes Dec 2, 2024

View reviewed changes

coliu-akamai approved these changes Dec 2, 2024

View reviewed changes

coliu-akamai added the Approved Multiple approvals and ready to merge! label Dec 2, 2024

abailly-akamai merged commit 2581eb5 into linode:develop Dec 2, 2024
23 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

tech-story: [M3-8939] - Get rid of `DrawerContent` #11338

tech-story: [M3-8939] - Get rid of `DrawerContent` #11338

abailly-akamai commented Nov 27, 2024 •

edited

Loading

abailly-akamai Nov 27, 2024

abailly-akamai Nov 27, 2024

abailly-akamai Nov 27, 2024 •

edited

Loading

abailly-akamai Nov 27, 2024

abailly-akamai Nov 27, 2024

bnussman-akamai Nov 27, 2024 •

edited

Loading

abailly-akamai Nov 27, 2024

abailly-akamai Nov 27, 2024

bnussman-akamai Nov 27, 2024 •

edited

Loading

bnussman-akamai Nov 27, 2024

github-actions bot commented Nov 27, 2024 •

edited

Loading

linode-gh-bot commented Nov 30, 2024

abailly-akamai Dec 2, 2024

abailly-akamai Dec 2, 2024

coliu-akamai left a comment

cypress bot commented Dec 2, 2024

tech-story: [M3-8939] - Get rid of DrawerContent #11338

tech-story: [M3-8939] - Get rid of DrawerContent #11338

Conversation

abailly-akamai commented Nov 27, 2024 • edited Loading

Description 📝

Changes 🔄

Preview 📷

How to test 🧪

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 ✅

Choose a reason for hiding this comment

Choose a reason for hiding this comment

abailly-akamai Nov 27, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bnussman-akamai Nov 27, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bnussman-akamai Nov 27, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Nov 27, 2024 • edited Loading

linode-gh-bot commented Nov 30, 2024

Cloud Manager UI test results

🎉 462 passing tests on test run #7 ↗︎

Choose a reason for hiding this comment

Choose a reason for hiding this comment

coliu-akamai left a comment

Choose a reason for hiding this comment

cypress bot commented Dec 2, 2024

tech-story: [M3-8939] - Get rid of `DrawerContent` #11338

tech-story: [M3-8939] - Get rid of `DrawerContent` #11338

abailly-akamai commented Nov 27, 2024 •

edited

Loading

abailly-akamai Nov 27, 2024 •

edited

Loading

bnussman-akamai Nov 27, 2024 •

edited

Loading

bnussman-akamai Nov 27, 2024 •

edited

Loading

github-actions bot commented Nov 27, 2024 •

edited

Loading