sadpandajoe commented on code in PR #39925:
URL: https://github.com/apache/superset/pull/39925#discussion_r3270376441
##########
superset-frontend/src/explore/components/useExploreAdditionalActionsMenu/DashboardsSubMenu.tsx:
##########
@@ -72,7 +72,7 @@ export const useDashboardsMenuItems = ({
<Link
target="_blank"
rel="noreferer noopener"
Review Comment:
Fixed in 1613e53aaf — `rel="noreferrer noopener"` so browsers honour the
token and the Referer header is suppressed.
##########
superset-frontend/src/features/home/SavedQueries.tsx:
##########
@@ -306,7 +305,7 @@ export const SavedQueries = ({
<CardStyles key={q.id}>
<ListViewCard
imgURL=""
- url={ensureAppRoot(`/sqllab?savedQueryId=${q.id}`)}
+ url={`/sqllab?savedQueryId=${q.id}`}
title={q.label}
Review Comment:
Fixed in 1613e53aaf — pass `linkComponent={Link}` so the Saved-Queries card
routes through react-router-dom (and its `basename`) instead of ListViewCard's
plain `<a href>` fallback.
##########
superset/views/core.py:
##########
@@ -134,6 +134,16 @@
class Superset(BaseSupersetView):
"""The base views for Superset!"""
+ # Flask-AppBuilder's BaseView auto-derives `route_base` from the class
+ # name, which would mount every @expose route under `/superset/...`. That
+ # collides catastrophically with subdirectory deployments: AppRootMiddle-
+ # ware strips the configured root from PATH_INFO and sets SCRIPT_NAME, so
+ # url_for emits `/superset/superset/...` (doubled), and the in-rule
+ # `/superset` prefix no longer matches the post-strip PATH_INFO — making
+ # the routes themselves unreachable. Mount at the root so the application
+ # root applies exactly once via SCRIPT_NAME / basename.
+ route_base = ""
+
Review Comment:
Fixed in 1613e53aaf — rebuild the redirect with `url_for("ExploreView.root",
...)` instead of `request.url.replace("/superset/explore", "/explore")`, so
SCRIPT_NAME is preserved under subdir deployments.
##########
superset-frontend/packages/superset-ui-core/src/connection/SupersetClientClass.ts:
##########
@@ -275,8 +275,23 @@ export default class SupersetClientClass {
const host = inputHost ?? this.host;
const cleanHost = host.slice(-1) === '/' ? host.slice(0, -1) : host; // no
backslash
+ // Strip a leading appRoot segment so callers that accidentally pre-prefix
+ // their endpoint (e.g. by wrapping with ensureAppRoot before passing to
the
+ // client) do not produce a doubled `/superset/superset/...` URL. The L2
+ // static invariant still flags this pattern as a migration issue; this is
+ // the runtime safety net.
+ let cleanEndpoint = endpoint;
+ const root = this.appRoot;
+ if (root) {
+ if (cleanEndpoint === root) {
+ cleanEndpoint = '';
+ } else if (cleanEndpoint.startsWith(`${root}/`)) {
+ cleanEndpoint = cleanEndpoint.slice(root.length);
+ }
Review Comment:
Fixed in 1613e53aaf — `getUrl()` now greedy-strips repeated leading appRoot
segments (mirroring `stripAppRoot`); contract test extended with multi-prefix
and segment-boundary cases.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
