From 9307b32f95db914a66770849a03ea4a1914507ab Mon Sep 17 00:00:00 2001
From: Kamil Gabryjelski <kamil.gabryjelski@gmail.com>
Date: Tue, 18 Apr 2023 17:42:14 +0200
Subject: [PATCH 1/2] feat: Drill by error management

---
 .../Chart/DrillBy/DrillByMenuItems.tsx        |  5 +-
 .../components/Chart/DrillBy/DrillByModal.tsx | 48 +++++++++++++++----
 2 files changed, 43 insertions(+), 10 deletions(-)

diff --git a/superset-frontend/src/components/Chart/DrillBy/DrillByMenuItems.tsx b/superset-frontend/src/components/Chart/DrillBy/DrillByMenuItems.tsx
index e5b919ff6d780..b503b1eed5db5 100644
--- a/superset-frontend/src/components/Chart/DrillBy/DrillByMenuItems.tsx
+++ b/superset-frontend/src/components/Chart/DrillBy/DrillByMenuItems.tsx
@@ -39,6 +39,7 @@ import {
 } from '@superset-ui/core';
 import Icons from 'src/components/Icons';
 import { Input } from 'src/components/Input';
+import { useToasts } from 'src/components/MessageToasts/withToasts';
 import {
   cachedSupersetGet,
   supersetGetCache,
@@ -80,12 +81,12 @@ export const DrillByMenuItems = ({
   ...rest
 }: DrillByMenuItemsProps) => {
   const theme = useTheme();
+  const { addDangerToast } = useToasts();
   const [searchInput, setSearchInput] = useState('');
   const [dataset, setDataset] = useState<Dataset>();
   const [columns, setColumns] = useState<Column[]>([]);
   const [showModal, setShowModal] = useState(false);
   const [currentColumn, setCurrentColumn] = useState();
-
   const handleSelection = useCallback(
     (event, column) => {
       onClick(event);
@@ -143,9 +144,11 @@ export const DrillByMenuItems = ({
         })
         .catch(() => {
           supersetGetCache.delete(`/api/v1/dataset/${datasetId}`);
+          addDangerToast(t('Failed to load dimensions for drill by'));
         });
     }
   }, [
+    addDangerToast,
     excludedColumns,
     formData,
     groupbyFieldName,
diff --git a/superset-frontend/src/components/Chart/DrillBy/DrillByModal.tsx b/superset-frontend/src/components/Chart/DrillBy/DrillByModal.tsx
index 7f65576854873..765e40db28532 100644
--- a/superset-frontend/src/components/Chart/DrillBy/DrillByModal.tsx
+++ b/superset-frontend/src/components/Chart/DrillBy/DrillByModal.tsx
@@ -47,6 +47,8 @@ import { noOp } from 'src/utils/common';
 import { simpleFilterToAdhoc } from 'src/utils/simpleFilterToAdhoc';
 import { useDatasetMetadataBar } from 'src/features/datasets/metadataBar/useDatasetMetadataBar';
 import { SingleQueryResultPane } from 'src/explore/components/DataTablesPane/components/SingleQueryResultPane';
+import { useToasts } from 'src/components/MessageToasts/withToasts';
+import Alert from 'src/components/Alert';
 import { Dataset, DrillByType } from '../types';
 import DrillByChart from './DrillByChart';
 import { ContextMenuItem } from '../ChartContextMenu/ChartContextMenu';
@@ -65,6 +67,7 @@ interface ModalFooterProps {
 }
 
 const ModalFooter = ({ formData, closeModal }: ModalFooterProps) => {
+  const { addDangerToast } = useToasts();
   const [url, setUrl] = useState('');
   const dashboardPageId = useContext(DashboardPageIdContext);
   const [datasource_id, datasource_type] = formData.datasource.split('__');
@@ -75,13 +78,24 @@ const ModalFooter = ({ formData, closeModal }: ModalFooterProps) => {
           `/explore/?form_data_key=${key}&dashboard_page_id=${dashboardPageId}`,
         );
       })
-      .catch(e => {
-        console.log(e);
+      .catch(() => {
+        addDangerToast(t('Failed to generate chart edit URL'));
       });
-  }, [dashboardPageId, datasource_id, datasource_type, formData]);
+  }, [
+    addDangerToast,
+    dashboardPageId,
+    datasource_id,
+    datasource_type,
+    formData,
+  ]);
   return (
     <>
-      <Button buttonStyle="secondary" buttonSize="small" onClick={noOp}>
+      <Button
+        buttonStyle="secondary"
+        buttonSize="small"
+        onClick={noOp}
+        disabled={!url}
+      >
         <Link
           css={css`
             &:hover {
@@ -126,6 +140,8 @@ export default function DrillByModal({
   onHideModal,
 }: DrillByModalProps) {
   const theme = useTheme();
+  const { addDangerToast } = useToasts();
+  const [isChartDataLoading, setIsChartDataLoading] = useState(false);
 
   const initialGroupbyColumns = useMemo(
     () =>
@@ -278,14 +294,22 @@ export default function DrillByModal({
 
   useEffect(() => {
     if (drilledFormData) {
+      setIsChartDataLoading(true);
       setChartDataResult(undefined);
       getChartDataRequest({
         formData: drilledFormData,
-      }).then(({ json }) => {
-        setChartDataResult(json.result);
-      });
+      })
+        .then(({ json }) => {
+          setChartDataResult(json.result);
+        })
+        .catch(() => {
+          addDangerToast(t('Failed to load chart data.'));
+        })
+        .finally(() => {
+          setIsChartDataLoading(false);
+        });
     }
-  }, [drilledFormData]);
+  }, [addDangerToast, drilledFormData]);
   const { metadataBar } = useDatasetMetadataBar({ dataset });
 
   return (
@@ -323,7 +347,13 @@ export default function DrillByModal({
         {metadataBar}
         {breadcrumbs}
         {displayModeToggle}
-        {!chartDataResult && <Loading />}
+        {isChartDataLoading && <Loading />}
+        {!isChartDataLoading && !chartDataResult && (
+          <Alert
+            type="error"
+            message={t('There was an error loading the chart data')}
+          />
+        )}
         {drillByDisplayMode === DrillByType.Chart && chartDataResult && (
           <DrillByChart
             formData={drilledFormData}

From 198b3b278753b5354033ffa322da24a861a09d32 Mon Sep 17 00:00:00 2001
From: Kamil Gabryjelski <kamil.gabryjelski@gmail.com>
Date: Tue, 18 Apr 2023 21:29:25 +0200
Subject: [PATCH 2/2] Fix tests

---
 .../Chart/DrillBy/DrillByModal.test.tsx       | 20 +++++++++++++++----
 .../components/Chart/DrillBy/DrillByModal.tsx |  2 +-
 2 files changed, 17 insertions(+), 5 deletions(-)

diff --git a/superset-frontend/src/components/Chart/DrillBy/DrillByModal.test.tsx b/superset-frontend/src/components/Chart/DrillBy/DrillByModal.test.tsx
index f0768253e0adf..66fd6a42ba63c 100644
--- a/superset-frontend/src/components/Chart/DrillBy/DrillByModal.test.tsx
+++ b/superset-frontend/src/components/Chart/DrillBy/DrillByModal.test.tsx
@@ -104,7 +104,7 @@ beforeEach(() => {
     .post(CHART_DATA_ENDPOINT, { body: {} }, {})
     .post(FORM_DATA_KEY_ENDPOINT, { key: '123' });
 });
-afterEach(fetchMock.restore);
+afterEach(() => fetchMock.restore());
 
 test('should render the title', async () => {
   await renderModal();
@@ -127,10 +127,22 @@ test('should close the modal', async () => {
 });
 
 test('should render loading indicator', async () => {
-  await renderModal();
-  await waitFor(() =>
-    expect(screen.getByLabelText('Loading')).toBeInTheDocument(),
+  fetchMock.post(
+    CHART_DATA_ENDPOINT,
+    { body: {} },
+    // delay is missing in fetch-mock types
+    // @ts-ignore
+    { overwriteRoutes: true, delay: 1000 },
   );
+  await renderModal();
+  expect(screen.getByLabelText('Loading')).toBeInTheDocument();
+});
+
+test('should render alert banner when results fail to load', async () => {
+  await renderModal();
+  expect(
+    await screen.findByText('There was an error loading the chart data'),
+  ).toBeInTheDocument();
 });
 
 test('should generate Explore url', async () => {
diff --git a/superset-frontend/src/components/Chart/DrillBy/DrillByModal.tsx b/superset-frontend/src/components/Chart/DrillBy/DrillByModal.tsx
index 765e40db28532..0a05635645f12 100644
--- a/superset-frontend/src/components/Chart/DrillBy/DrillByModal.tsx
+++ b/superset-frontend/src/components/Chart/DrillBy/DrillByModal.tsx
@@ -141,7 +141,7 @@ export default function DrillByModal({
 }: DrillByModalProps) {
   const theme = useTheme();
   const { addDangerToast } = useToasts();
-  const [isChartDataLoading, setIsChartDataLoading] = useState(false);
+  const [isChartDataLoading, setIsChartDataLoading] = useState(true);
 
   const initialGroupbyColumns = useMemo(
     () =>