ktmud commented on code in PR #19708:
URL: https://github.com/apache/superset/pull/19708#discussion_r850003432
##########
superset-frontend/packages/superset-ui-core/src/connection/SupersetClientClass.ts:
##########
@@ -161,7 +163,7 @@ export default class SupersetClientClass {
headers,
timeout,
fetchRetryOptions,
- ignoreUnauthorized,
+ ignoreUnauthorized = false,
Review Comment:
Be more explicit what is the default. I kind of feel this should probably
default to `true` but that's another topic. Async API requests should never
implicitly redirect users as it may often cause surprises. There should be
better error handling on the API user side instead.
cc @geido @kgabryje could you give more context on why #17597 made redirects
on 401 the default?
##########
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')) {
Review Comment:
Only redirect when we are not on the login page so we never see infinite
redirect caused by the API client ever again.
##########
superset-frontend/src/views/components/SubMenu.tsx:
##########
@@ -240,22 +240,22 @@ const SubMenuComponent:
React.FunctionComponent<SubMenuProps> = props => {
if ((props.usesRouter || hasHistory) && !!tab.usesRouter) {
return (
<Menu.Item key={tab.label}>
- <li
+ <div
Review Comment:
Bycatch: fix a React warning about nested `<li />`s.
##########
superset-frontend/src/views/components/MenuRight.tsx:
##########
@@ -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 && (
Review Comment:
Bycatch: only render the DatabaseModal when users can create new database
connection.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscr...@superset.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscr...@superset.apache.org
For additional commands, e-mail: notifications-h...@superset.apache.org
