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

change: [M3-6843] - Move manual snapshot error message to snapshot confirmation dialog #10791

Merged
merged 6 commits into from
Aug 19, 2024
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
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@linode/manager": Changed
---

Move manual snapshot error message from Linode Backups page to snapshot confirmation dialog ([#10791](https://github.com/linode/manager/pull/10791))
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import * as React from 'react';
import { Box } from 'src/components/Box';
import { Button } from 'src/components/Button/Button';
import { FormControl } from 'src/components/FormControl';
import { Notice } from 'src/components/Notice/Notice';
import { Paper } from 'src/components/Paper';
import { TextField } from 'src/components/TextField';
import { Typography } from 'src/components/Typography';
Expand All @@ -31,6 +30,7 @@ export const CaptureSnapshot = (props: Props) => {
error: snapshotError,
isLoading: isSnapshotLoading,
mutateAsync: takeSnapshot,
reset,
} = useLinodeBackupSnapshotMutation(linodeId);

const [
Expand Down Expand Up @@ -65,19 +65,14 @@ export const CaptureSnapshot = (props: Props) => {
manual snapshot will not be overwritten by automatic backups.
</Typography>
<FormControl>
{hasErrorFor.none && (
<Notice spacingBottom={8} variant="error">
{hasErrorFor.none}
</Notice>
)}
<StyledBox>
<TextField
sx={{ minWidth: 275 }}
data-qa-manual-name
errorText={hasErrorFor.label}
label="Name Snapshot"
name="label"
onChange={snapshotForm.handleChange}
sx={{ minWidth: 275 }}
value={snapshotForm.values.label}
/>
<Button
Expand All @@ -91,8 +86,10 @@ export const CaptureSnapshot = (props: Props) => {
</StyledBox>
</FormControl>
<CaptureSnapshotConfirmationDialog
error={hasErrorFor.none}
loading={isSnapshotLoading}
onClose={() => setIsSnapshotConfirmationDialogOpen(false)}
onExited={() => reset()}
onSnapshot={() => snapshotForm.handleSubmit()}
open={isSnapshotConfirmationDialogOpen}
/>
Expand Down
Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for adding this test!!!

Original file line number Diff line number Diff line change
@@ -0,0 +1,84 @@
import { fireEvent } from '@testing-library/react';
import * as React from 'react';

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

import { CaptureSnapshotConfirmationDialog } from './CaptureSnapshotConfirmationDialog';

const props = {
error: undefined,
loading: false,
onClose: vi.fn(),
onExited: vi.fn(),
onSnapshot: vi.fn(),
open: true,
};

describe('CaptureSnapshotConfirmationDialog', () => {
beforeEach(() => {
vi.clearAllMocks();
});

it('should render the dialog', () => {
const { getByTestId, getByText } = renderWithTheme(
<CaptureSnapshotConfirmationDialog {...props} />
);

const takeSnapshotButton = getByTestId('confirm');
const cancelButton = getByText('Cancel');
const title = getByText('Take a snapshot?');
const body = getByText(
'Taking a snapshot will back up your Linode in its current state, overriding your previous snapshot. Are you sure?'
);

expect(takeSnapshotButton).toBeVisible();
expect(cancelButton).toBeVisible();
expect(title).toBeVisible();
expect(body).toBeVisible();
});
coliu-akamai marked this conversation as resolved.
Show resolved Hide resolved

it('should send a request to create a snapshot if loading is false', () => {
const { getByTestId } = renderWithTheme(
<CaptureSnapshotConfirmationDialog {...props} />
);

const takeSnapshotButton = getByTestId('confirm');
expect(takeSnapshotButton).toBeVisible();
fireEvent.click(takeSnapshotButton);
expect(props.onSnapshot).toHaveBeenCalled();
});

it('should not send a request to create a snapshot if loading is true', () => {
const { getByTestId } = renderWithTheme(
<CaptureSnapshotConfirmationDialog {...props} loading={true} />
);

const takeSnapshotButton = getByTestId('confirm');
expect(takeSnapshotButton).toBeVisible();
fireEvent.click(takeSnapshotButton);
expect(props.onSnapshot).not.toHaveBeenCalled();
});

it('should close the dialog when the "Cancel" button is clicked', () => {
const { getByText } = renderWithTheme(
<CaptureSnapshotConfirmationDialog {...props} />
);

const cancelButton = getByText('Cancel');
expect(cancelButton).toBeVisible();
fireEvent.click(cancelButton);
expect(props.onClose).toHaveBeenCalled();
});

it('should render the dialog with an error message', () => {
const { getByText } = renderWithTheme(
<CaptureSnapshotConfirmationDialog
{...props}
error={'mock error message'}
/>
);

const errorMessage = getByText('mock error message');
expect(errorMessage).toBeVisible();
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -5,15 +5,16 @@ import { ConfirmationDialog } from 'src/components/ConfirmationDialog/Confirmati
import { Typography } from 'src/components/Typography';

interface Props {
error?: string;
error: string | undefined;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

there's no change in type/functionality of this prop, just made it non optional bc the one place we use this component now passes in an error

loading: boolean;
onClose: () => void;
onExited: () => void;
onSnapshot: () => void;
open: boolean;
}

export const CaptureSnapshotConfirmationDialog = (props: Props) => {
const { error, loading, onClose, onSnapshot, open } = props;
const { error, loading, onClose, onExited, onSnapshot, open } = props;

const actions = (
<ActionsPanel
Expand All @@ -37,6 +38,7 @@ export const CaptureSnapshotConfirmationDialog = (props: Props) => {
actions={actions}
error={error}
onClose={onClose}
onExited={onExited}
open={open}
title="Take a snapshot?"
>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import { LinodeBackup, PriceObject } from '@linode/api-v4/lib/linodes';
import { Box, Stack } from '@mui/material';
import { styled } from '@mui/material/styles';
import * as React from 'react';
Expand All @@ -23,13 +22,15 @@ import { useRegionsQuery } from 'src/queries/regions/regions';
import { useTypeQuery } from 'src/queries/types';
import { getMonthlyBackupsPrice } from 'src/utilities/pricing/backups';

import { BackupTableRow } from './BackupTableRow';
import { BackupsPlaceholder } from './BackupsPlaceholder';
import { BackupTableRow } from './BackupTableRow';
import { CancelBackupsDialog } from './CancelBackupsDialog';
Copy link
Contributor Author

@coliu-akamai coliu-akamai Aug 16, 2024

Choose a reason for hiding this comment

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

these are just eslint changes (same with line 75 of CaptureSnapshot.tsx)

import { CaptureSnapshot } from './CaptureSnapshot';
import { RestoreToLinodeDrawer } from './RestoreToLinodeDrawer';
import { ScheduleSettings } from './ScheduleSettings';

import type { LinodeBackup, PriceObject } from '@linode/api-v4/lib/linodes';

export const LinodeBackups = () => {
const { linodeId } = useParams<{ linodeId: string }>();
const id = Number(linodeId);
Expand Down
Loading