codeant-ai-for-open-source[bot] commented on code in PR #41119:
URL: https://github.com/apache/superset/pull/41119#discussion_r3422510642
##########
superset-frontend/src/features/home/Menu.tsx:
##########
@@ -294,7 +294,7 @@ export function Menu({
if (theme.brandLogoUrl) {
link = (
<StyledBrandWrapper margin={theme.brandLogoMargin}>
- <StyledBrandLink href={ensureAppRoot(theme.brandLogoHref)}>
+ <StyledBrandLink to={ensureAppRoot(theme.brandLogoHref)}>
Review Comment:
**Suggestion:** This change routes every non-external `brandLogoHref`
through React Router, but `GenericLink` only distinguishes external vs
non-external URLs, not frontend-route vs backend-route. If `brandLogoHref` is
configured to an internal backend URL that is not in `routes` (a previously
supported case with plain `<a>`), clicking the logo will do SPA navigation and
fail to load the server page. Keep client-side navigation only for known
frontend routes and fall back to a normal anchor for other internal paths.
[logic error]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ❌ Header logo fails navigating to configured backend URL.
- ⚠️ Custom themes cannot link logo to non-SPA pages.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Configure a custom theme so the brand logo uses an internal backend URL
that is not a
frontend route by setting `brandLogoHref` in `superset/config.py:1009` (under
`THEME_DEFAULT["token"]`) to a path like `/internal_backend/` instead of the
default
`'/'`.
2. Start the Superset frontend, which mounts `App` at
`superset-frontend/src/views/App.tsx:3-12`, where `<Router
basename={applicationRoot()}>`
wraps `<Menu data={bootstrapData.common.menu_data}
isFrontendRoute={isFrontendRoute} />`,
ensuring all navigation, including the brand logo, is handled by React
Router.
3. At runtime, `Menu` renders the brand logo via `renderBrand` in
`superset-frontend/src/features/home/Menu.tsx:53-66`, where
`StyledBrandLink` is a styled
`GenericLink` (defined in
`superset-frontend/src/components/GenericLink/index.tsx:24-49`)
and receives `to={ensureAppRoot(theme.brandLogoHref)}`; `ensureAppRoot`
(`superset-frontend/src/utils/pathUtils.ts:44-59`) turns
`/internal_backend/` into an
app-root-prefixed path like `/superset/internal_backend/`, which
`GenericLink` treats as
non-external and routes through `<Link>`.
4. Load any frontend page (e.g. `/superset/welcome/` from `routes` in
`superset-frontend/src/views/routes.tsx:197-224`), then click the header
logo; React
Router navigates SPA-style to `/superset/internal_backend/`, but because
this path is not
present in the `routes` array or `isFrontendRoute` map
(`routes.tsx:197-137`), no
`<Route>` matches, resulting in a blank or incorrect frontend view and
preventing the
intended backend page from ever loading.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=0f1d19c66b2c4ed89419d4c58c62aa23&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
[](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=0f1d19c66b2c4ed89419d4c58c62aa23&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset-frontend/src/features/home/Menu.tsx
**Line:** 297:297
**Comment:**
*Logic Error: This change routes every non-external `brandLogoHref`
through React Router, but `GenericLink` only distinguishes external vs
non-external URLs, not frontend-route vs backend-route. If `brandLogoHref` is
configured to an internal backend URL that is not in `routes` (a previously
supported case with plain `<a>`), clicking the logo will do SPA navigation and
fail to load the server page. Keep client-side navigation only for known
frontend routes and fall back to a normal anchor for other internal paths.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask
user if the user wants to fix the rest of the comments as well. if said yes,
then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41119&comment_hash=f9052b47524c2369b3a349076f0c81f70cb6248991b3cd0646db8f994cb7d74c&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41119&comment_hash=f9052b47524c2369b3a349076f0c81f70cb6248991b3cd0646db8f994cb7d74c&reaction=dislike'>👎</a>
--
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]
