Skip to content
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

feat: [M3-8099] - Improve Linode Clone Power Off based on initial data #10508

Merged
merged 10 commits into from
May 24, 2024
5 changes: 5 additions & 0 deletions packages/manager/.changeset/pr-10508-added-1716415682879.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@linode/manager": Added
---

Improvements to Clone flow to encourage powering down before cloning ([#10508](https://github.com/linode/manager/pull/10508))
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,13 @@ export const CloneWarning = () => {
<Notice variant="warning">
<List sx={{ listStyleType: 'disc', pl: 2.5 }}>
<ListItem sx={{ display: 'list-item', pl: 1, py: 0.5 }}>
This newly created Linode will be created with the same password and
SSH Keys (if any) as the original Linode.
To help <strong>avoid data corruption</strong> during the cloning
process, we recommend powering off your Compute Instance prior to
cloning.
</ListItem>
<ListItem sx={{ display: 'list-item', pl: 1, py: 0.5 }}>
To help avoid data corruption during the cloning process, we recommend
powering off your Compute Instance prior to cloning.
This newly created Linode will be created with the same password and
SSH Keys (if any) as the original Linode.
</ListItem>
</List>
</Notice>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import { PowerActionsDialog } from 'src/features/Linodes/PowerActionsDialogOrDra
import { useOrder } from 'src/hooks/useOrder';
import { usePagination } from 'src/hooks/usePagination';
import { useLinodesQuery } from 'src/queries/linodes/linodes';
import { sendLinodePowerOffEvent } from 'src/utilities/analytics/customEventAnalytics';
import { privateIPRegex } from 'src/utilities/ipUtils';
import { isNumeric } from 'src/utilities/stringUtils';

Expand All @@ -37,7 +38,8 @@ import type { Theme } from '@mui/material';

interface Props {
/**
* Adds an extra column that will dispay a "power off" option when the row is selected
* In desktop view, adds an extra column that will display a "power off" option when the row is selected.
* In mobile view, allows the "power off" button to display when the card is selected.
*/
enablePowerOff?: boolean;
}
Expand Down Expand Up @@ -102,6 +104,11 @@ export const LinodeSelectTable = (props: Props) => {
}));
};

const handlePowerOff = (linode: Linode) => {
setLinodeToPowerOff(linode);
sendLinodePowerOffEvent('Clone Linode');
};

const columns = enablePowerOff ? 6 : 5;

return (
Expand Down Expand Up @@ -162,7 +169,10 @@ export const LinodeSelectTable = (props: Props) => {
<LinodeSelectTableRow
onPowerOff={
enablePowerOff
? () => setLinodeToPowerOff(linode)
? () => {
setLinodeToPowerOff(linode);
sendLinodePowerOffEvent('Clone Linode');
}
: undefined
}
key={linode.id}
Expand All @@ -177,10 +187,12 @@ export const LinodeSelectTable = (props: Props) => {
<Grid container spacing={2}>
{data?.data.map((linode) => (
<SelectLinodeCard
handlePowerOff={() => handlePowerOff(linode)}
handleSelection={() => handleSelect(linode)}
key={linode.id}
linode={linode}
selected={linode.id === field.value?.id}
showPowerActions={Boolean(enablePowerOff)}
/>
))}
{data?.results === 0 && (
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
import { fireEvent } from '@testing-library/react';
import React from 'react';

import { linodeFactory } from 'src/factories';
import { renderWithTheme } from 'src/utilities/testHelpers';

import { SelectLinodeCard } from './SelectLinodeCard';

const mockPoweredOnLinode = linodeFactory.build({ status: 'running' });
const mockPoweredOffLinode = linodeFactory.build({ status: 'offline' });

const defaultProps = {
disabled: false,
handlePowerOff: vi.fn(),
handleSelection: vi.fn(),
key: mockPoweredOnLinode.id,
linode: mockPoweredOnLinode,
selected: false,
showPowerActions: false,
};

describe('SelectLinodeCard', () => {
it('displays the status of a linode', () => {
const { getByLabelText, getByText, queryByRole } = renderWithTheme(
<SelectLinodeCard {...defaultProps} showPowerActions={true} />
);
expect(getByLabelText('Linode status running')).toBeInTheDocument();
expect(getByText('Running')).toBeVisible();
// Should not display the Power Off button unless the card is selected.
expect(queryByRole('button')).not.toBeInTheDocument();
});

it('displays the status and Power Off button of a linode that is selected and running when power actions should be shown', () => {
const { getByLabelText, getByRole, getByText } = renderWithTheme(
<SelectLinodeCard
{...defaultProps}
selected={true}
showPowerActions={true}
/>
);

expect(getByLabelText('Linode status running')).toBeInTheDocument();
expect(getByText('Running')).toBeVisible();

const powerOffButton = getByRole('button');
expect(powerOffButton).toHaveTextContent('Power Off');
fireEvent.click(powerOffButton);
expect(defaultProps.handlePowerOff).toHaveBeenCalledTimes(1);
});

it('does not display the Power Off button when it should not be shown', () => {
const { queryByRole } = renderWithTheme(
<SelectLinodeCard
{...defaultProps}
selected={true}
showPowerActions={false}
/>
);
expect(queryByRole('button')).not.toBeInTheDocument();
});

it('does not display the Power Off button when a selected linode is powered off', () => {
const { getByLabelText, getByText, queryByRole } = renderWithTheme(
<SelectLinodeCard
{...defaultProps}
linode={mockPoweredOffLinode}
selected={true}
showPowerActions={true}
/>
);
expect(getByLabelText('Linode status offline')).toBeInTheDocument();
expect(getByText('Offline')).toBeVisible();
// Should not display the Power Off button unless the linode is running.
expect(queryByRole('button')).not.toBeInTheDocument();
});
});
Original file line number Diff line number Diff line change
@@ -1,26 +1,37 @@
import { Linode } from '@linode/api-v4';
import Grid from '@mui/material/Unstable_Grid2';
import React from 'react';

import { Button } from 'src/components/Button/Button';
import { SelectionCard } from 'src/components/SelectionCard/SelectionCard';
import { Stack } from 'src/components/Stack';
import { StatusIcon } from 'src/components/StatusIcon/StatusIcon';
import { useIsResourceRestricted } from 'src/hooks/useIsResourceRestricted';
import { useImageQuery } from 'src/queries/images';
import { useRegionsQuery } from 'src/queries/regions/regions';
import { useTypeQuery } from 'src/queries/types';
import { capitalizeAllWords } from 'src/utilities/capitalize';
import { formatStorageUnits } from 'src/utilities/formatStorageUnits';
import { isNotNullOrUndefined } from 'src/utilities/nullOrUndefined';

import { getLinodeIconStatus } from '../../LinodesLanding/utils';

interface Props {
disabled?: boolean;
handlePowerOff: () => void;
handleSelection: () => void;
linode: Linode;
selected?: boolean;
showPowerActions: boolean;
}

export const SelectLinodeCard = ({
disabled,
handlePowerOff,
handleSelection,
linode,
selected,
showPowerActions,
}: Props) => {
const { data: regions } = useRegionsQuery();

Expand All @@ -40,12 +51,35 @@ export const SelectLinodeCard = ({
id: linode?.id,
});

const iconStatus = getLinodeIconStatus(linode?.status);
const shouldShowPowerButton =
showPowerActions && linode?.status === 'running' && selected;

const type = linodeType ? formatStorageUnits(linodeType?.label) : linode.type;
const image = linodeImage?.label ?? linode.image;
const region =
regions?.find((region) => region.id == linode.region)?.label ??
linode.region;

const renderVariant = () => (
<Grid paddingTop={0} xs={12}>
<Stack direction="row" justifyContent="space-between" marginBottom={1}>
<Stack alignItems="center" direction="row" height={34}>
<StatusIcon
aria-label={`Linode status ${linode?.status ?? iconStatus}`}
status={iconStatus}
/>
{capitalizeAllWords(linode.status.replace('_', ' '))}
</Stack>
{shouldShowPowerButton && (
<Button buttonType="outlined" onClick={handlePowerOff}>
Power Off
</Button>
)}
</Stack>
</Grid>
);

return (
<SelectionCard
subheadings={[
Expand All @@ -56,6 +90,7 @@ export const SelectLinodeCard = ({
heading={linode.label}
key={`selection-card-${linode.id}`}
onClick={handleSelection}
renderVariant={renderVariant}
/>
);
};
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,9 @@ import { RenderLinodeProps } from './SelectLinodePanel';

export const SelectLinodeCards = ({
disabled,
handlePowerOff,
handleSelection,
showPowerActions,
orderBy: { data: linodes },
selectedLinodeId,
}: RenderLinodeProps) => (
Expand All @@ -20,9 +22,11 @@ export const SelectLinodeCards = ({
handleSelection(linode.id, linode.type, linode.specs.disk)
}
disabled={disabled}
handlePowerOff={() => handlePowerOff(linode.id)}
key={linode.id}
linode={linode}
selected={linode.id == selectedLinodeId}
showPowerActions={showPowerActions}
/>
))
) : (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -197,21 +197,6 @@ describe('SelectLinodePanel (cards, mobile)', () => {
).toHaveTextContent(defaultProps.linodes[0].label);
});

it('displays the heading, notices and error', () => {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was a duplicate test the linter notified me of. See L99-L112.

const { getByText } = renderWithTheme(
<SelectLinodePanel
{...defaultProps}
error={'Example error'}
header={'Example header'}
notices={['Example notice']}
/>
);

expect(getByText('Example error')).toBeInTheDocument();
expect(getByText('Example header')).toBeInTheDocument();
expect(getByText('Example notice')).toBeInTheDocument();
});

it('prefills the search box when mounted with a selected linode', async () => {
setupMocks();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import { Paper } from 'src/components/Paper';
import { Stack } from 'src/components/Stack';
import { Typography } from 'src/components/Typography';
import { useOrder } from 'src/hooks/useOrder';
import { sendLinodePowerOffEvent } from 'src/utilities/analytics/customEventAnalytics';

import { PowerActionsDialog } from '../../PowerActionsDialogOrDrawer';
import { SelectLinodeCards } from './SelectLinodeCards';
Expand All @@ -26,7 +27,7 @@ interface Props {
handleSelection: (id: number, type: null | string, diskSize?: number) => void;
header?: string;
linodes: Linode[];
notices?: string[];
notices?: (JSX.Element | string)[];
selectedLinodeID?: number;
showPowerActions?: boolean;
}
Expand Down Expand Up @@ -155,9 +156,10 @@ export const SelectLinodePanel = (props: Props) => {
/>
<StyledBox>
<SelectComponent
handlePowerOff={(linodeId) =>
setPowerOffLinode({ linodeId })
}
handlePowerOff={(linodeId) => {
setPowerOffLinode({ linodeId });
sendLinodePowerOffEvent('Clone Linode');
}}
orderBy={{
data: linodesData,
handleOrderChange,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -106,8 +106,12 @@ export const FromLinodeContent = (props: CombinedProps) => {
<StyledGrid>
<SelectLinodePanel
notices={[
<>
To help <strong>avoid data corruption</strong> during the
cloning process, we recommend powering off your Compute Instance
prior to cloning.
</>,
'This newly created Linode will be created with the same password and SSH Keys (if any) as the original Linode.',
'To help avoid data corruption during the cloning process, we recommend powering off your Compute Instance prior to cloning.',
]}
data-qa-linode-panel
disabled={userCannotCreateLinode}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -469,3 +469,13 @@ export const sendManageGravatarEvent = () => {
label: 'Manage photo',
});
};

// SelectLinodePanel.tsx
// LinodeSelectTable.tsx
export const sendLinodePowerOffEvent = (category: string) => {
sendEvent({
action: 'Click:button',
category,
label: 'Power Off',
});
};
Loading