refactor: [M3-8232] - Query Key Factory for Firewalls (#10568)

* initial work * event handler * a few fixes * Added changeset: Query Key Factory for Firewalls * improve and clean up --------- Co-authored-by: Banks Nussman <[email protected]>
linode · Jun 13, 2024 · 47dd6a7 · 47dd6a7
1 parent d244ecd
commit 47dd6a7
Show file tree

Hide file tree

Showing 13 changed files with 518 additions and 292 deletions.
diff --git a/packages/manager/.changeset/pr-10568-tech-stories-1718139515455.md b/packages/manager/.changeset/pr-10568-tech-stories-1718139515455.md
@@ -0,0 +1,5 @@
+---
+"@linode/manager": Tech Stories
+---
+
+Query Key Factory for Firewalls ([#10568](https://github.com/linode/manager/pull/10568))
diff --git a/packages/manager/src/features/Firewalls/FirewallDetail/Devices/AddNodebalancerDrawer.tsx b/packages/manager/src/features/Firewalls/FirewallDetail/Devices/AddNodebalancerDrawer.tsx
@@ -1,6 +1,4 @@
-import type { NodeBalancer } from '@linode/api-v4';
 import { useTheme } from '@mui/material';
-import { useQueryClient } from '@tanstack/react-query';
 import { useSnackbar } from 'notistack';
 import * as React from 'react';
 import { useParams } from 'react-router-dom';
@@ -16,12 +14,13 @@ import {
   useAddFirewallDeviceMutation,
   useAllFirewallsQuery,
 } from 'src/queries/firewalls';
-import { nodebalancerQueries } from 'src/queries/nodebalancers';
 import { useGrants, useProfile } from 'src/queries/profile/profile';
 import { getAPIErrorOrDefault } from 'src/utilities/errorUtils';
 import { getEntityIdsByPermission } from 'src/utilities/grants';
 import { sanitizeHTML } from 'src/utilities/sanitizeHTML';
 
+import type { NodeBalancer } from '@linode/api-v4';
+
 interface Props {
   helperText: string;
   onClose: () => void;
@@ -35,9 +34,8 @@ export const AddNodebalancerDrawer = (props: Props) => {
   const { data: grants } = useGrants();
   const { data: profile } = useProfile();
   const isRestrictedUser = Boolean(profile?.restricted);
-  const queryClient = useQueryClient();
 
-  const { data, error, isLoading } = useAllFirewallsQuery();
+  const { data, error, isLoading } = useAllFirewallsQuery(open);
 
   const firewall = data?.find((firewall) => firewall.id === Number(id));
 
@@ -73,10 +71,6 @@ export const AddNodebalancerDrawer = (props: Props) => {
         enqueueSnackbar(`NodeBalancer ${label} successfully added`, {
           variant: 'success',
         });
-        queryClient.invalidateQueries({
-          queryKey: nodebalancerQueries.nodebalancer(id)._ctx.firewalls
-            .queryKey,
-        });
         return;
       }
       failedNodebalancers.push(selectedNodebalancers[index]);

diff --git a/packages/manager/src/features/Firewalls/FirewallDetail/Devices/RemoveDeviceDialog.tsx b/packages/manager/src/features/Firewalls/FirewallDetail/Devices/RemoveDeviceDialog.tsx
@@ -6,7 +6,6 @@ import { ActionsPanel } from 'src/components/ActionsPanel/ActionsPanel';
 import { ConfirmationDialog } from 'src/components/ConfirmationDialog/ConfirmationDialog';
 import { Typography } from 'src/components/Typography';
 import { useRemoveFirewallDeviceMutation } from 'src/queries/firewalls';
-import { queryKey as firewallQueryKey } from 'src/queries/firewalls';
 import { queryKey as linodesQueryKey } from 'src/queries/linodes/linodes';
 import { nodebalancerQueries } from 'src/queries/nodebalancers';
 
@@ -57,12 +56,9 @@ export const RemoveDeviceDialog = React.memo((props: Props) => {
 
     // Since the linode was removed as a device, invalidate the linode-specific firewall query
     if (deviceType === 'linode') {
-      queryClient.invalidateQueries([
-        linodesQueryKey,
-        deviceType,
-        device.entity.id,
-        'firewalls',
-      ]);
+      queryClient.invalidateQueries({
+        queryKey: [linodesQueryKey, deviceType, device.entity.id, 'firewalls'],
+      });
     }
 
     if (deviceType === 'nodebalancer') {
@@ -72,8 +68,6 @@ export const RemoveDeviceDialog = React.memo((props: Props) => {
       });
     }
 
-    queryClient.invalidateQueries([firewallQueryKey]);
-
     onClose();
   };
 

diff --git a/packages/manager/src/features/Firewalls/FirewallDetail/Rules/FirewallRulesLanding.tsx b/packages/manager/src/features/Firewalls/FirewallDetail/Rules/FirewallRulesLanding.tsx
@@ -205,12 +205,14 @@ export const FirewallRulesLanding = React.memo((props: Props) => {
         if (devices) {
           for (const device of devices) {
             if (device.entity.type === 'linode') {
-              queryClient.invalidateQueries([
-                linodesQueryKey,
-                device.entity.type,
-                device.entity.id,
-                'firewalls',
-              ]);
+              queryClient.invalidateQueries({
+                queryKey: [
+                  linodesQueryKey,
+                  device.entity.type,
+                  device.entity.id,
+                  'firewalls',
+                ],
+              });
             }
             if (device.entity.type === 'nodebalancer') {
               queryClient.invalidateQueries({

diff --git a/packages/manager/src/features/Firewalls/FirewallLanding/CreateFirewallDrawer.tsx b/packages/manager/src/features/Firewalls/FirewallLanding/CreateFirewallDrawer.tsx
@@ -1,13 +1,5 @@
 /* eslint-disable jsx-a11y/anchor-is-valid */
-import { Linode } from '@linode/api-v4';
-import {
-  CreateFirewallPayload,
-  Firewall,
-  FirewallDeviceEntityType,
-} from '@linode/api-v4/lib/firewalls';
-import { NodeBalancer } from '@linode/api-v4/lib/nodebalancers';
 import { CreateFirewallSchema } from '@linode/validation/lib/firewalls.schema';
-import { useQueryClient } from '@tanstack/react-query';
 import { useFormik } from 'formik';
 import { useSnackbar } from 'notistack';
 import * as React from 'react';
@@ -27,12 +19,7 @@ import { FIREWALL_LIMITS_CONSIDERATIONS_LINK } from 'src/constants';
 import { LinodeSelect } from 'src/features/Linodes/LinodeSelect/LinodeSelect';
 import { NodeBalancerSelect } from 'src/features/NodeBalancers/NodeBalancerSelect';
 import { useAccountManagement } from 'src/hooks/useAccountManagement';
-import {
-  queryKey as firewallQueryKey,
-  useAllFirewallsQuery,
-  useCreateFirewall,
-} from 'src/queries/firewalls';
-import { queryKey as linodesQueryKey } from 'src/queries/linodes/linodes';
+import { useAllFirewallsQuery, useCreateFirewall } from 'src/queries/firewalls';
 import { useGrants } from 'src/queries/profile/profile';
 import { sendLinodeCreateFormStepEvent } from 'src/utilities/analytics/formEventAnalytics';
 import { getErrorMap } from 'src/utilities/errorUtils';
@@ -48,8 +35,14 @@ import {
   NODEBALANCER_CREATE_FLOW_TEXT,
 } from './constants';
 
+import type {
+  CreateFirewallPayload,
+  Firewall,
+  FirewallDeviceEntityType,
+  Linode,
+  NodeBalancer,
+} from '@linode/api-v4';
 import type { LinodeCreateType } from 'src/features/Linodes/LinodesCreate/types';
-import { nodebalancerQueries } from 'src/queries/nodebalancers';
 
 export const READ_ONLY_DEVICES_HIDDEN_MESSAGE =
   'Only services you have permission to modify are shown.';
@@ -81,10 +74,9 @@ export const CreateFirewallDrawer = React.memo(
     const { _hasGrant, _isRestrictedUser } = useAccountManagement();
     const { data: grants } = useGrants();
     const { mutateAsync } = useCreateFirewall();
-    const { data } = useAllFirewallsQuery();
+    const { data } = useAllFirewallsQuery(open);
 
     const { enqueueSnackbar } = useSnackbar();
-    const queryClient = useQueryClient();
 
     const location = useLocation();
     const isFromLinodeCreate = location.pathname.includes('/linodes/create');
@@ -132,33 +124,10 @@ export const CreateFirewallDrawer = React.memo(
         mutateAsync(payload)
           .then((response) => {
             setSubmitting(false);
-            queryClient.invalidateQueries([firewallQueryKey]);
             enqueueSnackbar(`Firewall ${payload.label} successfully created`, {
               variant: 'success',
             });
 
-            // Invalidate for Linodes
-            if (payload.devices?.linodes) {
-              payload.devices.linodes.forEach((linodeId) => {
-                queryClient.invalidateQueries([
-                  linodesQueryKey,
-                  'linode',
-                  linodeId,
-                  'firewalls',
-                ]);
-              });
-            }
-
-            // Invalidate for NodeBalancers
-            if (payload.devices?.nodebalancers) {
-              for (const id of payload.devices.nodebalancers) {
-                queryClient.invalidateQueries({
-                  queryKey: nodebalancerQueries.nodebalancer(id)._ctx.firewalls
-                    .queryKey,
-                });
-              }
-            }
-
             if (onFirewallCreated) {
               onFirewallCreated(response);
             }

diff --git a/packages/manager/src/features/Firewalls/FirewallLanding/FirewallDialog.tsx b/packages/manager/src/features/Firewalls/FirewallLanding/FirewallDialog.tsx
@@ -5,46 +5,37 @@ import * as React from 'react';
 import { ActionsPanel } from 'src/components/ActionsPanel/ActionsPanel';
 import { ConfirmationDialog } from 'src/components/ConfirmationDialog/ConfirmationDialog';
 import { useDeleteFirewall, useMutateFirewall } from 'src/queries/firewalls';
-import { queryKey as firewallQueryKey } from 'src/queries/firewalls';
-import { useAllFirewallDevicesQuery } from 'src/queries/firewalls';
 import { queryKey as linodesQueryKey } from 'src/queries/linodes/linodes';
 import { nodebalancerQueries } from 'src/queries/nodebalancers';
 import { capitalize } from 'src/utilities/capitalize';
 
+import type { Firewall } from '@linode/api-v4';
+
 export type Mode = 'delete' | 'disable' | 'enable';
 
 interface Props {
   mode: Mode;
   onClose: () => void;
   open: boolean;
-  selectedFirewallId: number;
-  selectedFirewallLabel: string;
+  selectedFirewall: Firewall;
 }
 
 export const FirewallDialog = React.memo((props: Props) => {
   const { enqueueSnackbar } = useSnackbar();
   const queryClient = useQueryClient();
 
-  const {
-    mode,
-    onClose,
-    open,
-    selectedFirewallId,
-    selectedFirewallLabel: label,
-  } = props;
-
-  const { data: devices } = useAllFirewallDevicesQuery(selectedFirewallId);
+  const { mode, onClose, open, selectedFirewall } = props;
 
   const {
     error: updateError,
     isLoading: isUpdating,
     mutateAsync: updateFirewall,
-  } = useMutateFirewall(selectedFirewallId);
+  } = useMutateFirewall(selectedFirewall.id);
   const {
     error: deleteError,
     isLoading: isDeleting,
     mutateAsync: deleteFirewall,
-  } = useDeleteFirewall(selectedFirewallId);
+  } = useDeleteFirewall(selectedFirewall.id);
 
   const requestMap = {
     delete: () => deleteFirewall(),
@@ -66,31 +57,30 @@ export const FirewallDialog = React.memo((props: Props) => {
 
   const onSubmit = async () => {
     await requestMap[mode]();
+
     // Invalidate Firewalls assigned to NodeBalancers and Linodes when Firewall is enabled, disabled, or deleted.
-    // eslint-disable-next-line no-unused-expressions
-    devices?.forEach((device) => {
-      const deviceType = device.entity.type;
-      if (deviceType === 'linode') {
-        queryClient.invalidateQueries([
-          linodesQueryKey,
-          deviceType,
-          device.entity.id,
-          'firewalls',
-        ]);
+    for (const entity of selectedFirewall.entities) {
+      if (entity.type === 'nodebalancer') {
+        queryClient.invalidateQueries({
+          queryKey: nodebalancerQueries.nodebalancer(entity.id)._ctx.firewalls
+            .queryKey,
+        });
       }
-      if (deviceType === 'nodebalancer') {
+
+      if (entity.type === 'linode') {
         queryClient.invalidateQueries({
-          queryKey: nodebalancerQueries.nodebalancer(device.entity.id)._ctx
-            .firewalls.queryKey,
+          queryKey: [linodesQueryKey, 'linode', entity.id, 'firewalls'],
         });
       }
-    });
-    if (mode === 'delete') {
-      queryClient.invalidateQueries([firewallQueryKey]);
     }
-    enqueueSnackbar(`Firewall ${label} successfully ${mode}d`, {
-      variant: 'success',
-    });
+
+    enqueueSnackbar(
+      `Firewall ${selectedFirewall.label} successfully ${mode}d`,
+      {
+        variant: 'success',
+      }
+    );
+
     onClose();
   };
 
@@ -109,7 +99,7 @@ export const FirewallDialog = React.memo((props: Props) => {
       error={errorMap[mode]?.[0].reason}
       onClose={onClose}
       open={open}
-      title={`${capitalize(mode)} Firewall ${label}?`}
+      title={`${capitalize(mode)} Firewall ${selectedFirewall.label}?`}
     >
       Are you sure you want to {mode} this firewall?
     </ConfirmationDialog>

diff --git a/packages/manager/src/features/Firewalls/FirewallLanding/FirewallLanding.tsx b/packages/manager/src/features/Firewalls/FirewallLanding/FirewallLanding.tsx
@@ -18,11 +18,13 @@ import { useFirewallsQuery } from 'src/queries/firewalls';
 import { getAPIErrorOrDefault } from 'src/utilities/errorUtils';
 
 import { CreateFirewallDrawer } from './CreateFirewallDrawer';
-import { ActionHandlers as FirewallHandlers } from './FirewallActionMenu';
-import { FirewallDialog, Mode } from './FirewallDialog';
+import { FirewallDialog } from './FirewallDialog';
 import { FirewallLandingEmptyState } from './FirewallLandingEmptyState';
 import { FirewallRow } from './FirewallRow';
 
+import type { ActionHandlers as FirewallHandlers } from './FirewallActionMenu';
+import type { Mode } from './FirewallDialog';
+
 const preferenceKey = 'firewalls';
 
 const FirewallLanding = () => {
@@ -175,13 +177,12 @@ const FirewallLanding = () => {
         onClose={onCloseCreateDrawer}
         open={isCreateFirewallDrawerOpen}
       />
-      {selectedFirewallId && (
+      {selectedFirewall && (
         <FirewallDialog
           mode={dialogMode}
           onClose={() => setIsModalOpen(false)}
           open={isModalOpen}
-          selectedFirewallId={selectedFirewallId}
-          selectedFirewallLabel={selectedFirewall?.label ?? ''}
+          selectedFirewall={selectedFirewall}
         />
       )}
     </React.Fragment>

diff --git a/packages/manager/src/features/Firewalls/FirewallLanding/FirewallRow.test.tsx b/packages/manager/src/features/Firewalls/FirewallLanding/FirewallRow.test.tsx
@@ -68,21 +68,21 @@ describe('FirewallRow', () => {
   describe('getDeviceLinks', () => {
     it('should return a single Link if one Device is attached', () => {
       const device = firewallDeviceFactory.build();
-      const links = getDeviceLinks([device]);
+      const links = getDeviceLinks([device.entity]);
       const { getByText } = renderWithTheme(links);
       expect(getByText(device.entity.label));
     });
 
     it('should render up to three comma-separated links', () => {
       const devices = firewallDeviceFactory.buildList(3);
-      const links = getDeviceLinks(devices);
+      const links = getDeviceLinks(devices.map((device) => device.entity));
       const { queryAllByTestId } = renderWithTheme(links);
       expect(queryAllByTestId('firewall-row-link')).toHaveLength(3);
     });
 
     it('should render "plus N more" text for any devices over three', () => {
       const devices = firewallDeviceFactory.buildList(13);
-      const links = getDeviceLinks(devices);
+      const links = getDeviceLinks(devices.map((device) => device.entity));
       const { getByText, queryAllByTestId } = renderWithTheme(links);
       expect(queryAllByTestId('firewall-row-link')).toHaveLength(3);
       expect(getByText(/10 more/));