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.

Sign up for GitHub

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

Jump to bottom

feat: Displaying details to Dataset/Database deletion modals #30016

Merged
merged 16 commits into from
Sep 10, 2024
Merged

feat: Displaying details to Dataset/Database deletion modals #30016

Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
888b77d
dataset deletion modal
rusackas Aug 26, 2024
f846e88
modal always centered (for better scrolling)
rusackas Aug 26, 2024
4861a10
Details/links on Database deletion
rusackas Aug 26, 2024
d65d3dc
type fixes
rusackas Aug 26, 2024
cd62fe1
fixin' some tests
rusackas Aug 26, 2024
b03641c
whitespace tweak
rusackas Aug 26, 2024
5320a2c
more tweaks
rusackas Aug 26, 2024
7d711ac
moving to snapshot from inlinesnapshot
rusackas Aug 27, 2024
b52698c
Fresh snapshot
rusackas Aug 27, 2024
482f045
skip license check on snap files
rusackas Aug 27, 2024
71383e0
Make prettier ignore test snapshots
rusackas Aug 27, 2024
5c65bac
don't run the precommit hook on .snap files
rusackas Aug 27, 2024
2c7f19d
fresh snap
rusackas Aug 27, 2024
16e6f3e
rat exclude adjustment
rusackas Aug 27, 2024
bf5ffb6
limiting results to 10 items *and more*
rusackas Sep 7, 2024
f864e49
deleting commented text.
rusackas Sep 10, 2024
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
2 changes: 1 addition & 1 deletion superset-frontend/src/components/ConfirmStatusChange/ConfirmStatusChange.test.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ test('calls the function on confirm', async () => {
const confirmInput = getByTestId('delete-modal-input');
fireEvent.change(confirmInput, { target: { value: 'DELETE' } });

const confirmButton = getByRole('button', { name: 'delete' });
const confirmButton = getByRole('button', { name: 'Delete' });
fireEvent.click(confirmButton);

await waitFor(() => expect(mockedProps.onConfirm).toHaveBeenCalledTimes(1));
Expand Down
4 changes: 2 additions & 2 deletions superset-frontend/src/components/DeleteModal/DeleteModal.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,12 +77,12 @@ test('Calling "onConfirm" only after typing "delete" in the input', () => {
expect(props.onConfirm).toBeCalledTimes(0);

// do not execute "onConfirm" if you have not typed "delete"
userEvent.click(screen.getByText('delete'));
userEvent.click(screen.getByText('Delete'));
expect(props.onConfirm).toBeCalledTimes(0);

// execute "onConfirm" if you have typed "delete"
userEvent.type(screen.getByTestId('delete-modal-input'), 'delete');
userEvent.click(screen.getByText('delete'));
userEvent.click(screen.getByText('Delete'));
expect(props.onConfirm).toBeCalledTimes(1);

// confirm input has been cleared
Expand Down
3 changes: 2 additions & 1 deletion superset-frontend/src/components/DeleteModal/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,10 +80,11 @@ export default function DeleteModal({
disablePrimaryButton={disableChange}
onHide={hide}
onHandledPrimaryAction={confirm}
primaryButtonName={t('delete')}
primaryButtonName={t('Delete')}
rusackas marked this conversation as resolved.
Show resolved Hide resolved
primaryButtonType="danger"
show={open}
title={title}
centered
>
<DescriptionContainer>{description}</DescriptionContainer>
<StyledDiv>
Expand Down
9 changes: 1 addition & 8 deletions superset-frontend/src/pages/DatabaseList/DatabaseList.test.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -164,14 +164,7 @@ describe('Admin DatabaseList', () => {
});
await waitForComponentToPaint(wrapper);

expect(wrapper.find(DeleteModal).props().description)
.toMatchInlineSnapshot(`
<React.Fragment>
<p>
The database db 0 is linked to 0 charts that appear on 0 dashboards and users have 0 SQL Lab tabs using this database open. Are you sure you want to continue? Deleting the database will break those objects.
</p>
</React.Fragment>
`);
expect(wrapper.find(DeleteModal).props().description).toMatchSnapshot();
Copy link
Member Author

Choose a reason for hiding this comment

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

InlineSnapshots are no longer supported by Jest. A few changed files in this PR allow us to use regular snapshots, which remain supported.

Copy link
Member Author

Choose a reason for hiding this comment

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

Note that I also loosened constraints (prettier, license check) on those snapshot files, so they can be committed as-generated.


act(() => {
wrapper
Expand Down
15 changes: 15 additions & 0 deletions superset-frontend/src/pages/DatabaseList/__snapshots__/DatabaseList.test.jsx.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`Admin DatabaseList deletes 1`] = `
<React.Fragment>
<p>
The database

<b>
db 0
</b>

is linked to 0 charts that appear on 0 dashboards and users have 0 SQL Lab tabs using this database open. Are you sure you want to continue? Deleting the database will break those objects.
</p>
</React.Fragment>
`;
65 changes: 56 additions & 9 deletions superset-frontend/src/pages/DatabaseList/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ import rison from 'rison';
import { useSelector } from 'react-redux';
import { useQueryParams, BooleanParam } from 'use-query-params';
import { LocalStorageKeys, setItem } from 'src/utils/localStorageHelpers';

import Loading from 'src/components/Loading';
import { useListViewResource } from 'src/views/CRUD/hooks';
import {
Expand Down Expand Up @@ -65,8 +64,8 @@ const dbConfigExtraExtension = extensionsRegistry.get(
const PAGE_SIZE = 25;

interface DatabaseDeleteObject extends DatabaseObject {
chart_count: number;
dashboard_count: number;
charts: any;
dashboards: any;
sqllab_tab_count: number;
}
interface DatabaseListProps {
Expand Down Expand Up @@ -170,8 +169,8 @@ function DatabaseList({
.then(({ json = {} }) => {
setDatabaseCurrentlyDeleting({
...database,
chart_count: json.charts.count,
dashboard_count: json.dashboards.count,
charts: json.charts,
dashboards: json.dashboards,
sqllab_tab_count: json.sqllab_tab_states.count,
});
})
Expand Down Expand Up @@ -609,14 +608,62 @@ function DatabaseList({
description={
<>
<p>
{t('The database')}{' '}
<b>{databaseCurrentlyDeleting.database_name}</b>{' '}
{t(
'The database %s is linked to %s charts that appear on %s dashboards and users have %s SQL Lab tabs using this database open. Are you sure you want to continue? Deleting the database will break those objects.',
databaseCurrentlyDeleting.database_name,
databaseCurrentlyDeleting.chart_count,
databaseCurrentlyDeleting.dashboard_count,
'is linked to %s charts that appear on %s dashboards and users have %s SQL Lab tabs using this database open. Are you sure you want to continue? Deleting the database will break those objects.',
databaseCurrentlyDeleting.charts.count,
databaseCurrentlyDeleting.dashboards.count,
databaseCurrentlyDeleting.sqllab_tab_count,
)}
</p>
{databaseCurrentlyDeleting.dashboards.count >= 1 && (
<>
<h4>{t('Affected Dashboards')}</h4>
<ul>
{databaseCurrentlyDeleting.dashboards.result.map(
(
result: { id: number; title: string },
index: number,
) => (
<li key={result.id}>
<a
href={`/superset/dashboard/${result.id}`}
target="_atRiskItem"
>
{result.title}
</a>
</li>
),
)}
</ul>
</>
)}
{databaseCurrentlyDeleting.charts.count >= 1 && (
<>
<h4>{t('Affected Charts')}</h4>
<ul>
{databaseCurrentlyDeleting.charts.result.map(
(
result: {
id: number;
slice_name: string;
},
index: number,
) => (
<li key={result.id}>
<a
href={`/explore/?slice_id=${result.id}`}
target="_atRiskItem"
>
{result.slice_name}
</a>
</li>
),
)}
</ul>
</>
)}
{DatabaseDeleteRelatedExtension && (
<DatabaseDeleteRelatedExtension
database={databaseCurrentlyDeleting}
Expand Down
68 changes: 60 additions & 8 deletions superset-frontend/src/pages/DatasetList/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ import {
SupersetClient,
t,
} from '@superset-ui/core';
import { FunctionComponent, useState, useMemo, useCallback } from 'react';
import { FunctionComponent, useState, useMemo, useCallback, Key } from 'react';
import { Link, useHistory } from 'react-router-dom';
import rison from 'rison';
import {
Expand Down Expand Up @@ -154,7 +154,11 @@ const DatasetList: FunctionComponent<DatasetListProps> = ({
} = useListViewResource<Dataset>('dataset', t('dataset'), addDangerToast);

const [datasetCurrentlyDeleting, setDatasetCurrentlyDeleting] = useState<
(Dataset & { chart_count: number; dashboard_count: number }) | null
| (Dataset & {
charts: any;
dashboards: any;
})
| null
>(null);

const [datasetCurrentlyEditing, setDatasetCurrentlyEditing] =
Expand Down Expand Up @@ -243,8 +247,8 @@ const DatasetList: FunctionComponent<DatasetListProps> = ({
.then(({ json = {} }) => {
setDatasetCurrentlyDeleting({
...dataset,
chart_count: json.charts.count,
dashboard_count: json.dashboards.count,
charts: json.charts,
dashboards: json.dashboards,
});
})
.catch(
Expand Down Expand Up @@ -742,13 +746,61 @@ const DatasetList: FunctionComponent<DatasetListProps> = ({
description={
<>
<p>
{t('The dataset')}
<b> {datasetCurrentlyDeleting.table_name} </b>
{t(
'The dataset %s is linked to %s charts that appear on %s dashboards. Are you sure you want to continue? Deleting the dataset will break those objects.',
datasetCurrentlyDeleting.table_name,
datasetCurrentlyDeleting.chart_count,
datasetCurrentlyDeleting.dashboard_count,
'is linked to %s charts that appear on %s dashboards. Are you sure you want to continue? Deleting the dataset will break those objects.',
datasetCurrentlyDeleting.charts.count,
datasetCurrentlyDeleting.dashboards.count,
)}
</p>
{datasetCurrentlyDeleting.dashboards.count >= 1 && (
<>
<h4>{t('Affected Dashboards')}</h4>
<ul>
{datasetCurrentlyDeleting.dashboards.result.map(
(
result: { id: Key | null | undefined; title: string },
index: number,
) => (
<li key={result.id}>
<a
href={`/superset/dashboard/${result.id}`}
target="_atRiskItem"
>
{result.title}
</a>
</li>
),
)}
</ul>
</>
)}
{datasetCurrentlyDeleting.charts.count >= 1 && (
<>
<h4>{t('Affected Charts')}</h4>
<ul>
{datasetCurrentlyDeleting.charts.result.map(
(
result: {
id: Key | null | undefined;
slice_name: string;
},
index: number,
) => (
<li key={result.id}>
<a
href={`/explore/?slice_id=${result.id}`}
target="_atRiskItem"
>
{result.slice_name}
</a>
</li>
),
)}
</ul>
</>
)}
{DatasetDeleteRelatedExtension && (
<DatasetDeleteRelatedExtension
dataset={datasetCurrentlyDeleting}
Expand Down
Loading