chore: Add permission to view and drill on Dashboard context (apache#26798)

Author: geido

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

@@ -90,6 +90,12 @@ const ChartContextMenu = (
   const canExplore = useSelector((state: RootState) =>
     findPermission('can_explore', 'Superset', state.user?.roles),
   );
+  const canViewDrill = useSelector((state: RootState) =>
+    findPermission('can_view_and_drill', 'Dashboard', state.user?.roles),
+  );
+  const canDatasourceSamples = useSelector((state: RootState) =>
+    findPermission('can_samples', 'Datasource', state.user?.roles),
+  );
   const crossFiltersEnabled = useSelector<RootState, boolean>(
     ({ dashboardInfo }) => dashboardInfo.crossFiltersEnabled,
   );
@@ -105,15 +111,17 @@ const ChartContextMenu = (
   }>({ clientX: 0, clientY: 0 });
 
   const menuItems = [];
+  const canExploreOrView = canExplore || canViewDrill;
 
   const showDrillToDetail =
     isFeatureEnabled(FeatureFlag.DRILL_TO_DETAIL) &&
-    canExplore &&
+    canExploreOrView &&
+    canDatasourceSamples &&
     isDisplayed(ContextMenuItem.DrillToDetail);
 
   const showDrillBy =
     isFeatureEnabled(FeatureFlag.DRILL_BY) &&
-    canExplore &&
+    canExploreOrView &&
     isDisplayed(ContextMenuItem.DrillBy);
 
   const showCrossFilters =

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

@@ -38,10 +38,12 @@ const setup = ({
   onSelection = noOp,
   displayedItems = ContextMenuItem.All,
   additionalConfig = {},
+  roles = undefined,
 }: {
   onSelection?: () => void;
   displayedItems?: ContextMenuItem | ContextMenuItem[];
   additionalConfig?: Record<string, any>;
+  roles?: Record<string, string[][]>;
 } = {}) => {
   const { result } = renderHook(() =>
     useContextMenu(
@@ -58,7 +60,12 @@ const setup = ({
       ...mockState,
       user: {
         ...mockState.user,
-        roles: { Admin: [['can_explore', 'Superset']] },
+        roles: roles ?? {
+          Admin: [
+            ['can_explore', 'Superset'],
+            ['can_samples', 'Datasource'],
+          ],
+        },
       },
     },
   });
@@ -75,7 +82,7 @@ test('Context menu renders', () => {
   expect(screen.getByText('Drill by')).toBeInTheDocument();
 });
 
-test('Context menu contains all items only', () => {
+test('Context menu contains all displayed items only', () => {
   const result = setup({
     displayedItems: [ContextMenuItem.DrillToDetail, ContextMenuItem.DrillBy],
   });
@@ -84,3 +91,28 @@ test('Context menu contains all items only', () => {
   expect(screen.getByText('Drill to detail')).toBeInTheDocument();
   expect(screen.getByText('Drill by')).toBeInTheDocument();
 });
+
+test('Context menu shows all items tied to can_view_and_drill permission', () => {
+  const result = setup({
+    roles: {
+      Admin: [
+        ['can_view_and_drill', 'Dashboard'],
+        ['can_samples', 'Datasource'],
+      ],
+    },
+  });
+  result.current.onContextMenu(0, 0, {});
+  expect(screen.getByText('Drill to detail')).toBeInTheDocument();
+  expect(screen.getByText('Drill by')).toBeInTheDocument();
+  expect(screen.getByText('Add cross-filter')).toBeInTheDocument();
+});
+
+test('Context menu does not show "Drill to detail" without proper permissions', () => {
+  const result = setup({
+    roles: { Admin: [['can_view_and_drill', 'Dashboard']] },
+  });
+  result.current.onContextMenu(0, 0, {});
+  expect(screen.queryByText('Drill to detail')).not.toBeInTheDocument();
+  expect(screen.getByText('Drill by')).toBeInTheDocument();
+  expect(screen.getByText('Add cross-filter')).toBeInTheDocument();
+});

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

@@ -68,7 +68,10 @@ const dataset = {
   ],
 };
 
-const renderModal = async (modalProps: Partial<DrillByModalProps> = {}) => {
+const renderModal = async (
+  modalProps: Partial<DrillByModalProps> = {},
+  overrideState: Record<string, any> = {},
+) => {
   const DrillByModalWrapper = () => {
     const [showModal, setShowModal] = useState(false);
 
@@ -93,7 +96,10 @@ const renderModal = async (modalProps: Partial<DrillByModalProps> = {}) => {
     useDnd: true,
     useRedux: true,
     useRouter: true,
-    initialState: drillByModalState,
+    initialState: {
+      ...drillByModalState,
+      ...overrideState,
+    },
   });
 
   userEvent.click(screen.getByRole('button', { name: 'Show modal' }));
@@ -233,3 +239,29 @@ test('render breadcrumbs', async () => {
   expect(newBreadcrumbItems).toHaveLength(1);
   expect(within(breadcrumbItems[0]).getByText('gender')).toBeInTheDocument();
 });
+
+test('should render "Edit chart" as disabled without can_explore permission', async () => {
+  await renderModal(
+    {},
+    {
+      user: {
+        ...drillByModalState.user,
+        roles: { Admin: [['test_invalid_role', 'Superset']] },
+      },
+    },
+  );
+  expect(screen.getByRole('button', { name: 'Edit chart' })).toBeDisabled();
+});
+
+test('should render "Edit chart" enabled with can_explore permission', async () => {
+  await renderModal(
+    {},
+    {
+      user: {
+        ...drillByModalState.user,
+        roles: { Admin: [['can_explore', 'Superset']] },
+      },
+    },
+  );
+  expect(screen.getByRole('button', { name: 'Edit chart' })).toBeEnabled();
+});

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

@@ -54,6 +54,7 @@ import {
   LOG_ACTIONS_DRILL_BY_MODAL_OPENED,
   LOG_ACTIONS_FURTHER_DRILL_BY,
 } from 'src/logger/LogUtils';
+import { findPermission } from 'src/utils/findPermission';
 import { Dataset, DrillByType } from '../types';
 import DrillByChart from './DrillByChart';
 import { ContextMenuItem } from '../ChartContextMenu/ChartContextMenu';
@@ -75,6 +76,7 @@ interface ModalFooterProps {
 const ModalFooter = ({ formData, closeModal }: ModalFooterProps) => {
   const dispatch = useDispatch();
   const { addDangerToast } = useToasts();
+  const theme = useTheme();
   const [url, setUrl] = useState('');
   const dashboardPageId = useContext(DashboardPageIdContext);
   const onEditChartClick = useCallback(() => {
@@ -84,6 +86,9 @@ const ModalFooter = ({ formData, closeModal }: ModalFooterProps) => {
       }),
     );
   }, [dispatch, formData.slice_id]);
+  const canExplore = useSelector((state: RootState) =>
+    findPermission('can_explore', 'Superset', state.user?.roles),
+  );
 
   const [datasource_id, datasource_type] = formData.datasource.split('__');
   useEffect(() => {
@@ -103,13 +108,20 @@ const ModalFooter = ({ formData, closeModal }: ModalFooterProps) => {
     datasource_type,
     formData,
   ]);
+  const isEditDisabled = !url || !canExplore;
+
   return (
     <>
       <Button
         buttonStyle="secondary"
         buttonSize="small"
         onClick={onEditChartClick}
-        disabled={!url}
+        disabled={isEditDisabled}
+        tooltip={
+          isEditDisabled
+            ? t('You do not have sufficient permissions to edit the chart')
+            : undefined
+        }
       >
         <Link
           css={css`
@@ -128,6 +140,9 @@ const ModalFooter = ({ formData, closeModal }: ModalFooterProps) => {
         buttonSize="small"
         onClick={closeModal}
         data-test="close-drill-by-modal"
+        css={css`
+          margin-left: ${theme.gridUnit * 2}px;
+        `}
       >
         {t('Close')}
       </Button>

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

@@ -35,9 +35,16 @@ jest.mock('react-router-dom', () => ({
 
 const { id: chartId, form_data: formData } = chartQueries[sliceId];
 const { slice_name: chartName } = formData;
+const store = getMockStoreWithNativeFilters();
+const drillToDetailModalState = {
+  ...store.getState(),
+  user: {
+    ...store.getState().user,
+    roles: { Admin: [['can_explore', 'Superset']] },
+  },
+};
 
-const renderModal = async () => {
-  const store = getMockStoreWithNativeFilters();
+const renderModal = async (overrideState: Record<string, any> = {}) => {
   const DrillDetailModalWrapper = () => {
     const [showModal, setShowModal] = useState(false);
     return (
@@ -59,7 +66,10 @@ const renderModal = async () => {
   render(<DrillDetailModalWrapper />, {
     useRouter: true,
     useRedux: true,
-    store,
+    initialState: {
+      ...drillToDetailModalState,
+      ...overrideState,
+    },
   });
 
   userEvent.click(screen.getByRole('button', { name: 'Show modal' }));
@@ -93,3 +103,13 @@ test('should forward to Explore', async () => {
     `/explore/?dashboard_page_id=&slice_id=${sliceId}`,
   );
 });
+
+test('should render "Edit chart" as disabled without can_explore permission', async () => {
+  await renderModal({
+    user: {
+      ...drillToDetailModalState.user,
+      roles: { Admin: [['test_invalid_role', 'Superset']] },
+    },
+  });
+  expect(screen.getByRole('button', { name: 'Edit chart' })).toBeDisabled();
+});

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

@@ -31,28 +31,52 @@ import Button from 'src/components/Button';
 import { useSelector } from 'react-redux';
 import { DashboardPageIdContext } from 'src/dashboard/containers/DashboardPage';
 import { Slice } from 'src/types/Chart';
+import { RootState } from 'src/dashboard/types';
+import { findPermission } from 'src/utils/findPermission';
 import DrillDetailPane from './DrillDetailPane';
 
 interface ModalFooterProps {
-  exploreChart: () => void;
+  canExplore: boolean;
   closeModal?: () => void;
+  exploreChart: () => void;
 }
 
-const ModalFooter = ({ exploreChart, closeModal }: ModalFooterProps) => (
-  <>
-    <Button buttonStyle="secondary" buttonSize="small" onClick={exploreChart}>
-      {t('Edit chart')}
-    </Button>
-    <Button
-      buttonStyle="primary"
-      buttonSize="small"
-      onClick={closeModal}
-      data-test="close-drilltodetail-modal"
-    >
-      {t('Close')}
-    </Button>
-  </>
-);
+const ModalFooter = ({
+  canExplore,
+  closeModal,
+  exploreChart,
+}: ModalFooterProps) => {
+  const theme = useTheme();
+
+  return (
+    <>
+      <Button
+        buttonStyle="secondary"
+        buttonSize="small"
+        onClick={exploreChart}
+        disabled={!canExplore}
+        tooltip={
+          !canExplore
+            ? t('You do not have sufficient permissions to edit the chart')
+            : undefined
+        }
+      >
+        {t('Edit chart')}
+      </Button>
+      <Button
+        buttonStyle="primary"
+        buttonSize="small"
+        onClick={closeModal}
+        data-test="close-drilltodetail-modal"
+        css={css`
+          margin-left: ${theme.gridUnit * 2}px;
+        `}
+      >
+        {t('Close')}
+      </Button>
+    </>
+  );
+};
 
 interface DrillDetailModalProps {
   chartId: number;
@@ -76,6 +100,9 @@ export default function DrillDetailModal({
     (state: { sliceEntities: { slices: Record<number, Slice> } }) =>
       state.sliceEntities.slices[chartId],
   );
+  const canExplore = useSelector((state: RootState) =>
+    findPermission('can_explore', 'Superset', state.user?.roles),
+  );
 
   const exploreUrl = useMemo(
     () => `/explore/?dashboard_page_id=${dashboardPageId}&slice_id=${chartId}`,
@@ -97,7 +124,9 @@ export default function DrillDetailModal({
         }
       `}
       title={t('Drill to detail: %s', chartName)}
-      footer={<ModalFooter exploreChart={exploreChart} />}
+      footer={
+        <ModalFooter exploreChart={exploreChart} canExplore={canExplore} />
+      }
       responsive
       resizable
       resizableConfig={{