diff --git a/superset-frontend/src/SqlLab/components/ResultSet/index.tsx b/superset-frontend/src/SqlLab/components/ResultSet/index.tsx index 3297aa7b9467..e981a1b5b581 100644 --- a/superset-frontend/src/SqlLab/components/ResultSet/index.tsx +++ b/superset-frontend/src/SqlLab/components/ResultSet/index.tsx @@ -286,9 +286,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 { diff --git a/superset-frontend/src/explore/exploreUtils/index.js b/superset-frontend/src/explore/exploreUtils/index.js index d3bd62ef412a..41fd7ec16deb 100644 --- a/superset-frontend/src/explore/exploreUtils/index.js +++ b/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]; diff --git a/superset-frontend/src/features/home/Menu.test.tsx b/superset-frontend/src/features/home/Menu.test.tsx index 4dd01c66e1b1..4b7e744188e7 100644 --- a/superset-frontend/src/features/home/Menu.test.tsx +++ b/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(, { - 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(, { + 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 }); diff --git a/superset-frontend/src/features/home/Menu.tsx b/superset-frontend/src/features/home/Menu.tsx index 9d8c35341bb2..96270b03ecc3 100644 --- a/superset-frontend/src/features/home/Menu.tsx +++ b/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 = ( - + @@ -303,17 +305,25 @@ export function Menu({ // Kept as is for backwards compatibility with the old theme system / superset_config.py link = ( - + ); } else { link = ( - + ); } diff --git a/superset-frontend/src/features/home/RightMenu.tsx b/superset-frontend/src/features/home/RightMenu.tsx index 6222df3d04f7..aa0cd55d2711 100644 --- a/superset-frontend/src/features/home/RightMenu.tsx +++ b/superset-frontend/src/features/home/RightMenu.tsx @@ -479,7 +479,7 @@ const RightMenu = ({ userItems.push({ key: 'info', label: ( - + {t('Info')} ), diff --git a/superset-frontend/src/utils/assetUrl.test.ts b/superset-frontend/src/utils/assetUrl.test.ts new file mode 100644 index 000000000000..ac501899b7a1 --- /dev/null +++ b/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); + }); +}); diff --git a/superset-frontend/src/utils/assetUrl.ts b/superset-frontend/src/utils/assetUrl.ts index 5e834527838c..a7fc2553b50d 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 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; +} diff --git a/superset/security/manager.py b/superset/security/manager.py index fd758d235b1a..92e43392c243 100644 --- a/superset/security/manager.py +++ b/superset/security/manager.py @@ -43,7 +43,6 @@ 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 @@ -109,27 +108,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 @@ -170,9 +148,6 @@ def pre_delete(self, item: Model) -> None: 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} diff --git a/superset/views/base.py b/superset/views/base.py index bf493628ad5d..bfc2ac727564 100644 --- a/superset/views/base.py +++ b/superset/views/base.py @@ -39,7 +39,6 @@ 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 @@ -589,13 +588,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()