fix(menu): Use app prefix in Menu brand icon url

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

@@ -21,6 +21,7 @@ import fetchMock from 'fetch-mock';
 import { render, screen, userEvent } from 'spec/helpers/testing-library';
 import setupExtensions from 'src/setup/setupExtensions';
 import { getExtensionsRegistry } from '@superset-ui/core';
+import * as getBootstrapData from 'src/utils/getBootstrapData';
 import { Menu } from './Menu';
 
 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 app root
+  staticAssetsPrefixMock.mockReturnValue('');
 });
 
 test('should render', async () => {
@@ -272,22 +279,25 @@ 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,
+describe('should render the brand', () => {
+  it.each(['', '/myapp'])("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}`);
   });
-  expect(await screen.findByAltText(alt)).toBeInTheDocument();
-  const image = screen.getByAltText(alt);
-  expect(image).toHaveAttribute('src', icon);
 });
 
 test('should render the environment tag', async () => {

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

@@ -19,6 +19,7 @@
 import { useState, useEffect } from 'react';
 import { styled, css, useTheme } from '@superset-ui/core';
 import { debounce } from 'lodash';
+import { assetUrlIf } from 'src/utils/assetUrl';
 import { getUrlParam } from 'src/utils/urlUtils';
 import { MainNav, MenuMode } from '@superset-ui/core/components/Menu';
 import { Tooltip, Grid, Row, Col, Image } from '@superset-ui/core/components';
@@ -285,7 +286,7 @@ export function Menu({
         >
           <Image
             preview={false}
-            src={theme.brandLogoUrl}
+            src={assetUrlIf(theme.brandLogoUrl)}
             alt={theme.brandLogoAlt || 'Apache Superset'}
           />
         </Typography.Link>
@@ -296,7 +297,7 @@ 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}>
-          <Image preview={false} src={brand.icon} alt={brand.alt} />
+          <Image preview={false} src={assetUrlIf(brand.icon)} alt={brand.alt} />
         </GenericLink>
       );
     } else {
@@ -306,7 +307,7 @@ export function Menu({
           href={brand.path}
           tabIndex={-1}
         >
-          <Image preview={false} src={brand.icon} alt={brand.alt} />
+          <Image preview={false} src={assetUrlIf(brand.icon)} alt={brand.alt} />
         </Typography.Link>
       );
     }

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

@@ -0,0 +1,50 @@
+/**
+ * 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, assetUrlIf } from './assetUrl';
+
+const staticAssetsPrefixMock = jest.spyOn(
+  getBootstrapData,
+  'staticAssetsPrefix',
+);
+const resourcePath = '/endpoint/img.png';
+const absoluteResourcePath = `https://cdn.domain.com/static${resourcePath}`;
+
+beforeEach(() => {
+  // Clear app root
+  staticAssetsPrefixMock.mockReturnValue('');
+});
+
+describe('assetUrl should prepend static asset prefix', () => {
+  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('assetUrlIf should ignore static asset prefix', () => {
+  it.each(['', '/myapp'])("'%s' for absolute url", app_root => {
+    staticAssetsPrefixMock.mockReturnValue(app_root);
+    expect(assetUrlIf(resourcePath)).toBe(`${app_root}${resourcePath}`);
+    expect(assetUrlIf(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 stirng 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 assetUrlIf(url_or_path: string): string {
+  if (URL.canParse(url_or_path)) return url_or_path;
+
+  return assetUrl(url_or_path);
+}