You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by yj...@apache.org on 2022/04/14 06:30:08 UTC
[superset] branch master updated: fix(nav): infinite redirect and upload dataset nav permissions (#19708)
This is an automated email from the ASF dual-hosted git repository.
yjc 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 32a9265cc0 fix(nav): infinite redirect and upload dataset nav permissions (#19708)
32a9265cc0 is described below
commit 32a9265cc0cb850910e55b6f49a73169fc7ed377
Author: Jesse Yang <je...@airbnb.com>
AuthorDate: Wed Apr 13 23:29:56 2022 -0700
fix(nav): infinite redirect and upload dataset nav permissions (#19708)
---
.../src/connection/SupersetClientClass.ts | 10 ++-
.../test/connection/SupersetClientClass.test.ts | 27 ++++++-
superset-frontend/src/views/CRUD/utils.tsx | 22 ++++--
.../src/views/CRUD/welcome/ActivityTable.test.tsx | 4 +-
.../src/views/CRUD/welcome/ChartTable.test.tsx | 2 +-
.../src/views/CRUD/welcome/DashboardTable.test.tsx | 4 +-
.../src/views/CRUD/welcome/SavedQueries.test.tsx | 4 +-
.../src/views/components/MenuRight.tsx | 86 +++++++++++++---------
superset-frontend/src/views/components/SubMenu.tsx | 8 +-
9 files changed, 104 insertions(+), 63 deletions(-)
diff --git a/superset-frontend/packages/superset-ui-core/src/connection/SupersetClientClass.ts b/superset-frontend/packages/superset-ui-core/src/connection/SupersetClientClass.ts
index bf0c02a81d..5e046bce7e 100644
--- a/superset-frontend/packages/superset-ui-core/src/connection/SupersetClientClass.ts
+++ b/superset-frontend/packages/superset-ui-core/src/connection/SupersetClientClass.ts
@@ -34,9 +34,11 @@ import {
import { DEFAULT_FETCH_RETRY_OPTIONS, DEFAULT_BASE_URL } from './constants';
const defaultUnauthorizedHandler = () => {
- window.location.href = `/login?next=${
- window.location.pathname + window.location.search
- }`;
+ if (!window.location.pathname.startsWith('/login')) {
+ window.location.href = `/login?next=${
+ window.location.pathname + window.location.search
+ }`;
+ }
};
export default class SupersetClientClass {
@@ -161,7 +163,7 @@ export default class SupersetClientClass {
headers,
timeout,
fetchRetryOptions,
- ignoreUnauthorized,
+ ignoreUnauthorized = false,
...rest
}: RequestConfig & { parseMethod?: T }) {
await this.ensureAuth();
diff --git a/superset-frontend/packages/superset-ui-core/test/connection/SupersetClientClass.test.ts b/superset-frontend/packages/superset-ui-core/test/connection/SupersetClientClass.test.ts
index 921061b22f..a17cbceb22 100644
--- a/superset-frontend/packages/superset-ui-core/test/connection/SupersetClientClass.test.ts
+++ b/superset-frontend/packages/superset-ui-core/test/connection/SupersetClientClass.test.ts
@@ -505,7 +505,8 @@ describe('SupersetClientClass', () => {
const mockRequestUrl = 'https://host/get/url';
const mockRequestPath = '/get/url';
const mockRequestSearch = '?param=1¶m=2';
- const mockHref = `http://localhost${mockRequestPath + mockRequestSearch}`;
+ const mockRequestRelativeUrl = mockRequestPath + mockRequestSearch;
+ const mockHref = `http://localhost${mockRequestRelativeUrl}`;
beforeEach(() => {
originalLocation = window.location;
@@ -541,13 +542,31 @@ describe('SupersetClientClass', () => {
error = err;
} finally {
const redirectURL = window.location.href;
- expect(redirectURL).toBe(
- `/login?next=${mockRequestPath + mockRequestSearch}`,
- );
+ expect(redirectURL).toBe(`/login?next=${mockRequestRelativeUrl}`);
expect(error.status).toBe(401);
}
});
+ it('should not redirect again if already on login page', async () => {
+ const client = new SupersetClientClass({});
+
+ // @ts-expect-error
+ window.location = {
+ href: '/login?next=something',
+ pathname: '/login',
+ search: '?next=something',
+ };
+
+ let error;
+ try {
+ await client.request({ url: mockRequestUrl, method: 'GET' });
+ } catch (err) {
+ error = err;
+ } finally {
+ expect(window.location.href).toBe('/login?next=something');
+ expect(error.status).toBe(401);
+ }
+ });
it('does nothing if instructed to ignoreUnauthorized', async () => {
const client = new SupersetClientClass({});
diff --git a/superset-frontend/src/views/CRUD/utils.tsx b/superset-frontend/src/views/CRUD/utils.tsx
index 1a7b3ca1fa..31f3d4c9ed 100644
--- a/superset-frontend/src/views/CRUD/utils.tsx
+++ b/superset-frontend/src/views/CRUD/utils.tsx
@@ -427,14 +427,20 @@ export const uploadUserPerms = (
colExt: Array<string>,
excelExt: Array<string>,
allowedExt: Array<string>,
-) => ({
- canUploadCSV:
+) => {
+ const canUploadCSV =
findPermission('can_this_form_get', 'CsvToDatabaseView', roles) &&
- checkUploadExtensions(csvExt, allowedExt),
- canUploadColumnar:
+ checkUploadExtensions(csvExt, allowedExt);
+ const canUploadColumnar =
checkUploadExtensions(colExt, allowedExt) &&
- findPermission('can_this_form_get', 'ColumnarToDatabaseView', roles),
- canUploadExcel:
+ findPermission('can_this_form_get', 'ColumnarToDatabaseView', roles);
+ const canUploadExcel =
checkUploadExtensions(excelExt, allowedExt) &&
- findPermission('can_this_form_get', 'ExcelToDatabaseView', roles),
-});
+ findPermission('can_this_form_get', 'ExcelToDatabaseView', roles);
+ return {
+ canUploadCSV,
+ canUploadColumnar,
+ canUploadExcel,
+ canUploadData: canUploadCSV || canUploadColumnar || canUploadExcel,
+ };
+};
diff --git a/superset-frontend/src/views/CRUD/welcome/ActivityTable.test.tsx b/superset-frontend/src/views/CRUD/welcome/ActivityTable.test.tsx
index 2fd7feb977..71067b817a 100644
--- a/superset-frontend/src/views/CRUD/welcome/ActivityTable.test.tsx
+++ b/superset-frontend/src/views/CRUD/welcome/ActivityTable.test.tsx
@@ -100,14 +100,14 @@ describe('ActivityTable', () => {
expect(wrapper.find(ActivityTable)).toExist();
});
it('renders tabs with three buttons', () => {
- expect(wrapper.find('li.no-router')).toHaveLength(3);
+ expect(wrapper.find('[role="tab"]')).toHaveLength(3);
});
it('renders ActivityCards', async () => {
expect(wrapper.find('ListViewCard')).toExist();
});
it('calls the getEdited batch call when edited tab is clicked', async () => {
act(() => {
- const handler = wrapper.find('li.no-router a').at(1).prop('onClick');
+ const handler = wrapper.find('[role="tab"] a').at(1).prop('onClick');
if (handler) {
handler({} as any);
}
diff --git a/superset-frontend/src/views/CRUD/welcome/ChartTable.test.tsx b/superset-frontend/src/views/CRUD/welcome/ChartTable.test.tsx
index c61cb1b33e..cfa9230328 100644
--- a/superset-frontend/src/views/CRUD/welcome/ChartTable.test.tsx
+++ b/superset-frontend/src/views/CRUD/welcome/ChartTable.test.tsx
@@ -79,7 +79,7 @@ describe('ChartTable', () => {
it('fetches chart favorites and renders chart cards', async () => {
act(() => {
- const handler = wrapper.find('li.no-router a').at(0).prop('onClick');
+ const handler = wrapper.find('[role="tab"] a').at(0).prop('onClick');
if (handler) {
handler({} as any);
}
diff --git a/superset-frontend/src/views/CRUD/welcome/DashboardTable.test.tsx b/superset-frontend/src/views/CRUD/welcome/DashboardTable.test.tsx
index 26fd3a13d3..79f88e3128 100644
--- a/superset-frontend/src/views/CRUD/welcome/DashboardTable.test.tsx
+++ b/superset-frontend/src/views/CRUD/welcome/DashboardTable.test.tsx
@@ -71,10 +71,10 @@ describe('DashboardTable', () => {
it('render a submenu with clickable tabs and buttons', async () => {
expect(wrapper.find('SubMenu')).toExist();
- expect(wrapper.find('li.no-router')).toHaveLength(2);
+ expect(wrapper.find('[role="tab"]')).toHaveLength(2);
expect(wrapper.find('Button')).toHaveLength(6);
act(() => {
- const handler = wrapper.find('li.no-router a').at(0).prop('onClick');
+ const handler = wrapper.find('[role="tab"] a').at(0).prop('onClick');
if (handler) {
handler({} as any);
}
diff --git a/superset-frontend/src/views/CRUD/welcome/SavedQueries.test.tsx b/superset-frontend/src/views/CRUD/welcome/SavedQueries.test.tsx
index dd883a1aa1..f656a9e8e1 100644
--- a/superset-frontend/src/views/CRUD/welcome/SavedQueries.test.tsx
+++ b/superset-frontend/src/views/CRUD/welcome/SavedQueries.test.tsx
@@ -81,7 +81,7 @@ describe('SavedQueries', () => {
const clickTab = (idx: number) => {
act(() => {
- const handler = wrapper.find('li.no-router a').at(idx).prop('onClick');
+ const handler = wrapper.find('[role="tab"] a').at(idx).prop('onClick');
if (handler) {
handler({} as any);
}
@@ -105,7 +105,7 @@ describe('SavedQueries', () => {
it('renders a submenu with clickable tables and buttons', async () => {
expect(wrapper.find(SubMenu)).toExist();
- expect(wrapper.find('li.no-router')).toHaveLength(1);
+ expect(wrapper.find('[role="tab"]')).toHaveLength(1);
expect(wrapper.find('button')).toHaveLength(2);
clickTab(0);
await waitForComponentToPaint(wrapper);
diff --git a/superset-frontend/src/views/components/MenuRight.tsx b/superset-frontend/src/views/components/MenuRight.tsx
index 9d657aa9b7..1c46f6bcf0 100644
--- a/superset-frontend/src/views/components/MenuRight.tsx
+++ b/superset-frontend/src/views/components/MenuRight.tsx
@@ -105,15 +105,15 @@ const RightMenu = ({
const canChart = findPermission('can_write', 'Chart', roles);
const canDatabase = findPermission('can_write', 'Database', roles);
- const { canUploadCSV, canUploadColumnar, canUploadExcel } = uploadUserPerms(
- roles,
- CSV_EXTENSIONS,
- COLUMNAR_EXTENSIONS,
- EXCEL_EXTENSIONS,
- ALLOWED_EXTENSIONS,
- );
+ const { canUploadData, canUploadCSV, canUploadColumnar, canUploadExcel } =
+ uploadUserPerms(
+ roles,
+ CSV_EXTENSIONS,
+ COLUMNAR_EXTENSIONS,
+ EXCEL_EXTENSIONS,
+ ALLOWED_EXTENSIONS,
+ );
- const canUpload = canUploadCSV || canUploadColumnar || canUploadExcel;
const showActionDropdown = canSql || canChart || canDashboard;
const [allowUploads, setAllowUploads] = useState<boolean>(false);
const isAdmin = isUserAdmin(user);
@@ -137,19 +137,19 @@ const RightMenu = ({
label: t('Upload CSV to database'),
name: 'Upload a CSV',
url: '/csvtodatabaseview/form',
- perm: CSV_EXTENSIONS && showUploads,
+ perm: canUploadCSV && showUploads,
},
{
label: t('Upload columnar file to database'),
name: 'Upload a Columnar file',
url: '/columnartodatabaseview/form',
- perm: COLUMNAR_EXTENSIONS && showUploads,
+ perm: canUploadColumnar && showUploads,
},
{
label: t('Upload Excel file to database'),
name: 'Upload Excel',
url: '/exceltodatabaseview/form',
- perm: EXCEL_EXTENSIONS && showUploads,
+ perm: canUploadExcel && showUploads,
},
],
},
@@ -176,7 +176,7 @@ const RightMenu = ({
},
];
- const hasFileUploadEnabled = () => {
+ const checkAllowUploads = () => {
const payload = {
filters: [
{ col: 'allow_file_upload', opr: 'upload_is_enabled', value: true },
@@ -189,7 +189,11 @@ const RightMenu = ({
});
};
- useEffect(() => hasFileUploadEnabled(), []);
+ useEffect(() => {
+ if (canUploadData) {
+ checkAllowUploads();
+ }
+ }, [canUploadData]);
const menuIconAndLabel = (menu: MenuObjectProps) => (
<>
@@ -234,19 +238,21 @@ const RightMenu = ({
};
const onMenuOpen = (openKeys: string[]) => {
- if (openKeys.length) {
- return hasFileUploadEnabled();
+ if (openKeys.length && canUploadData) {
+ return checkAllowUploads();
}
return null;
};
return (
<StyledDiv align={align}>
- <DatabaseModal
- onHide={handleOnHideModal}
- show={showModal}
- dbEngine={engine}
- />
+ {canDatabase && (
+ <DatabaseModal
+ onHide={handleOnHideModal}
+ show={showModal}
+ dbEngine={engine}
+ />
+ )}
<Menu
selectable={false}
mode="horizontal"
@@ -262,23 +268,31 @@ const RightMenu = ({
icon={<Icons.TriangleDown />}
>
{dropdownItems.map(menu => {
+ const canShowChild = menu.childs?.some(
+ item => typeof item === 'object' && !!item.perm,
+ );
if (menu.childs) {
- return canDatabase || canUpload ? (
- <SubMenu
- key={`sub2_${menu.label}`}
- className="data-menu"
- title={menuIconAndLabel(menu)}
- >
- {menu.childs.map((item, idx) =>
- typeof item !== 'string' && item.name && item.perm ? (
- <Fragment key={item.name}>
- {idx === 2 && <Menu.Divider />}
- {buildMenuItem(item)}
- </Fragment>
- ) : null,
- )}
- </SubMenu>
- ) : null;
+ if (canShowChild) {
+ return (
+ <SubMenu
+ key={`sub2_${menu.label}`}
+ className="data-menu"
+ title={menuIconAndLabel(menu)}
+ >
+ {menu.childs.map((item, idx) =>
+ typeof item !== 'string' && item.name && item.perm ? (
+ <Fragment key={item.name}>
+ {idx === 2 && <Menu.Divider />}
+ {buildMenuItem(item)}
+ </Fragment>
+ ) : null,
+ )}
+ </SubMenu>
+ );
+ }
+ if (!menu.url) {
+ return null;
+ }
}
return (
findPermission(
diff --git a/superset-frontend/src/views/components/SubMenu.tsx b/superset-frontend/src/views/components/SubMenu.tsx
index 9dc5b41cdc..2ae897196b 100644
--- a/superset-frontend/src/views/components/SubMenu.tsx
+++ b/superset-frontend/src/views/components/SubMenu.tsx
@@ -240,7 +240,7 @@ const SubMenuComponent: React.FunctionComponent<SubMenuProps> = props => {
if ((props.usesRouter || hasHistory) && !!tab.usesRouter) {
return (
<Menu.Item key={tab.label}>
- <li
+ <div
role="tab"
data-test={tab['data-test']}
className={tab.name === props.activeChild ? 'active' : ''}
@@ -248,14 +248,14 @@ const SubMenuComponent: React.FunctionComponent<SubMenuProps> = props => {
<div>
<Link to={tab.url || ''}>{tab.label}</Link>
</div>
- </li>
+ </div>
</Menu.Item>
);
}
return (
<Menu.Item key={tab.label}>
- <li
+ <div
className={cx('no-router', {
active: tab.name === props.activeChild,
})}
@@ -264,7 +264,7 @@ const SubMenuComponent: React.FunctionComponent<SubMenuProps> = props => {
<a href={tab.url} onClick={tab.onClick}>
{tab.label}
</a>
- </li>
+ </div>
</Menu.Item>
);
})}