diff --git a/superset-frontend/src/components/Chart/DrillBy/DrillByChart.tsx b/superset-frontend/src/components/Chart/DrillBy/DrillByChart.tsx index 1b588c1bfe251..58a19996b97fd 100644 --- a/superset-frontend/src/components/Chart/DrillBy/DrillByChart.tsx +++ b/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%" /> ) : ( diff --git a/superset-frontend/src/components/Chart/DrillBy/DrillByMenuItems.test.tsx b/superset-frontend/src/components/Chart/DrillBy/DrillByMenuItems.test.tsx index e7db5efe8ea85..f22472e17a489 100644 --- a/superset-frontend/src/components/Chart/DrillBy/DrillByMenuItems.test.tsx +++ b/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 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 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 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 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(); - }); - - const searchbox = screen.getByRole('textbox'); - expect(searchbox).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(); + }); - userEvent.type(searchbox, 'col1'); + const searchbox = screen.getByRole('textbox'); + expect(searchbox).toBeInTheDocument(); - await screen.findByText('col1'); + userEvent.type(searchbox, 'col1'); - const expectedFilteredColumnNames = ['col1', 'col10', 'col11']; + await screen.findByText('col1'); - defaultColumns - .filter(col => !expectedFilteredColumnNames.includes(col.column_name)) - .forEach(col => { - expect(screen.queryByText(col.column_name)).not.toBeInTheDocument(); - }); + const expectedFilteredColumnNames = ['col1', 'col10', 'col11']; - expectedFilteredColumnNames.forEach(colName => { - expect(screen.getByText(colName)).toBeInTheDocument(); + 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(); }); }); diff --git a/superset-frontend/src/components/Chart/DrillBy/DrillByMenuItems.tsx b/superset-frontend/src/components/Chart/DrillBy/DrillByMenuItems.tsx index 2253ff5deaa59..d1a27d5ea9db6 100644 --- a/superset-frontend/src/components/Chart/DrillBy/DrillByMenuItems.tsx +++ b/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(); const [columns, setColumns] = useState([]); 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!} /> ); diff --git a/superset-frontend/src/components/Chart/DrillBy/DrillByModal.test.tsx b/superset-frontend/src/components/Chart/DrillBy/DrillByModal.test.tsx index e3e96426f8d8a..dd3cddec71a94 100644 --- a/superset-frontend/src/components/Chart/DrillBy/DrillByModal.test.tsx +++ b/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} /> ); diff --git a/superset-frontend/src/components/Chart/DrillBy/DrillByModal.tsx b/superset-frontend/src/components/Chart/DrillBy/DrillByModal.tsx index f870cdb01c3e7..aab89f6accb84 100644 --- a/superset-frontend/src/components/Chart/DrillBy/DrillByModal.tsx +++ b/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( @@ -80,6 +84,7 @@ export default function DrillByModal({ chartLayoutItem?.meta.sliceNameOverride || chartLayoutItem?.meta.sliceName; const exploreChart = () => {}; + const { metadataBar } = useDatasetMetadataBar({ dataset }); return ( - +
+ {metadataBar} + +
); } diff --git a/superset-frontend/src/components/Chart/DrillDetail/DrillDetailPane.test.tsx b/superset-frontend/src/components/Chart/DrillDetail/DrillDetailPane.test.tsx index a6c8fd06b07f0..582f7b15e9c79 100644 --- a/superset-frontend/src/components/Chart/DrillDetail/DrillDetailPane.test.tsx +++ b/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'), diff --git a/superset-frontend/src/components/Chart/DrillDetail/DrillDetailPane.tsx b/superset-frontend/src/components/Chart/DrillDetail/DrillDetailPane.tsx index f03a8c8b9fab9..daf9f3f1cf4b5 100644 --- a/superset-frontend/src/components/Chart/DrillDetail/DrillDetailPane.tsx +++ b/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); diff --git a/superset-frontend/src/components/Chart/DrillDetail/types.ts b/superset-frontend/src/components/Chart/DrillDetail/types.ts index ea49c22ce3f10..06b9b1bab9543 100644 --- a/superset-frontend/src/components/Chart/DrillDetail/types.ts +++ b/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; - }[]; -}; diff --git a/superset-frontend/src/components/Chart/types.ts b/superset-frontend/src/components/Chart/types.ts new file mode 100644 index 0000000000000..83983df94336c --- /dev/null +++ b/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; + }[]; +}; diff --git a/superset-frontend/src/features/datasets/metadataBar/DatasetMetadataBar.stories.tsx b/superset-frontend/src/features/datasets/metadataBar/DatasetMetadataBar.stories.tsx index 252bb22bb2b98..80c17ca2eeec6 100644 --- a/superset-frontend/src/features/datasets/metadataBar/DatasetMetadataBar.stories.tsx +++ b/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 ( diff --git a/superset-frontend/src/features/datasets/metadataBar/useDatasetMetadataBar.test.tsx b/superset-frontend/src/features/datasets/metadataBar/useDatasetMetadataBar.test.tsx index fdca8fb7f554c..c5c2d55f15456 100644 --- a/superset-frontend/src/features/datasets/metadataBar/useDatasetMetadataBar.test.tsx +++ b/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(); }); diff --git a/superset-frontend/src/features/datasets/metadataBar/useDatasetMetadataBar.tsx b/superset-frontend/src/features/datasets/metadataBar/useDatasetMetadataBar.tsx index 280b7a398960d..8c2f83977ac0d 100644 --- a/superset-frontend/src/features/datasets/metadataBar/useDatasetMetadataBar.tsx +++ b/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(`/api/v1/dataset/${datasetId}`); + const [result, setResult] = useState(); + const [status, setStatus] = useState( + 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) => { )} ); - }, [result, status, theme.gridUnit]); + }, [datasetProps, result, status, theme.gridUnit]); return { metadataBar,