Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: update Permissions for right nav #19051

Merged
merged 13 commits into from
Apr 11, 2022
2 changes: 1 addition & 1 deletion superset-frontend/src/dashboard/util/findPermission.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 = (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -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({});

Expand Down Expand Up @@ -63,10 +64,6 @@ jest.mock('react-redux', () => ({
useSelector: jest.fn(),
}));

const mockUser = {
userId: 1,
};

fetchMock.get(databasesInfoEndpoint, {
permissions: ['can_write'],
});
Expand All @@ -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({
Expand All @@ -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>,
);

Expand All @@ -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)"`,
);
Expand Down
42 changes: 35 additions & 7 deletions superset-frontend/src/views/CRUD/data/database/DatabaseList.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -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';
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -163,6 +171,8 @@ function DatabaseList({ addDangerToast, addSuccessToast }: DatabaseListProps) {
ALLOWED_EXTENSIONS,
);

const isDisabled = isAdmin && !allowUploads;

const uploadDropdownMenu = [
{
label: t('Upload file to database'),
Expand All @@ -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);
Expand Down
6 changes: 6 additions & 0 deletions superset-frontend/src/views/components/Menu.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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();
Expand Down
1 change: 1 addition & 0 deletions superset-frontend/src/views/components/Menu.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@ export interface MenuObjectChildProps {
isFrontendRoute?: boolean;
perm?: string | boolean;
view?: string;
disable?: boolean;
}

export interface MenuObjectProps extends MenuObjectChildProps {
Expand Down
94 changes: 79 additions & 15 deletions superset-frontend/src/views/components/MenuRight.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -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;
Expand All @@ -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,
Expand All @@ -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'),
Expand All @@ -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,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice like the dual perms checking!

},
{
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,
},
],
},
Expand All @@ -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}`} />
Expand All @@ -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>) => {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

awesome! so glad you split this out

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"
Expand All @@ -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,
)}
Expand Down
Loading