chore(D2D): Add granular permission for dashboard drilling operations (…

…#28435)
apache · Dec 3, 2024 · 54c89cd · 54c89cd
1 parent 833421e
commit 54c89cd
Show file tree

Hide file tree

Showing 5 changed files with 142 additions and 10 deletions.
diff --git a/superset-frontend/src/components/Chart/ChartContextMenu/ChartContextMenu.tsx b/superset-frontend/src/components/Chart/ChartContextMenu/ChartContextMenu.tsx
@@ -96,8 +96,11 @@ const ChartContextMenu = (
   const canDatasourceSamples = useSelector((state: RootState) =>
     findPermission('can_samples', 'Datasource', state.user?.roles),
   );
-  const canDrillBy = canExplore && canWriteExploreFormData;
-  const canDrillToDetail = canExplore && canDatasourceSamples;
+  const canDrill = useSelector((state: RootState) =>
+    findPermission('can_drill', 'Dashboard', state.user?.roles),
+  );
+  const canDrillBy = (canExplore || canDrill) && canWriteExploreFormData;
+  const canDrillToDetail = (canExplore || canDrill) && canDatasourceSamples;
   const crossFiltersEnabled = useSelector<RootState, boolean>(
     ({ dashboardInfo }) => dashboardInfo.crossFiltersEnabled,
   );

diff --git a/superset-frontend/src/components/Chart/ChartContextMenu/useContextMenu.test.tsx b/superset-frontend/src/components/Chart/ChartContextMenu/useContextMenu.test.tsx
@@ -93,7 +93,7 @@ test('Context menu contains all displayed items only', () => {
   expect(screen.getByText('Drill by')).toBeInTheDocument();
 });
 
-test('Context menu shows "Drill by"', () => {
+test('Context menu shows "Drill by" with `can_explore` & `can_write` perms', () => {
   const result = setup({
     roles: {
       Admin: [
@@ -106,7 +106,34 @@ test('Context menu shows "Drill by"', () => {
   expect(screen.getByText('Drill by')).toBeInTheDocument();
 });
 
-test('Context menu does not show "Drill by"', () => {
+test('Context menu shows "Drill by" with `can_drill` & `can_write` perms', () => {
+  const result = setup({
+    roles: {
+      Admin: [
+        ['can_write', 'ExploreFormDataRestApi'],
+        ['can_drill', 'Dashboard'],
+      ],
+    },
+  });
+  result.current.onContextMenu(0, 0, {});
+  expect(screen.getByText('Drill by')).toBeInTheDocument();
+});
+
+test('Context menu shows "Drill by" with `can_drill` & `can_explore` + `can_write` perms', () => {
+  const result = setup({
+    roles: {
+      Admin: [
+        ['can_write', 'ExploreFormDataRestApi'],
+        ['can_explore', 'Superset'],
+        ['can_drill', 'Dashboard'],
+      ],
+    },
+  });
+  result.current.onContextMenu(0, 0, {});
+  expect(screen.getByText('Drill by')).toBeInTheDocument();
+});
+
+test('Context menu does not show "Drill by" with neither of required perms', () => {
   const result = setup({
     roles: {
       Admin: [['invalid_permission', 'Dashboard']],
@@ -116,7 +143,17 @@ test('Context menu does not show "Drill by"', () => {
   expect(screen.queryByText('Drill by')).not.toBeInTheDocument();
 });
 
-test('Context menu shows "Drill to detail"', () => {
+test('Context menu does not show "Drill by" with just `can_dril` perm', () => {
+  const result = setup({
+    roles: {
+      Admin: [['can_drill', 'Dashboard']],
+    },
+  });
+  result.current.onContextMenu(0, 0, {});
+  expect(screen.queryByText('Drill by')).not.toBeInTheDocument();
+});
+
+test('Context menu shows "Drill to detail" with `can_samples` and `can_explore` perms', () => {
   const result = setup({
     roles: {
       Admin: [
@@ -129,10 +166,47 @@ test('Context menu shows "Drill to detail"', () => {
   expect(screen.getByText('Drill to detail')).toBeInTheDocument();
 });
 
-test('Context menu does not show "Drill to detail"', () => {
+test('Context menu shows "Drill to detail" with `can_drill` & `can_samples` perms', () => {
+  const result = setup({
+    roles: {
+      Admin: [
+        ['can_samples', 'Datasource'],
+        ['can_drill', 'Dashboard'],
+      ],
+    },
+  });
+  result.current.onContextMenu(0, 0, {});
+  expect(screen.getByText('Drill to detail')).toBeInTheDocument();
+});
+
+test('Context menu shows "Drill to detail" with `can_drill` & `can_explore` + `can_write` perms', () => {
+  const result = setup({
+    roles: {
+      Admin: [
+        ['can_samples', 'Datasource'],
+        ['can_explore', 'Superset'],
+        ['can_drill', 'Dashboard'],
+      ],
+    },
+  });
+  result.current.onContextMenu(0, 0, {});
+  expect(screen.getByText('Drill to detail')).toBeInTheDocument();
+});
+
+test('Context menu does not show "Drill to detail" with neither of required perms', () => {
+  const result = setup({
+    roles: {
+      Admin: [['invalid_permission', 'Dashboard']],
+    },
+  });
+  result.current.onContextMenu(0, 0, {});
+  expect(screen.queryByText('Drill to detail')).not.toBeInTheDocument();
+});
+
+test('Context menu does not show "Drill to detail" with just `can_dril` perm', () => {
   const result = setup({
     roles: {
-      Admin: [['can_explore', 'Superset']],
+      Admin: [['can_drill', 'Dashboard']],
     },
   });
   result.current.onContextMenu(0, 0, {});

diff --git a/superset-frontend/src/dashboard/components/SliceHeaderControls/SliceHeaderControls.test.tsx b/superset-frontend/src/dashboard/components/SliceHeaderControls/SliceHeaderControls.test.tsx
@@ -302,7 +302,7 @@ test('Drill to detail modal is under featureflag', () => {
   expect(screen.queryByText('Drill to detail')).not.toBeInTheDocument();
 });
 
-test('Should show "Drill to detail"', () => {
+test('Should show "Drill to detail" with `can_explore` & `can_samples` perms', () => {
   (global as any).featureFlags = {
     [FeatureFlag.DrillToDetail]: true,
   };
@@ -317,7 +317,43 @@ test('Should show "Drill to detail"', () => {
   expect(screen.getByText('Drill to detail')).toBeInTheDocument();
 });
 
-test('Should not show "Drill to detail"', () => {
+test('Should show "Drill to detail" with `can_drill` & `can_samples` perms', () => {
+  (global as any).featureFlags = {
+    [FeatureFlag.DrillToDetail]: true,
+  };
+  const props = {
+    ...createProps(),
+    supersetCanExplore: false,
+  };
+  props.slice.slice_id = 18;
+  renderWrapper(props, {
+    Admin: [
+      ['can_samples', 'Datasource'],
+      ['can_drill', 'Dashboard'],
+    ],
+  });
+  expect(screen.getByText('Drill to detail')).toBeInTheDocument();
+});
+
+test('Should show "Drill to detail" with both `canexplore` + `can_drill` & `can_samples` perms', () => {
+  (global as any).featureFlags = {
+    [FeatureFlag.DrillToDetail]: true,
+  };
+  const props = {
+    ...createProps(),
+    supersetCanExplore: true,
+  };
+  props.slice.slice_id = 18;
+  renderWrapper(props, {
+    Admin: [
+      ['can_samples', 'Datasource'],
+      ['can_drill', 'Dashboard'],
+    ],
+  });
+  expect(screen.getByText('Drill to detail')).toBeInTheDocument();
+});
+
+test('Should not show "Drill to detail" with neither of required perms', () => {
   (global as any).featureFlags = {
     [FeatureFlag.DrillToDetail]: true,
   };
@@ -332,6 +368,21 @@ test('Should not show "Drill to detail"', () => {
   expect(screen.queryByText('Drill to detail')).not.toBeInTheDocument();
 });
 
+test('Should not show "Drill to detail" only `can_dril` perm', () => {
+  (global as any).featureFlags = {
+    [FeatureFlag.DrillToDetail]: true,
+  };
+  const props = {
+    ...createProps(),
+    supersetCanExplore: false,
+  };
+  props.slice.slice_id = 18;
+  renderWrapper(props, {
+    Admin: [['can_drill', 'Dashboard']],
+  });
+  expect(screen.queryByText('Drill to detail')).not.toBeInTheDocument();
+});
+
 test('Should show "View query"', () => {
   const props = {
     ...createProps(),

diff --git a/superset-frontend/src/dashboard/components/SliceHeaderControls/index.tsx b/superset-frontend/src/dashboard/components/SliceHeaderControls/index.tsx
@@ -557,7 +557,10 @@ const SliceHeaderControls = (props: SliceHeaderControlsPropsWithRouter) => {
   const canDatasourceSamples = useSelector((state: RootState) =>
     findPermission('can_samples', 'Datasource', state.user?.roles),
   );
-  const canDrillToDetail = canExplore && canDatasourceSamples;
+  const canDrill = useSelector((state: RootState) =>
+    findPermission('can_drill', 'Dashboard', state.user?.roles),
+  );
+  const canDrillToDetail = (canExplore || canDrill) && canDatasourceSamples;
   const canViewQuery = useSelector((state: RootState) =>
     findPermission('can_view_query', 'Dashboard', state.user?.roles),
   );

diff --git a/superset/security/manager.py b/superset/security/manager.py
@@ -967,6 +967,7 @@ def create_custom_permissions(self) -> None:
         self.add_permission_view_menu("can_sqllab", "Superset")
         self.add_permission_view_menu("can_view_query", "Dashboard")
         self.add_permission_view_menu("can_view_chart_as_table", "Dashboard")
+        self.add_permission_view_menu("can_drill", "Dashboard")
 
     def create_missing_perms(self) -> None:
         """