From 896b5fbb16c86ff098281fe23d304c39ecaff342 Mon Sep 17 00:00:00 2001 From: amitmiran137 Date: Sun, 11 Apr 2021 15:01:45 +0300 Subject: [PATCH 1/6] feat: share chart - can_share --- .../components/SliceHeader/index.tsx | 3 ++ .../components/SliceHeaderControls/index.jsx | 29 ++++++++++--------- .../components/gridComponents/Chart.jsx | 3 ++ .../src/dashboard/containers/Chart.jsx | 1 + .../src/dashboard/reducers/getInitialState.js | 1 + superset/security/manager.py | 1 + superset/views/core.py | 2 ++ tests/security_tests.py | 3 ++ 8 files changed, 30 insertions(+), 13 deletions(-) diff --git a/superset-frontend/src/dashboard/components/SliceHeader/index.tsx b/superset-frontend/src/dashboard/components/SliceHeader/index.tsx index 875852c578f07..72bd0897f4064 100644 --- a/superset-frontend/src/dashboard/components/SliceHeader/index.tsx +++ b/superset-frontend/src/dashboard/components/SliceHeader/index.tsx @@ -46,6 +46,7 @@ type SliceHeaderProps = { annotationError?: object; sliceName?: string; supersetCanExplore?: boolean; + supersetCanShare?: boolean; supersetCanCSV?: boolean; sliceCanEdit?: boolean; componentId: string; @@ -83,6 +84,7 @@ const SliceHeader: FC = ({ isExpanded = [], sliceName = '', supersetCanExplore = false, + supersetCanShare = false, supersetCanCSV = false, sliceCanEdit = false, slice, @@ -172,6 +174,7 @@ const SliceHeader: FC = ({ exploreChart={exploreChart} exportCSV={exportCSV} supersetCanExplore={supersetCanExplore} + supersetCanShare={supersetCanShare} supersetCanCSV={supersetCanCSV} sliceCanEdit={sliceCanEdit} componentId={componentId} diff --git a/superset-frontend/src/dashboard/components/SliceHeaderControls/index.jsx b/superset-frontend/src/dashboard/components/SliceHeaderControls/index.jsx index edef021377118..22199916d8329 100644 --- a/superset-frontend/src/dashboard/components/SliceHeaderControls/index.jsx +++ b/superset-frontend/src/dashboard/components/SliceHeaderControls/index.jsx @@ -43,6 +43,7 @@ const propTypes = { isExpanded: PropTypes.bool, updatedDttm: PropTypes.number, supersetCanExplore: PropTypes.bool, + supersetCanShare: PropTypes.bool, supersetCanCSV: PropTypes.bool, sliceCanEdit: PropTypes.bool, toggleExpandSlice: PropTypes.func, @@ -61,6 +62,7 @@ const defaultProps = { isCached: [], isExpanded: false, supersetCanExplore: false, + supersetCanShare: false, supersetCanCSV: false, sliceCanEdit: false, }; @@ -72,7 +74,6 @@ const MENU_KEYS = { EXPLORE_CHART: 'explore_chart', EXPORT_CSV: 'export_csv', RESIZE_LABEL: 'resize_label', - SHARE_CHART: 'share_chart', DOWNLOAD_AS_IMAGE: 'download_as_image', }; @@ -253,18 +254,20 @@ class SliceHeaderControls extends React.PureComponent { )} - + {this.props.supersetCanShare && ( + + )} {resizeLabel} diff --git a/superset-frontend/src/dashboard/components/gridComponents/Chart.jsx b/superset-frontend/src/dashboard/components/gridComponents/Chart.jsx index 18cec534189dd..bfe775eb8e89d 100644 --- a/superset-frontend/src/dashboard/components/gridComponents/Chart.jsx +++ b/superset-frontend/src/dashboard/components/gridComponents/Chart.jsx @@ -65,6 +65,7 @@ const propTypes = { isExpanded: PropTypes.bool.isRequired, isCached: PropTypes.bool, supersetCanExplore: PropTypes.bool.isRequired, + supersetCanShare: PropTypes.bool.isRequired, supersetCanCSV: PropTypes.bool.isRequired, sliceCanEdit: PropTypes.bool.isRequired, addSuccessToast: PropTypes.func.isRequired, @@ -256,6 +257,7 @@ export default class Chart extends React.Component { toggleExpandSlice, timeout, supersetCanExplore, + supersetCanShare, supersetCanCSV, sliceCanEdit, addSuccessToast, @@ -311,6 +313,7 @@ export default class Chart extends React.Component { updateSliceName={updateSliceName} sliceName={sliceName} supersetCanExplore={supersetCanExplore} + supersetCanShare={supersetCanShare} supersetCanCSV={supersetCanCSV} sliceCanEdit={sliceCanEdit} componentId={componentId} diff --git a/superset-frontend/src/dashboard/containers/Chart.jsx b/superset-frontend/src/dashboard/containers/Chart.jsx index 94f88d3a9ef7f..31c0e3255c755 100644 --- a/superset-frontend/src/dashboard/containers/Chart.jsx +++ b/superset-frontend/src/dashboard/containers/Chart.jsx @@ -84,6 +84,7 @@ function mapStateToProps( editMode: dashboardState.editMode, isExpanded: !!dashboardState.expandedSlices[id], supersetCanExplore: !!dashboardInfo.superset_can_explore, + supersetCanShare: !!dashboardInfo.superset_can_share, supersetCanCSV: !!dashboardInfo.superset_can_csv, sliceCanEdit: !!dashboardInfo.slice_can_edit, ownCurrentState: dataMask.ownFilters?.[id]?.currentState, diff --git a/superset-frontend/src/dashboard/reducers/getInitialState.js b/superset-frontend/src/dashboard/reducers/getInitialState.js index 19ea54e789f1f..ecbb4fa9b347b 100644 --- a/superset-frontend/src/dashboard/reducers/getInitialState.js +++ b/superset-frontend/src/dashboard/reducers/getInitialState.js @@ -276,6 +276,7 @@ export default function getInitialState(bootstrapData) { dash_edit_perm: dashboard.dash_edit_perm, dash_save_perm: dashboard.dash_save_perm, superset_can_explore: dashboard.superset_can_explore, + superset_can_share: dashboard.superset_can_share, superset_can_csv: dashboard.superset_can_csv, slice_can_edit: dashboard.slice_can_edit, common: { diff --git a/superset/security/manager.py b/superset/security/manager.py index 1c4419cfc5241..42f70eafdbfa6 100644 --- a/superset/security/manager.py +++ b/superset/security/manager.py @@ -556,6 +556,7 @@ def create_custom_permissions(self) -> None: self.add_permission_view_menu("all_datasource_access", "all_datasource_access") self.add_permission_view_menu("all_database_access", "all_database_access") self.add_permission_view_menu("all_query_access", "all_query_access") + self.add_permission_view_menu("can_share", "Superset") def create_missing_perms(self) -> None: """ diff --git a/superset/views/core.py b/superset/views/core.py index 0093f50421f3c..13dfcc5df1b17 100755 --- a/superset/views/core.py +++ b/superset/views/core.py @@ -1838,6 +1838,7 @@ def dashboard( # pylint: disable=too-many-locals ) and security_manager.can_access("can_save_dash", "Superset") dash_save_perm = security_manager.can_access("can_save_dash", "Superset") superset_can_explore = security_manager.can_access("can_explore", "Superset") + superset_can_share = security_manager.can_access("can_share", "Superset") superset_can_csv = security_manager.can_access("can_csv", "Superset") slice_can_edit = security_manager.can_access("can_edit", "SliceModelView") standalone_mode = ReservedUrlParameters.is_standalone_mode() @@ -1876,6 +1877,7 @@ def dashboard( # pylint: disable=too-many-locals "dash_save_perm": dash_save_perm, "dash_edit_perm": dash_edit_perm, "superset_can_explore": superset_can_explore, + "superset_can_share": superset_can_share, "superset_can_csv": superset_can_csv, "slice_can_edit": slice_can_edit, }, diff --git a/tests/security_tests.py b/tests/security_tests.py index 0648104101cb2..0ed8ddede2701 100644 --- a/tests/security_tests.py +++ b/tests/security_tests.py @@ -670,12 +670,14 @@ def assert_can_gamma(self, perm_set): self.assertIn(("can_csv", "Superset"), perm_set) self.assertIn(("can_dashboard", "Superset"), perm_set) self.assertIn(("can_explore", "Superset"), perm_set) + self.assertIn(("can_share", "Superset"), perm_set) self.assertIn(("can_explore_json", "Superset"), perm_set) self.assertIn(("can_fave_dashboards", "Superset"), perm_set) self.assertIn(("can_fave_slices", "Superset"), perm_set) self.assertIn(("can_save_dash", "Superset"), perm_set) self.assertIn(("can_slice", "Superset"), perm_set) self.assertIn(("can_explore", "Superset"), perm_set) + self.assertIn(("can_share", "Superset"), perm_set) self.assertIn(("can_explore_json", "Superset"), perm_set) self.assertIn(("can_userinfo", "UserDBModelView"), perm_set) self.assert_can_menu("Databases", perm_set) @@ -868,6 +870,7 @@ def test_gamma_permissions(self): self.assertIn(("can_csv", "Superset"), gamma_perm_set) self.assertIn(("can_dashboard", "Superset"), gamma_perm_set) self.assertIn(("can_explore", "Superset"), gamma_perm_set) + self.assertIn(("can_share", "Superset"), gamma_perm_set) self.assertIn(("can_explore_json", "Superset"), gamma_perm_set) self.assertIn(("can_fave_dashboards", "Superset"), gamma_perm_set) self.assertIn(("can_fave_slices", "Superset"), gamma_perm_set) From 1c6134ff4a14d4aa879daea6b018aaa335ac5503 Mon Sep 17 00:00:00 2001 From: amitmiran137 Date: Sun, 11 Apr 2021 20:19:40 +0300 Subject: [PATCH 2/6] feat: share chart - can_share_chart share dashboard can_share_dashboard --- .../src/dashboard/components/Header.jsx | 2 ++ .../components/HeaderActionsDropdown.jsx | 22 +++++++++++-------- .../src/dashboard/reducers/getInitialState.js | 1 + superset/security/manager.py | 3 ++- superset/views/core.py | 6 +++-- tests/security_tests.py | 8 +++---- 6 files changed, 26 insertions(+), 16 deletions(-) diff --git a/superset-frontend/src/dashboard/components/Header.jsx b/superset-frontend/src/dashboard/components/Header.jsx index 8ad6c57e69ad1..3bc68b0823db7 100644 --- a/superset-frontend/src/dashboard/components/Header.jsx +++ b/superset-frontend/src/dashboard/components/Header.jsx @@ -376,6 +376,7 @@ class Header extends React.PureComponent { } = this.props; const userCanEdit = dashboardInfo.dash_edit_perm; + const userCanShare = dashboardInfo.dash_share_perm; const userCanSaveAs = dashboardInfo.dash_save_perm; const refreshLimit = dashboardInfo.common.conf.SUPERSET_DASHBOARD_PERIODICAL_REFRESH_LIMIT; @@ -543,6 +544,7 @@ class Header extends React.PureComponent { editMode={editMode} hasUnsavedChanges={hasUnsavedChanges} userCanEdit={userCanEdit} + userCanShare={userCanShare} userCanSave={userCanSaveAs} isLoading={isLoading} showPropertiesModal={this.showPropertiesModal} diff --git a/superset-frontend/src/dashboard/components/HeaderActionsDropdown.jsx b/superset-frontend/src/dashboard/components/HeaderActionsDropdown.jsx index 5d27f1f7f5917..ba255fb3224aa 100644 --- a/superset-frontend/src/dashboard/components/HeaderActionsDropdown.jsx +++ b/superset-frontend/src/dashboard/components/HeaderActionsDropdown.jsx @@ -54,6 +54,7 @@ const propTypes = { startPeriodicRender: PropTypes.func.isRequired, editMode: PropTypes.bool.isRequired, userCanEdit: PropTypes.bool.isRequired, + userCanShare: PropTypes.bool.isRequired, userCanSave: PropTypes.bool.isRequired, isLoading: PropTypes.bool.isRequired, layout: PropTypes.object.isRequired, @@ -192,6 +193,7 @@ class HeaderActionsDropdown extends React.PureComponent { expandedSlices, onSave, userCanEdit, + userCanShare, userCanSave, isLoading, refreshLimit, @@ -241,15 +243,17 @@ class HeaderActionsDropdown extends React.PureComponent { /> )} - + {userCanShare && ( + + )} None: self.add_permission_view_menu("all_datasource_access", "all_datasource_access") self.add_permission_view_menu("all_database_access", "all_database_access") self.add_permission_view_menu("all_query_access", "all_query_access") - self.add_permission_view_menu("can_share", "Superset") + self.add_permission_view_menu("can_share_dashboard", "Superset") + self.add_permission_view_menu("can_share_chart", "Superset") def create_missing_perms(self) -> None: """ diff --git a/superset/views/core.py b/superset/views/core.py index 13dfcc5df1b17..5f6d093241f9f 100755 --- a/superset/views/core.py +++ b/superset/views/core.py @@ -1838,7 +1838,8 @@ def dashboard( # pylint: disable=too-many-locals ) and security_manager.can_access("can_save_dash", "Superset") dash_save_perm = security_manager.can_access("can_save_dash", "Superset") superset_can_explore = security_manager.can_access("can_explore", "Superset") - superset_can_share = security_manager.can_access("can_share", "Superset") + superset_can_share_chart = security_manager.can_access("can_share_chart", "Superset") + superset_can_share_dashboard = security_manager.can_access("can_share_dashboard", "Superset") superset_can_csv = security_manager.can_access("can_csv", "Superset") slice_can_edit = security_manager.can_access("can_edit", "SliceModelView") standalone_mode = ReservedUrlParameters.is_standalone_mode() @@ -1875,9 +1876,10 @@ def dashboard( # pylint: disable=too-many-locals **data["dashboard"], "standalone_mode": standalone_mode, "dash_save_perm": dash_save_perm, + "dash_share_perm": superset_can_share_dashboard, "dash_edit_perm": dash_edit_perm, "superset_can_explore": superset_can_explore, - "superset_can_share": superset_can_share, + "superset_can_share": superset_can_share_chart, "superset_can_csv": superset_can_csv, "slice_can_edit": slice_can_edit, }, diff --git a/tests/security_tests.py b/tests/security_tests.py index 0ed8ddede2701..f7e55174db924 100644 --- a/tests/security_tests.py +++ b/tests/security_tests.py @@ -670,14 +670,13 @@ def assert_can_gamma(self, perm_set): self.assertIn(("can_csv", "Superset"), perm_set) self.assertIn(("can_dashboard", "Superset"), perm_set) self.assertIn(("can_explore", "Superset"), perm_set) - self.assertIn(("can_share", "Superset"), perm_set) + self.assertIn(("can_share_chart", "Superset"), perm_set) + self.assertIn(("can_share_dashboard", "Superset"), perm_set) self.assertIn(("can_explore_json", "Superset"), perm_set) self.assertIn(("can_fave_dashboards", "Superset"), perm_set) self.assertIn(("can_fave_slices", "Superset"), perm_set) self.assertIn(("can_save_dash", "Superset"), perm_set) self.assertIn(("can_slice", "Superset"), perm_set) - self.assertIn(("can_explore", "Superset"), perm_set) - self.assertIn(("can_share", "Superset"), perm_set) self.assertIn(("can_explore_json", "Superset"), perm_set) self.assertIn(("can_userinfo", "UserDBModelView"), perm_set) self.assert_can_menu("Databases", perm_set) @@ -870,7 +869,8 @@ def test_gamma_permissions(self): self.assertIn(("can_csv", "Superset"), gamma_perm_set) self.assertIn(("can_dashboard", "Superset"), gamma_perm_set) self.assertIn(("can_explore", "Superset"), gamma_perm_set) - self.assertIn(("can_share", "Superset"), gamma_perm_set) + self.assertIn(("can_share_chart", "Superset"), gamma_perm_set) + self.assertIn(("can_share_dashboard", "Superset"), gamma_perm_set) self.assertIn(("can_explore_json", "Superset"), gamma_perm_set) self.assertIn(("can_fave_dashboards", "Superset"), gamma_perm_set) self.assertIn(("can_fave_slices", "Superset"), gamma_perm_set) From 514efe3ade8acf54bb0c633cbe7abefe71270fba Mon Sep 17 00:00:00 2001 From: amitmiran137 Date: Sun, 11 Apr 2021 20:36:43 +0300 Subject: [PATCH 3/6] fix: pre-commit --- superset/views/core.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/superset/views/core.py b/superset/views/core.py index 5f6d093241f9f..931c415c32627 100755 --- a/superset/views/core.py +++ b/superset/views/core.py @@ -1838,8 +1838,12 @@ def dashboard( # pylint: disable=too-many-locals ) and security_manager.can_access("can_save_dash", "Superset") dash_save_perm = security_manager.can_access("can_save_dash", "Superset") superset_can_explore = security_manager.can_access("can_explore", "Superset") - superset_can_share_chart = security_manager.can_access("can_share_chart", "Superset") - superset_can_share_dashboard = security_manager.can_access("can_share_dashboard", "Superset") + superset_can_share_chart = security_manager.can_access( + "can_share_chart", "Superset" + ) + superset_can_share_dashboard = security_manager.can_access( + "can_share_dashboard", "Superset" + ) superset_can_csv = security_manager.can_access("can_csv", "Superset") slice_can_edit = security_manager.can_access("can_edit", "SliceModelView") standalone_mode = ReservedUrlParameters.is_standalone_mode() From f3f2c4e16d78394640f354a0795e2a1ab3be371b Mon Sep 17 00:00:00 2001 From: amitmiran137 Date: Sun, 11 Apr 2021 21:01:13 +0300 Subject: [PATCH 4/6] fix: userCanShare tests --- .../components/HeaderActionsDropdown_spec.jsx | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/superset-frontend/spec/javascripts/dashboard/components/HeaderActionsDropdown_spec.jsx b/superset-frontend/spec/javascripts/dashboard/components/HeaderActionsDropdown_spec.jsx index ea3b65ca28eb8..fa60986ceaa64 100644 --- a/superset-frontend/spec/javascripts/dashboard/components/HeaderActionsDropdown_spec.jsx +++ b/superset-frontend/spec/javascripts/dashboard/components/HeaderActionsDropdown_spec.jsx @@ -67,7 +67,7 @@ describe('HeaderActionsDropdown', () => { } describe('readonly-user', () => { - const overrideProps = { userCanSave: false }; + const overrideProps = { userCanSave: false, userCanShare: false }; it('should render the DropdownButton', () => { const { wrapper } = setup(overrideProps); @@ -89,9 +89,9 @@ describe('HeaderActionsDropdown', () => { expect(menu.find(RefreshIntervalModal)).toExist(); }); - it('should render the ShareMenuItems', () => { + it('should not render the ShareMenuItems', () => { const { menu } = setup(overrideProps); - expect(menu.find(ShareMenuItems)).toExist(); + expect(menu.find(ShareMenuItems)).not.toExist(); }); it('should not render the CssEditor', () => { @@ -101,7 +101,7 @@ describe('HeaderActionsDropdown', () => { }); describe('write-user', () => { - const overrideProps = { userCanSave: true }; + const overrideProps = { userCanSave: true, userCanShare: true }; it('should render the DropdownButton', () => { const { wrapper } = setup(overrideProps); @@ -135,7 +135,11 @@ describe('HeaderActionsDropdown', () => { }); describe('write-user-with-edit-mode', () => { - const overrideProps = { userCanSave: true, editMode: true }; + const overrideProps = { + userCanSave: true, + editMode: true, + userCanShare: true, + }; it('should render the DropdownButton', () => { const { wrapper } = setup(overrideProps); From dc98bf64dc09b8785af5dece520482db86e4e23d Mon Sep 17 00:00:00 2001 From: amitmiran137 Date: Tue, 13 Apr 2021 08:29:46 +0300 Subject: [PATCH 5/6] fix: after hugh CR --- .../src/dashboard/components/SliceHeaderControls/index.jsx | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/superset-frontend/src/dashboard/components/SliceHeaderControls/index.jsx b/superset-frontend/src/dashboard/components/SliceHeaderControls/index.jsx index 22199916d8329..650a590ac15b0 100644 --- a/superset-frontend/src/dashboard/components/SliceHeaderControls/index.jsx +++ b/superset-frontend/src/dashboard/components/SliceHeaderControls/index.jsx @@ -189,6 +189,7 @@ class SliceHeaderControls extends React.PureComponent { addSuccessToast, addDangerToast, isFullSize, + supersetCanShare, } = this.props; const crossFilterItems = getChartMetadataRegistry().items; const isCrossFilter = Object.entries(crossFilterItems) @@ -254,7 +255,7 @@ class SliceHeaderControls extends React.PureComponent { )} - {this.props.supersetCanShare && ( + {supersetCanShare && ( Date: Tue, 13 Apr 2021 09:38:46 +0300 Subject: [PATCH 6/6] fix: adjust after spa refactor --- superset-frontend/src/dashboard/actions/hydrate.js | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/superset-frontend/src/dashboard/actions/hydrate.js b/superset-frontend/src/dashboard/actions/hydrate.js index 7cdaa2396d3ef..8065bf625fe3e 100644 --- a/superset-frontend/src/dashboard/actions/hydrate.js +++ b/superset-frontend/src/dashboard/actions/hydrate.js @@ -313,7 +313,17 @@ export const hydrateDashboard = (dashboardData, chartData, datasourcesData) => ( userId: String(user.userId), // legacy, please use state.user instead dash_edit_perm: getPermissions('can_write', 'Dashboard', roles), dash_save_perm: getPermissions('can_save_dash', 'Superset', roles), + dash_share_perm: getPermissions( + 'can_share_dashboard', + 'Superset', + roles, + ), superset_can_explore: getPermissions('can_explore', 'Superset', roles), + superset_can_share: getPermissions( + 'can_share_chart', + 'Superset', + roles, + ), superset_can_csv: getPermissions('can_csv', 'Superset', roles), slice_can_edit: getPermissions('can_slice', 'Superset', roles), common: {