feat: Displaying details to Dataset/Database deletion modals (apache#…

…30016)
justinpark · Sep 10, 2024 · 7bb6a14 · 7bb6a14
1 parent 5a0e887
commit 7bb6a14
Show file tree

Hide file tree

Showing 8 changed files with 177 additions and 25 deletions.
diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml
@@ -53,6 +53,7 @@ repos:
       - id: debug-statements
       - id: end-of-file-fixer
       - id: trailing-whitespace
+        exclude: ^.*\.(snap)
         args: ["--markdown-linebreak-ext=md"]
   - repo: https://github.com/pre-commit/mirrors-prettier
     rev: v3.1.0 # Use the sha or tag you want to point at

diff --git a/.rat-excludes b/.rat-excludes
@@ -61,6 +61,9 @@ tsconfig.tsbuildinfo
 generator-superset/*
 temporary_superset_ui/*
 
+# skip license checks for auto-generated test snapshots
+.*snap
+
 # docs overrides for third party logos we don't have the rights to
 google-big-query.svg
 google-sheets.svg

diff --git a/superset-frontend/.prettierignore b/superset-frontend/.prettierignore
@@ -25,5 +25,6 @@ CHANGELOG/
 *.geojson
 *-topo.json
 storybook-static/
+*.snap
 
 /.nx/workspace-data
diff --git a/superset-frontend/src/components/DeleteModal/index.tsx b/superset-frontend/src/components/DeleteModal/index.tsx
@@ -84,6 +84,7 @@ export default function DeleteModal({
       primaryButtonType="danger"
       show={open}
       title={title}
+      centered
     >
       <DescriptionContainer>{description}</DescriptionContainer>
       <StyledDiv>

diff --git a/superset-frontend/src/pages/DatabaseList/DatabaseList.test.jsx b/superset-frontend/src/pages/DatabaseList/DatabaseList.test.jsx
@@ -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();
 
     act(() => {
       wrapper

diff --git a/superset-frontend/src/pages/DatabaseList/__snapshots__/DatabaseList.test.jsx.snap b/superset-frontend/src/pages/DatabaseList/__snapshots__/DatabaseList.test.jsx.snap
@@ -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>
+`;
diff --git a/superset-frontend/src/pages/DatabaseList/index.tsx b/superset-frontend/src/pages/DatabaseList/index.tsx
@@ -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 {
@@ -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 {
@@ -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,
         });
       })
@@ -609,14 +608,82 @@ 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
+                      .slice(0, 10)
+                      .map(
+                        (
+                          result: { id: number; title: string },
+                          index: number,
+                        ) => (
+                          <li key={result.id}>
+                            <a
+                              href={`/superset/dashboard/${result.id}`}
+                              target="_atRiskItem"
+                            >
+                              {result.title}
+                            </a>
+                          </li>
+                        ),
+                      )}
+                    {databaseCurrentlyDeleting.dashboards.result.length >
+                      10 && (
+                      <li>
+                        {t(
+                          '... and %s others',
+                          databaseCurrentlyDeleting.dashboards.result.length -
+                            10,
+                        )}
+                      </li>
+                    )}
+                  </ul>
+                </>
+              )}
+              {databaseCurrentlyDeleting.charts.count >= 1 && (
+                <>
+                  <h4>{t('Affected Charts')}</h4>
+                  <ul>
+                    {databaseCurrentlyDeleting.charts.result.slice(0, 10).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>
+                      ),
+                    )}
+                    {databaseCurrentlyDeleting.charts.result.length > 10 && (
+                      <li>
+                        {t(
+                          '... and %s others',
+                          databaseCurrentlyDeleting.charts.result.length - 10,
+                        )}
+                      </li>
+                    )}
+                  </ul>
+                </>
+              )}
               {DatabaseDeleteRelatedExtension && (
                 <DatabaseDeleteRelatedExtension
                   database={databaseCurrentlyDeleting}

diff --git a/superset-frontend/src/pages/DatasetList/index.tsx b/superset-frontend/src/pages/DatasetList/index.tsx
@@ -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 {
@@ -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] =
@@ -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(
@@ -742,13 +746,80 @@ 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
+                      .slice(0, 10)
+                      .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>
+                        ),
+                      )}
+                    {datasetCurrentlyDeleting.dashboards.result.length > 10 && (
+                      <li>
+                        {t(
+                          '... and %s others',
+                          datasetCurrentlyDeleting.dashboards.result.length -
+                            10,
+                        )}
+                      </li>
+                    )}
+                  </ul>
+                </>
+              )}
+              {datasetCurrentlyDeleting.charts.count >= 1 && (
+                <>
+                  <h4>{t('Affected Charts')}</h4>
+                  <ul>
+                    {datasetCurrentlyDeleting.charts.result.slice(0, 10).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>
+                      ),
+                    )}
+                    {datasetCurrentlyDeleting.charts.result.length > 10 && (
+                      <li>
+                        {t(
+                          '... and %s others',
+                          datasetCurrentlyDeleting.charts.result.length - 10,
+                        )}
+                      </li>
+                    )}
+                  </ul>
+                </>
+              )}
               {DatasetDeleteRelatedExtension && (
                 <DatasetDeleteRelatedExtension
                   dataset={datasetCurrentlyDeleting}