fix: Revert "fix: RBAC hide right menu (#16902)" (#16968)

* Revert "fix: RBAC hide right menu (#16902)"

This reverts commit 87baac7650.

* fix failing test

(cherry picked from commit 5866d5ebb0)

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', () => {
+  it('should render a div.hover-menu', () => {
-    const rendered = render(<HoverMenu />);
+    const wrapper = shallow(<HoverMenu />);
-    const hoverMenu = rendered.container.querySelector('.hover-menu');
+    expect(wrapper.find('.hover-menu')).toExist();
-    expect(hoverMenu).toBeVisible();
   });
 });

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();
-});

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) => {
+}: RightMenuProps) => (
-  const { roles } = useSelector<any, UserWithPermissionsAndRoles>(
+  <StyledDiv align={align}>
-    state => state.user,
+    <Menu mode="horizontal">
-  );
+      {!navbarRight.user_is_anonymous && (
-
+        <SubMenu
-  // if user has any of these roles the dropdown will appear
+          data-test="new-dropdown"
-  const canSql = findPermission('can_sqllab', 'Superset', roles);
+          title={
-  const canDashboard = findPermission('can_write', 'Dashboard', roles);
+            <StyledI data-test="new-dropdown-icon" className="fa fa-plus" />
-  const canChart = findPermission('can_write', 'Chart', roles);
+          }
-  const showActionDropdown = canSql || canChart || canDashboard;
+          icon={<Icons.TriangleDown />}
-  return (
+        >
-    <StyledDiv align={align}>
+          {dropdownItems.map(menu => (
-      <Menu mode="horizontal">
+            <Menu.Item key={menu.label}>
-        {!navbarRight.user_is_anonymous && showActionDropdown && (
+              <a href={menu.url}>
-          <SubMenu
+                <i
-            data-test="new-dropdown"
+                  data-test={`menu-item-${menu.label}`}
-            title={
+                  className={`fa ${menu.icon}`}
-              <StyledI data-test="new-dropdown-icon" className="fa fa-plus" />
+                />{' '}
-            }
+                {menu.label}
-            icon={<Icons.TriangleDown />}
+              </a>
-          >
+            </Menu.Item>
-            {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>
-                  </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 />,
-          ])}
-
-          {!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) && [
-            <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" />
-          &nbsp;
-        </StyledAnchor>
       )}
-      {navbarRight.bug_report_url && (
+      <SubMenu title="Settings" icon={<Icons.TriangleDown iconSize="xl" />}>
-        <StyledAnchor
+        {settings.map((section, index) => [
-          href={navbarRight.bug_report_url}
+          <Menu.ItemGroup key={`${section.label}`} title={section.label}>
-          target="_blank"
+            {section.childs?.map(child => {
-          rel="noreferrer"
+              if (typeof child !== 'string') {
-          title={t('Report a bug')}
+                return (
-        >
+                  <Menu.Item key={`${child.label}`}>
-          <i className="fa fa-bug" />
+                    {isFrontendRoute(child.url) ? (
-        </StyledAnchor>
+                      <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 />,
+        ])}
+
+        {!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.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}
+        />
       )}
-      {navbarRight.user_is_anonymous && (
+    </Menu>
-        <StyledAnchor href={navbarRight.user_login_url}>
+    {navbarRight.documentation_url && (
-          <i className="fa fa-fw fa-sign-in" />
+      <StyledAnchor
-          {t('Login')}
+        href={navbarRight.documentation_url}
-        </StyledAnchor>
+        target="_blank"
-      )}
+        rel="noreferrer"
-    </StyledDiv>
+        title={t('Documentation')}
-  );
+      >
-};
+        <i className="fa fa-question" />
+        &nbsp;
+      </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;

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} />
-        <Menu data={menu} />
-      </Provider>
     </ThemeProvider>
   </CacheProvider>
 );