diff --git a/superset-frontend/src/components/Chart/ChartContextMenu/ChartContextMenu.tsx b/superset-frontend/src/components/Chart/ChartContextMenu/ChartContextMenu.tsx index 20c65effed3a..d1774be1e2be 100644 --- a/superset-frontend/src/components/Chart/ChartContextMenu/ChartContextMenu.tsx +++ b/superset-frontend/src/components/Chart/ChartContextMenu/ChartContextMenu.tsx @@ -161,6 +161,7 @@ const ChartContextMenu = ( formData.datasource, dashboardId, formData, + !canDrillToDetail && !canDrillBy, ); const isLoadingDataset = datasetResource.status === ResourceStatus.Loading; diff --git a/superset-frontend/src/components/Chart/ChartContextMenu/useContextMenu.test.tsx b/superset-frontend/src/components/Chart/ChartContextMenu/useContextMenu.test.tsx index db812201f104..416d15a01f83 100644 --- a/superset-frontend/src/components/Chart/ChartContextMenu/useContextMenu.test.tsx +++ b/superset-frontend/src/components/Chart/ChartContextMenu/useContextMenu.test.tsx @@ -22,9 +22,15 @@ import { renderHook } from '@testing-library/react-hooks'; import mockState from 'spec/fixtures/mockState'; import { sliceId } from 'spec/fixtures/mockChartQueries'; import { noOp } from 'src/utils/common'; +import { cachedSupersetGet } from 'src/utils/cachedSupersetGet'; import { useContextMenu } from './useContextMenu'; import { ContextMenuItem } from './ChartContextMenu'; +jest.mock('src/utils/cachedSupersetGet'); + +const mockCachedSupersetGet = cachedSupersetGet as jest.MockedFunction< + typeof cachedSupersetGet +>; const CONTEXT_MENU_TEST_ID = 'chart-context-menu'; // @ts-ignore @@ -73,6 +79,19 @@ const setup = ({ return result; }; +beforeEach(() => { + mockCachedSupersetGet.mockClear(); + mockCachedSupersetGet.mockResolvedValue({ + response: {} as Response, + json: { + result: { + columns: [], + metrics: [], + }, + }, + }); +}); + test('Context menu renders', () => { const result = setup(); expect(screen.queryByTestId(CONTEXT_MENU_TEST_ID)).not.toBeInTheDocument(); @@ -271,3 +290,42 @@ test('Context menu does not show "Drill to detail" with `can_drill`, `can_explor result.current.onContextMenu(0, 0, {}); expect(screen.queryByText('Drill to detail')).not.toBeInTheDocument(); }); + +test('Dataset drill info API call is made when user has drill permissions', async () => { + const result = setup({ + roles: { + Admin: [ + ['can_explore', 'Superset'], + ['can_samples', 'Datasource'], + ['can_write', 'ExploreFormDataRestApi'], + ['can_get_drill_info', 'Dataset'], + ], + }, + }); + + result.current.onContextMenu(0, 0, {}); + + await new Promise(resolve => setTimeout(resolve, 0)); + + expect(mockCachedSupersetGet).toHaveBeenCalledWith({ + endpoint: expect.stringContaining( + '/api/v1/dataset/1/drill_info/?q=(dashboard_id:', + ), + }); +}); + +test('Dataset drill info API call is not made when user lacks drill permissions', async () => { + const result = setup({ + roles: { + Admin: [['invalid_permission', 'Dashboard']], + }, + }); + + result.current.onContextMenu(0, 0, {}); + + await new Promise(resolve => setTimeout(resolve, 0)); + + expect(mockCachedSupersetGet).not.toHaveBeenCalled(); + expect(screen.queryByText('Drill by')).not.toBeInTheDocument(); + expect(screen.queryByText('Drill to detail')).not.toBeInTheDocument(); +}); diff --git a/superset-frontend/src/components/Chart/types.ts b/superset-frontend/src/components/Chart/types.ts index 85a4668dc819..4f13de71cc94 100644 --- a/superset-frontend/src/components/Chart/types.ts +++ b/superset-frontend/src/components/Chart/types.ts @@ -32,11 +32,11 @@ export type Dataset = { first_name: string; last_name: string; }; - changed_on_humanized: string; - created_on_humanized: string; - description: string; - table_name: string; - owners: { + 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/dashboard/components/SliceHeaderControls/SliceHeaderControls.test.tsx b/superset-frontend/src/dashboard/components/SliceHeaderControls/SliceHeaderControls.test.tsx index af59b26f5af4..8b966a092444 100644 --- a/superset-frontend/src/dashboard/components/SliceHeaderControls/SliceHeaderControls.test.tsx +++ b/superset-frontend/src/dashboard/components/SliceHeaderControls/SliceHeaderControls.test.tsx @@ -20,8 +20,14 @@ import { render, screen, userEvent } from 'spec/helpers/testing-library'; import { FeatureFlag, VizType } from '@superset-ui/core'; import mockState from 'spec/fixtures/mockState'; +import { cachedSupersetGet } from 'src/utils/cachedSupersetGet'; import SliceHeaderControls, { SliceHeaderControlsProps } from '.'; +jest.mock('src/utils/cachedSupersetGet'); + +const mockCachedSupersetGet = cachedSupersetGet as jest.MockedFunction< + typeof cachedSupersetGet +>; const SLICE_ID = 371; const createProps = (viz_type = VizType.Sunburst) => @@ -116,6 +122,19 @@ const openMenu = () => { userEvent.click(screen.getByRole('button', { name: 'More Options' })); }; +beforeEach(() => { + mockCachedSupersetGet.mockClear(); + mockCachedSupersetGet.mockResolvedValue({ + response: {} as Response, + json: { + result: { + columns: [], + metrics: [], + }, + }, + }); +}); + test('Should render', () => { renderWrapper(); openMenu(); @@ -526,3 +545,37 @@ test('Should not show the "Edit chart" button', () => { openMenu(); expect(screen.queryByText('Edit chart')).not.toBeInTheDocument(); }); + +test('Dataset drill info API call is made when user has drill permissions', async () => { + (global as any).featureFlags = { + [FeatureFlag.DrillToDetail]: true, + }; + renderWrapper(undefined, { + Admin: [ + ['can_samples', 'Datasource'], + ['can_explore', 'Superset'], + ['can_get_drill_info', 'Dataset'], + ], + }); + + await new Promise(resolve => setTimeout(resolve, 0)); + + expect(mockCachedSupersetGet).toHaveBeenCalledWith({ + endpoint: expect.stringContaining( + '/api/v1/dataset/58/drill_info/?q=(dashboard_id:26)', + ), + }); +}); + +test('Dataset drill info API call is not made when user lacks drill permissions', async () => { + (global as any).featureFlags = { + [FeatureFlag.DrillToDetail]: true, + }; + renderWrapper(undefined, { + Admin: [['invalid_permission', 'Dashboard']], + }); + + await new Promise(resolve => setTimeout(resolve, 0)); + + expect(mockCachedSupersetGet).not.toHaveBeenCalled(); +}); diff --git a/superset-frontend/src/dashboard/components/SliceHeaderControls/index.tsx b/superset-frontend/src/dashboard/components/SliceHeaderControls/index.tsx index 2dcc08966d3b..60c9012ec04b 100644 --- a/superset-frontend/src/dashboard/components/SliceHeaderControls/index.tsx +++ b/superset-frontend/src/dashboard/components/SliceHeaderControls/index.tsx @@ -184,6 +184,7 @@ const SliceHeaderControls = ( props.slice.datasource, props.dashboardId, props.formData, + !canDrillToDetail, ); const datasetWithVerboseMap = diff --git a/superset-frontend/src/features/datasets/metadataBar/useDatasetMetadataBar.tsx b/superset-frontend/src/features/datasets/metadataBar/useDatasetMetadataBar.tsx index 0faa24c97107..75ea53b43495 100644 --- a/superset-frontend/src/features/datasets/metadataBar/useDatasetMetadataBar.tsx +++ b/superset-frontend/src/features/datasets/metadataBar/useDatasetMetadataBar.tsx @@ -60,23 +60,23 @@ export const useDatasetMetadataBar = ({ ? `${changed_by.first_name} ${changed_by.last_name}` : notAvailable; const formattedOwners = - owners?.length > 0 + owners && owners.length > 0 ? owners.map(owner => `${owner.first_name} ${owner.last_name}`) : [notAvailable]; items.push({ type: MetadataType.Table, - title: table_name, + title: table_name || notAvailable, }); items.push({ type: MetadataType.LastModified, - value: changed_on_humanized, + value: changed_on_humanized || notAvailable, modifiedBy, }); items.push({ type: MetadataType.Owner, createdBy, owners: formattedOwners, - createdOn: created_on_humanized, + createdOn: created_on_humanized || notAvailable, }); if (description) { items.push({ diff --git a/superset-frontend/src/hooks/apiResources/datasets.ts b/superset-frontend/src/hooks/apiResources/datasets.ts index ddd091fbe252..d48dac46fd37 100644 --- a/superset-frontend/src/hooks/apiResources/datasets.ts +++ b/superset-frontend/src/hooks/apiResources/datasets.ts @@ -63,6 +63,7 @@ export const useDatasetDrillInfo = ( datasetId: string | number, dashboardId: number, formData?: QueryFormData, + skip: boolean = false, ): Resource => { const [resource, setResource] = useState>({ status: ResourceStatus.Loading, @@ -71,6 +72,15 @@ export const useDatasetDrillInfo = ( }); useEffect(() => { + if (skip) { + // short circuit if `skip` is `true` + setResource({ + status: ResourceStatus.Complete, + result: {} as Dataset, + error: null, + }); + return; + } const fetchDataset = async () => { try { const numericDatasetId = getDatasetId(datasetId); @@ -115,7 +125,7 @@ export const useDatasetDrillInfo = ( }; fetchDataset(); - }, [datasetId, dashboardId, formData]); + }, [datasetId, dashboardId, formData, skip]); return resource; };