You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by di...@apache.org on 2022/08/22 13:18:12 UTC

[superset] 23/36: fix: Use Home page in SPA (#21006)

This is an automated email from the ASF dual-hosted git repository.

diegopucci pushed a commit to branch chore/drill-to-detail-modal-tests
in repository https://gitbox.apache.org/repos/asf/superset.git

commit 6fd422a4d57f3bc870dffb997218fd32d909424f
Author: EugeneTorap <ev...@gmail.com>
AuthorDate: Thu Aug 18 13:19:15 2022 +0300

    fix: Use Home page in SPA (#21006)
    
    * fix: Use Welcome page in SPA
    
    * Fix Menu.test.tsx
    
    * Handle not frontend routes
---
 .../src/views/components/Menu.test.tsx             | 119 +++++++++++++++++----
 superset-frontend/src/views/components/Menu.tsx    |  13 ++-
 superset/views/base.py                             |   2 +-
 3 files changed, 109 insertions(+), 25 deletions(-)

diff --git a/superset-frontend/src/views/components/Menu.test.tsx b/superset-frontend/src/views/components/Menu.test.tsx
index 3fea75eb46..5d785f91d4 100644
--- a/superset-frontend/src/views/components/Menu.test.tsx
+++ b/superset-frontend/src/views/components/Menu.test.tsx
@@ -253,13 +253,18 @@ test('should render', () => {
   const { container } = render(<Menu {...mockedProps} />, {
     useRedux: true,
     useQueryParams: true,
+    useRouter: true,
   });
   expect(container).toBeInTheDocument();
 });
 
 test('should render the navigation', () => {
   useSelectorMock.mockReturnValue({ roles: user.roles });
-  render(<Menu {...mockedProps} />, { useRedux: true, useQueryParams: true });
+  render(<Menu {...mockedProps} />, {
+    useRedux: true,
+    useQueryParams: true,
+    useRouter: true,
+  });
   expect(screen.getByRole('navigation')).toBeInTheDocument();
 });
 
@@ -270,7 +275,11 @@ test('should render the brand', () => {
       brand: { alt, icon },
     },
   } = mockedProps;
-  render(<Menu {...mockedProps} />, { useRedux: true, useQueryParams: true });
+  render(<Menu {...mockedProps} />, {
+    useRedux: true,
+    useQueryParams: true,
+    useRouter: true,
+  });
   const image = screen.getByAltText(alt);
   expect(image).toHaveAttribute('src', icon);
 });
@@ -280,7 +289,11 @@ test('should render all the top navbar menu items', () => {
   const {
     data: { menu },
   } = mockedProps;
-  render(<Menu {...mockedProps} />, { useRedux: true, useQueryParams: true });
+  render(<Menu {...mockedProps} />, {
+    useRedux: true,
+    useQueryParams: true,
+    useRouter: true,
+  });
   menu.forEach(item => {
     expect(screen.getByText(item.label)).toBeInTheDocument();
   });
@@ -291,7 +304,11 @@ test('should render the top navbar child menu items', async () => {
   const {
     data: { menu },
   } = mockedProps;
-  render(<Menu {...mockedProps} />, { useRedux: true, useQueryParams: true });
+  render(<Menu {...mockedProps} />, {
+    useRedux: true,
+    useQueryParams: true,
+    useRouter: true,
+  });
   const sources = screen.getByText('Sources');
   userEvent.hover(sources);
   const datasets = await screen.findByText('Datasets');
@@ -305,7 +322,11 @@ test('should render the top navbar child menu items', async () => {
 
 test('should render the dropdown items', async () => {
   useSelectorMock.mockReturnValue({ roles: user.roles });
-  render(<Menu {...notanonProps} />, { useRedux: true, useQueryParams: true });
+  render(<Menu {...notanonProps} />, {
+    useRedux: true,
+    useQueryParams: true,
+    useRouter: true,
+  });
   const dropdown = screen.getByTestId('new-dropdown-icon');
   userEvent.hover(dropdown);
   // todo (philip): test data submenu
@@ -331,14 +352,22 @@ test('should render the dropdown items', async () => {
 
 test('should render the Settings', async () => {
   useSelectorMock.mockReturnValue({ roles: user.roles });
-  render(<Menu {...mockedProps} />, { useRedux: true, useQueryParams: true });
+  render(<Menu {...mockedProps} />, {
+    useRedux: true,
+    useQueryParams: true,
+    useRouter: true,
+  });
   const settings = await screen.findByText('Settings');
   expect(settings).toBeInTheDocument();
 });
 
 test('should render the Settings menu item', async () => {
   useSelectorMock.mockReturnValue({ roles: user.roles });
-  render(<Menu {...mockedProps} />, { useRedux: true, useQueryParams: true });
+  render(<Menu {...mockedProps} />, {
+    useRedux: true,
+    useQueryParams: true,
+    useRouter: true,
+  });
   userEvent.hover(screen.getByText('Settings'));
   const label = await screen.findByText('Security');
   expect(label).toBeInTheDocument();
@@ -349,7 +378,11 @@ test('should render the Settings dropdown child menu items', async () => {
   const {
     data: { settings },
   } = mockedProps;
-  render(<Menu {...mockedProps} />, { useRedux: true, useQueryParams: true });
+  render(<Menu {...mockedProps} />, {
+    useRedux: true,
+    useQueryParams: true,
+    useRouter: true,
+  });
   userEvent.hover(screen.getByText('Settings'));
   const listUsers = await screen.findByText('List Users');
   expect(listUsers).toHaveAttribute('href', settings[0].childs[0].url);
@@ -357,13 +390,21 @@ test('should render the Settings dropdown child menu items', async () => {
 
 test('should render the plus menu (+) when user is not anonymous', () => {
   useSelectorMock.mockReturnValue({ roles: user.roles });
-  render(<Menu {...notanonProps} />, { useRedux: true, useQueryParams: true });
+  render(<Menu {...notanonProps} />, {
+    useRedux: true,
+    useQueryParams: true,
+    useRouter: true,
+  });
   expect(screen.getByTestId('new-dropdown')).toBeInTheDocument();
 });
 
 test('should NOT render the plus menu (+) when user is anonymous', () => {
   useSelectorMock.mockReturnValue({ roles: user.roles });
-  render(<Menu {...mockedProps} />, { useRedux: true, useQueryParams: true });
+  render(<Menu {...mockedProps} />, {
+    useRedux: true,
+    useQueryParams: true,
+    useRouter: true,
+  });
   expect(screen.queryByTestId('new-dropdown')).not.toBeInTheDocument();
 });
 
@@ -375,7 +416,11 @@ test('should render the user actions when user is not anonymous', async () => {
     },
   } = mockedProps;
 
-  render(<Menu {...notanonProps} />, { useRedux: true, useQueryParams: true });
+  render(<Menu {...notanonProps} />, {
+    useRedux: true,
+    useQueryParams: true,
+    useRouter: true,
+  });
   userEvent.hover(screen.getByText('Settings'));
   const user = await screen.findByText('User');
   expect(user).toBeInTheDocument();
@@ -389,7 +434,11 @@ test('should render the user actions when user is not anonymous', async () => {
 
 test('should NOT render the user actions when user is anonymous', () => {
   useSelectorMock.mockReturnValue({ roles: user.roles });
-  render(<Menu {...mockedProps} />, { useRedux: true, useQueryParams: true });
+  render(<Menu {...mockedProps} />, {
+    useRedux: true,
+    useQueryParams: true,
+    useRouter: true,
+  });
   expect(screen.queryByText('User')).not.toBeInTheDocument();
 });
 
@@ -401,7 +450,11 @@ test('should render the Profile link when available', async () => {
     },
   } = mockedProps;
 
-  render(<Menu {...notanonProps} />, { useRedux: true, useQueryParams: true });
+  render(<Menu {...notanonProps} />, {
+    useRedux: true,
+    useQueryParams: true,
+    useRouter: true,
+  });
 
   userEvent.hover(screen.getByText('Settings'));
   const profile = await screen.findByText('Profile');
@@ -416,7 +469,11 @@ test('should render the About section and version_string, sha or build_number wh
     },
   } = mockedProps;
 
-  render(<Menu {...mockedProps} />, { useRedux: true, useQueryParams: true });
+  render(<Menu {...mockedProps} />, {
+    useRedux: true,
+    useQueryParams: true,
+    useRouter: true,
+  });
   userEvent.hover(screen.getByText('Settings'));
   const about = await screen.findByText('About');
   const version = await screen.findByText(`Version: ${version_string}`);
@@ -435,7 +492,11 @@ test('should render the Documentation link when available', async () => {
       navbar_right: { documentation_url },
     },
   } = mockedProps;
-  render(<Menu {...mockedProps} />, { useRedux: true, useQueryParams: true });
+  render(<Menu {...mockedProps} />, {
+    useRedux: true,
+    useQueryParams: true,
+    useRouter: true,
+  });
   userEvent.hover(screen.getByText('Settings'));
   const doc = await screen.findByTitle('Documentation');
   expect(doc).toHaveAttribute('href', documentation_url);
@@ -449,7 +510,11 @@ test('should render the Bug Report link when available', async () => {
     },
   } = mockedProps;
 
-  render(<Menu {...mockedProps} />, { useRedux: true, useQueryParams: true });
+  render(<Menu {...mockedProps} />, {
+    useRedux: true,
+    useQueryParams: true,
+    useRouter: true,
+  });
   const bugReport = await screen.findByTitle('Report a bug');
   expect(bugReport).toHaveAttribute('href', bug_report_url);
 });
@@ -462,26 +527,38 @@ test('should render the Login link when user is anonymous', () => {
     },
   } = mockedProps;
 
-  render(<Menu {...mockedProps} />, { useRedux: true, useQueryParams: true });
+  render(<Menu {...mockedProps} />, {
+    useRedux: true,
+    useQueryParams: true,
+    useRouter: true,
+  });
   const login = screen.getByText('Login');
   expect(login).toHaveAttribute('href', user_login_url);
 });
 
 test('should render the Language Picker', () => {
   useSelectorMock.mockReturnValue({ roles: user.roles });
-  render(<Menu {...mockedProps} />, { useRedux: true, useQueryParams: true });
+  render(<Menu {...mockedProps} />, {
+    useRedux: true,
+    useQueryParams: true,
+    useRouter: true,
+  });
   expect(screen.getByLabelText('Languages')).toBeInTheDocument();
 });
 
 test('should hide create button without proper roles', () => {
   useSelectorMock.mockReturnValue({ roles: [] });
-  render(<Menu {...mockedProps} />, { useRedux: true, useQueryParams: true });
+  render(<Menu {...mockedProps} />, {
+    useRedux: true,
+    useQueryParams: true,
+    useRouter: true,
+  });
   expect(screen.queryByTestId('new-dropdown')).not.toBeInTheDocument();
 });
 
 test('should render without QueryParamProvider', () => {
   useSelectorMock.mockReturnValue({ roles: [] });
-  render(<Menu {...mockedProps} />, { useRedux: true });
+  render(<Menu {...mockedProps} />, { useRedux: true, useRouter: true });
   expect(screen.queryByTestId('new-dropdown')).not.toBeInTheDocument();
 });
 
@@ -494,7 +571,7 @@ test('should render an extension component if one is supplied', () => {
 
   setupExtensions();
 
-  render(<Menu {...mockedProps} />);
+  render(<Menu {...mockedProps} />, { useRouter: true });
 
   expect(
     screen.getByText('navbar.right extension component'),
diff --git a/superset-frontend/src/views/components/Menu.tsx b/superset-frontend/src/views/components/Menu.tsx
index 739d7258c6..59d243879d 100644
--- a/superset-frontend/src/views/components/Menu.tsx
+++ b/superset-frontend/src/views/components/Menu.tsx
@@ -25,6 +25,7 @@ import { Row, Col, Grid } from 'src/components';
 import { MainNav as DropdownMenu, MenuMode } from 'src/components/Menu';
 import { Tooltip } from 'src/components/Tooltip';
 import { Link } from 'react-router-dom';
+import { GenericLink } from 'src/components/GenericLink/GenericLink';
 import Icons from 'src/components/Icons';
 import { useUiConfig } from 'src/components/UiConfigContext';
 import { URL_PARAMS } from 'src/constants';
@@ -282,9 +283,15 @@ export function Menu({
             title={brand.tooltip}
             arrowPointAtCenter
           >
-            <a className="navbar-brand" href={brand.path}>
-              <img src={brand.icon} alt={brand.alt} />
-            </a>
+            {isFrontendRoute(window.location.pathname) ? (
+              <GenericLink className="navbar-brand" to={brand.path}>
+                <img src={brand.icon} alt={brand.alt} />
+              </GenericLink>
+            ) : (
+              <a className="navbar-brand" href={brand.path}>
+                <img src={brand.icon} alt={brand.alt} />
+              </a>
+            )}
           </Tooltip>
           {brand.text && (
             <div className="navbar-brand-text">
diff --git a/superset/views/base.py b/superset/views/base.py
index 73b9096cd2..73b97d9f41 100644
--- a/superset/views/base.py
+++ b/superset/views/base.py
@@ -310,7 +310,7 @@ def menu_data() -> Dict[str, Any]:
     return {
         "menu": menu,
         "brand": {
-            "path": appbuilder.app.config["LOGO_TARGET_PATH"] or "/",
+            "path": appbuilder.app.config["LOGO_TARGET_PATH"] or "/superset/welcome/",
             "icon": appbuilder.app_icon,
             "alt": appbuilder.app_name,
             "tooltip": appbuilder.app.config["LOGO_TOOLTIP"],