-
Notifications
You must be signed in to change notification settings - Fork 358
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. Weβll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: [M3-7718] - Stale assigned Firewall data displaying on Linode and NodeBalancer details pages #10534
fix: [M3-7718] - Stale assigned Firewall data displaying on Linode and NodeBalancer details pages #10534
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
--- | ||
"@linode/manager": Fixed | ||
--- | ||
|
||
Stale assigned Firewall data displaying on Linode and NodeBalancer details pages ([#10534](https://github.com/linode/manager/pull/10534)) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,4 +1,5 @@ | ||
import { styled } from '@mui/material/styles'; | ||
import { useQueryClient } from '@tanstack/react-query'; | ||
import { useSnackbar } from 'notistack'; | ||
import * as React from 'react'; | ||
|
||
|
@@ -7,11 +8,15 @@ import { ConfirmationDialog } from 'src/components/ConfirmationDialog/Confirmati | |
import { Notice } from 'src/components/Notice/Notice'; | ||
import { Prompt } from 'src/components/Prompt/Prompt'; | ||
import { Typography } from 'src/components/Typography'; | ||
import { useUpdateFirewallRulesMutation } from 'src/queries/firewalls'; | ||
import { | ||
useAllFirewallDevicesQuery, | ||
useUpdateFirewallRulesMutation, | ||
} from 'src/queries/firewalls'; | ||
import { queryKey as linodesQueryKey } from 'src/queries/linodes/linodes'; | ||
import { queryKey as nodebalancersQueryKey } from 'src/queries/nodebalancers'; | ||
import { getAPIErrorOrDefault } from 'src/utilities/errorUtils'; | ||
|
||
import { FirewallRuleDrawer } from './FirewallRuleDrawer'; | ||
import { FirewallRuleTable } from './FirewallRuleTable'; | ||
import { | ||
hasModified as _hasModified, | ||
curriedFirewallRuleEditorReducer, | ||
|
@@ -20,6 +25,7 @@ import { | |
prepareRules, | ||
stripExtendedFields, | ||
} from './firewallRuleEditor'; | ||
import { FirewallRuleTable } from './FirewallRuleTable'; | ||
import { parseFirewallRuleError } from './shared'; | ||
|
||
import type { FirewallRuleDrawerMode } from './FirewallRuleDrawer.types'; | ||
|
@@ -51,6 +57,8 @@ export const FirewallRulesLanding = React.memo((props: Props) => { | |
const { mutateAsync: updateFirewallRules } = useUpdateFirewallRulesMutation( | ||
firewallID | ||
); | ||
const { data: devices } = useAllFirewallDevicesQuery(firewallID); | ||
const queryClient = useQueryClient(); | ||
|
||
const { enqueueSnackbar } = useSnackbar(); | ||
|
||
|
@@ -193,6 +201,19 @@ export const FirewallRulesLanding = React.memo((props: Props) => { | |
updateFirewallRules(finalRules) | ||
.then((_rules) => { | ||
setSubmitting(false); | ||
// Invalidate Firewalls assigned to NodeBalancers and Linodes. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No invalidation was happening on Firewall rule updates, which was the initial issue identified in the ticket. |
||
// eslint-disable-next-line no-unused-expressions | ||
devices?.forEach((device) => | ||
queryClient.invalidateQueries([ | ||
device.entity.type === 'linode' | ||
? linodesQueryKey | ||
: nodebalancersQueryKey, | ||
device.entity.type, | ||
device.entity.id, | ||
'firewalls', | ||
]) | ||
); | ||
|
||
// Reset editor state. | ||
inboundDispatch({ rules: _rules.inbound ?? [], type: 'RESET' }); | ||
outboundDispatch({ rules: _rules.outbound ?? [], type: 'RESET' }); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -8,7 +8,7 @@ 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 { queryKey as nodebalancerQueryKey } from 'src/queries/nodebalancers'; | ||
import { queryKey as nodebalancersQueryKey } from 'src/queries/nodebalancers'; | ||
import { capitalize } from 'src/utilities/capitalize'; | ||
|
||
export type Mode = 'delete' | 'disable' | 'enable'; | ||
|
@@ -66,17 +66,19 @@ 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) => { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Previously, we were only invalidating the Firewalls assigned to a device when deleting the Firewall. Updating the status of the Firewall (e.g. enabling it or disabling it) should be reason to invalidate the Firewall too. |
||
const deviceType = device.entity.type; | ||
queryClient.invalidateQueries([ | ||
deviceType === 'linode' ? linodesQueryKey : nodebalancersQueryKey, | ||
deviceType, | ||
device.entity.id, | ||
'firewalls', | ||
]); | ||
}); | ||
if (mode === 'delete') { | ||
devices?.forEach((device) => { | ||
const deviceType = device.entity.type; | ||
queryClient.invalidateQueries([ | ||
deviceType === 'linode' ? linodesQueryKey : nodebalancerQueryKey, | ||
deviceType, | ||
device.entity.id, | ||
'firewalls', | ||
]); | ||
queryClient.invalidateQueries([firewallQueryKey]); | ||
}); | ||
queryClient.invalidateQueries([firewallQueryKey]); | ||
} | ||
enqueueSnackbar(`Firewall ${label} successfully ${mode}d`, { | ||
variant: 'success', | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just some linting and renaming in this file. The query key for nodebalancers is plural, so updated the name of the import accordingly. This is done in a few other files, too.