Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions packages/manager/.changeset/pr-9785-fixed-1697059201989.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@linode/manager": Fixed
---

Firewall Create Drawer opens in the same tab in the Linode Create Flow ([#9785](https://github.com/linode/manager/pull/9785))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Firewall Create Drawer opens in the same tab in the Linode Create Flow ([#9785](https://github.com/linode/manager/pull/9785))
Firewall Create Drawer opening in a new tab in the Linode Create Flow ([#9785](https://github.com/linode/manager/pull/9785))

Similar suggestion here with describing the thing that was fixed.

Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
import { waitFor } from '@testing-library/react';
import { fireEvent, waitFor } from '@testing-library/react';
import * as React from 'react';
import { QueryClient } from 'react-query';

import { mockMatchMedia, renderWithTheme } from 'src/utilities/testHelpers';

import { SelectFirewallPanel } from './SelectFirewallPanel';
import { createFirewallLabel } from './SelectFirewallPanel';

const queryClient = new QueryClient();

Expand All @@ -21,6 +22,7 @@ describe('SelectFirewallPanel', () => {
<SelectFirewallPanel
handleFirewallChange={jest.fn()}
helperText={<span>Testing</span>}
selectedFirewallId={-1}
/>,
{
queryClient,
Expand All @@ -31,4 +33,25 @@ describe('SelectFirewallPanel', () => {
expect(wrapper.getByTestId(testId)).toBeInTheDocument();
});
});

it('should open a Create Firewall drawer when the link is clicked', async () => {
const wrapper = renderWithTheme(
<SelectFirewallPanel
handleFirewallChange={jest.fn()}
helperText={<span>Testing</span>}
selectedFirewallId={-1}
/>,
{
queryClient,
}
);

const createFirewallLink = wrapper.getByText('Create Firewall');

fireEvent.click(createFirewallLink);

await waitFor(() => {
expect(wrapper.getByLabelText(createFirewallLabel)).toBeInTheDocument();
});
});
});
Original file line number Diff line number Diff line change
@@ -1,21 +1,35 @@
import { Firewall } from '@linode/api-v4';
import Stack from '@mui/material/Stack';
import * as React from 'react';

import Select from 'src/components/EnhancedSelect';
import { Paper } from 'src/components/Paper';
import { Typography } from 'src/components/Typography';
import { APP_ROOT } from 'src/constants';
import { CreateFirewallDrawer } from 'src/features/Firewalls/FirewallLanding/CreateFirewallDrawer';
import { useFirewallsQuery } from 'src/queries/firewalls';

import { StyledCreateLink } from '../../features/Linodes/LinodesCreate/LinodeCreate.styles';
import { Autocomplete } from '../Autocomplete/Autocomplete';
import { LinkButton } from '../LinkButton';

interface Props {
handleFirewallChange: (firewallID: number) => void;
helperText: JSX.Element;
selectedFirewallId: number;
}

export const createFirewallLabel = 'Additional Linodes (Optional)';
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

UX wanted to ensure the user:

  • Wasnt looking for current Linode in the dropdown --> 'Additional'
  • Knew that the input was optional --> '(Optional)'


export const SelectFirewallPanel = (props: Props) => {
const { handleFirewallChange, helperText } = props;
const { handleFirewallChange, helperText, selectedFirewallId } = props;

const [isDrawerOpen, setIsDrawerOpen] = React.useState(false);

const handleCreateFirewallClick = () => {
setIsDrawerOpen(true);
};

const handleFirewallCreated = (firewall: Firewall) => {
handleFirewallChange(firewall.id);
};

const { data: firewallsData, error, isLoading } = useFirewallsQuery();

Expand All @@ -25,10 +39,12 @@ export const SelectFirewallPanel = (props: Props) => {
value: firewall.id,
}));

firewallsDropdownOptions.unshift({
label: 'None',
value: -1,
});
const selectedFirewall =
selectedFirewallId !== -1
? firewallsDropdownOptions.find(
(option) => option.value === selectedFirewallId
) || null
: null;

return (
<Paper
Expand All @@ -43,20 +59,30 @@ export const SelectFirewallPanel = (props: Props) => {
</Typography>
<Stack>
{helperText}
<Select
defaultValue={firewallsDropdownOptions[0]}
<Autocomplete
onChange={(_, selection) => {
handleFirewallChange(selection?.value ?? -1);
}}
errorText={error?.[0].reason}
isClearable={false}
isLoading={isLoading}
label="Assign Firewall"
noOptionsMessage={() => 'Create a Firewall to assign to this Linode.'}
onChange={(selection) => handleFirewallChange(selection.value)}
loading={isLoading}
noOptionsText="No Firewalls available"
options={firewallsDropdownOptions}
placeholder={''}
placeholder={'None'}
value={selectedFirewall}
/>
<StyledCreateLink to={`${APP_ROOT}/firewalls/create`}>
<LinkButton
onClick={handleCreateFirewallClick}
style={{ marginBottom: 16, marginTop: 12, textAlign: 'left' }}
>
Create Firewall
</StyledCreateLink>
</LinkButton>
<CreateFirewallDrawer
label={createFirewallLabel}
onClose={() => setIsDrawerOpen(false)}
onFirewallCreated={handleFirewallCreated}
open={isDrawerOpen}
/>
</Stack>
</Paper>
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,13 @@ import { getEntityIdsByPermission } from 'src/utilities/grants';
import { READ_ONLY_LINODES_HIDDEN_MESSAGE } from '../../FirewallLanding/CreateFirewallDrawer';

interface Props {
label?: string;
onClose: () => void;
open: boolean;
}

export const AddDeviceDrawer = (props: Props) => {
const { onClose, open } = props;
const { label, onClose, open } = props;

const { id } = useParams<{ id: string }>();

Expand Down Expand Up @@ -136,6 +137,7 @@ export const AddDeviceDrawer = (props: Props) => {
![...readOnlyLinodeIds, ...currentLinodeIds].includes(linode.id)
}
disabled={currentDevicesLoading}
label={label}
loading={currentDevicesLoading}
multiple
noOptionsMessage="No Linodes available to add"
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { CreateFirewallPayload } from '@linode/api-v4/lib/firewalls';
import { CreateFirewallPayload, Firewall } from '@linode/api-v4/lib/firewalls';
import { CreateFirewallSchema } from '@linode/validation/lib/firewalls.schema';
import { useFormik } from 'formik';
import * as React from 'react';
Expand All @@ -22,7 +22,9 @@ export const READ_ONLY_LINODES_HIDDEN_MESSAGE =
'Only Linodes you have permission to modify are shown.';

export interface CreateFirewallDrawerProps {
label?: string;
onClose: () => void;
onFirewallCreated?: (firewall: Firewall) => void;
open: boolean;
}

Expand All @@ -39,15 +41,10 @@ const initialValues: CreateFirewallPayload = {

export const CreateFirewallDrawer = React.memo(
(props: CreateFirewallDrawerProps) => {
const { onClose, open } = props;

/**
* We'll eventually want to check the read_write firewall
* grant here too, but it doesn't exist yet.
*/
// TODO: NBFW - We'll eventually want to check the read_write firewall grant here too, but it doesn't exist yet.
const { label, onClose, onFirewallCreated, open } = props;
const { _hasGrant, _isRestrictedUser } = useAccountManagement();
const { data: grants } = useGrants();

const { mutateAsync } = useCreateFirewall();

const {
Expand Down Expand Up @@ -90,8 +87,11 @@ export const CreateFirewallDrawer = React.memo(
}

mutateAsync(payload)
.then(() => {
.then((response) => {
setSubmitting(false);
if (onFirewallCreated) {
onFirewallCreated(response);
}
onClose();
})
.catch((err) => {
Expand Down Expand Up @@ -181,6 +181,7 @@ export const CreateFirewallDrawer = React.memo(
disabled={userCannotAddFirewall}
errorText={errors['devices.linodes']}
helperText={firewallHelperText}
label={label}
multiple
onBlur={handleBlur}
optionsFilter={(linode) => !readOnlyLinodeIds.includes(linode.id)}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,11 +37,11 @@ import { WithTypesProps } from 'src/containers/types.container';
import { FeatureFlagConsumerProps } from 'src/containers/withFeatureFlagConsumer.container';
import { WithLinodesProps } from 'src/containers/withLinodes.container';
import EUAgreementCheckbox from 'src/features/Account/Agreements/EUAgreementCheckbox';
import { regionSupportsMetadata } from 'src/features/Linodes/LinodesCreate/utilities';
import {
getMonthlyAndHourlyNodePricing,
utoa,
} from 'src/features/Linodes/LinodesCreate/utilities';
import { regionSupportsMetadata } from 'src/features/Linodes/LinodesCreate/utilities';
import { SMTPRestrictionText } from 'src/features/Linodes/SMTPRestrictionText';
import {
getCommunityStackscripts,
Expand All @@ -62,6 +62,7 @@ import { getErrorMap } from 'src/utilities/errorUtils';
import { extendType } from 'src/utilities/extendType';
import { filterCurrentTypes } from 'src/utilities/filterCurrentLinodeTypes';
import { getMonthlyBackupsPrice } from 'src/utilities/pricing/backups';
import { UNKNOWN_PRICE } from 'src/utilities/pricing/constants';
import { renderMonthlyPriceToCorrectDecimalPlace } from 'src/utilities/pricing/dynamicPricing';
import { getQueryParamsFromQueryString } from 'src/utilities/queryParams';

Expand Down Expand Up @@ -97,14 +98,13 @@ import {
} from './types';

import type { Tab } from 'src/components/TabLinkList/TabLinkList';
import { UNKNOWN_PRICE } from 'src/utilities/pricing/constants';

export interface LinodeCreateProps {
assignPublicIPv4Address: boolean;
autoassignIPv4WithinVPC: boolean;
checkValidation: LinodeCreateValidation;
createType: CreateTypes;
firewallId: number | undefined;
firewallId?: number;
handleAgreementChange: () => void;
handleFirewallChange: (firewallId: number) => void;
handleShowApiAwarenessModal: () => void;
Expand Down Expand Up @@ -381,7 +381,11 @@ export class LinodeCreate extends React.PureComponent<
});
}

if (this.props.firewallId !== undefined && this.props.firewallId !== -1) {
if (
this.props.firewallId !== null &&
this.props.firewallId !== undefined &&
this.props.firewallId !== -1
) {
displaySections.push({
title: 'Firewall Assigned',
});
Expand Down Expand Up @@ -638,6 +642,7 @@ export class LinodeCreate extends React.PureComponent<
// @TODO VPC: Update "Learn More" link
}
handleFirewallChange={this.props.handleFirewallChange}
selectedFirewallId={this.props.firewallId || -1}
/>
)}
<AddonsPanel
Expand Down