fix: add subdirectory deployment support for app icon and reports urls (#35098)

Co-authored-by: Claude <noreply@anthropic.com>
Co-authored-by: Daniel Gaspar <danielvazgaspar@gmail.com>
This commit is contained in:
Elizabeth Thompson
2025-12-08 16:06:08 -08:00
committed by GitHub
parent 0131e542e9
commit b35b1d7633
13 changed files with 333 additions and 21 deletions

View File

@@ -41,6 +41,7 @@ import {
import { fDuration, extendedDayjs } from '@superset-ui/core/utils/dates';
import { SqlLabRootState } from 'src/SqlLab/types';
import { UserWithPermissionsAndRoles as User } from 'src/types/bootstrapTypes';
import { makeUrl } from 'src/utils/pathUtils';
import ResultSet from '../ResultSet';
import HighlightedSql from '../HighlightedSql';
import { StaticPosition, StyledTooltip } from './styles';
@@ -68,7 +69,7 @@ interface QueryTableProps {
}
const openQuery = (id: number) => {
const url = `/sqllab?queryId=${id}`;
const url = makeUrl(`/sqllab?queryId=${id}`);
window.open(url);
};

View File

@@ -85,8 +85,8 @@ import { Icons } from '@superset-ui/core/components/Icons';
import { findPermission } from 'src/utils/findPermission';
import { StreamingExportModal } from 'src/components/StreamingExportModal';
import { useStreamingExport } from 'src/components/StreamingExportModal/useStreamingExport';
import { ensureAppRoot } from 'src/utils/pathUtils';
import { useConfirmModal } from 'src/hooks/useConfirmModal';
import { makeUrl } from 'src/utils/pathUtils';
import ExploreCtasResultsButton from '../ExploreCtasResultsButton';
import ExploreResultsButton from '../ExploreResultsButton';
import HighlightedSql from '../HighlightedSql';
@@ -314,7 +314,7 @@ const ResultSet = ({
};
const getExportCsvUrl = (clientId: string) =>
ensureAppRoot(`/api/v1/sqllab/export/${clientId}/`);
makeUrl(`/api/v1/sqllab/export/${clientId}/`);
const handleCloseStreamingModal = () => {
cancelExport();

View File

@@ -72,6 +72,7 @@ import {
} from 'src/database/actions';
import Mousetrap from 'mousetrap';
import { clearDatasetCache } from 'src/utils/cachedSupersetGet';
import { makeUrl } from 'src/utils/pathUtils';
import { DatabaseSelector } from '../../../DatabaseSelector';
import CollectionTable from '../CollectionTable';
import Fieldset from '../Fieldset';
@@ -787,7 +788,7 @@ class DatasourceEditor extends PureComponent {
autorun: true,
isDataset: true,
});
return `/sqllab/?${queryParams.toString()}`;
return makeUrl(`/sqllab/?${queryParams.toString()}`);
}
openOnSqlLab() {

View File

@@ -39,6 +39,7 @@ import {
import { CopyToClipboard } from 'src/components';
import { RootState } from 'src/dashboard/types';
import { findPermission } from 'src/utils/findPermission';
import { makeUrl } from 'src/utils/pathUtils';
import CodeSyntaxHighlighter, {
SupportedLanguage,
preloadLanguages,
@@ -137,7 +138,9 @@ const ViewQuery: FC<ViewQueryProps> = props => {
if (domEvent.metaKey || domEvent.ctrlKey) {
domEvent.preventDefault();
window.open(
`/sqllab?datasourceKey=${datasource}&sql=${encodeURIComponent(currentSQL)}`,
makeUrl(
`/sqllab?datasourceKey=${datasource}&sql=${encodeURIComponent(currentSQL)}`,
),
'_blank',
);
} else {

View File

@@ -33,6 +33,7 @@ import { CheckboxChangeEvent } from '@superset-ui/core/components/Checkbox/types
import { useHistory } from 'react-router-dom';
import { setItem, LocalStorageKeys } from 'src/utils/localStorageHelpers';
import { makeUrl } from 'src/utils/pathUtils';
import Tabs from '@superset-ui/core/components/Tabs';
import {
Button,
@@ -1736,7 +1737,7 @@ const DatabaseModal: FunctionComponent<DatabaseModalProps> = ({
onClick={() => {
setLoading(true);
fetchAndSetDB();
redirectURL(`/sqllab?db=true`);
redirectURL(makeUrl(`/sqllab?db=true`));
}}
>
{t('Query data in SQL Lab')}

View File

@@ -24,6 +24,7 @@ import { TableTab } from 'src/views/CRUD/types';
import { t } from '@superset-ui/core';
import { styled } from '@apache-superset/core/ui';
import { navigateTo } from 'src/utils/navigationUtils';
import { makeUrl } from 'src/utils/pathUtils';
import { WelcomeTable } from './types';
const EmptyContainer = styled.div`
@@ -58,7 +59,7 @@ const REDIRECTS = {
create: {
[WelcomeTable.Charts]: '/chart/add',
[WelcomeTable.Dashboards]: '/dashboard/new',
[WelcomeTable.SavedQueries]: '/sqllab?new=true',
[WelcomeTable.SavedQueries]: makeUrl('/sqllab?new=true'),
},
viewAll: {
[WelcomeTable.Charts]: '/chart/list',

View File

@@ -33,7 +33,7 @@ import {
TelemetryPixel,
} from '@superset-ui/core/components';
import type { ItemType, MenuItem } from '@superset-ui/core/components/Menu';
import { ensureAppRoot } from 'src/utils/pathUtils';
import { ensureAppRoot, makeUrl } from 'src/utils/pathUtils';
import { findPermission } from 'src/utils/findPermission';
import { isUserAdmin } from 'src/dashboard/util/permissionUtils';
import {
@@ -201,7 +201,7 @@ const RightMenu = ({
},
{
label: t('SQL query'),
url: '/sqllab?new=true',
url: makeUrl('/sqllab?new=true'),
icon: <Icons.SearchOutlined data-test={`menu-item-${t('SQL query')}`} />,
perm: 'can_sqllab',
view: 'Superset',

View File

@@ -65,6 +65,7 @@ import copyTextToClipboard from 'src/utils/copy';
import { UserWithPermissionsAndRoles } from 'src/types/bootstrapTypes';
import SavedQueryPreviewModal from 'src/features/queries/SavedQueryPreviewModal';
import { findPermission } from 'src/utils/findPermission';
import { makeUrl } from 'src/utils/pathUtils';
const PAGE_SIZE = 25;
const PASSWORDS_NEEDED_MESSAGE = t(
@@ -222,7 +223,7 @@ function SavedQueryList({
name: t('Query'),
buttonStyle: 'primary',
onClick: () => {
history.push('/sqllab?new=true');
history.push(makeUrl('/sqllab?new=true'));
},
});
@@ -231,7 +232,9 @@ function SavedQueryList({
// Action methods
const openInSqlLab = (id: number, openInNewWindow: boolean) => {
copyTextToClipboard(() =>
Promise.resolve(`${window.location.origin}/sqllab?savedQueryId=${id}`),
Promise.resolve(
`${window.location.origin}${makeUrl(`/sqllab?savedQueryId=${id}`)}`,
),
)
.then(() => {
addSuccessToast(t('Link Copied!'));
@@ -240,9 +243,9 @@ function SavedQueryList({
addDangerToast(t('Sorry, your browser does not support copying.'));
});
if (openInNewWindow) {
window.open(`/sqllab?savedQueryId=${id}`);
window.open(makeUrl(`/sqllab?savedQueryId=${id}`));
} else {
history.push(`/sqllab?savedQueryId=${id}`);
history.push(makeUrl(`/sqllab?savedQueryId=${id}`));
}
};
@@ -335,7 +338,9 @@ function SavedQueryList({
row: {
original: { id, label },
},
}: any) => <Link to={`/sqllab?savedQueryId=${id}`}>{label}</Link>,
}: any) => (
<Link to={makeUrl(`/sqllab?savedQueryId=${id}`)}>{label}</Link>
),
id: 'label',
},
{

View File

@@ -0,0 +1,160 @@
/**
* 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.
*/
afterEach(() => {
// Clean up the DOM
document.body.innerHTML = '';
jest.resetModules();
});
test('ensureAppRoot should add application root prefix to path with default root', async () => {
document.body.innerHTML = '';
jest.resetModules();
const { ensureAppRoot } = await import('./pathUtils');
expect(ensureAppRoot('/sqllab')).toBe('/sqllab');
expect(ensureAppRoot('/api/v1/chart')).toBe('/api/v1/chart');
});
test('ensureAppRoot should add application root prefix to path with custom subdirectory', async () => {
const customData = {
common: {
application_root: '/superset/',
},
};
document.body.innerHTML = `<div id="app" data-bootstrap='${JSON.stringify(customData)}'></div>`;
jest.resetModules();
// Import getBootstrapData first to initialize the cache
await import('./getBootstrapData');
const { ensureAppRoot } = await import('./pathUtils');
expect(ensureAppRoot('/sqllab')).toBe('/superset/sqllab');
expect(ensureAppRoot('/api/v1/chart')).toBe('/superset/api/v1/chart');
});
test('ensureAppRoot should handle paths without leading slash', async () => {
const customData = {
common: {
application_root: '/superset/',
},
};
document.body.innerHTML = `<div id="app" data-bootstrap='${JSON.stringify(customData)}'></div>`;
jest.resetModules();
await import('./getBootstrapData');
const { ensureAppRoot } = await import('./pathUtils');
expect(ensureAppRoot('sqllab')).toBe('/superset/sqllab');
expect(ensureAppRoot('api/v1/chart')).toBe('/superset/api/v1/chart');
});
test('ensureAppRoot should handle paths with query strings', async () => {
const customData = {
common: {
application_root: '/superset/',
},
};
document.body.innerHTML = `<div id="app" data-bootstrap='${JSON.stringify(customData)}'></div>`;
jest.resetModules();
await import('./getBootstrapData');
const { ensureAppRoot } = await import('./pathUtils');
expect(ensureAppRoot('/sqllab?new=true')).toBe('/superset/sqllab?new=true');
expect(ensureAppRoot('/api/v1/chart/export/123/')).toBe(
'/superset/api/v1/chart/export/123/',
);
});
test('makeUrl should create URL with default application root', async () => {
document.body.innerHTML = '';
jest.resetModules();
const { makeUrl } = await import('./pathUtils');
expect(makeUrl('/sqllab')).toBe('/sqllab');
expect(makeUrl('/api/v1/chart')).toBe('/api/v1/chart');
});
test('makeUrl should create URL with subdirectory prefix', async () => {
const customData = {
common: {
application_root: '/superset/',
},
};
document.body.innerHTML = `<div id="app" data-bootstrap='${JSON.stringify(customData)}'></div>`;
jest.resetModules();
await import('./getBootstrapData');
const { makeUrl } = await import('./pathUtils');
expect(makeUrl('/sqllab')).toBe('/superset/sqllab');
expect(makeUrl('/sqllab?new=true')).toBe('/superset/sqllab?new=true');
expect(makeUrl('/api/v1/sqllab/export/123/')).toBe(
'/superset/api/v1/sqllab/export/123/',
);
});
test('makeUrl should handle paths without leading slash', async () => {
const customData = {
common: {
application_root: '/superset/',
},
};
document.body.innerHTML = `<div id="app" data-bootstrap='${JSON.stringify(customData)}'></div>`;
jest.resetModules();
await import('./getBootstrapData');
const { makeUrl } = await import('./pathUtils');
expect(makeUrl('sqllab?queryId=123')).toBe('/superset/sqllab?queryId=123');
});
test('makeUrl should work with different subdirectory paths', async () => {
const customData = {
common: {
application_root: '/my-app/superset/',
},
};
document.body.innerHTML = `<div id="app" data-bootstrap='${JSON.stringify(customData)}'></div>`;
jest.resetModules();
await import('./getBootstrapData');
const { makeUrl } = await import('./pathUtils');
expect(makeUrl('/sqllab')).toBe('/my-app/superset/sqllab');
expect(makeUrl('/dashboard/list')).toBe('/my-app/superset/dashboard/list');
});
test('makeUrl should handle URLs with anchors', async () => {
const customData = {
common: {
application_root: '/superset/',
},
};
document.body.innerHTML = `<div id="app" data-bootstrap='${JSON.stringify(customData)}'></div>`;
jest.resetModules();
await import('./getBootstrapData');
const { makeUrl } = await import('./pathUtils');
expect(makeUrl('/dashboard/123#anchor')).toBe(
'/superset/dashboard/123#anchor',
);
});

View File

@@ -26,3 +26,19 @@ import { applicationRoot } from 'src/utils/getBootstrapData';
export function ensureAppRoot(path: string): string {
return `${applicationRoot()}${path.startsWith('/') ? path : `/${path}`}`;
}
/**
* Creates a URL with the proper application root prefix for subdirectory deployments.
* Use this when constructing URLs for navigation, API calls, or file downloads.
*
* @param path - The path to convert to a full URL (e.g., '/sqllab', '/api/v1/chart/123')
* @returns The path prefixed with the application root (e.g., '/superset/sqllab')
*
* @example
* // In a subdirectory deployment at /superset
* makeUrl('/sqllab?new=true') // returns '/superset/sqllab?new=true'
* makeUrl('/api/v1/chart/export/123/') // returns '/superset/api/v1/chart/export/123/'
*/
export function makeUrl(path: string): string {
return ensureAppRoot(path);
}

View File

@@ -68,6 +68,22 @@ def create_app(
# value of app_root so things work out of the box
if not app.config["STATIC_ASSETS_PREFIX"]:
app.config["STATIC_ASSETS_PREFIX"] = app_root
# Prefix APP_ICON path with subdirectory root for subdirectory deployments
if (
app.config.get("APP_ICON", "").startswith("/static/")
and app_root != "/"
):
app.config["APP_ICON"] = f"{app_root}{app.config['APP_ICON']}"
# Also update theme tokens for subdirectory deployments
for theme_key in ("THEME_DEFAULT", "THEME_DARK"):
theme = app.config[theme_key]
token = theme.get("token", {})
# Update brandLogoUrl if it points to /static/
if token.get("brandLogoUrl", "").startswith("/static/"):
token["brandLogoUrl"] = f"{app_root}{token['brandLogoUrl']}"
# Update brandLogoHref if it's the default "/"
if token.get("brandLogoHref") == "/":
token["brandLogoHref"] = app_root
if app.config["APPLICATION_ROOT"] == "/":
app.config["APPLICATION_ROOT"] = app_root

View File

@@ -15,7 +15,6 @@
# specific language governing permissions and limitations
# under the License.
import urllib
from contextlib import nullcontext
from typing import Any
from urllib.parse import urlparse
@@ -33,13 +32,21 @@ def headless_url(path: str, user_friendly: bool = False) -> str:
def get_url_path(view: str, user_friendly: bool = False, **kwargs: Any) -> str:
if has_request_context():
request_context = nullcontext
else:
request_context = app.test_request_context
in_request_context = has_request_context()
with request_context():
return headless_url(url_for(view, **kwargs), user_friendly=user_friendly)
# When already in a request context, Flask's url_for respects SCRIPT_NAME from
# the WSGI environment, so the prefix is already included. Only add APPLICATION_ROOT
# prefix when creating a new request context.
if in_request_context:
url = url_for(view, **kwargs)
else:
with app.test_request_context():
url = url_for(view, **kwargs)
app_root = app.config.get("APPLICATION_ROOT", "/")
if app_root != "/" and not url.startswith(app_root):
url = app_root.rstrip("/") + url
return headless_url(url, user_friendly=user_friendly)
def modify_url_query(url: str, **kwargs: Any) -> str:

View File

@@ -0,0 +1,101 @@
# 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.
"""Tests for subdirectory deployment features."""
from unittest.mock import MagicMock
from werkzeug.test import EnvironBuilder
from superset.app import AppRootMiddleware
from tests.integration_tests.base_tests import SupersetTestCase
class TestSubdirectoryDeployments(SupersetTestCase):
"""Test subdirectory deployment features including middleware."""
def setUp(self):
super().setUp()
# AppRootMiddleware tests (core subdirectory deployment functionality)
def test_app_root_middleware_path_handling(self):
"""Test middleware correctly handles path prefixes."""
# Create a mock WSGI app
mock_app = MagicMock()
mock_app.return_value = [b"response"]
middleware = AppRootMiddleware(mock_app, "/superset")
# Test with correct prefix
environ = EnvironBuilder("/superset/dashboard").get_environ()
start_response = MagicMock()
result = list(middleware(environ, start_response))
# Should call the wrapped app
mock_app.assert_called_once()
called_environ = mock_app.call_args[0][0]
# PATH_INFO should be stripped of prefix
assert called_environ["PATH_INFO"] == "/dashboard"
# SCRIPT_NAME should be set to the prefix
assert called_environ["SCRIPT_NAME"] == "/superset"
assert result == [b"response"]
def test_app_root_middleware_wrong_path_returns_404(self):
"""Test middleware returns 404 for incorrect paths."""
# Create a mock WSGI app
mock_app = MagicMock()
middleware = AppRootMiddleware(mock_app, "/superset")
# Test with incorrect prefix
environ = EnvironBuilder("/wrong/path").get_environ()
start_response = MagicMock()
list(middleware(environ, start_response))
# Should not call the wrapped app
mock_app.assert_not_called()
# Should return 404 response
start_response.assert_called_once()
status = start_response.call_args[0][0]
assert "404" in status
def test_app_root_middleware_root_path_handling(self):
"""Test middleware handles root path correctly."""
# Create a mock WSGI app
mock_app = MagicMock()
mock_app.return_value = [b"response"]
middleware = AppRootMiddleware(mock_app, "/superset")
# Test with exact prefix path
environ = EnvironBuilder("/superset").get_environ()
start_response = MagicMock()
list(middleware(environ, start_response))
# Should call the wrapped app
mock_app.assert_called_once()
called_environ = mock_app.call_args[0][0]
# PATH_INFO should be empty
assert called_environ["PATH_INFO"] == ""
# SCRIPT_NAME should be set to the prefix
assert called_environ["SCRIPT_NAME"] == "/superset"