chore: move data nav menu to plus menu (#18629)

* more data nav menu * fix lint and fix nav css * update test and remove icons * Update superset-frontend/src/views/components/Menu.test.tsx Co-authored-by: Hugh A. Miles II <hughmil3s@gmail.com> * Apply suggestions from code review * use backend app.link to show new nav changes * fix lint * update test * usetheme and remove chaining * add more suggestions * fix lint * add allowed extensions to bootstrap and hard code links * remove backend links * fix test * add extensions to frontend conf * fix test and add be changes * test is python test passes * update python test and reremove app links * fix ts and add t's Co-authored-by: Hugh A. Miles II <hughmil3s@gmail.com>
apache · Feb 17, 2022 · 2421d17 · 2421d17
1 parent 3cccc63
commit 2421d17
Show file tree

Hide file tree

Showing 7 changed files with 132 additions and 76 deletions.
diff --git a/superset-frontend/src/views/App.tsx b/superset-frontend/src/views/App.tsx
@@ -38,7 +38,9 @@ import { RootContextProviders } from './RootContextProviders';
 setupApp();
 
 const user = { ...bootstrapData.user };
-const menu = { ...bootstrapData.common.menu_data };
+const menu = {
+  ...bootstrapData.common.menu_data,
+};
 let lastLocationPathname: string;
 initFeatureFlags(bootstrapData.common.feature_flags);
 

diff --git a/superset-frontend/src/views/components/Menu.test.tsx b/superset-frontend/src/views/components/Menu.test.tsx
@@ -89,6 +89,26 @@ const mockedProps = {
         url: '/dashboard/list/',
         index: 4,
       },
+      {
+        name: 'Data',
+        icon: 'fa-database',
+        label: 'Data',
+        childs: [
+          {
+            name: 'Databases',
+            icon: 'fa-database',
+            label: 'Databases',
+            url: '/databaseview/list/',
+          },
+          {
+            name: 'Datasets',
+            icon: 'fa-table',
+            label: 'Datasets',
+            url: '/tablemodelview/list/',
+          },
+          '-',
+        ],
+      },
     ],
     brand: {
       path: '/superset/profile/admin/',
@@ -220,13 +240,11 @@ test('should render the dropdown items', async () => {
   render(<Menu {...notanonProps} />);
   const dropdown = screen.getByTestId('new-dropdown-icon');
   userEvent.hover(dropdown);
-  expect(await screen.findByText(dropdownItems[0].label)).toHaveAttribute(
+  // todo (philip): test data submenu
+  expect(await screen.findByText(dropdownItems[1].label)).toHaveAttribute(
     'href',
-    dropdownItems[0].url,
+    dropdownItems[1].url,
   );
-  expect(
-    screen.getByTestId(`menu-item-${dropdownItems[0].label}`),
-  ).toBeInTheDocument();
   expect(await screen.findByText(dropdownItems[1].label)).toHaveAttribute(
     'href',
     dropdownItems[1].url,

diff --git a/superset-frontend/src/views/components/Menu.tsx b/superset-frontend/src/views/components/Menu.tsx
@@ -17,7 +17,7 @@
  * under the License.
  */
 import React, { useState, useEffect } from 'react';
-import { styled, css } from '@superset-ui/core';
+import { styled, css, useTheme, SupersetTheme } from '@superset-ui/core';
 import { debounce } from 'lodash';
 import { Global } from '@emotion/react';
 import { getUrlParam } from 'src/utils/urlUtils';
@@ -75,10 +75,12 @@ export interface MenuProps {
 interface MenuObjectChildProps {
   label: string;
   name?: string;
-  icon: string;
-  index: number;
+  icon?: string;
+  index?: number;
   url?: string;
   isFrontendRoute?: boolean;
+  perm?: string;
+  view?: string;
 }
 
 export interface MenuObjectProps extends MenuObjectChildProps {
@@ -172,7 +174,21 @@ const StyledHeader = styled.header`
     }
   }
 `;
-
+const globalStyles = (theme: SupersetTheme) => css`
+  .ant-menu-submenu.ant-menu-submenu-popup.ant-menu.ant-menu-light.ant-menu-submenu-placement-bottomLeft {
+    border-radius: 0px;
+  }
+  .ant-menu-submenu.ant-menu-submenu-popup.ant-menu.ant-menu-light {
+    border-radius: 0px;
+  }
+  .ant-menu-vertical > .ant-menu-submenu.data-menu > .ant-menu-submenu-title {
+    height: 28px;
+    i {
+      padding-right: ${theme.gridUnit * 2}px;
+      margin-left: ${theme.gridUnit * 1.75}px;
+    }
+  }
+`;
 const { SubMenu } = DropdownMenu;
 
 const { useBreakpoint } = Grid;
@@ -184,6 +200,7 @@ export function Menu({
   const [showMenu, setMenu] = useState<MenuMode>('horizontal');
   const screens = useBreakpoint();
   const uiConig = useUiConfig();
+  const theme = useTheme();
 
   useEffect(() => {
     function handleResize() {
@@ -251,16 +268,7 @@ export function Menu({
   };
   return (
     <StyledHeader className="top" id="main-menu" role="navigation">
-      <Global
-        styles={css`
-          .ant-menu-submenu.ant-menu-submenu-popup.ant-menu.ant-menu-light.ant-menu-submenu-placement-bottomLeft {
-            border-radius: 0px;
-          }
-          .ant-menu-submenu.ant-menu-submenu-popup.ant-menu.ant-menu-light {
-            border-radius: 0px;
-          }
-        `}
-      />
+      <Global styles={globalStyles(theme)} />
       <Row>
         <Col md={16} xs={24}>
           <Tooltip

diff --git a/superset-frontend/src/views/components/MenuRight.tsx b/superset-frontend/src/views/components/MenuRight.tsx
@@ -23,11 +23,38 @@ 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 {
+  UserWithPermissionsAndRoles,
+  CommonBootstrapData,
+} from 'src/types/bootstrapTypes';
 import LanguagePicker from './LanguagePicker';
 import { NavBarProps, MenuObjectProps } from './Menu';
 
-export const dropdownItems = [
+export const dropdownItems: MenuObjectProps[] = [
+  {
+    label: t('Data'),
+    icon: 'fa-database',
+    childs: [
+      {
+        icon: 'fa-upload',
+        label: t('Upload a CSV'),
+        name: 'Upload a CSV',
+        url: '/csvtodatabaseview/form',
+      },
+      {
+        icon: 'fa-upload',
+        label: t('Upload a Columnar File'),
+        name: 'Upload a Columnar file',
+        url: '/columnartodatabaseview/form',
+      },
+      {
+        icon: 'fa-upload',
+        label: t('Upload Excel'),
+        name: 'Upload Excel',
+        url: '/exceltodatabaseview/form',
+      },
+    ],
+  },
   {
     label: t('SQL query'),
     url: '/superset/sqllab?new=true',
@@ -96,12 +123,27 @@ const RightMenu = ({
   const { roles } = useSelector<any, UserWithPermissionsAndRoles>(
     state => state.user,
   );
-
+  // @ts-ignore
+  const { CSV_EXTENSIONS, COLUMNAR_EXTENSIONS, EXCEL_EXTENSIONS } = useSelector<
+    any,
+    CommonBootstrapData
+  >(state => state.common.conf);
   // if user has any of these roles the dropdown will appear
+  const configMap = {
+    'Upload a CSV': CSV_EXTENSIONS,
+    'Upload a Columnar file': COLUMNAR_EXTENSIONS,
+    'Upload Excel': EXCEL_EXTENSIONS,
+  };
   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;
+  const menuIconAndLabel = (menu: MenuObjectProps) => (
+    <>
+      <i data-test={`menu-item-${menu.label}`} className={`fa ${menu.icon}`} />
+      {menu.label}
+    </>
+  );
   return (
     <StyledDiv align={align}>
       <Menu mode="horizontal">
@@ -113,9 +155,32 @@ const RightMenu = ({
             }
             icon={<Icons.TriangleDown />}
           >
-            {dropdownItems.map(
-              menu =>
-                findPermission(menu.perm, menu.view, roles) && (
+            {dropdownItems.map(menu => {
+              if (menu.childs) {
+                return (
+                  <SubMenu
+                    key="sub2"
+                    className="data-menu"
+                    title={menuIconAndLabel(menu)}
+                  >
+                    {menu.childs.map(item =>
+                      typeof item !== 'string' &&
+                      item.name &&
+                      configMap[item.name] === true ? (
+                        <Menu.Item key={item.name}>
+                          <a href={item.url}> {item.label} </a>
+                        </Menu.Item>
+                      ) : null,
+                    )}
+                  </SubMenu>
+                );
+              }
+              return (
+                findPermission(
+                  menu.perm as string,
+                  menu.view as string,
+                  roles,
+                ) && (
                   <Menu.Item key={menu.label}>
                     <a href={menu.url}>
                       <i
@@ -125,8 +190,9 @@ const RightMenu = ({
                       {menu.label}
                     </a>
                   </Menu.Item>
-                ),
-            )}
+                )
+              );
+            })}
           </SubMenu>
         )}
         <SubMenu

diff --git a/superset/initialization/__init__.py b/superset/initialization/__init__.py
@@ -367,53 +367,6 @@ def init_views(self) -> None:
             category_icon="fa-table",
         )
         appbuilder.add_separator("Data")
-        appbuilder.add_link(
-            "Upload a CSV",
-            label=__("Upload a CSV"),
-            href="/csvtodatabaseview/form",
-            icon="fa-upload",
-            category="Data",
-            category_label=__("Data"),
-            category_icon="fa-wrench",
-            cond=lambda: bool(
-                self.config["CSV_EXTENSIONS"].intersection(
-                    self.config["ALLOWED_EXTENSIONS"]
-                )
-            ),
-        )
-        appbuilder.add_link(
-            "Upload a Columnar file",
-            label=__("Upload a Columnar File"),
-            href="/columnartodatabaseview/form",
-            icon="fa-upload",
-            category="Data",
-            category_label=__("Data"),
-            category_icon="fa-wrench",
-            cond=lambda: bool(
-                self.config["COLUMNAR_EXTENSIONS"].intersection(
-                    self.config["ALLOWED_EXTENSIONS"]
-                )
-            ),
-        )
-        try:
-            import xlrd  # pylint: disable=unused-import
-
-            appbuilder.add_link(
-                "Upload Excel",
-                label=__("Upload Excel"),
-                href="/exceltodatabaseview/form",
-                icon="fa-upload",
-                category="Data",
-                category_label=__("Data"),
-                category_icon="fa-wrench",
-                cond=lambda: bool(
-                    self.config["EXCEL_EXTENSIONS"].intersection(
-                        self.config["ALLOWED_EXTENSIONS"]
-                    )
-                ),
-            )
-        except ImportError:
-            pass
 
         appbuilder.add_api(LogRestApi)
         appbuilder.add_view(

diff --git a/superset/views/base.py b/superset/views/base.py
@@ -346,6 +346,16 @@ def common_bootstrap_payload() -> Dict[str, Any]:
 
     # should not expose API TOKEN to frontend
     frontend_config = {k: conf.get(k) for k in FRONTEND_CONF_KEYS}
+    frontend_config["EXCEL_EXTENSIONS"] = bool(
+        bool(conf["EXCEL_EXTENSIONS"].intersection(conf["ALLOWED_EXTENSIONS"])),
+    )
+    frontend_config["CSV_EXTENSIONS"] = bool(
+        bool(conf["CSV_EXTENSIONS"].intersection(conf["ALLOWED_EXTENSIONS"])),
+    )
+    frontend_config["COLUMNAR_EXTENSIONS"] = bool(
+        bool(conf["COLUMNAR_EXTENSIONS"].intersection(conf["ALLOWED_EXTENSIONS"])),
+    )
+
     if conf.get("SLACK_API_TOKEN"):
         frontend_config["ALERT_REPORTS_NOTIFICATION_METHODS"] = [
             ReportRecipientType.EMAIL,

diff --git a/tests/integration_tests/security_tests.py b/tests/integration_tests/security_tests.py
@@ -706,7 +706,6 @@ def assert_can_alpha(self, perm_set):
         self.assert_can_menu("Manage", perm_set)
         self.assert_can_menu("Annotation Layers", perm_set)
         self.assert_can_menu("CSS Templates", perm_set)
-        self.assert_can_menu("Upload a CSV", perm_set)
         self.assertIn(("all_datasource_access", "all_datasource_access"), perm_set)
 
     def assert_cannot_alpha(self, perm_set):