Skip to content

Commit ef127cb

Browse files
upcoming: [UIE-9793] - UI/UX enhancements and fixes for Rule Sets & Prefix Lists (part-2) (#13188)
* Fix Rule Set Id copy from Rules table row * Change `RuleSets` to `Rule Sets` text * Update Ruleset ID to Rule Set ID for edge case * Exclude Marked for deletion rulesets from the dropdown * Update default state of newly choosen PL or Special PL that supports both Ipv4 & IPv6 * Added changeset: UI/UX enhancements and fixes for Rule Sets & Prefix Lists (part-2) * Fix styling for long PL/RS labels in PL/RS Drawer * Add flex wrap * Minor styling bug fix * Copy Icon display style update * Clean up: avoid repetitive styles * Add a comment for clarity * Add unit tests for Rule Sets filter logic
1 parent ce761d2 commit ef127cb

File tree

10 files changed

+159
-38
lines changed

10 files changed

+159
-38
lines changed
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"@linode/manager": Upcoming Features
3+
---
4+
5+
UI/UX enhancements and fixes for Rule Sets & Prefix Lists (part-2) ([#13188](https://github.com/linode/manager/pull/13188))

packages/manager/src/features/Firewalls/FirewallDetail/Rules/FirewallPrefixListDrawer.tsx

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -204,7 +204,7 @@ export const FirewallPrefixListDrawer = React.memo(
204204
column?: boolean;
205205
copy?: boolean;
206206
label: string;
207-
value: React.ReactNode | string;
207+
value: React.ReactNode;
208208
}[];
209209

210210
return (
@@ -227,13 +227,10 @@ export const FirewallPrefixListDrawer = React.memo(
227227
<>
228228
{fields.map((item, idx) => (
229229
<StyledListItem
230+
fieldsMode
230231
key={`item-${idx}`}
231232
paddingMultiplier={2}
232-
sx={
233-
item.column
234-
? { flexDirection: 'column', alignItems: 'flex-start' }
235-
: {}
236-
}
233+
sx={item.column ? { flexDirection: 'column' } : {}}
237234
>
238235
{item.label && (
239236
<StyledLabel component="span">{item.label}:</StyledLabel>

packages/manager/src/features/Firewalls/FirewallDetail/Rules/FirewallRuleDrawer.test.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -143,7 +143,7 @@ describe('AddRuleSetDrawer', () => {
143143
// Description
144144
expect(
145145
getByText(
146-
'RuleSets are reusable collections of Cloud Firewall rules that use the same fields as individual rules. They let you manage and update multiple rules as a group. You can then apply them across different firewalls by reference.'
146+
'Rule Sets are reusable collections of Cloud Firewall rules that use the same fields as individual rules. They let you manage and update multiple rules as a group. You can then apply them across different firewalls by reference.'
147147
)
148148
).toBeVisible();
149149

packages/manager/src/features/Firewalls/FirewallDetail/Rules/FirewallRuleSetDetailsView.tsx

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -107,13 +107,10 @@ export const FirewallRuleSetDetailsView = (
107107
},
108108
].map((item, idx) => (
109109
<StyledListItem
110+
fieldsMode
110111
key={`item-${idx}`}
111112
paddingMultiplier={2}
112-
sx={
113-
item.column
114-
? { flexDirection: 'column', alignItems: 'flex-start' }
115-
: {}
116-
}
113+
sx={item.column ? { flexDirection: 'column' } : {}}
117114
>
118115
{item.label && (
119116
<StyledLabel component="span">{item.label}:</StyledLabel>
Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,64 @@
1+
import { firewallRuleSetFactory } from 'src/factories';
2+
3+
import { filterRuleSets } from './FirewallRuleSetForm';
4+
5+
import type { FirewallRuleSet } from '@linode/api-v4';
6+
7+
describe('filterRuleSets', () => {
8+
const mockRuleSets: FirewallRuleSet[] = [
9+
firewallRuleSetFactory.build({
10+
id: 1,
11+
type: 'inbound',
12+
label: 'Inbound RS',
13+
deleted: null,
14+
}),
15+
firewallRuleSetFactory.build({
16+
id: 2,
17+
type: 'outbound',
18+
label: 'Outbound RS',
19+
deleted: null,
20+
}),
21+
firewallRuleSetFactory.build({
22+
id: 3,
23+
type: 'inbound',
24+
label: 'Deleted RS',
25+
deleted: '2025-11-18T18:51:11', // Marked for Deletion Rule Set
26+
}),
27+
firewallRuleSetFactory.build({
28+
id: 4,
29+
type: 'inbound',
30+
label: 'Already Used RS',
31+
deleted: null,
32+
}),
33+
];
34+
35+
it('returns only ruleSets of the correct type', () => {
36+
const result = filterRuleSets({
37+
ruleSets: mockRuleSets,
38+
category: 'inbound',
39+
inboundAndOutboundRules: [],
40+
});
41+
42+
expect(result.map((r) => r.id)).toEqual([1, 4]);
43+
});
44+
45+
it('excludes ruleSets already referenced by the firewall', () => {
46+
const result = filterRuleSets({
47+
ruleSets: mockRuleSets,
48+
category: 'inbound',
49+
inboundAndOutboundRules: [{ ruleset: 4 }],
50+
});
51+
52+
expect(result.map((r) => r.id)).toEqual([1]); // excludes id=4
53+
});
54+
55+
it('excludes ruleSets marked for deletion', () => {
56+
const result = filterRuleSets({
57+
ruleSets: mockRuleSets,
58+
category: 'inbound',
59+
inboundAndOutboundRules: [],
60+
});
61+
62+
expect(result.some((r) => r.id === 3)).toBe(false);
63+
});
64+
});

packages/manager/src/features/Firewalls/FirewallDetail/Rules/FirewallRuleSetForm.tsx

Lines changed: 42 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,39 @@ import {
2222
import { StyledLabel, StyledListItem, useStyles } from './shared.styles';
2323

2424
import type { FirewallRuleSetFormProps } from './FirewallRuleDrawer.types';
25+
import type { Category } from './shared';
26+
import type { FirewallRuleSet, FirewallRuleType } from '@linode/api-v4';
27+
28+
interface FilterRuleSetsArgs {
29+
category: Category;
30+
inboundAndOutboundRules: FirewallRuleType[];
31+
ruleSets: FirewallRuleSet[];
32+
}
33+
34+
/**
35+
* Display only those Rule Sets that:
36+
* - are applicable to the given category
37+
* - are not already referenced by the firewall
38+
* - are not marked for deletion
39+
*/
40+
export const filterRuleSets = ({
41+
ruleSets,
42+
category,
43+
inboundAndOutboundRules,
44+
}: FilterRuleSetsArgs) => {
45+
return ruleSets.filter((ruleSet) => {
46+
// TODO: Firewall RuleSets: Remove this client-side filter once the API supports filtering by the 'type' field
47+
const isCorrectType = ruleSet.type === category;
48+
49+
const isNotAlreadyReferenced = !inboundAndOutboundRules.some(
50+
(rule) => rule.ruleset === ruleSet.id
51+
);
52+
53+
const isNotMarkedForDeletion = ruleSet.deleted === null;
54+
55+
return isCorrectType && isNotAlreadyReferenced && isNotMarkedForDeletion;
56+
});
57+
};
2558

2659
export const FirewallRuleSetForm = React.memo(
2760
(props: FirewallRuleSetFormProps) => {
@@ -58,19 +91,14 @@ export const FirewallRuleSetForm = React.memo(
5891
// Build dropdown options
5992
const ruleSetDropdownOptions = React.useMemo(
6093
() =>
61-
ruleSets
62-
// TODO: Firewall RuleSets: Remove this client-side filter once the API supports filtering by the 'type' field
63-
.filter(
64-
(ruleSet) =>
65-
ruleSet.type === category &&
66-
!inboundAndOutboundRules.some(
67-
(rule) => rule.ruleset === ruleSet.id
68-
)
69-
) // Display only rule sets applicable to the given category and filter out rule sets already referenced by the FW
70-
.map((ruleSet) => ({
71-
label: ruleSet.label,
72-
value: ruleSet.id,
73-
})),
94+
filterRuleSets({
95+
ruleSets,
96+
category,
97+
inboundAndOutboundRules,
98+
}).map((ruleSet) => ({
99+
label: ruleSet.label,
100+
value: ruleSet.id,
101+
})),
74102
[ruleSets]
75103
);
76104

@@ -83,7 +111,7 @@ export const FirewallRuleSetForm = React.memo(
83111
<Typography
84112
sx={(theme) => ({ marginTop: theme.spacingFunction(16) })}
85113
>
86-
RuleSets are reusable collections of Cloud Firewall rules that use
114+
Rule Sets are reusable collections of Cloud Firewall rules that use
87115
the same fields as individual rules. They let you manage and update
88116
multiple rules as a group. You can then apply them across different
89117
firewalls by reference.

packages/manager/src/features/Firewalls/FirewallDetail/Rules/FirewallRuleTable.tsx

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -397,8 +397,6 @@ const FirewallRuleTableRow = React.memo((props: FirewallRuleTableRowProps) => {
397397
return <TableRowLoading columns={smDown ? 3 : lgDown ? 5 : 6} />;
398398
}
399399

400-
const ruleSetCopyableId = `${rulesetDetails ? 'ID:' : 'Ruleset ID:'} ${ruleset}`;
401-
402400
return (
403401
<StyledTableRow
404402
aria-label={label ?? `firewall rule ${id}`}
@@ -475,7 +473,8 @@ const FirewallRuleTableRow = React.memo((props: FirewallRuleTableRowProps) => {
475473
)}
476474
</Box>
477475
<Box sx={{ whiteSpace: 'nowrap' }}>
478-
<IPAddress ips={[ruleSetCopyableId]} isHovered={isHovered} />
476+
<span>{rulesetDetails ? 'ID: ' : 'Rule Set ID: '}</span>
477+
<IPAddress ips={[String(ruleset)]} isHovered={isHovered} />
479478
</Box>
480479
</Box>
481480
</TableCell>

packages/manager/src/features/Firewalls/FirewallDetail/Rules/MultiplePrefixListSelect.test.tsx

Lines changed: 27 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,7 @@ describe('MultiplePrefixListSelect', () => {
5454
}, // supported (supports only ipv4)
5555
{ name: 'pl:system:supports-both', ipv4: [], ipv6: [] }, // supported (supports both)
5656
{ name: 'pl:system:not-supported', ipv4: null, ipv6: null }, // unsupported
57+
{ name: 'pl::vpcs:<current>' }, // Special Prefix List (Doesn't have IPv4 & IPv6)
5758
];
5859

5960
queryMocks.useAllFirewallPrefixListsQuery.mockReturnValue({
@@ -177,7 +178,7 @@ describe('MultiplePrefixListSelect', () => {
177178
getByDisplayValue('pl:system:supports-only-ipv4');
178179
});
179180

180-
it('defaults to IPv4 selected and IPv6 unselected when choosing a PL that supports both', async () => {
181+
it('defaults to both IPv4 and IPv6 selected when choosing a PL that supports both', async () => {
181182
const pls = [{ address: '', inIPv4Rule: false, inIPv6Rule: false }];
182183
const { findByText, getByRole } = renderWithTheme(
183184
<MultiplePrefixListSelect {...baseProps} pls={pls} />
@@ -196,7 +197,31 @@ describe('MultiplePrefixListSelect', () => {
196197
{
197198
address: 'pl::supports-both',
198199
inIPv4Rule: true,
199-
inIPv6Rule: false,
200+
inIPv6Rule: true,
201+
},
202+
]);
203+
});
204+
205+
it('defaults to both IPv4 and IPv6 selected when choosing a Special PL', async () => {
206+
const pls = [{ address: '', inIPv4Rule: false, inIPv6Rule: false }];
207+
const { findByText, getByRole } = renderWithTheme(
208+
<MultiplePrefixListSelect {...baseProps} pls={pls} />
209+
);
210+
211+
const input = getByRole('combobox');
212+
213+
// Type the Special PL name to filter the dropdown
214+
await userEvent.type(input, 'pl::vpcs:<current>');
215+
216+
// Select the option from the autocomplete dropdown
217+
const option = await findByText('pl::vpcs:<current>');
218+
await userEvent.click(option);
219+
220+
expect(baseProps.onChange).toHaveBeenCalledWith([
221+
{
222+
address: 'pl::vpcs:<current>',
223+
inIPv4Rule: true,
224+
inIPv6Rule: true,
200225
},
201226
]);
202227
});

packages/manager/src/features/Firewalls/FirewallDetail/Rules/MultiplePrefixListSelect.tsx

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -84,17 +84,20 @@ const getDefaultPLReferenceState = (
8484
): { inIPv4Rule: boolean; inIPv6Rule: boolean } => {
8585
if (support === null) {
8686
// Special Prefix List case
87-
return { inIPv4Rule: true, inIPv6Rule: false };
87+
return { inIPv4Rule: true, inIPv6Rule: true };
8888
}
8989

9090
const { isPLIPv4Unsupported, isPLIPv6Unsupported } = support;
9191

92+
// Supports both IPv4 & IPv6
9293
if (!isPLIPv4Unsupported && !isPLIPv6Unsupported)
93-
return { inIPv4Rule: true, inIPv6Rule: false };
94+
return { inIPv4Rule: true, inIPv6Rule: true };
9495

96+
// Supports only IPv4
9597
if (!isPLIPv4Unsupported && isPLIPv6Unsupported)
9698
return { inIPv4Rule: true, inIPv6Rule: false };
9799

100+
// Supports only Ipv6
98101
if (isPLIPv4Unsupported && !isPLIPv6Unsupported)
99102
return { inIPv4Rule: false, inIPv6Rule: true };
100103

packages/manager/src/features/Firewalls/FirewallDetail/Rules/shared.styles.ts

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -12,16 +12,20 @@ import type { FirewallPolicyType } from '@linode/api-v4';
1212
import type { Theme } from '@linode/ui';
1313

1414
interface StyledListItemProps {
15+
fieldsMode?: boolean;
1516
paddingMultiplier?: number; // optional, default 1
1617
}
1718

1819
export const StyledListItem = styled(Typography, {
1920
label: 'StyledTypography',
20-
shouldForwardProp: omittedProps(['paddingMultiplier']),
21-
})<StyledListItemProps>(({ theme, paddingMultiplier = 1 }) => ({
22-
alignItems: 'center',
21+
shouldForwardProp: omittedProps(['fieldsMode', 'paddingMultiplier']),
22+
})<StyledListItemProps>(({ theme, fieldsMode, paddingMultiplier = 1 }) => ({
23+
alignItems: fieldsMode ? 'flex-start' : 'center',
2324
display: 'flex',
2425
padding: `${theme.spacingFunction(4 * paddingMultiplier)} 0`,
26+
...(fieldsMode && {
27+
flexWrap: 'wrap', // Longer labels start on the next line
28+
}),
2529
}));
2630

2731
export const StyledLabel = styled(Box, {
@@ -71,8 +75,7 @@ export const useStyles = makeStyles()((theme: Theme) => ({
7175
width: '1em',
7276
},
7377
color: theme.palette.primary.main,
74-
display: 'inline-block',
78+
display: 'flex',
7579
position: 'relative',
76-
marginTop: theme.spacingFunction(4),
7780
},
7881
}));

0 commit comments

Comments
 (0)