From 7cee9e90d6d57585bcdc3fc1768eb75eb4340aee Mon Sep 17 00:00:00 2001 From: Elizabeth Thompson Date: Mon, 4 Oct 2021 19:18:12 -0700 Subject: [PATCH] fix: Revert "fix: RBAC hide right menu (#16902)" (#16968) * Revert "fix: RBAC hide right menu (#16902)" This reverts commit 87baac7650bdb6a8f8f82cf4992567a2c5c73cba. * fix failing test --- .../components/menu/HoverMenu_spec.tsx | 9 +- .../src/components/Menu/Menu.test.tsx | 52 ---- .../src/components/Menu/MenuRight.tsx | 269 ++++++++---------- superset-frontend/src/views/menu.tsx | 7 +- 4 files changed, 129 insertions(+), 208 deletions(-) diff --git a/superset-frontend/spec/javascripts/dashboard/components/menu/HoverMenu_spec.tsx b/superset-frontend/spec/javascripts/dashboard/components/menu/HoverMenu_spec.tsx index 220952bf8dc56..24106f1641db5 100644 --- a/superset-frontend/spec/javascripts/dashboard/components/menu/HoverMenu_spec.tsx +++ b/superset-frontend/spec/javascripts/dashboard/components/menu/HoverMenu_spec.tsx @@ -17,14 +17,13 @@ * under the License. */ import React from 'react'; -import { render } from 'spec/helpers/testing-library'; +import { shallow } from 'enzyme'; import HoverMenu from 'src/dashboard/components/menu/HoverMenu'; describe('HoverMenu', () => { - it('should render a hover menu', () => { - const rendered = render(); - const hoverMenu = rendered.container.querySelector('.hover-menu'); - expect(hoverMenu).toBeVisible(); + it('should render a div.hover-menu', () => { + const wrapper = shallow(); + expect(wrapper.find('.hover-menu')).toExist(); }); }); diff --git a/superset-frontend/src/components/Menu/Menu.test.tsx b/superset-frontend/src/components/Menu/Menu.test.tsx index 8dd8f56b89024..f3a2fd1ff5ad8 100644 --- a/superset-frontend/src/components/Menu/Menu.test.tsx +++ b/superset-frontend/src/components/Menu/Menu.test.tsx @@ -17,32 +17,12 @@ * under the License. */ import React from 'react'; -import * as reactRedux from 'react-redux'; import { render, screen } from 'spec/helpers/testing-library'; import userEvent from '@testing-library/user-event'; import { Menu } from './Menu'; import { dropdownItems } from './MenuRight'; -const user = { - createdOn: '2021-04-27T18:12:38.952304', - email: 'admin', - firstName: 'admin', - isActive: true, - lastName: 'admin', - permissions: {}, - roles: { - Admin: [ - ['can_sqllab', 'Superset'], - ['can_write', 'Dashboard'], - ['can_write', 'Chart'], - ], - }, - userId: 1, - username: 'admin', -}; - const mockedProps = { - user, data: { menu: [ { @@ -156,27 +136,17 @@ const notanonProps = { }, }; -const useSelectorMock = jest.spyOn(reactRedux, 'useSelector'); - -beforeEach(() => { - // setup a DOM element as a render target - useSelectorMock.mockClear(); -}); - test('should render', () => { - useSelectorMock.mockReturnValue({ roles: user.roles }); const { container } = render(); expect(container).toBeInTheDocument(); }); test('should render the navigation', () => { - useSelectorMock.mockReturnValue({ roles: user.roles }); render(); expect(screen.getByRole('navigation')).toBeInTheDocument(); }); test('should render the brand', () => { - useSelectorMock.mockReturnValue({ roles: user.roles }); const { data: { brand: { alt, icon }, @@ -188,7 +158,6 @@ test('should render the brand', () => { }); test('should render all the top navbar menu items', () => { - useSelectorMock.mockReturnValue({ roles: user.roles }); const { data: { menu }, } = mockedProps; @@ -199,7 +168,6 @@ test('should render all the top navbar menu items', () => { }); test('should render the top navbar child menu items', async () => { - useSelectorMock.mockReturnValue({ roles: user.roles }); const { data: { menu }, } = mockedProps; @@ -216,7 +184,6 @@ test('should render the top navbar child menu items', async () => { }); test('should render the dropdown items', async () => { - useSelectorMock.mockReturnValue({ roles: user.roles }); render(); const dropdown = screen.getByTestId('new-dropdown-icon'); userEvent.hover(dropdown); @@ -244,14 +211,12 @@ test('should render the dropdown items', async () => { }); test('should render the Settings', async () => { - useSelectorMock.mockReturnValue({ roles: user.roles }); render(); const settings = await screen.findByText('Settings'); expect(settings).toBeInTheDocument(); }); test('should render the Settings menu item', async () => { - useSelectorMock.mockReturnValue({ roles: user.roles }); render(); userEvent.hover(screen.getByText('Settings')); const label = await screen.findByText('Security'); @@ -259,7 +224,6 @@ test('should render the Settings menu item', async () => { }); test('should render the Settings dropdown child menu items', async () => { - useSelectorMock.mockReturnValue({ roles: user.roles }); const { data: { settings }, } = mockedProps; @@ -270,19 +234,16 @@ test('should render the Settings dropdown child menu items', async () => { }); test('should render the plus menu (+) when user is not anonymous', () => { - useSelectorMock.mockReturnValue({ roles: user.roles }); render(); expect(screen.getByTestId('new-dropdown')).toBeInTheDocument(); }); test('should NOT render the plus menu (+) when user is anonymous', () => { - useSelectorMock.mockReturnValue({ roles: user.roles }); render(); expect(screen.queryByTestId('new-dropdown')).not.toBeInTheDocument(); }); test('should render the user actions when user is not anonymous', async () => { - useSelectorMock.mockReturnValue({ roles: mockedProps.user.roles }); const { data: { navbar_right: { user_info_url, user_logout_url }, @@ -302,13 +263,11 @@ test('should render the user actions when user is not anonymous', async () => { }); test('should NOT render the user actions when user is anonymous', () => { - useSelectorMock.mockReturnValue({ roles: user.roles }); render(); expect(screen.queryByText('User')).not.toBeInTheDocument(); }); test('should render the Profile link when available', async () => { - useSelectorMock.mockReturnValue({ roles: user.roles }); const { data: { navbar_right: { user_profile_url }, @@ -323,7 +282,6 @@ test('should render the Profile link when available', async () => { }); test('should render the About section and version_string, sha or build_number when available', async () => { - useSelectorMock.mockReturnValue({ roles: user.roles }); const { data: { navbar_right: { version_sha, version_string, build_number }, @@ -343,7 +301,6 @@ test('should render the About section and version_string, sha or build_number wh }); test('should render the Documentation link when available', async () => { - useSelectorMock.mockReturnValue({ roles: user.roles }); const { data: { navbar_right: { documentation_url }, @@ -356,7 +313,6 @@ test('should render the Documentation link when available', async () => { }); test('should render the Bug Report link when available', async () => { - useSelectorMock.mockReturnValue({ roles: user.roles }); const { data: { navbar_right: { bug_report_url }, @@ -369,7 +325,6 @@ test('should render the Bug Report link when available', async () => { }); test('should render the Login link when user is anonymous', () => { - useSelectorMock.mockReturnValue({ roles: user.roles }); const { data: { navbar_right: { user_login_url }, @@ -382,13 +337,6 @@ test('should render the Login link when user is anonymous', () => { }); test('should render the Language Picker', () => { - useSelectorMock.mockReturnValue({ roles: user.roles }); render(); expect(screen.getByLabelText('Languages')).toBeInTheDocument(); }); - -test('should hide create button without proper roles', () => { - useSelectorMock.mockReturnValue({ roles: [] }); - render(); - expect(screen.queryByTestId('new-dropdown')).not.toBeInTheDocument(); -}); diff --git a/superset-frontend/src/components/Menu/MenuRight.tsx b/superset-frontend/src/components/Menu/MenuRight.tsx index 348bc466f9b09..1626713f54283 100644 --- a/superset-frontend/src/components/Menu/MenuRight.tsx +++ b/superset-frontend/src/components/Menu/MenuRight.tsx @@ -21,9 +21,6 @@ import { MainNav as Menu } from 'src/common/components'; import { t, styled, css, SupersetTheme } from '@superset-ui/core'; import { Link } from 'react-router-dom'; import Icons from 'src/components/Icons'; -import findPermission from 'src/dashboard/util/findPermission'; -import { useSelector } from 'react-redux'; -import { UserWithPermissionsAndRoles } from 'src/types/bootstrapTypes'; import LanguagePicker from './LanguagePicker'; import { NavBarProps, MenuObjectProps } from './Menu'; @@ -32,22 +29,16 @@ export const dropdownItems = [ label: t('SQL query'), url: '/superset/sqllab?new=true', icon: 'fa-fw fa-search', - perm: 'can_sqllab', - view: 'Superset', }, { label: t('Chart'), url: '/chart/add', icon: 'fa-fw fa-bar-chart', - perm: 'can_write', - view: 'Dashboard', }, { label: t('Dashboard'), url: '/dashboard/new', icon: 'fa-fw fa-dashboard', - perm: 'can_write', - view: 'Chart', }, ]; @@ -92,146 +83,134 @@ const RightMenu = ({ settings, navbarRight, isFrontendRoute, -}: RightMenuProps) => { - const { roles } = useSelector( - state => state.user, - ); - - // if user has any of these roles the dropdown will appear - const canSql = findPermission('can_sqllab', 'Superset', roles); - const canDashboard = findPermission('can_write', 'Dashboard', roles); - const canChart = findPermission('can_write', 'Chart', roles); - const showActionDropdown = canSql || canChart || canDashboard; - return ( - - - {!navbarRight.user_is_anonymous && showActionDropdown && ( - - } - icon={} - > - {dropdownItems.map( - menu => - findPermission(menu.perm, menu.view, roles) && ( - - - {' '} - {menu.label} - +}: RightMenuProps) => ( + + + {!navbarRight.user_is_anonymous && ( + + } + icon={} + > + {dropdownItems.map(menu => ( + + + {' '} + {menu.label} + + + ))} + + )} + }> + {settings.map((section, index) => [ + + {section.childs?.map(child => { + if (typeof child !== 'string') { + return ( + + {isFrontendRoute(child.url) ? ( + {child.label} + ) : ( + {child.label} + )} - ), - )} - - )} - }> - {settings.map((section, index) => [ - - {section.childs?.map(child => { - if (typeof child !== 'string') { - return ( - - {isFrontendRoute(child.url) ? ( - {child.label} - ) : ( - {child.label} - )} - - ); - } - return null; - })} - , - index < settings.length - 1 && , - ])} + ); + } + return null; + })} + , + index < settings.length - 1 && , + ])} - {!navbarRight.user_is_anonymous && [ - , - - {navbarRight.user_profile_url && ( - - {t('Profile')} - + {!navbarRight.user_is_anonymous && [ + , + + {navbarRight.user_profile_url && ( + + {t('Profile')} + + )} + {navbarRight.user_info_url && ( + + {t('Info')} + + )} + + {t('Logout')} + + , + ]} + {(navbarRight.version_string || + navbarRight.version_sha || + navbarRight.build_number) && [ + , + +
+ {navbarRight.show_watermark && ( +
+ {t('Powered by Apache Superset')} +
)} - {navbarRight.user_info_url && ( - - {t('Info')} - + {navbarRight.version_string && ( +
+ Version: {navbarRight.version_string} +
)} - - {t('Logout')} - - , - ]} - {(navbarRight.version_string || navbarRight.version_sha) && [ - , - -
- {navbarRight.show_watermark && ( -
- {t('Powered by Apache Superset')} -
- )} - {navbarRight.version_string && ( -
- Version: {navbarRight.version_string} -
- )} - {navbarRight.version_sha && ( -
- SHA: {navbarRight.version_sha} -
- )} - {navbarRight.build_number && ( -
- Build: {navbarRight.build_number} -
- )} -
- , - ]} - - {navbarRight.show_language_picker && ( - - )} -
- {navbarRight.documentation_url && ( - - -   - - )} - {navbarRight.bug_report_url && ( - - - - )} - {navbarRight.user_is_anonymous && ( - - - {t('Login')} - + {navbarRight.version_sha && ( +
+ SHA: {navbarRight.version_sha} +
+ )} + {navbarRight.build_number && ( +
+ Build: {navbarRight.build_number} +
+ )} + + , + ]} + + {navbarRight.show_language_picker && ( + )} - - ); -}; + + {navbarRight.documentation_url && ( + + +   + + )} + {navbarRight.bug_report_url && ( + + + + )} + {navbarRight.user_is_anonymous && ( + + + {t('Login')} + + )} + +); export default RightMenu; diff --git a/superset-frontend/src/views/menu.tsx b/superset-frontend/src/views/menu.tsx index 68b72d70ec68d..35f62526d5f94 100644 --- a/superset-frontend/src/views/menu.tsx +++ b/superset-frontend/src/views/menu.tsx @@ -27,9 +27,6 @@ import { ThemeProvider } from '@superset-ui/core'; import Menu from 'src/components/Menu/Menu'; import { theme } from 'src/preamble'; -import { Provider } from 'react-redux'; -import { store } from './store'; - const container = document.getElementById('app'); const bootstrapJson = container?.getAttribute('data-bootstrap') ?? '{}'; const bootstrap = JSON.parse(bootstrapJson); @@ -43,9 +40,7 @@ const app = ( // @ts-ignore: emotion types defs are incompatible between core and cache - - - + );