diff --git a/superset-frontend/src/features/home/Menu.test.tsx b/superset-frontend/src/features/home/Menu.test.tsx index 9b8102ee3811..0fe15c7d6e3c 100644 --- a/superset-frontend/src/features/home/Menu.test.tsx +++ b/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(, { - 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(, { + 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 () => { diff --git a/superset-frontend/src/features/home/Menu.tsx b/superset-frontend/src/features/home/Menu.tsx index b9705851e4cd..050701605446 100644 --- a/superset-frontend/src/features/home/Menu.tsx +++ b/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({ > {theme.brandLogoAlt @@ -296,7 +297,7 @@ export function Menu({ // Kept as is for backwards compatibility with the old theme system / superset_config.py link = ( - {brand.alt} + {brand.alt} ); } else { @@ -306,7 +307,7 @@ export function Menu({ href={brand.path} tabIndex={-1} > - {brand.alt} + {brand.alt} ); } diff --git a/superset-frontend/src/utils/assetUrl.test.ts b/superset-frontend/src/utils/assetUrl.test.ts new file mode 100644 index 000000000000..442560f0e0a6 --- /dev/null +++ b/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); + }); +}); diff --git a/superset-frontend/src/utils/assetUrl.ts b/superset-frontend/src/utils/assetUrl.ts index 5e834527838c..5daa9561c858 100644 --- a/superset-frontend/src/utils/assetUrl.ts +++ b/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_or_path.startsWith('/')) return assetUrl(url_or_path); + + return url_or_path; +}