fix(superset-frontend): Fixes for broken functionality when an application root is defined (#36058)

superset-frontend/src/SqlLab/components/ResultSet/index.tsx

@@ -288,9 +288,16 @@ const ResultSet = ({
 
         all_columns: results.columns.map(column => column.column_name),
       });
-      const url = mountExploreUrl(null, {
-        [URL_PARAMS.formDataKey.name]: key,
-      });
+      const force = false;
+      const includeAppRoot = openInNewWindow;
+      const url = mountExploreUrl(
+        null,
+        {
+          [URL_PARAMS.formDataKey.name]: key,
+        },
+        force,
+        includeAppRoot,
+      );
       if (openInNewWindow) {
         window.open(url, '_blank', 'noreferrer');
       } else {

superset-frontend/src/explore/exploreUtils/index.js

@@ -78,21 +78,24 @@ export function getAnnotationJsonUrl(slice_id, force) {
     .toString();
 }
 
-export function getURIDirectory(endpointType = 'base') {
+export function getURIDirectory(endpointType = 'base', includeAppRoot = true) {
   // Building the directory part of the URI
-  if (
-    ['full', 'json', 'csv', 'query', 'results', 'samples'].includes(
-      endpointType,
-    )
-  ) {
-    return ensureAppRoot('/superset/explore_json/');
-  }
-  return ensureAppRoot('/explore/');
+  const uri = ['full', 'json', 'csv', 'query', 'results', 'samples'].includes(
+    endpointType,
+  )
+    ? '/superset/explore_json/'
+    : '/explore/';
+  return includeAppRoot ? ensureAppRoot(uri) : uri;
 }
 
-export function mountExploreUrl(endpointType, extraSearch = {}, force = false) {
+export function mountExploreUrl(
+  endpointType,
+  extraSearch = {},
+  force = false,
+  includeAppRoot = true,
+) {
   const uri = new URI('/');
-  const directory = getURIDirectory(endpointType);
+  const directory = getURIDirectory(endpointType, includeAppRoot);
   const search = uri.search(true);
   Object.keys(extraSearch).forEach(key => {
     search[key] = extraSearch[key];

superset-frontend/src/features/home/Menu.test.tsx

@@ -22,6 +22,7 @@ import { render, screen, userEvent } from 'spec/helpers/testing-library';
 import setupCodeOverrides from 'src/setup/setupCodeOverrides';
 import { getExtensionsRegistry } from '@superset-ui/core';
 import { Menu } from './Menu';
+import * as getBootstrapData from 'src/utils/getBootstrapData';
 
 const dropdownItems = [
   {
@@ -238,6 +239,10 @@ const notanonProps = {
 };
 
 const useSelectorMock = jest.spyOn(reactRedux, 'useSelector');
+const staticAssetsPrefixMock = jest.spyOn(
+  getBootstrapData,
+  'staticAssetsPrefix',
+);
 
 fetchMock.get(
   'glob:*api/v1/database/?q=(filters:!((col:allow_file_upload,opr:upload_is_enabled,value:!t)))',
@@ -247,6 +252,8 @@ fetchMock.get(
 beforeEach(() => {
   // setup a DOM element as a render target
   useSelectorMock.mockClear();
+  // By default use empty static assets prefix
+  staticAssetsPrefixMock.mockReturnValue('');
 });
 
 test('should render', async () => {
@@ -272,23 +279,27 @@ test('should render the navigation', async () => {
   expect(await screen.findByRole('navigation')).toBeInTheDocument();
 });
 
-test('should render the brand', async () => {
-  useSelectorMock.mockReturnValue({ roles: user.roles });
-  const {
-    data: {
-      brand: { alt, icon },
-    },
-  } = mockedProps;
-  render(<Menu {...mockedProps} />, {
-    useRedux: true,
-    useQueryParams: true,
-    useRouter: true,
-    useTheme: true,
-  });
-  expect(await screen.findByAltText(alt)).toBeInTheDocument();
-  const image = screen.getByAltText(alt);
-  expect(image).toHaveAttribute('src', icon);
-});
+test.each(['', '/myapp'])(
+  'should render the brand, including app_root "%s"',
+  async app_root => {
+    staticAssetsPrefixMock.mockReturnValue(app_root);
+    useSelectorMock.mockReturnValue({ roles: user.roles });
+    const {
+      data: {
+        brand: { alt, icon },
+      },
+    } = mockedProps;
+    render(<Menu {...mockedProps} />, {
+      useRedux: true,
+      useQueryParams: true,
+      useRouter: true,
+      useTheme: true,
+    });
+    expect(await screen.findByAltText(alt)).toBeInTheDocument();
+    const image = screen.getByAltText(alt);
+    expect(image).toHaveAttribute('src', `${app_root}${icon}`);
+  },
+);
 
 test('should render the environment tag', async () => {
   useSelectorMock.mockReturnValue({ roles: user.roles });

superset-frontend/src/features/home/Menu.tsx

@@ -18,6 +18,8 @@
  */
 import { useState, useEffect } from 'react';
 import { styled, css, useTheme } from '@apache-superset/core/ui';
+import { ensureStaticPrefix } from 'src/utils/assetUrl';
+import { ensureAppRoot } from 'src/utils/pathUtils';
 import { getUrlParam } from 'src/utils/urlUtils';
 import { MainNav, MenuItem } from '@superset-ui/core/components/Menu';
 import { Tooltip, Grid, Row, Col, Image } from '@superset-ui/core/components';
@@ -287,10 +289,10 @@ export function Menu({
     if (theme.brandLogoUrl) {
       link = (
         <StyledBrandWrapper margin={theme.brandLogoMargin}>
-          <StyledBrandLink href={theme.brandLogoHref}>
+          <StyledBrandLink href={ensureAppRoot(theme.brandLogoHref)}>
             <StyledImage
               preview={false}
-              src={theme.brandLogoUrl}
+              src={ensureStaticPrefix(theme.brandLogoUrl)}
               alt={theme.brandLogoAlt || 'Apache Superset'}
               height={theme.brandLogoHeight}
             />
@@ -303,17 +305,25 @@ export function Menu({
       // Kept as is for backwards compatibility with the old theme system / superset_config.py
       link = (
         <GenericLink className="navbar-brand" to={brand.path}>
-          <StyledImage preview={false} src={brand.icon} alt={brand.alt} />
+          <StyledImage
+            preview={false}
+            src={ensureStaticPrefix(brand.icon)}
+            alt={brand.alt}
+          />
         </GenericLink>
       );
     } else {
       link = (
         <Typography.Link
           className="navbar-brand"
-          href={brand.path}
+          href={ensureAppRoot(brand.path)}
           tabIndex={-1}
         >
-          <StyledImage preview={false} src={brand.icon} alt={brand.alt} />
+          <StyledImage
+            preview={false}
+            src={ensureStaticPrefix(brand.icon)}
+            alt={brand.alt}
+          />
         </Typography.Link>
       );
     }

superset-frontend/src/features/home/RightMenu.tsx

@@ -483,7 +483,7 @@ const RightMenu = ({
           userItems.push({
             key: 'info',
             label: (
-              <Typography.Link href={navbarRight.user_info_url}>
+              <Typography.Link href={ensureAppRoot(navbarRight.user_info_url)}>
                 {t('Info')}
               </Typography.Link>
             ),

superset-frontend/src/utils/assetUrl.test.ts

@@ -0,0 +1,48 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import * as getBootstrapData from 'src/utils/getBootstrapData';
+import { assetUrl, ensureStaticPrefix } from './assetUrl';
+
+const staticAssetsPrefixMock = jest.spyOn(
+  getBootstrapData,
+  'staticAssetsPrefix',
+);
+const resourcePath = '/endpoint/img.png';
+const absoluteResourcePath = `https://cdn.domain.com/static${resourcePath}`;
+
+beforeEach(() => {
+  staticAssetsPrefixMock.mockReturnValue('');
+});
+
+describe('assetUrl should prepend static asset prefix for relative paths', () => {
+  it.each(['', '/myapp'])("'%s' for relative path", app_root => {
+    staticAssetsPrefixMock.mockReturnValue(app_root);
+    expect(assetUrl(resourcePath)).toBe(`${app_root}${resourcePath}`);
+    expect(assetUrl(absoluteResourcePath)).toBe(
+      `${app_root}/${absoluteResourcePath}`,
+    );
+  });
+});
+
+describe('assetUrl should ignore static asset prefix for absolute URLs', () => {
+  it.each(['', '/myapp'])("'%s' for absolute url", app_root => {
+    staticAssetsPrefixMock.mockReturnValue(app_root);
+    expect(ensureStaticPrefix(absoluteResourcePath)).toBe(absoluteResourcePath);
+  });
+});

superset-frontend/src/utils/assetUrl.ts

@@ -23,6 +23,17 @@ import { staticAssetsPrefix } from 'src/utils/getBootstrapData';
  * defined in the bootstrap data
  * @param path A string path to a resource
  */
-export function assetUrl(path: string) {
+export function assetUrl(path: string): string {
   return `${staticAssetsPrefix()}${path.startsWith('/') ? path : `/${path}`}`;
 }
+
+/**
+ * Returns the path prepended with the staticAssetsPrefix if the string is a relative path else it returns
+ * the string as is.
+ * @param url_or_path A url or relative path to a resource
+ */
+export function ensureStaticPrefix(url_or_path: string): string {
+  if (url_or_path.startsWith('/')) return assetUrl(url_or_path);
+
+  return url_or_path;
+}

superset/security/manager.py

@@ -44,7 +44,6 @@ from flask_appbuilder.security.views import (
     PermissionViewModelView,
     ViewMenuModelView,
 )
-from flask_appbuilder.widgets import ListWidget
 from flask_babel import lazy_gettext as _
 from flask_login import AnonymousUserMixin, LoginManager
 from jwt.api_jwt import _jwt_global_obj
@@ -111,27 +110,6 @@ class DatabaseCatalogSchema(NamedTuple):
     schema: str
 
 
-class SupersetSecurityListWidget(ListWidget):  # pylint: disable=too-few-public-methods
-    """
-    Redeclaring to avoid circular imports
-    """
-
-    template = "superset/fab_overrides/list.html"
-
-
-class SupersetRoleListWidget(ListWidget):  # pylint: disable=too-few-public-methods
-    """
-    Role model view from FAB already uses a custom list widget override
-    So we override the override
-    """
-
-    template = "superset/fab_overrides/list_role.html"
-
-    def __init__(self, **kwargs: Any) -> None:
-        kwargs["appbuilder"] = current_app.appbuilder
-        super().__init__(**kwargs)
-
-
 class SupersetRoleApi(RoleApi):
     """
     Overriding the RoleApi to be able to delete roles with permissions
@@ -196,9 +174,6 @@ class SupersetUserApi(UserApi):
         item.roles = []
 
 
-PermissionViewModelView.list_widget = SupersetSecurityListWidget
-PermissionModelView.list_widget = SupersetSecurityListWidget
-
 # Limiting routes on FAB model views
 PermissionViewModelView.include_route_methods = {RouteMethod.LIST}
 PermissionModelView.include_route_methods = {RouteMethod.LIST}

superset/views/base.py

@@ -40,7 +40,6 @@ from flask_appbuilder.const import AUTH_OAUTH
 from flask_appbuilder.forms import DynamicForm
 from flask_appbuilder.models.sqla.filters import BaseFilter
 from flask_appbuilder.security.sqla.models import User
-from flask_appbuilder.widgets import ListWidget
 from flask_babel import get_locale, gettext as __
 from flask_jwt_extended.exceptions import NoAuthorizationError
 from flask_wtf.form import FlaskForm
@@ -626,13 +625,8 @@ def get_spa_template_context(
     }
 
 
-class SupersetListWidget(ListWidget):  # pylint: disable=too-few-public-methods
-    template = "superset/fab_overrides/list.html"
-
-
 class SupersetModelView(ModelView):
     page_size = 100
-    list_widget = SupersetListWidget
 
     def render_app_template(self) -> FlaskResponse:
         context = get_spa_template_context()