fix: Avoid dataset drill request if no perm (#34665)

superset-frontend/src/components/Chart/ChartContextMenu/ChartContextMenu.tsx

@@ -194,6 +194,7 @@ const ChartContextMenu = (
     formData.datasource,
     dashboardId,
     formData,
+    !canDrillToDetail && !canDrillBy,
   );
 
   const isLoadingDataset = datasetResource.status === ResourceStatus.Loading;

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();
+});

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;
   }[];

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();
+});

superset-frontend/src/dashboard/components/SliceHeaderControls/index.tsx

@@ -184,6 +184,7 @@ const SliceHeaderControls = (
     props.slice.datasource,
     props.dashboardId,
     props.formData,
+    !canDrillToDetail,
   );
 
   const datasetWithVerboseMap =

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({

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<Dataset> => {
   const [resource, setResource] = useState<Resource<Dataset>>({
     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;
 };