fix: update Permissions for right nav (#19051)

* draft pr * finished styling * add filter * added testing * added tests * added permissions tests * Empty-Commit * new test * Update superset-frontend/src/views/components/MenuRight.tsx Co-authored-by: Elizabeth Thompson <eschutho@gmail.com> * revisions * added to CRUD view Co-authored-by: Elizabeth Thompson <eschutho@gmail.com>
apache · Apr 11, 2022 · 4bf4d58 · 4bf4d58
1 parent d693f4e
commit 4bf4d58
Show file tree

Hide file tree

Showing 11 changed files with 602 additions and 48 deletions.
diff --git a/superset-frontend/src/dashboard/util/findPermission.ts b/superset-frontend/src/dashboard/util/findPermission.ts
@@ -36,7 +36,7 @@ export default findPermission;
 // but is hardcoded in backend logic already, so...
 const ADMIN_ROLE_NAME = 'admin';
 
-const isUserAdmin = (user: UserWithPermissionsAndRoles) =>
+export const isUserAdmin = (user: UserWithPermissionsAndRoles) =>
   Object.keys(user.roles).some(role => role.toLowerCase() === ADMIN_ROLE_NAME);
 
 const isUserDashboardOwner = (

diff --git a/superset-frontend/src/views/CRUD/data/database/DatabaseList.test.jsx b/superset-frontend/src/views/CRUD/data/database/DatabaseList.test.jsx
@@ -18,7 +18,7 @@
  */
 import React from 'react';
 import thunk from 'redux-thunk';
-import * as redux from 'react-redux';
+import * as reactRedux from 'react-redux';
 import configureStore from 'redux-mock-store';
 import fetchMock from 'fetch-mock';
 import { Provider } from 'react-redux';
@@ -34,6 +34,7 @@ import waitForComponentToPaint from 'spec/helpers/waitForComponentToPaint';
 import { act } from 'react-dom/test-utils';
 
 // store needed for withToasts(DatabaseList)
+
 const mockStore = configureStore([thunk]);
 const store = mockStore({});
 
@@ -63,10 +64,6 @@ jest.mock('react-redux', () => ({
   useSelector: jest.fn(),
 }));
 
-const mockUser = {
-  userId: 1,
-};
-
 fetchMock.get(databasesInfoEndpoint, {
   permissions: ['can_write'],
 });
@@ -91,7 +88,13 @@ fetchMock.get(databaseRelatedEndpoint, {
   },
 });
 
-const useSelectorMock = jest.spyOn(redux, 'useSelector');
+fetchMock.get(
+  'glob:*api/v1/database/?q=(filters:!((col:allow_file_upload,opr:upload_is_enabled,value:!t)))',
+  {},
+);
+
+const useSelectorMock = jest.spyOn(reactRedux, 'useSelector');
+const userSelectorMock = jest.spyOn(reactRedux, 'useSelector');
 
 describe('DatabaseList', () => {
   useSelectorMock.mockReturnValue({
@@ -100,10 +103,27 @@ describe('DatabaseList', () => {
     COLUMNAR_EXTENSIONS: ['parquet', 'zip'],
     ALLOWED_EXTENSIONS: ['parquet', 'zip', 'xls', 'xlsx', 'csv'],
   });
+  userSelectorMock.mockReturnValue({
+    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 wrapper = mount(
     <Provider store={store}>
-      <DatabaseList user={mockUser} />
+      <DatabaseList />
     </Provider>,
   );
 
@@ -129,7 +149,7 @@ describe('DatabaseList', () => {
 
   it('fetches Databases', () => {
     const callsD = fetchMock.calls(/database\/\?q/);
-    expect(callsD).toHaveLength(1);
+    expect(callsD).toHaveLength(2);
     expect(callsD[0][0]).toMatchInlineSnapshot(
       `"http://localhost/api/v1/database/?q=(order_column:changed_on_delta_humanized,order_direction:desc,page:0,page_size:25)"`,
     );

diff --git a/superset-frontend/src/views/CRUD/data/database/DatabaseList.tsx b/superset-frontend/src/views/CRUD/data/database/DatabaseList.tsx
@@ -17,7 +17,8 @@
  * under the License.
  */
 import { SupersetClient, t, styled } from '@superset-ui/core';
-import React, { useState, useMemo } from 'react';
+import React, { useState, useMemo, useEffect } from 'react';
+import rison from 'rison';
 import { useSelector } from 'react-redux';
 import Loading from 'src/components/Loading';
 import { isFeatureEnabled, FeatureFlag } from 'src/featureFlags';
@@ -28,6 +29,7 @@ import SubMenu, { SubMenuProps } from 'src/views/components/SubMenu';
 import DeleteModal from 'src/components/DeleteModal';
 import { Tooltip } from 'src/components/Tooltip';
 import Icons from 'src/components/Icons';
+import { isUserAdmin } from 'src/dashboard/util/findPermission';
 import ListView, { FilterOperator, Filters } from 'src/components/ListView';
 import { commonMenuData } from 'src/views/CRUD/data/common';
 import handleResourceExport from 'src/utils/export';
@@ -85,16 +87,22 @@ function DatabaseList({ addDangerToast, addSuccessToast }: DatabaseListProps) {
     t('database'),
     addDangerToast,
   );
+  const user = useSelector<any, UserWithPermissionsAndRoles>(
+    state => state.user,
+  );
+
   const [databaseModalOpen, setDatabaseModalOpen] = useState<boolean>(false);
   const [databaseCurrentlyDeleting, setDatabaseCurrentlyDeleting] =
     useState<DatabaseDeleteObject | null>(null);
   const [currentDatabase, setCurrentDatabase] = useState<DatabaseObject | null>(
     null,
   );
+  const [allowUploads, setAllowUploads] = useState<boolean>(false);
+  const isAdmin = isUserAdmin(user);
+  const showUploads = allowUploads || isAdmin;
+
   const [preparingExport, setPreparingExport] = useState<boolean>(false);
-  const { roles } = useSelector<any, UserWithPermissionsAndRoles>(
-    state => state.user,
-  );
+  const { roles } = user;
   const {
     CSV_EXTENSIONS,
     COLUMNAR_EXTENSIONS,
@@ -163,6 +171,8 @@ function DatabaseList({ addDangerToast, addSuccessToast }: DatabaseListProps) {
     ALLOWED_EXTENSIONS,
   );
 
+  const isDisabled = isAdmin && !allowUploads;
+
   const uploadDropdownMenu = [
     {
       label: t('Upload file to database'),
@@ -171,24 +181,42 @@ function DatabaseList({ addDangerToast, addSuccessToast }: DatabaseListProps) {
           label: t('Upload CSV'),
           name: 'Upload CSV file',
           url: '/csvtodatabaseview/form',
-          perm: canUploadCSV,
+          perm: canUploadCSV && showUploads,
+          disable: isDisabled,
         },
         {
           label: t('Upload columnar file'),
           name: 'Upload columnar file',
           url: '/columnartodatabaseview/form',
-          perm: canUploadColumnar,
+          perm: canUploadColumnar && showUploads,
+          disable: isDisabled,
         },
         {
           label: t('Upload Excel file'),
           name: 'Upload Excel file',
           url: '/exceltodatabaseview/form',
-          perm: canUploadExcel,
+          perm: canUploadExcel && showUploads,
+          disable: isDisabled,
         },
       ],
     },
   ];
 
+  const hasFileUploadEnabled = () => {
+    const payload = {
+      filters: [
+        { col: 'allow_file_upload', opr: 'upload_is_enabled', value: true },
+      ],
+    };
+    SupersetClient.get({
+      endpoint: `/api/v1/database/?q=${rison.encode(payload)}`,
+    }).then(({ json }: Record<string, any>) => {
+      setAllowUploads(json.count >= 1);
+    });
+  };
+
+  useEffect(() => hasFileUploadEnabled(), [databaseModalOpen]);
+
   const filteredDropDown = uploadDropdownMenu.map(link => {
     // eslint-disable-next-line no-param-reassign
     link.childs = link.childs.filter(item => item.perm);

diff --git a/superset-frontend/src/views/components/Menu.test.tsx b/superset-frontend/src/views/components/Menu.test.tsx
@@ -18,6 +18,7 @@
  */
 import React from 'react';
 import * as reactRedux from 'react-redux';
+import fetchMock from 'fetch-mock';
 import { render, screen } from 'spec/helpers/testing-library';
 import userEvent from '@testing-library/user-event';
 import { Menu } from './Menu';
@@ -235,6 +236,11 @@ const notanonProps = {
 
 const useSelectorMock = jest.spyOn(reactRedux, 'useSelector');
 
+fetchMock.get(
+  'glob:*api/v1/database/?q=(filters:!((col:allow_file_upload,opr:upload_is_enabled,value:!t)))',
+  {},
+);
+
 beforeEach(() => {
   // setup a DOM element as a render target
   useSelectorMock.mockClear();

diff --git a/superset-frontend/src/views/components/Menu.tsx b/superset-frontend/src/views/components/Menu.tsx
@@ -75,6 +75,7 @@ export interface MenuObjectChildProps {
   isFrontendRoute?: boolean;
   perm?: string | boolean;
   view?: string;
+  disable?: boolean;
 }
 
 export interface MenuObjectProps extends MenuObjectChildProps {

diff --git a/superset-frontend/src/views/components/MenuRight.tsx b/superset-frontend/src/views/components/MenuRight.tsx
@@ -16,12 +16,20 @@
  * specific language governing permissions and limitations
  * under the License.
  */
-import React, { Fragment, useState } from 'react';
+import React, { Fragment, useState, useEffect } from 'react';
+import rison from 'rison';
 import { MainNav as Menu } from 'src/components/Menu';
-import { t, styled, css, SupersetTheme } from '@superset-ui/core';
+import {
+  t,
+  styled,
+  css,
+  SupersetTheme,
+  SupersetClient,
+} from '@superset-ui/core';
+import { Tooltip } from 'src/components/Tooltip';
 import { Link } from 'react-router-dom';
 import Icons from 'src/components/Icons';
-import findPermission from 'src/dashboard/util/findPermission';
+import findPermission, { isUserAdmin } from 'src/dashboard/util/findPermission';
 import { useSelector } from 'react-redux';
 import { UserWithPermissionsAndRoles } from 'src/types/bootstrapTypes';
 import LanguagePicker from './LanguagePicker';
@@ -45,6 +53,15 @@ const StyledI = styled.div`
   color: ${({ theme }) => theme.colors.primary.dark1};
 `;
 
+const styledDisabled = (theme: SupersetTheme) => css`
+  color: ${theme.colors.grayscale.base};
+  backgroundColor: ${theme.colors.grayscale.light2}};
+  .ant-menu-item:hover {
+    color: ${theme.colors.grayscale.base};
+    cursor: default;
+  }
+`;
+
 const StyledDiv = styled.div<{ align: string }>`
   display: flex;
   flex-direction: row;
@@ -69,9 +86,11 @@ const RightMenu = ({
   navbarRight,
   isFrontendRoute,
 }: RightMenuProps) => {
-  const { roles } = useSelector<any, UserWithPermissionsAndRoles>(
+  const user = useSelector<any, UserWithPermissionsAndRoles>(
     state => state.user,
   );
+
+  const { roles } = user;
   const {
     CSV_EXTENSIONS,
     COLUMNAR_EXTENSIONS,
@@ -96,6 +115,9 @@ const RightMenu = ({
 
   const canUpload = canUploadCSV || canUploadColumnar || canUploadExcel;
   const showActionDropdown = canSql || canChart || canDashboard;
+  const [allowUploads, setAllowUploads] = useState<boolean>(false);
+  const isAdmin = isUserAdmin(user);
+  const showUploads = allowUploads || isAdmin;
   const dropdownItems: MenuObjectProps[] = [
     {
       label: t('Data'),
@@ -115,19 +137,19 @@ const RightMenu = ({
           label: t('Upload CSV to database'),
           name: 'Upload a CSV',
           url: '/csvtodatabaseview/form',
-          perm: canUploadCSV,
+          perm: CSV_EXTENSIONS && showUploads,
         },
         {
           label: t('Upload columnar file to database'),
           name: 'Upload a Columnar file',
           url: '/columnartodatabaseview/form',
-          perm: canUploadColumnar,
+          perm: COLUMNAR_EXTENSIONS && showUploads,
         },
         {
           label: t('Upload Excel file to database'),
           name: 'Upload Excel',
           url: '/exceltodatabaseview/form',
-          perm: canUploadExcel,
+          perm: EXCEL_EXTENSIONS && showUploads,
         },
       ],
     },
@@ -154,6 +176,21 @@ const RightMenu = ({
     },
   ];
 
+  const hasFileUploadEnabled = () => {
+    const payload = {
+      filters: [
+        { col: 'allow_file_upload', opr: 'upload_is_enabled', value: true },
+      ],
+    };
+    SupersetClient.get({
+      endpoint: `/api/v1/database/?q=${rison.encode(payload)}`,
+    }).then(({ json }: Record<string, any>) => {
+      setAllowUploads(json.count >= 1);
+    });
+  };
+
+  useEffect(() => hasFileUploadEnabled(), []);
+
   const menuIconAndLabel = (menu: MenuObjectProps) => (
     <>
       <i data-test={`menu-item-${menu.label}`} className={`fa ${menu.icon}`} />
@@ -175,14 +212,47 @@ const RightMenu = ({
     setShowModal(false);
   };
 
+  const isDisabled = isAdmin && !allowUploads;
+
+  const tooltipText = t(
+    "Enable 'Allow data upload' in any database's settings",
+  );
+
+  const buildMenuItem = (item: Record<string, any>) => {
+    const disabledText = isDisabled && item.url;
+    return disabledText ? (
+      <Menu.Item key={item.name} css={styledDisabled}>
+        <Tooltip placement="top" title={tooltipText}>
+          {item.label}
+        </Tooltip>
+      </Menu.Item>
+    ) : (
+      <Menu.Item key={item.name}>
+        {item.url ? <a href={item.url}> {item.label} </a> : item.label}
+      </Menu.Item>
+    );
+  };
+
+  const onMenuOpen = (openKeys: string[]) => {
+    if (openKeys.length) {
+      return hasFileUploadEnabled();
+    }
+    return null;
+  };
+
   return (
     <StyledDiv align={align}>
       <DatabaseModal
         onHide={handleOnHideModal}
         show={showModal}
         dbEngine={engine}
       />
-      <Menu selectable={false} mode="horizontal" onClick={handleMenuSelection}>
+      <Menu
+        selectable={false}
+        mode="horizontal"
+        onClick={handleMenuSelection}
+        onOpenChange={onMenuOpen}
+      >
         {!navbarRight.user_is_anonymous && showActionDropdown && (
           <SubMenu
             data-test="new-dropdown"
@@ -203,13 +273,7 @@ const RightMenu = ({
                       typeof item !== 'string' && item.name && item.perm ? (
                         <Fragment key={item.name}>
                           {idx === 2 && <Menu.Divider />}
-                          <Menu.Item key={item.name}>
-                            {item.url ? (
-                              <a href={item.url}> {item.label} </a>
-                            ) : (
-                              item.label
-                            )}
-                          </Menu.Item>
+                          {buildMenuItem(item)}
                         </Fragment>
                       ) : null,
                     )}