fix(subdirectory): align SPA + sibling view routes after Superset.route_base=""

Round-4 follow-up to 756458f031. Four user-reported symptoms on /superset/
deployments — blank welcome, blank dashboard edit mode, doubled explore
copy-permalink, JSON 404 on dashboard discard — all trace to round-2
leftovers:

- superset-frontend/src/views/routes.tsx: six SPA routes still hard-coded
  the /superset/ prefix on top of <Router basename={applicationRoot()}>.
  React Router never matched them post-basename. Drop the prefix on
  welcome, file-handler, dashboard/:idOrSlug, explore/p, all_entities,
  and tags. isFrontendRoute stripAppRoots its input so menu URLs that
  carry the appRoot still resolve.
- Menu.tsx + RightMenu.tsx: SPA-route menu branches handed already-rooted
  URLs to <NavLink>/<Link>, doubling them via basename. Mirror the
  round-3 brand-link stripAppRoot pattern.
- superset/views/{explore,tags,all_entities}.py: three sibling view
  classes still declared route_base = "/superset/...". Mirror
  Superset.route_base="".
- superset/models/dashboard.py: Dashboard.url / get_url returned
  "/superset/dashboard/<id>/", producing doubled DashboardList row hrefs
  that caused the discard-edit 404. Return "/dashboard/<id>/" so
  downstream ensureAppRoot-aware consumers prepend exactly once.
- superset/mcp_service/dashboard/{schemas,tool/*}.py: seven sibling
  hard-codes of "/superset/dashboard/<id>/" updated identically.

Tests pin shapes for ExplorePermalinkView.permalink, TagModelView.list,
TaggedObjectsModelView.list, Dashboard.get_url, and the MCP dashboard
url emission. routes.test.tsx adds appRoot-prefixed lookup coverage and
documents the dict-lookup-no-pattern-match limitation.

UPDATING.md notes the new sibling route_base moves and the model URL
change alongside the round-2 Superset.route_base entry.

Live Playwright re-validation confirmed all four bugs fixed.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
Joe Li
2026-05-14 14:52:28 -07:00
parent b7c4d1e999
commit c531185d0a
18 changed files with 178 additions and 43 deletions

View File

@@ -28,6 +28,8 @@ assists people when migrating to a new version.
- **Breaking — `Superset` view class route prefix removed.** The `Superset` view in `superset/views/core.py` now declares `route_base = ""`, overriding Flask-AppBuilder's auto-derived `/superset` prefix. Routes that previously lived at `/superset/welcome/`, `/superset/dashboard/<id>/`, `/superset/dashboard/p/<key>/`, `/superset/explore/`, etc. now respond at `/welcome/`, `/dashboard/<id>/`, `/dashboard/p/<key>/`, `/explore/`, etc. Under subdirectory deployment (`SUPERSET_APP_ROOT=/superset`) the URLs are unchanged from end-user perspective — `AppRootMiddleware` re-applies the prefix via `SCRIPT_NAME`. Under root deployments, any external integration or bookmark that hard-codes `/superset/<endpoint>/` paths must be updated to drop the prefix. This fixes the doubled `/superset/superset/...` URLs that `url_for` emitted for these endpoints under subdirectory deployment and the related 404s on the routes themselves.
- **Breaking — Three sibling view classes route prefix removed.** Following the same rationale as the `Superset` class above, `ExplorePermalinkView` (`superset/views/explore.py`), `TagModelView`, and `TaggedObjectsModelView` (`superset/views/tags.py`, `superset/views/all_entities.py`) now mount at the application root rather than a hard-coded `/superset/...`. The user-visible URLs `/superset/explore/p/<key>/`, `/superset/tags/`, and `/superset/all_entities/` are unchanged under subdirectory deployment; under root deployments these views now serve `/explore/p/<key>/`, `/tags/`, and `/all_entities/`, so any external integration or bookmark must drop the `/superset/` prefix. `Dashboard.url` and `Dashboard.get_url` likewise return `/dashboard/<id>/` instead of the prior `/superset/dashboard/<id>/` literal so downstream consumers (DashboardList row hrefs, MCP service `dashboard_url`) emit a single, deployment-correct prefix.
### Granular Export Controls
A new feature flag `GRANULAR_EXPORT_CONTROLS` introduces three fine-grained permissions that replace the legacy `can_csv` permission:

View File

@@ -82,7 +82,7 @@ const SupersetTag = ({
{' '}
{id ? (
<Link
to={`/superset/all_entities/?id=${id}`}
to={`/all_entities/?id=${id}`}
target="_blank"
rel="noreferrer"
>

View File

@@ -244,10 +244,18 @@ export function Menu({
isFrontendRoute,
}: MenuObjectProps): MenuItem => {
if (url && isFrontendRoute) {
// `<Router basename={applicationRoot()}>` re-prepends the app root to
// `to`, so handing it the already-rooted `url` from bootstrap_data
// would render a doubled `/superset/superset/...` anchor. Strip the
// root first; mirrors the brand-link treatment below.
return {
key: label,
label: (
<NavLink role="button" to={url} activeClassName="is-active">
<NavLink
role="button"
to={stripAppRoot(url)}
activeClassName="is-active"
>
{label}
</NavLink>
),
@@ -269,7 +277,11 @@ export function Menu({
childItems.push({
key: `${child.label}`,
label: child.isFrontendRoute ? (
<NavLink to={child.url || ''} exact activeClassName="is-active">
<NavLink
to={stripAppRoot(child.url || '')}
exact
activeClassName="is-active"
>
{child.label}
</NavLink>
) : (

View File

@@ -44,7 +44,7 @@ import {
TelemetryPixel,
} from '@superset-ui/core/components';
import type { ItemType, MenuItem } from '@superset-ui/core/components/Menu';
import { ensureAppRoot } from 'src/utils/navigationUtils';
import { ensureAppRoot, stripAppRoot } from 'src/utils/navigationUtils';
import { isEmbedded } from 'src/dashboard/util/isEmbedded';
import { findPermission } from 'src/utils/findPermission';
import { isUserAdmin } from 'src/dashboard/util/permissionUtils';
@@ -409,7 +409,7 @@ const RightMenu = ({
items.push({
key: menu.label,
label: isFrontendRoute(menu.url) ? (
<Link to={menu.url || ''}>{menu.label}</Link>
<Link to={stripAppRoot(menu.url || '')}>{menu.label}</Link>
) : (
<Typography.Link href={ensureAppRoot(menu.url || '')}>
{menu.label}
@@ -425,7 +425,7 @@ const RightMenu = ({
items.push({
key: menu.label,
label: isFrontendRoute(menu.url) ? (
<Link to={menu.url || ''}>{menu.label}</Link>
<Link to={stripAppRoot(menu.url || '')}>{menu.label}</Link>
) : (
<Typography.Link href={ensureAppRoot(menu.url || '')}>
{menu.label}
@@ -460,7 +460,9 @@ const RightMenu = ({
sectionItems.push({
key: child.label,
label: isFrontendRoute(child.url) ? (
<Link to={child.url || ''}>{menuItemDisplay}</Link>
<Link to={stripAppRoot(child.url || '')}>
{menuItemDisplay}
</Link>
) : (
<Typography.Link
href={child.url || ''}

View File

@@ -31,4 +31,53 @@ describe('isFrontendRoute', () => {
test('returns false if a route does not match', () => {
expect(isFrontendRoute('/nonexistent/path/')).toBe(false);
});
test('does not pattern-match parameterised routes', () => {
// Documented limitation: lookup is a literal dictionary check, so a
// concrete `/dashboard/123/` does not match the registered
// `/dashboard/:idOrSlug/` key. Callers must accept the full-page-reload
// fallback for these URLs.
expect(isFrontendRoute('/dashboard/123/')).toBe(false);
});
});
test('routes used to mount the Superset.* SPA pages no longer carry a /superset prefix', () => {
// After `Superset.route_base = ""` (round-2 backend fix), the SPA paths
// must mirror the post-basename pathname seen by React Router. The legacy
// shapes still appearing here would mean Home or Dashboard renders blank
// under subdirectory deployment, exactly the round-4 user report.
const paths = routes.map(r => r.path);
expect(paths).toContain('/welcome/');
expect(paths).toContain('/dashboard/:idOrSlug/');
expect(paths).not.toContain('/superset/welcome/');
expect(paths).not.toContain('/superset/dashboard/:idOrSlug/');
});
test('isFrontendRoute accepts both bare-route and appRoot-prefixed menu URLs', async () => {
// Menu URLs flow from bootstrap_data via `url_for(...)` and already carry
// the active appRoot (e.g. `/superset/welcome/`). The check must succeed
// against both the bare route shape and the app-root-prefixed shape so
// Menu.tsx routes them through React Router instead of forcing a reload.
const previousBody = document.body.innerHTML;
try {
const bootstrapData = {
common: {
application_root: '/superset',
conf: { AUTH_USER_REGISTRATION: false },
},
};
document.body.innerHTML = `<div id="app" data-bootstrap='${JSON.stringify(bootstrapData)}'></div>`;
jest.resetModules();
const { isFrontendRoute: scopedIsFrontendRoute } = await import('./routes');
// Bare route shape (post-basename) — what React Router sees.
expect(scopedIsFrontendRoute('/welcome/')).toBe(true);
expect(scopedIsFrontendRoute('/dashboard/list/')).toBe(true);
// App-root-prefixed shape — what bootstrap_data emits via url_for(...).
expect(scopedIsFrontendRoute('/superset/welcome/')).toBe(true);
expect(scopedIsFrontendRoute('/superset/dashboard/list/')).toBe(true);
expect(scopedIsFrontendRoute('/superset/welcome/?edit=true')).toBe(true);
} finally {
document.body.innerHTML = previousBody;
jest.resetModules();
}
});

View File

@@ -26,6 +26,7 @@ import {
} from 'react';
import { isUserAdmin } from 'src/dashboard/util/permissionUtils';
import getBootstrapData from 'src/utils/getBootstrapData';
import { stripAppRoot } from 'src/utils/navigationUtils';
// not lazy loaded since this is the home page.
import Home from 'src/pages/Home';
@@ -216,11 +217,11 @@ export const routes: Routes = [
Component: Login,
},
{
path: '/superset/welcome/',
path: '/welcome/',
Component: Home,
},
{
path: '/superset/file-handler',
path: '/file-handler',
Component: FileHandler,
},
{
@@ -228,7 +229,7 @@ export const routes: Routes = [
Component: DashboardList,
},
{
path: '/superset/dashboard/:idOrSlug/',
path: '/dashboard/:idOrSlug/',
Component: Dashboard,
},
{
@@ -298,7 +299,7 @@ export const routes: Routes = [
Component: Chart,
},
{
path: '/superset/explore/p',
path: '/explore/p',
Component: Chart,
},
{
@@ -334,11 +335,11 @@ export const routes: Routes = [
if (isFeatureEnabled(FeatureFlag.TaggingSystem)) {
routes.push({
path: '/superset/all_entities/',
path: '/all_entities/',
Component: AllEntities,
});
routes.push({
path: '/superset/tags/',
path: '/tags/',
Component: Tags,
});
}
@@ -391,7 +392,16 @@ const frontEndRoutes: Record<string, boolean> = routes
export const isFrontendRoute = (path?: string): boolean => {
if (path) {
const basePath = path.split(/[?#]/)[0]; // strip out query params and link bookmarks
// Strip query / hash, then strip the application-root segment so menu URLs
// emitted by the backend (`url_for(...)` → `/<appRoot>/<route>`) match
// against the route table, which is keyed by post-basename paths.
//
// Note: this is a literal dictionary lookup, not a path-pattern match —
// parameterised routes such as `/dashboard/:idOrSlug/` are NOT matched
// by a concrete `/dashboard/123/` URL. Callers relying on that behaviour
// (e.g., the brand-link SPA-route check in `Menu.tsx`) accept a
// full-page-reload fallback for those URLs.
const basePath = stripAppRoot(path.split(/[?#]/)[0]);
return !!frontEndRoutes[basePath];
}
return false;

View File

@@ -841,7 +841,7 @@ def dashboard_serializer(dashboard: "Dashboard") -> DashboardInfo:
include_data_model_metadata = user_can_view_data_model_metadata()
base_url = get_superset_base_url()
relative_url = dashboard.url # e.g. "/superset/dashboard/{slug_or_id}/"
relative_url = dashboard.url # e.g. "/dashboard/{slug_or_id}/"
absolute_url = f"{base_url}{relative_url}" if relative_url else None
json_metadata_str = getattr(dashboard, "json_metadata", None)
position_json_str = getattr(dashboard, "position_json", None)
@@ -914,9 +914,7 @@ def serialize_dashboard_object(dashboard: Any) -> DashboardInfo:
slug = getattr(dashboard, "slug", None)
dashboard_url = None
if dashboard_id is not None:
dashboard_url = (
f"{get_superset_base_url()}/superset/dashboard/{slug or dashboard_id}/"
)
dashboard_url = f"{get_superset_base_url()}/dashboard/{slug or dashboard_id}/"
json_metadata_str = getattr(dashboard, "json_metadata", None)
position_json_str = getattr(dashboard, "position_json", None)

View File

@@ -513,7 +513,7 @@ def add_chart_to_existing_dashboard(
exc_info=True,
)
dashboard_url = (
f"{get_superset_base_url()}/superset/dashboard/{updated_dashboard.id}/"
f"{get_superset_base_url()}/dashboard/{updated_dashboard.id}/"
)
position_info = {
"row": row_key,
@@ -548,7 +548,7 @@ def add_chart_to_existing_dashboard(
created_on=updated_dashboard.created_on,
changed_on=updated_dashboard.changed_on,
uuid=str(updated_dashboard.uuid) if updated_dashboard.uuid else None,
url=f"{get_superset_base_url()}/superset/dashboard/{updated_dashboard.id}/",
url=f"{get_superset_base_url()}/dashboard/{updated_dashboard.id}/",
chart_count=len(updated_dashboard.slices),
tags=[
serialize_tag_object(tag)
@@ -568,9 +568,7 @@ def add_chart_to_existing_dashboard(
],
)
dashboard_url = (
f"{get_superset_base_url()}/superset/dashboard/{updated_dashboard.id}/"
)
dashboard_url = f"{get_superset_base_url()}/dashboard/{updated_dashboard.id}/"
logger.info(
"Added chart %s to dashboard %s ", request.chart_id, request.dashboard_id

View File

@@ -387,9 +387,7 @@ def generate_dashboard( # noqa: C901
"Database rollback failed during dashboard re-fetch error handling",
exc_info=True,
)
dashboard_url = (
f"{get_superset_base_url()}/superset/dashboard/{dashboard.id}/"
)
dashboard_url = f"{get_superset_base_url()}/dashboard/{dashboard.id}/"
return GenerateDashboardResponse(
dashboard=DashboardInfo(
id=dashboard.id,
@@ -419,7 +417,7 @@ def generate_dashboard( # noqa: C901
created_on=dashboard.created_on,
changed_on=dashboard.changed_on,
uuid=str(dashboard.uuid) if dashboard.uuid else None,
url=f"{get_superset_base_url()}/superset/dashboard/{dashboard.id}/",
url=f"{get_superset_base_url()}/dashboard/{dashboard.id}/",
chart_count=len(request.chart_ids),
tags=[
serialize_tag_object(tag)
@@ -439,7 +437,7 @@ def generate_dashboard( # noqa: C901
],
)
dashboard_url = f"{get_superset_base_url()}/superset/dashboard/{dashboard.id}/"
dashboard_url = f"{get_superset_base_url()}/dashboard/{dashboard.id}/"
logger.info(
"Created dashboard %s with %s charts", dashboard.id, len(request.chart_ids)

View File

@@ -197,12 +197,12 @@ class Dashboard(CoreDashboard, AuditMixinNullable, ImportExportMixin):
@property
def url(self) -> str:
return f"/superset/dashboard/{self.slug or self.id}/"
return f"/dashboard/{self.slug or self.id}/"
@staticmethod
def get_url(id_: int, slug: str | None = None) -> str:
# To be able to generate URL's without instantiating a Dashboard object
return f"/superset/dashboard/{slug or id_}/"
return f"/dashboard/{slug or id_}/"
@property
def datasources(self) -> set[BaseDatasource]:

View File

@@ -31,7 +31,7 @@ logger = logging.getLogger(__name__)
class TaggedObjectsModelView(SupersetModelView):
route_base = "/superset/all_entities"
route_base = "/all_entities"
datamodel = SQLAInterface(Tag)
class_permission_name = "Tags"
include_route_methods = {RouteMethod.LIST}

View File

@@ -37,7 +37,12 @@ class ExploreView(BaseSupersetView):
class ExplorePermalinkView(BaseSupersetView):
route_base = "/superset"
# Mirror `Superset.route_base = ""` (see `views/core.py`): Flask-AppBuilder
# auto-derives `/explorepermalink` from the class name, but the rule pattern
# already encodes the full path. Leaving the auto-derived value collides
# with `AppRootMiddleware` under subdirectory deployments and doubles the
# prefix emitted by `url_for("ExplorePermalinkView.permalink")`.
route_base = ""
class_permission_name = "Explore"
@expose("/explore/p/<key>/")

View File

@@ -35,7 +35,7 @@ logger = logging.getLogger(__name__)
class TagModelView(SupersetModelView):
route_base = "/superset/tags"
route_base = "/tags"
datamodel = SQLAInterface(Tag)
class_permission_name = "Tags"
include_route_methods = {"list"}

View File

@@ -126,7 +126,7 @@ class TestSerializeDashboardObject:
dashboard = _mock_dashboard(id=42, slug=None)
result = serialize_dashboard_object(dashboard)
assert result.url == "http://localhost:8088/superset/dashboard/42/"
assert result.url == "http://localhost:8088/dashboard/42/"
@patch("superset.mcp_service.utils.url_utils.get_superset_base_url")
def test_url_uses_slug_when_available(self, mock_base_url):
@@ -136,7 +136,7 @@ class TestSerializeDashboardObject:
dashboard = _mock_dashboard(id=42, slug="my-dashboard")
result = serialize_dashboard_object(dashboard)
assert result.url == "http://localhost:8088/superset/dashboard/my-dashboard/"
assert result.url == "http://localhost:8088/dashboard/my-dashboard/"
@patch("superset.mcp_service.utils.url_utils.get_superset_base_url")
def test_no_json_metadata_or_position_json_in_response(self, mock_base_url):
@@ -318,7 +318,7 @@ class TestSerializeDashboardObject:
"cross_filters_enabled": True,
}
dashboard = _mock_dashboard(id=1, slices=[chart])
dashboard.url = "/superset/dashboard/1/"
dashboard.url = "/dashboard/1/"
dashboard.json_metadata = json_dumps(metadata)
result = dashboard_serializer(dashboard)
@@ -377,7 +377,7 @@ class TestSerializeDashboardObject:
assert result.certified_by == _wrapped("Analytics Team")
assert result.certification_details == _wrapped("Certified by analytics")
assert result.slug == "safe-slug"
assert result.url == "http://localhost:8088/superset/dashboard/safe-slug/"
assert result.url == "http://localhost:8088/dashboard/safe-slug/"
assert result.uuid == "dashboard-uuid-7"
assert result.native_filters[0].id == "NATIVE_FILTER-abc123"
assert result.native_filters[0].name == _wrapped("Region Filter")

View File

@@ -292,6 +292,7 @@ async def test_successful_add(
assert content["error"] is None
assert content["permission_denied"] is False
assert content["dashboard_url"] is not None
assert "/superset/dashboard/1/" in content["dashboard_url"]
assert content["dashboard_url"].endswith("/dashboard/1/")
assert "/superset/superset/dashboard/" not in content["dashboard_url"]
assert content["position"] is not None
assert "chart_key" in content["position"]

View File

@@ -204,8 +204,10 @@ class TestGenerateDashboard:
== "Analytics Dashboard"
)
assert result.structured_content["dashboard"]["chart_count"] == 2
assert result.structured_content["dashboard_url"].endswith("/dashboard/10/")
assert (
"/superset/dashboard/10/" in result.structured_content["dashboard_url"]
"/superset/superset/dashboard/"
not in result.structured_content["dashboard_url"]
)
@patch("superset.models.dashboard.Dashboard")
@@ -650,8 +652,10 @@ class TestAddChartToExistingDashboard:
assert "chart_key" in result.structured_content["position"]
row_key = result.structured_content["position"]["row_key"]
assert row_key.startswith("ROW-")
assert result.structured_content["dashboard_url"].endswith("/dashboard/1/")
assert (
"/superset/dashboard/1/" in result.structured_content["dashboard_url"]
"/superset/superset/dashboard/"
not in result.structured_content["dashboard_url"]
)
call_args = mock_update_command.call_args[0][1]

View File

@@ -1188,9 +1188,8 @@ async def test_list_dashboards_sanitizes_dashboard_descriptions_and_filter_text(
]
assert dashboard_payload["slug"] == "quarterly-dashboard"
assert dashboard_payload["uuid"] == "uuid-quarterly-3"
assert dashboard_payload["url"].endswith(
"/superset/dashboard/quarterly-dashboard/"
)
assert dashboard_payload["url"].endswith("/dashboard/quarterly-dashboard/")
assert "/superset/superset/dashboard/" not in dashboard_payload["url"]
assert "uuid" in data["columns_requested"]
assert "slug" in data["columns_requested"]

View File

@@ -109,3 +109,60 @@ def test_dashboard_permalink_external_url_is_single_prefixed(
url = url_for("Superset.dashboard_permalink", key="abc123", _external=True)
assert url.endswith("/superset/dashboard/p/abc123/")
assert "/superset/superset/" not in url
def test_explore_permalink_url_is_single_prefixed(app_context: None) -> None:
"""ExplorePermalinkView previously hard-coded route_base = "/superset".
The `superset/explore/permalink/api.py` endpoint serves the clipboard URL
via `url_for("ExplorePermalinkView.permalink", _external=True)`. Under
SCRIPT_NAME=/superset that combination doubled to
`/superset/superset/explore/p/<key>/`. Mirroring the Superset.route_base=""
decision pins the single-prefix shape and prevents the doubled emission
from regressing.
"""
from flask import current_app
with current_app.test_request_context(
"/",
environ_overrides={"SCRIPT_NAME": "/superset"},
):
url = url_for("ExplorePermalinkView.permalink", key="abc123", _external=True)
assert url.endswith("/superset/explore/p/abc123/")
assert "/superset/superset/" not in url
def test_tag_views_urls_are_single_prefixed(app_context: None) -> None:
"""TagModelView and TaggedObjectsModelView previously hard-coded
`route_base = "/superset/tags"` / `/superset/all_entities"`.
Mirroring the Superset.route_base="" decision, the rule is now
`/tags/` / `/all_entities/` and `url_for` under SCRIPT_NAME=/superset
must carry the prefix exactly once.
"""
from flask import current_app
with current_app.test_request_context(
"/",
environ_overrides={"SCRIPT_NAME": "/superset"},
):
assert url_for("TagModelView.list") == "/superset/tags/"
assert url_for("TaggedObjectsModelView.list") == "/superset/all_entities/"
def test_dashboard_model_url_has_no_route_prefix() -> None:
"""`Dashboard.url` / `Dashboard.get_url` previously hard-coded
`/superset/dashboard/<id>/` as a string literal.
After `Superset.route_base = ""` the backend rule is `/dashboard/<id>/`,
so the literal was wrong on both root deployments (no matching rule) and
on subdirectory deployments (downstream callers re-applied the app root
and produced doubled paths in DashboardList row hrefs, ultimately leading
to the user-visible discard-edit 404). The method now returns a
prefix-free relative URL that downstream `ensureAppRoot`-aware helpers
can prepend exactly once.
"""
from superset.models.dashboard import Dashboard
assert Dashboard.get_url(11) == "/dashboard/11/"
assert Dashboard.get_url(11, "births") == "/dashboard/births/"