Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: Add granular permissions for actions in Dashboard #27029

Merged
merged 19 commits into from
Feb 9, 2024
Merged
Show file tree
Hide file tree
Changes from 16 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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;
kgabryje marked this conversation as resolved.
Show resolved Hide resolved
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
Loading