Skip to content

Commit

Permalink
chore: Add granular permissions for actions in Dashboard (apache#27029)
Browse files Browse the repository at this point in the history
  • Loading branch information
geido authored and sfirke committed Mar 22, 2024
1 parent b37f210 commit 5a9ce3f
Show file tree
Hide file tree
Showing 10 changed files with 272 additions and 49 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -90,12 +90,14 @@ 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 canWriteExploreFormData = useSelector((state: RootState) =>
findPermission('can_write', 'ExploreFormDataRestApi', state.user?.roles),
);
const canDatasourceSamples = useSelector((state: RootState) =>
findPermission('can_samples', 'Datasource', state.user?.roles),
);
const canDrillBy = canExplore && canWriteExploreFormData;
const canDrillToDetail = canExplore && canDatasourceSamples;
const crossFiltersEnabled = useSelector<RootState, boolean>(
({ dashboardInfo }) => dashboardInfo.crossFiltersEnabled,
);
Expand All @@ -111,17 +113,15 @@ const ChartContextMenu = (
}>({ clientX: 0, clientY: 0 });

const menuItems = [];
const canExploreOrView = canExplore || canViewDrill;

const showDrillToDetail =
isFeatureEnabled(FeatureFlag.DrillToDetail) &&
canExploreOrView &&
canDatasourceSamples &&
canDrillToDetail &&
isDisplayed(ContextMenuItem.DrillToDetail);

const showDrillBy =
isFeatureEnabled(FeatureFlag.DrillBy) &&
canExploreOrView &&
canDrillBy &&
isDisplayed(ContextMenuItem.DrillBy);

const showCrossFilters =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ const setup = ({
Admin: [
['can_explore', 'Superset'],
['can_samples', 'Datasource'],
['can_write', 'ExploreFormDataRestApi'],
],
},
},
Expand Down Expand Up @@ -92,27 +93,48 @@ test('Context menu contains all displayed items only', () => {
expect(screen.getByText('Drill by')).toBeInTheDocument();
});

test('Context menu shows all items tied to can_view_and_drill permission', () => {
test('Context menu shows "Drill by"', () => {
const result = setup({
roles: {
Admin: [
['can_write', 'ExploreFormDataRestApi'],
['can_explore', 'Superset'],
],
},
});
result.current.onContextMenu(0, 0, {});
expect(screen.getByText('Drill by')).toBeInTheDocument();
});

test('Context menu does not show "Drill by"', () => {
const result = setup({
roles: {
Admin: [['invalid_permission', 'Dashboard']],
},
});
result.current.onContextMenu(0, 0, {});
expect(screen.queryByText('Drill by')).not.toBeInTheDocument();
});

test('Context menu shows "Drill to detail"', () => {
const result = setup({
roles: {
Admin: [
['can_view_and_drill', 'Dashboard'],
['can_samples', 'Datasource'],
['can_explore', 'Superset'],
],
},
});
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', () => {
test('Context menu does not show "Drill to detail"', () => {
const result = setup({
roles: { Admin: [['can_view_and_drill', 'Dashboard']] },
roles: {
Admin: [['can_explore', 'Superset']],
},
});
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();
});
Original file line number Diff line number Diff line change
Expand Up @@ -246,7 +246,7 @@ test('should render "Edit chart" as disabled without can_explore permission', as
{
user: {
...drillByModalState.user,
roles: { Admin: [['test_invalid_role', 'Superset']] },
roles: { Admin: [['invalid_permission', 'Superset']] },
},
},
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ test('should render "Edit chart" as disabled without can_explore permission', as
await renderModal({
user: {
...drillToDetailModalState.user,
roles: { Admin: [['test_invalid_role', 'Superset']] },
roles: { Admin: [['invalid_permission', 'Superset']] },
},
});
expect(screen.getByRole('button', { name: 'Edit chart' })).toBeDisabled();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,9 @@

import userEvent from '@testing-library/user-event';
import React from 'react';
import { getMockStore } from 'spec/fixtures/mockStore';
import { render, screen } from 'spec/helpers/testing-library';
import { FeatureFlag } from '@superset-ui/core';
import mockState from 'spec/fixtures/mockState';
import SliceHeaderControls, { SliceHeaderControlsProps } from '.';

jest.mock('src/components/Dropdown', () => {
Expand Down Expand Up @@ -104,16 +104,16 @@ const renderWrapper = (
roles?: Record<string, string[][]>,
) => {
const props = overrideProps || createProps();
const store = getMockStore();
const mockState = store.getState();
return render(<SliceHeaderControls {...props} />, {
useRedux: true,
useRouter: true,
initialState: {
...mockState,
user: {
...mockState.user,
roles: roles ?? mockState.user.roles,
roles: roles ?? {
Admin: [['can_samples', 'Datasource']],
},
},
},
});
Expand Down Expand Up @@ -310,18 +310,18 @@ test('Should show "Drill to detail"', () => {
global.featureFlags = {
[FeatureFlag.DrillToDetail]: true,
};
const props = createProps();
const props = {
...createProps(),
supersetCanExplore: true,
};
props.slice.slice_id = 18;
renderWrapper(props, {
Admin: [
['can_view_and_drill', 'Dashboard'],
['can_samples', 'Datasource'],
],
Admin: [['can_samples', 'Datasource']],
});
expect(screen.getByText('Drill to detail')).toBeInTheDocument();
});

test('Should show menu items tied to can_view_and_drill permission', () => {
test('Should not show "Drill to detail"', () => {
// @ts-ignore
global.featureFlags = {
[FeatureFlag.DrillToDetail]: true,
Expand All @@ -332,21 +332,71 @@ test('Should show menu items tied to can_view_and_drill permission', () => {
};
props.slice.slice_id = 18;
renderWrapper(props, {
Admin: [['can_view_and_drill', 'Dashboard']],
Admin: [['invalid_permission', 'Dashboard']],
});
expect(screen.queryByText('Drill to detail')).not.toBeInTheDocument();
});

test('Should show "View query"', () => {
const props = {
...createProps(),
supersetCanExplore: false,
};
props.slice.slice_id = 18;
renderWrapper(props, {
Admin: [['can_view_query', 'Dashboard']],
});
expect(screen.getByText('View query')).toBeInTheDocument();
});

test('Should not show "View query"', () => {
const props = {
...createProps(),
supersetCanExplore: false,
};
props.slice.slice_id = 18;
renderWrapper(props, {
Admin: [['invalid_permission', 'Dashboard']],
});
expect(screen.queryByText('View query')).not.toBeInTheDocument();
});

test('Should show "View as table"', () => {
const props = {
...createProps(),
supersetCanExplore: false,
};
props.slice.slice_id = 18;
renderWrapper(props, {
Admin: [['can_view_chart_as_table', 'Dashboard']],
});
expect(screen.getByText('View as table')).toBeInTheDocument();
expect(screen.queryByText('Drill to detail')).not.toBeInTheDocument();
});

test('Should not show the "Edit chart" without proper permissions', () => {
test('Should not show "View as table"', () => {
const props = {
...createProps(),
supersetCanExplore: false,
};
props.slice.slice_id = 18;
renderWrapper(props, {
Admin: [['can_view_and_drill', 'Dashboard']],
Admin: [['invalid_permission', 'Dashboard']],
});
expect(screen.queryByText('View as table')).not.toBeInTheDocument();
});

test('Should not show the "Edit chart" button', () => {
const props = {
...createProps(),
supersetCanExplore: false,
};
props.slice.slice_id = 18;
renderWrapper(props, {
Admin: [
['can_samples', 'Datasource'],
['can_view_query', 'Dashboard'],
['can_view_chart_as_table', 'Dashboard'],
],
});
expect(screen.queryByText('Edit chart')).not.toBeInTheDocument();
});
Original file line number Diff line number Diff line change
Expand Up @@ -273,13 +273,17 @@ const SliceHeaderControls = (props: SliceHeaderControlsPropsWithRouter) => {
getChartMetadataRegistry()
.get(props.slice.viz_type)
?.behaviors?.includes(Behavior.InteractiveChart);
const canViewDrill = useSelector((state: RootState) =>
findPermission('can_view_and_drill', 'Dashboard', state.user?.roles),
);
const canExploreOrView = props.supersetCanExplore || canViewDrill;
const canExplore = props.supersetCanExplore;
const canDatasourceSamples = useSelector((state: RootState) =>
findPermission('can_samples', 'Datasource', state.user?.roles),
);
const canDrillToDetail = canExplore && canDatasourceSamples;
const canViewQuery = useSelector((state: RootState) =>
findPermission('can_view_query', 'Dashboard', state.user?.roles),
);
const canViewTable = useSelector((state: RootState) =>
findPermission('can_view_chart_as_table', 'Dashboard', state.user?.roles),
);
const refreshChart = () => {
if (props.updatedDttm) {
props.forceRefresh(props.slice.slice_id, props.dashboardId);
Expand Down Expand Up @@ -429,7 +433,7 @@ const SliceHeaderControls = (props: SliceHeaderControlsPropsWithRouter) => {
</Menu.Item>
)}

{props.supersetCanExplore && (
{canExplore && (
<Menu.Item key={MENU_KEYS.EXPLORE_CHART}>
<Link to={props.exploreUrl}>
<Tooltip title={getSliceHeaderTooltip(props.slice.slice_name)}>
Expand All @@ -448,7 +452,7 @@ const SliceHeaderControls = (props: SliceHeaderControlsPropsWithRouter) => {
</>
)}

{canExploreOrView && (
{(canExplore || canViewQuery) && (
<Menu.Item key={MENU_KEYS.VIEW_QUERY}>
<ModalTrigger
triggerNode={
Expand All @@ -463,7 +467,7 @@ const SliceHeaderControls = (props: SliceHeaderControlsPropsWithRouter) => {
</Menu.Item>
)}

{canExploreOrView && (
{(canExplore || canViewTable) && (
<Menu.Item key={MENU_KEYS.VIEW_RESULTS}>
<ViewResultsModalTrigger
canExplore={props.supersetCanExplore}
Expand All @@ -485,16 +489,14 @@ const SliceHeaderControls = (props: SliceHeaderControlsPropsWithRouter) => {
</Menu.Item>
)}

{isFeatureEnabled(FeatureFlag.DrillToDetail) &&
canExploreOrView &&
canDatasourceSamples && (
<DrillDetailMenuItems
chartId={slice.slice_id}
formData={props.formData}
/>
)}
{isFeatureEnabled(FeatureFlag.DrillToDetail) && canDrillToDetail && (
<DrillDetailMenuItems
chartId={slice.slice_id}
formData={props.formData}
/>
)}

{(slice.description || canExploreOrView) && <Menu.Divider />}
{(slice.description || canExplore) && <Menu.Divider />}

{supersetCanShare && (
<Menu.SubMenu title={t('Share')}>
Expand Down
Loading

0 comments on commit 5a9ce3f

Please sign in to comment.