feat: Add metadata bar to drill by modal (#23542)

superset-frontend/src/components/Chart/DrillBy/DrillByChart.tsx

@@ -45,7 +45,7 @@ export default function DrillByChart({
   let groupbyField: any = [];
   const [chartDataResult, setChartDataResult] = useState();
 
-  if (groupbyFieldName && column) {
+  if (column) {
     groupbyField = Array.isArray(formData[groupbyFieldName])
       ? [column.column_name]
       : column.column_name;
@@ -82,8 +82,8 @@ export default function DrillByChart({
           chartType={formData.viz_type}
           enableNoResults
           formData={updatedFormData}
-          height="100%"
           queriesData={chartDataResult}
+          height="100%"
           width="100%"
         />
       ) : (

superset-frontend/src/components/Chart/DrillBy/DrillByMenuItems.test.tsx

@@ -114,77 +114,73 @@ getChartMetadataRegistry().registerValue(
   }),
 );
 
-describe('Drill by menu items', () => {
-  afterEach(() => {
-    supersetGetCache.clear();
-    fetchMock.restore();
+afterEach(() => {
+  supersetGetCache.clear();
+  fetchMock.restore();
+});
+
+test('render disabled menu item for unsupported chart', async () => {
+  renderMenu({
+    formData: { ...defaultFormData, viz_type: 'unsupported_viz' },
+  });
+  await expectDrillByDisabled(
+    'Drill by is not yet supported for this chart type',
+  );
+});
+
+test('render disabled menu item for supported chart, no filters', async () => {
+  renderMenu({ filters: [] });
+  await expectDrillByDisabled('Drill by is not available for this data point');
+});
+
+test('render disabled menu item for supported chart, no columns', async () => {
+  fetchMock.get(datasetEndpointMatcher, { result: { columns: [] } });
+  renderMenu({});
+  await waitFor(() => fetchMock.called(datasetEndpointMatcher));
+  await expectDrillByDisabled('No dimensions available for drill by');
+});
+
+test('render menu item with submenu without searchbox', async () => {
+  const slicedColumns = defaultColumns.slice(0, 9);
+  fetchMock.get(datasetEndpointMatcher, {
+    result: { columns: slicedColumns },
+  });
+  renderMenu({});
+  await waitFor(() => fetchMock.called(datasetEndpointMatcher));
+  await expectDrillByEnabled();
+  slicedColumns.forEach(column => {
+    expect(screen.getByText(column.column_name)).toBeInTheDocument();
+  });
+  expect(screen.queryByRole('textbox')).not.toBeInTheDocument();
+});
+
+test('render menu item with submenu and searchbox', async () => {
+  fetchMock.get(datasetEndpointMatcher, {
+    result: { columns: defaultColumns },
+  });
+  renderMenu({});
+  await waitFor(() => fetchMock.called(datasetEndpointMatcher));
+  await expectDrillByEnabled();
+  defaultColumns.forEach(column => {
+    expect(screen.getByText(column.column_name)).toBeInTheDocument();
   });
 
-  test('render disabled menu item for unsupported chart', async () => {
-    renderMenu({
-      formData: { ...defaultFormData, viz_type: 'unsupported_viz' },
-    });
-    await expectDrillByDisabled(
-      'Drill by is not yet supported for this chart type',
-    );
-  });
+  const searchbox = screen.getByRole('textbox');
+  expect(searchbox).toBeInTheDocument();
 
-  test('render disabled menu item for supported chart, no filters', async () => {
-    renderMenu({ filters: [] });
-    await expectDrillByDisabled(
-      'Drill by is not available for this data point',
-    );
-  });
+  userEvent.type(searchbox, 'col1');
 
-  test('render disabled menu item for supported chart, no columns', async () => {
-    fetchMock.get(datasetEndpointMatcher, { result: { columns: [] } });
-    renderMenu({});
-    await waitFor(() => fetchMock.called(datasetEndpointMatcher));
-    await expectDrillByDisabled('No dimensions available for drill by');
-  });
+  await screen.findByText('col1');
 
-  test('render menu item with submenu without searchbox', async () => {
-    const slicedColumns = defaultColumns.slice(0, 9);
-    fetchMock.get(datasetEndpointMatcher, {
-      result: { columns: slicedColumns },
-    });
-    renderMenu({});
-    await waitFor(() => fetchMock.called(datasetEndpointMatcher));
-    await expectDrillByEnabled();
-    slicedColumns.forEach(column => {
-      expect(screen.getByText(column.column_name)).toBeInTheDocument();
-    });
-    expect(screen.queryByRole('textbox')).not.toBeInTheDocument();
-  });
+  const expectedFilteredColumnNames = ['col1', 'col10', 'col11'];
 
-  test('render menu item with submenu and searchbox', async () => {
-    fetchMock.get(datasetEndpointMatcher, {
-      result: { columns: defaultColumns },
-    });
-    renderMenu({});
-    await waitFor(() => fetchMock.called(datasetEndpointMatcher));
-    await expectDrillByEnabled();
-    defaultColumns.forEach(column => {
-      expect(screen.getByText(column.column_name)).toBeInTheDocument();
+  defaultColumns
+    .filter(col => !expectedFilteredColumnNames.includes(col.column_name))
+    .forEach(col => {
+      expect(screen.queryByText(col.column_name)).not.toBeInTheDocument();
     });
 
-    const searchbox = screen.getByRole('textbox');
-    expect(searchbox).toBeInTheDocument();
-
-    userEvent.type(searchbox, 'col1');
-
-    await screen.findByText('col1');
-
-    const expectedFilteredColumnNames = ['col1', 'col10', 'col11'];
-
-    defaultColumns
-      .filter(col => !expectedFilteredColumnNames.includes(col.column_name))
-      .forEach(col => {
-        expect(screen.queryByText(col.column_name)).not.toBeInTheDocument();
-      });
-
-    expectedFilteredColumnNames.forEach(colName => {
-      expect(screen.getByText(colName)).toBeInTheDocument();
-    });
+  expectedFilteredColumnNames.forEach(colName => {
+    expect(screen.getByText(colName)).toBeInTheDocument();
   });
 });

superset-frontend/src/components/Chart/DrillBy/DrillByMenuItems.tsx

@@ -47,6 +47,7 @@ import { MenuItemTooltip } from '../DisabledMenuItemTooltip';
 import DrillByModal from './DrillByModal';
 import { getSubmenuYOffset } from '../utils';
 import { MenuItemWithTruncation } from '../MenuItemWithTruncation';
+import { Dataset } from '../types';
 
 const MAX_SUBMENU_HEIGHT = 200;
 const SHOW_COLUMNS_SEARCH_THRESHOLD = 10;
@@ -74,6 +75,7 @@ export const DrillByMenuItems = ({
 }: DrillByMenuItemsProps) => {
   const theme = useTheme();
   const [searchInput, setSearchInput] = useState('');
+  const [dataset, setDataset] = useState<Dataset>();
   const [columns, setColumns] = useState<Column[]>([]);
   const [showModal, setShowModal] = useState(false);
   const [currentColumn, setCurrentColumn] = useState();
@@ -114,6 +116,7 @@ export const DrillByMenuItems = ({
         endpoint: `/api/v1/dataset/${datasetId}`,
       })
         .then(({ json: { result } }) => {
+          setDataset(result);
           setColumns(
             ensureIsArray(result.columns)
               .filter(column => column.groupby)
@@ -248,6 +251,7 @@ export const DrillByMenuItems = ({
         groupbyFieldName={groupbyFieldName}
         onHideModal={closeModal}
         showModal={showModal}
+        dataset={dataset!}
       />
     </>
   );

superset-frontend/src/components/Chart/DrillBy/DrillByModal.test.tsx

@@ -44,6 +44,18 @@ const drillByModalState = {
     },
   },
 };
+const dataset = {
+  changed_on_humanized: '01-01-2001',
+  created_on_humanized: '01-01-2001',
+  description: 'desc',
+  table_name: 'my_dataset',
+  owners: [
+    {
+      first_name: 'Sarah',
+      last_name: 'Connor',
+    },
+  ],
+};
 const renderModal = async (state?: object) => {
   const DrillByModalWrapper = () => {
     const [showModal, setShowModal] = useState(false);
@@ -57,6 +69,7 @@ const renderModal = async (state?: object) => {
           formData={formData}
           showModal={showModal}
           onHideModal={() => setShowModal(false)}
+          dataset={dataset}
         />
       </>
     );

superset-frontend/src/components/Chart/DrillBy/DrillByModal.tsx

@@ -29,6 +29,8 @@ import Modal from 'src/components/Modal';
 import Button from 'src/components/Button';
 import { useSelector } from 'react-redux';
 import { DashboardLayout, RootState } from 'src/dashboard/types';
+import { useDatasetMetadataBar } from 'src/features/datasets/metadataBar/useDatasetMetadataBar';
+import { Dataset } from '../types';
 import DrillByChart from './DrillByChart';
 
 interface ModalFooterProps {
@@ -59,6 +61,7 @@ interface DrillByModalProps {
   groupbyFieldName?: string;
   onHideModal: () => void;
   showModal: boolean;
+  dataset: Dataset;
 }
 
 export default function DrillByModal({
@@ -68,6 +71,7 @@ export default function DrillByModal({
   groupbyFieldName,
   onHideModal,
   showModal,
+  dataset,
 }: DrillByModalProps) {
   const theme = useTheme();
   const dashboardLayout = useSelector<RootState, DashboardLayout>(
@@ -80,6 +84,7 @@ export default function DrillByModal({
     chartLayoutItem?.meta.sliceNameOverride || chartLayoutItem?.meta.sliceName;
   const exploreChart = () => {};
 
+  const { metadataBar } = useDatasetMetadataBar({ dataset });
   return (
     <Modal
       css={css`
@@ -105,12 +110,21 @@ export default function DrillByModal({
       destroyOnClose
       maskClosable={false}
     >
-      <DrillByChart
-        column={column}
-        filters={filters}
-        formData={formData}
-        groupbyFieldName={groupbyFieldName}
-      />
+      <div
+        css={css`
+          display: flex;
+          flex-direction: column;
+          height: 100%;
+        `}
+      >
+        {metadataBar}
+        <DrillByChart
+          column={column}
+          filters={filters}
+          formData={formData}
+          groupbyFieldName={groupbyFieldName}
+        />
+      </div>
     </Modal>
   );
 }

superset-frontend/src/components/Chart/DrillDetail/DrillDetailPane.test.tsx

@@ -17,11 +17,12 @@
  * under the License.
  */
 import React from 'react';
+import fetchMock from 'fetch-mock';
+import { QueryFormData, SupersetClient } from '@superset-ui/core';
 import { render, screen, waitFor } from 'spec/helpers/testing-library';
 import { getMockStoreWithNativeFilters } from 'spec/fixtures/mockStore';
 import chartQueries, { sliceId } from 'spec/fixtures/mockChartQueries';
-import { QueryFormData, SupersetClient } from '@superset-ui/core';
-import fetchMock from 'fetch-mock';
+import { supersetGetCache } from 'src/utils/cachedSupersetGet';
 import DrillDetailPane from './DrillDetailPane';
 
 const chart = chartQueries[sliceId];
@@ -114,7 +115,10 @@ const fetchWithData = () => {
   });
 };
 
-afterEach(fetchMock.restore);
+afterEach(() => {
+  fetchMock.restore();
+  supersetGetCache.clear();
+});
 
 test('should render', async () => {
   fetchWithNoData();
@@ -180,11 +184,7 @@ test('should render the metadata bar', async () => {
 
 test('should render an error message when fails to load the metadata', async () => {
   fetchWithNoData();
-  fetchMock.get(
-    DATASET_ENDPOINT,
-    { status: 'error', error: 'Some error' },
-    { overwriteRoutes: true },
-  );
+  fetchMock.get(DATASET_ENDPOINT, { status: 400 }, { overwriteRoutes: true });
   setup();
   expect(
     await screen.findByText('There was an error loading the dataset metadata'),

superset-frontend/src/components/Chart/DrillDetail/DrillDetailPane.tsx

@@ -102,8 +102,9 @@ export default function DrillDetailPane({
     [formData.datasource],
   );
 
-  const { metadataBar, status: metadataBarStatus } =
-    useDatasetMetadataBar(datasourceId);
+  const { metadataBar, status: metadataBarStatus } = useDatasetMetadataBar({
+    datasetId: datasourceId,
+  });
   // Get page of results
   const resultsPage = useMemo(() => {
     const nextResultsPage = resultsPages.get(pageIndex);

superset-frontend/src/components/Chart/DrillDetail/types.ts

@@ -24,22 +24,3 @@ export type ResultsPage = {
   colNames: string[];
   colTypes: GenericDataType[];
 };
-
-export type Dataset = {
-  changed_by?: {
-    first_name: string;
-    last_name: string;
-  };
-  created_by?: {
-    first_name: string;
-    last_name: string;
-  };
-  changed_on_humanized: string;
-  created_on_humanized: string;
-  description: string;
-  table_name: string;
-  owners: {
-    first_name: string;
-    last_name: string;
-  }[];
-};

superset-frontend/src/components/Chart/types.ts

@@ -0,0 +1,37 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+export type Dataset = {
+  changed_by?: {
+    first_name: string;
+    last_name: string;
+  };
+  created_by?: {
+    first_name: string;
+    last_name: string;
+  };
+  changed_on_humanized: string;
+  created_on_humanized: string;
+  description: string;
+  table_name: string;
+  owners: {
+    first_name: string;
+    last_name: string;
+  }[];
+};

superset-frontend/src/features/datasets/metadataBar/DatasetMetadataBar.stories.tsx

@@ -53,7 +53,7 @@ export default {
 export const DatasetSpecific = () => {
   SupersetClient.reset();
   SupersetClient.configure({ csrfToken: '1234' }).init();
-  const { metadataBar } = useDatasetMetadataBar(1);
+  const { metadataBar } = useDatasetMetadataBar({ datasetId: 1 });
   const { width, height, ref } = useResizeDetector();
   // eslint-disable-next-line no-param-reassign
   return (

superset-frontend/src/features/datasets/metadataBar/useDatasetMetadataBar.test.tsx

@@ -20,28 +20,36 @@
 import fetchMock from 'fetch-mock';
 import { renderHook } from '@testing-library/react-hooks';
 import { createWrapper, render } from 'spec/helpers/testing-library';
+import { supersetGetCache } from 'src/utils/cachedSupersetGet';
 import { useDatasetMetadataBar } from './useDatasetMetadataBar';
 
-test('renders dataset metadata bar', async () => {
+const MOCK_DATASET = {
+  changed_on: '2023-01-26T12:06:58.733316',
+  changed_on_humanized: 'a month ago',
+  changed_by: { first_name: 'Han', last_name: 'Solo' },
+  created_by: { first_name: 'Luke', last_name: 'Skywalker' },
+  created_on: '2023-01-26T12:06:54.965034',
+  created_on_humanized: 'a month ago',
+  table_name: `This is dataset's name`,
+  owners: [
+    { first_name: 'John', last_name: 'Doe' },
+    { first_name: 'Luke', last_name: 'Skywalker' },
+  ],
+  description: 'This is a dataset description',
+};
+
+afterEach(() => {
+  fetchMock.restore();
+  supersetGetCache.clear();
+});
+
+test('renders dataset metadata bar from request', async () => {
   fetchMock.get('glob:*/api/v1/dataset/1', {
-    result: {
-      changed_on: '2023-01-26T12:06:58.733316',
-      changed_on_humanized: 'a month ago',
-      changed_by: { first_name: 'Han', last_name: 'Solo' },
-      created_by: { first_name: 'Luke', last_name: 'Skywalker' },
-      created_on: '2023-01-26T12:06:54.965034',
-      created_on_humanized: 'a month ago',
-      table_name: `This is dataset's name`,
-      owners: [
-        { first_name: 'John', last_name: 'Doe' },
-        { first_name: 'Luke', last_name: 'Skywalker' },
-      ],
-      description: 'This is a dataset description',
-    },
+    result: MOCK_DATASET,
   });
 
   const { result, waitForValueToChange } = renderHook(
-    () => useDatasetMetadataBar(1),
+    () => useDatasetMetadataBar({ datasetId: 1 }),
     {
       wrapper: createWrapper(),
     },
@@ -50,13 +58,36 @@ test('renders dataset metadata bar', async () => {
   await waitForValueToChange(() => result.current.status);
   expect(result.current.status).toEqual('complete');
 
+  expect(fetchMock.called()).toBeTruthy();
+  const { findByText, findAllByRole } = render(result.current.metadataBar);
+  expect(await findByText(`This is dataset's name`)).toBeVisible();
+  expect(await findByText('This is a dataset description')).toBeVisible();
+  expect(await findByText('Luke Skywalker')).toBeVisible();
+  expect(await findByText('a month ago')).toBeVisible();
+  expect(await findAllByRole('img')).toHaveLength(4);
+});
+
+test('renders dataset metadata bar without request', async () => {
+  fetchMock.get('glob:*/api/v1/dataset/1', {
+    result: {},
+  });
+
+  const { result } = renderHook(
+    () => useDatasetMetadataBar({ dataset: MOCK_DATASET }),
+    {
+      wrapper: createWrapper(),
+    },
+  );
+
+  expect(result.current.status).toEqual('complete');
+
+  expect(fetchMock.called()).toBeFalsy();
   const { findByText, findAllByRole } = render(result.current.metadataBar);
   expect(await findByText(`This is dataset's name`)).toBeVisible();
   expect(await findByText('This is a dataset description')).toBeVisible();
   expect(await findByText('Luke Skywalker')).toBeVisible();
   expect(await findByText('a month ago')).toBeVisible();
   expect(await findAllByRole('img')).toHaveLength(4);
-  fetchMock.restore();
 });
 
 test('renders dataset metadata bar without description and owners', async () => {
@@ -71,7 +102,7 @@ test('renders dataset metadata bar without description and owners', async () =>
   });
 
   const { result, waitForValueToChange } = renderHook(
-    () => useDatasetMetadataBar(1),
+    () => useDatasetMetadataBar({ datasetId: 1 }),
     {
       wrapper: createWrapper(),
     },
@@ -80,6 +111,7 @@ test('renders dataset metadata bar without description and owners', async () =>
   await waitForValueToChange(() => result.current.status);
   expect(result.current.status).toEqual('complete');
 
+  expect(fetchMock.called()).toBeTruthy();
   const { findByText, queryByText, findAllByRole } = render(
     result.current.metadataBar,
   );
@@ -88,6 +120,4 @@ test('renders dataset metadata bar without description and owners', async () =>
   expect(await findByText('Not available')).toBeVisible();
   expect(await findByText('a month ago')).toBeVisible();
   expect(await findAllByRole('img')).toHaveLength(3);
-
-  fetchMock.restore();
 });

superset-frontend/src/features/datasets/metadataBar/useDatasetMetadataBar.tsx

@@ -16,27 +16,50 @@
  * specific language governing permissions and limitations
  * under the License.
  */
-import React, { useMemo } from 'react';
+import React, { useEffect, useMemo, useState } from 'react';
 import { css, t, useTheme } from '@superset-ui/core';
 import Alert from 'src/components/Alert';
-import { useApiV1Resource } from 'src/hooks/apiResources';
-import { Dataset } from 'src/components/Chart/DrillDetail/types';
+import { Dataset } from 'src/components/Chart/types';
 import MetadataBar from 'src/components/MetadataBar';
 import {
   ContentType,
   MetadataType,
 } from 'src/components/MetadataBar/ContentType';
 import { ResourceStatus } from 'src/hooks/apiResources/apiResources';
+import { cachedSupersetGet } from 'src/utils/cachedSupersetGet';
 
-export const useDatasetMetadataBar = (datasetId: number | string) => {
+export type UseDatasetMetadataBarProps =
+  | { datasetId?: undefined; dataset: Dataset }
+  | { datasetId: number | string; dataset?: undefined };
+export const useDatasetMetadataBar = ({
+  dataset: datasetProps,
+  datasetId,
+}: UseDatasetMetadataBarProps) => {
   const theme = useTheme();
-  const response = useApiV1Resource<Dataset>(`/api/v1/dataset/${datasetId}`);
+  const [result, setResult] = useState<Dataset>();
+  const [status, setStatus] = useState<ResourceStatus>(
+    datasetProps ? ResourceStatus.COMPLETE : ResourceStatus.LOADING,
+  );
 
-  const { status, result } = response;
+  useEffect(() => {
+    if (!datasetProps && datasetId) {
+      cachedSupersetGet({
+        endpoint: `/api/v1/dataset/${datasetId}`,
+      })
+        .then(({ json: { result } }) => {
+          setResult(result);
+          setStatus(ResourceStatus.COMPLETE);
+        })
+        .catch(() => {
+          setStatus(ResourceStatus.ERROR);
+        });
+    }
+  }, [datasetId, datasetProps]);
 
   const metadataBar = useMemo(() => {
     const items: ContentType[] = [];
-    if (result) {
+    const dataset = datasetProps || result;
+    if (dataset) {
       const {
         changed_on_humanized,
         created_on_humanized,
@@ -45,7 +68,7 @@ export const useDatasetMetadataBar = (datasetId: number | string) => {
         changed_by,
         created_by,
         owners,
-      } = result;
+      } = dataset;
       const notAvailable = t('Not available');
       const createdBy =
         `${created_by?.first_name ?? ''} ${
@@ -98,7 +121,7 @@ export const useDatasetMetadataBar = (datasetId: number | string) => {
         )}
       </div>
     );
-  }, [result, status, theme.gridUnit]);
+  }, [datasetProps, result, status, theme.gridUnit]);
 
   return {
     metadataBar,