This is an automated email from the ASF dual-hosted git repository.
elizabeth pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/superset.git
The following commit(s) were added to refs/heads/master by this push:
new 5866d5e fix: Revert "fix: RBAC hide right menu (#16902)" (#16968)
5866d5e is described below
commit 5866d5ebb064648b6ac23c498cc252207f62dfd1
Author: Elizabeth Thompson <[email protected]>
AuthorDate: Mon Oct 4 19:18:12 2021 -0700
fix: Revert "fix: RBAC hide right menu (#16902)" (#16968)
* Revert "fix: RBAC hide right menu (#16902)"
This reverts commit 87baac7650bdb6a8f8f82cf4992567a2c5c73cba.
* fix failing test
---
.../dashboard/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 220952b..24106f1 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(<HoverMenu />);
- const hoverMenu = rendered.container.querySelector('.hover-menu');
- expect(hoverMenu).toBeVisible();
+ it('should render a div.hover-menu', () => {
+ const wrapper = shallow(<HoverMenu />);
+ 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 8dd8f56..f3a2fd1 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(<Menu {...mockedProps} />);
expect(container).toBeInTheDocument();
});
test('should render the navigation', () => {
- useSelectorMock.mockReturnValue({ roles: user.roles });
render(<Menu {...mockedProps} />);
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(<Menu {...notanonProps} />);
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(<Menu {...mockedProps} />);
const settings = await screen.findByText('Settings');
expect(settings).toBeInTheDocument();
});
test('should render the Settings menu item', async () => {
- useSelectorMock.mockReturnValue({ roles: user.roles });
render(<Menu {...mockedProps} />);
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(<Menu {...notanonProps} />);
expect(screen.getByTestId('new-dropdown')).toBeInTheDocument();
});
test('should NOT render the plus menu (+) when user is anonymous', () => {
- useSelectorMock.mockReturnValue({ roles: user.roles });
render(<Menu {...mockedProps} />);
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(<Menu {...mockedProps} />);
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(<Menu {...mockedProps} />);
expect(screen.getByLabelText('Languages')).toBeInTheDocument();
});
-
-test('should hide create button without proper roles', () => {
- useSelectorMock.mockReturnValue({ roles: [] });
- render(<Menu {...notanonProps} />);
- 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 348bc46..1626713 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<any, UserWithPermissionsAndRoles>(
- 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 (
- <StyledDiv align={align}>
- <Menu mode="horizontal">
- {!navbarRight.user_is_anonymous && showActionDropdown && (
- <SubMenu
- data-test="new-dropdown"
- title={
- <StyledI data-test="new-dropdown-icon" className="fa fa-plus" />
- }
- icon={<Icons.TriangleDown />}
- >
- {dropdownItems.map(
- menu =>
- findPermission(menu.perm, menu.view, roles) && (
- <Menu.Item key={menu.label}>
- <a href={menu.url}>
- <i
- data-test={`menu-item-${menu.label}`}
- className={`fa ${menu.icon}`}
- />{' '}
- {menu.label}
- </a>
+}: RightMenuProps) => (
+ <StyledDiv align={align}>
+ <Menu mode="horizontal">
+ {!navbarRight.user_is_anonymous && (
+ <SubMenu
+ data-test="new-dropdown"
+ title={
+ <StyledI data-test="new-dropdown-icon" className="fa fa-plus" />
+ }
+ icon={<Icons.TriangleDown />}
+ >
+ {dropdownItems.map(menu => (
+ <Menu.Item key={menu.label}>
+ <a href={menu.url}>
+ <i
+ data-test={`menu-item-${menu.label}`}
+ className={`fa ${menu.icon}`}
+ />{' '}
+ {menu.label}
+ </a>
+ </Menu.Item>
+ ))}
+ </SubMenu>
+ )}
+ <SubMenu title="Settings" icon={<Icons.TriangleDown iconSize="xl" />}>
+ {settings.map((section, index) => [
+ <Menu.ItemGroup key={`${section.label}`} title={section.label}>
+ {section.childs?.map(child => {
+ if (typeof child !== 'string') {
+ return (
+ <Menu.Item key={`${child.label}`}>
+ {isFrontendRoute(child.url) ? (
+ <Link to={child.url || ''}>{child.label}</Link>
+ ) : (
+ <a href={child.url}>{child.label}</a>
+ )}
</Menu.Item>
- ),
- )}
- </SubMenu>
- )}
- <SubMenu title="Settings" icon={<Icons.TriangleDown iconSize="xl" />}>
- {settings.map((section, index) => [
- <Menu.ItemGroup key={`${section.label}`} title={section.label}>
- {section.childs?.map(child => {
- if (typeof child !== 'string') {
- return (
- <Menu.Item key={`${child.label}`}>
- {isFrontendRoute(child.url) ? (
- <Link to={child.url || ''}>{child.label}</Link>
- ) : (
- <a href={child.url}>{child.label}</a>
- )}
- </Menu.Item>
- );
- }
- return null;
- })}
- </Menu.ItemGroup>,
- index < settings.length - 1 && <Menu.Divider />,
- ])}
+ );
+ }
+ return null;
+ })}
+ </Menu.ItemGroup>,
+ index < settings.length - 1 && <Menu.Divider />,
+ ])}
- {!navbarRight.user_is_anonymous && [
- <Menu.Divider key="user-divider" />,
- <Menu.ItemGroup key="user-section" title={t('User')}>
- {navbarRight.user_profile_url && (
- <Menu.Item key="profile">
- <a href={navbarRight.user_profile_url}>{t('Profile')}</a>
- </Menu.Item>
+ {!navbarRight.user_is_anonymous && [
+ <Menu.Divider key="user-divider" />,
+ <Menu.ItemGroup key="user-section" title={t('User')}>
+ {navbarRight.user_profile_url && (
+ <Menu.Item key="profile">
+ <a href={navbarRight.user_profile_url}>{t('Profile')}</a>
+ </Menu.Item>
+ )}
+ {navbarRight.user_info_url && (
+ <Menu.Item key="info">
+ <a href={navbarRight.user_info_url}>{t('Info')}</a>
+ </Menu.Item>
+ )}
+ <Menu.Item key="logout">
+ <a href={navbarRight.user_logout_url}>{t('Logout')}</a>
+ </Menu.Item>
+ </Menu.ItemGroup>,
+ ]}
+ {(navbarRight.version_string ||
+ navbarRight.version_sha ||
+ navbarRight.build_number) && [
+ <Menu.Divider key="version-info-divider" />,
+ <Menu.ItemGroup key="about-section" title={t('About')}>
+ <div className="about-section">
+ {navbarRight.show_watermark && (
+ <div css={versionInfoStyles}>
+ {t('Powered by Apache Superset')}
+ </div>
)}
- {navbarRight.user_info_url && (
- <Menu.Item key="info">
- <a href={navbarRight.user_info_url}>{t('Info')}</a>
- </Menu.Item>
+ {navbarRight.version_string && (
+ <div css={versionInfoStyles}>
+ Version: {navbarRight.version_string}
+ </div>
)}
- <Menu.Item key="logout">
- <a href={navbarRight.user_logout_url}>{t('Logout')}</a>
- </Menu.Item>
- </Menu.ItemGroup>,
- ]}
- {(navbarRight.version_string || navbarRight.version_sha) && [
- <Menu.Divider key="version-info-divider" />,
- <Menu.ItemGroup key="about-section" title={t('About')}>
- <div className="about-section">
- {navbarRight.show_watermark && (
- <div css={versionInfoStyles}>
- {t('Powered by Apache Superset')}
- </div>
- )}
- {navbarRight.version_string && (
- <div css={versionInfoStyles}>
- Version: {navbarRight.version_string}
- </div>
- )}
- {navbarRight.version_sha && (
- <div css={versionInfoStyles}>
- SHA: {navbarRight.version_sha}
- </div>
- )}
- {navbarRight.build_number && (
- <div css={versionInfoStyles}>
- Build: {navbarRight.build_number}
- </div>
- )}
- </div>
- </Menu.ItemGroup>,
- ]}
- </SubMenu>
- {navbarRight.show_language_picker && (
- <LanguagePicker
- locale={navbarRight.locale}
- languages={navbarRight.languages}
- />
- )}
- </Menu>
- {navbarRight.documentation_url && (
- <StyledAnchor
- href={navbarRight.documentation_url}
- target="_blank"
- rel="noreferrer"
- title={t('Documentation')}
- >
- <i className="fa fa-question" />
-
- </StyledAnchor>
- )}
- {navbarRight.bug_report_url && (
- <StyledAnchor
- href={navbarRight.bug_report_url}
- target="_blank"
- rel="noreferrer"
- title={t('Report a bug')}
- >
- <i className="fa fa-bug" />
- </StyledAnchor>
- )}
- {navbarRight.user_is_anonymous && (
- <StyledAnchor href={navbarRight.user_login_url}>
- <i className="fa fa-fw fa-sign-in" />
- {t('Login')}
- </StyledAnchor>
+ {navbarRight.version_sha && (
+ <div css={versionInfoStyles}>
+ SHA: {navbarRight.version_sha}
+ </div>
+ )}
+ {navbarRight.build_number && (
+ <div css={versionInfoStyles}>
+ Build: {navbarRight.build_number}
+ </div>
+ )}
+ </div>
+ </Menu.ItemGroup>,
+ ]}
+ </SubMenu>
+ {navbarRight.show_language_picker && (
+ <LanguagePicker
+ locale={navbarRight.locale}
+ languages={navbarRight.languages}
+ />
)}
- </StyledDiv>
- );
-};
+ </Menu>
+ {navbarRight.documentation_url && (
+ <StyledAnchor
+ href={navbarRight.documentation_url}
+ target="_blank"
+ rel="noreferrer"
+ title={t('Documentation')}
+ >
+ <i className="fa fa-question" />
+
+ </StyledAnchor>
+ )}
+ {navbarRight.bug_report_url && (
+ <StyledAnchor
+ href={navbarRight.bug_report_url}
+ target="_blank"
+ rel="noreferrer"
+ title={t('Report a bug')}
+ >
+ <i className="fa fa-bug" />
+ </StyledAnchor>
+ )}
+ {navbarRight.user_is_anonymous && (
+ <StyledAnchor href={navbarRight.user_login_url}>
+ <i className="fa fa-fw fa-sign-in" />
+ {t('Login')}
+ </StyledAnchor>
+ )}
+ </StyledDiv>
+);
export default RightMenu;
diff --git a/superset-frontend/src/views/menu.tsx
b/superset-frontend/src/views/menu.tsx
index 68b72d7..35f6252 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
<CacheProvider value={emotionCache}>
<ThemeProvider theme={theme}>
- <Provider store={store}>
- <Menu data={menu} />
- </Provider>
+ <Menu data={menu} />
</ThemeProvider>
</CacheProvider>
);
