You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by "aminghadersohi (via GitHub)" <gi...@apache.org> on 2026/08/14 21:14:44 UTC
[PR] fix(dataset): preserve legacy default dashboard URLs [superset]
aminghadersohi opened a new pull request, #43190:
URL: https://github.com/apache/superset/pull/43190
### SUMMARY
The dataset editor recommended the legacy `/superset/dashboard/{id}/` path even though the frontend router registers `/dashboard/:idOrSlug/`. Update the example to the registered route and normalize existing saved legacy dashboard Default URLs before passing them to React Router, preserving query strings and fragments.
This intentionally handles the known legacy dashboard path rather than adding a global fallback for every unmatched frontend URL. A general fallback needs broader routing ownership and behavior decisions because the existing route detector does not pattern-match parameterized routes; it is better handled separately than expanded into this focused compatibility fix.
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
**Before:** following the Default URL help text and clicking the dataset name navigated to `/superset/dashboard/<id>/`, leaving only the persistent navigation bar and a blank page.
**After:** the help text recommends `/dashboard/{id}/`. Existing datasets saved with `/superset/dashboard/<id>/` are transparently routed to `/dashboard/<id>/`, so the dashboard renders instead of the blank shell.
### TESTING INSTRUCTIONS
1. Edit a dataset and open the Settings tab; verify Default URL recommends `/dashboard/{id}/`.
2. Save `/superset/dashboard/<valid-dashboard-id>/?standalone=1` as its Default URL.
3. From the Datasets list, click the dataset name.
4. Verify the dashboard loads at `/dashboard/<valid-dashboard-id>/?standalone=1`.
5. Run:
- `npm test -- --runInBand src/pages/DatasetList/DatasetList.listview.test.tsx src/components/Datasource/components/DatasourceEditor/tests/DatasourceEditor.test.tsx`
Regression coverage verifies both the canonical help text and navigation of a saved legacy URL, including its query string and fragment.
### ADDITIONAL INFORMATION
- [ ] Has associated issue:
- [x] Required feature flags: `PREVENT_UNSAFE_DEFAULT_URLS_ON_DATASET` (enabled by default)
- [x] Changes UI
- [ ] Includes DB Migration (follow approval process in [SIP-59](https://github.com/apache/superset/issues/13351))
- [ ] Migration is atomic, supports rollback & is backwards-compatible
- [ ] Confirm DB migration upgrade and downgrade tested
- [ ] Runtime estimates and downtime expectations provided
- [ ] Introduces new feature or API
- [ ] Removes existing feature or API
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(dataset): preserve legacy default dashboard URLs [superset]
Posted by "aminghadersohi (via GitHub)" <gi...@apache.org>.
aminghadersohi commented on code in PR #43190:
URL: https://github.com/apache/superset/pull/43190#discussion_r3806926766
##########
superset-frontend/src/pages/DatasetList/DatasetList.subdirectory.test.tsx:
##########
@@ -115,6 +123,27 @@ test('explore link is single-prefixed under a subdirectory deployment', async ()
expect(exploreLink.getAttribute('href')).not.toContain('/superset/superset');
});
+test('legacy dashboard default URL uses the router basename once', async () => {
+ const dataset = {
+ ...mockDatasets[0],
+ explore_url: '/superset/dashboard/123/?standalone=1#section',
Review Comment:
Good catch — you are right that `stripAppRoot` was eating the only `/superset` prefix, so the assertion held with `index.tsx` reverted. Applied your suggestion in afc8841: the saved value is now the double-prefixed `${APP_ROOT}/superset/dashboard/123/?standalone=1#section`, which is what a subdirectory user pasting the full browser path would actually store. `stripAppRoot` removes the application root (single-pass, per its documented invariant), `normalizeLegacyDashboardUrl` removes the legacy prefix, and the basename re-adds the root once — same expected href, but the assertion now depends on the new normalization.
Note: I could not run the suite in this worktree (no `node_modules` installed here), so I am relying on your local both-directions run plus CI for confirmation.
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(dataset): preserve legacy default dashboard URLs [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #43190:
URL: https://github.com/apache/superset/pull/43190#issuecomment-5304172748
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
<details>
<summary>
Bito Usage Guide
</summary>
**Commands**
Type the following command in the pull request comment and save the comment.
- `/review` - Manually triggers a full AI review.
- `/pause` - Pauses automatic reviews on this pull request.
- `/resume` - Resumes automatic reviews.
- `/resolve` - Marks all Bito-posted review comments as resolved.
- `/abort` - Cancels all in-progress reviews.
Refer to the <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/available-commands" target="_blank" rel="noopener noreferrer">documentation</a> for additional commands.
**Configuration**
This repository uses `Superset` You can customize the agent settings <a href="https://alpha.bito.ai/home/ai-agents/code-review-agent" target="_blank" rel="noopener noreferrer">here</a> or contact your Bito workspace admin at evan@preset.io.
**Documentation & Help**
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/getting-started/install-run-using-bito-cloud/create-or-customize-an-agent-instance" target="_blank" rel="noopener noreferrer">Customize agent settings</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/implementing-custom-code-review-rules" target="_blank" rel="noopener noreferrer">Review rules</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent" target="_blank" rel="noopener noreferrer">General documentation</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/faqs" target="_blank" rel="noopener noreferrer">FAQ</a>
</details>
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(dataset): preserve legacy default dashboard URLs [superset]
Posted by "codeant-ai-for-open-source[bot] (via GitHub)" <gi...@apache.org>.
codeant-ai-for-open-source[bot] commented on code in PR #43190:
URL: https://github.com/apache/superset/pull/43190#discussion_r3806986901
##########
superset-frontend/src/pages/DatasetList/index.tsx:
##########
@@ -722,7 +725,9 @@ const DatasetList: FunctionComponent<DatasetListProps> = ({
// Router basename, which re-prefixes the root — so strip it here to
// avoid a doubled `/superset/superset/...`. External
// `default_endpoint` URLs pass through unchanged.
- const exploreTo = stripAppRoot(exploreURL);
+ const exploreTo = normalizeLegacyDashboardUrl(
+ stripAppRoot(exploreURL),
+ );
Review Comment:
**Suggestion:** The normalization is applied to every relative `explore_url`, but the backend returns a configured `default_endpoint` verbatim. A user-configured endpoint such as `/superset/dashboard/custom` is therefore indistinguishable from a legacy dashboard URL and is silently changed to `/dashboard/custom`, redirecting the dataset to a different destination. Restrict this rewrite to URLs known to represent the legacy dashboard route, or preserve explicitly configured endpoints separately from generated dashboard URLs. [api mismatch]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ❌ Custom relative dataset destinations are silently changed.
- ⚠️ Dataset links may reach the wrong dashboard or endpoint.
- ⚠️ Existing Default URL behavior becomes configuration-dependent.
```
</details>
[](https://docs.codeant.ai/cli/resolve-pr-comments-skill)
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset-frontend/src/pages/DatasetList/index.tsx
**Line:** 728:730
**Comment:**
*Api Mismatch: The normalization is applied to every relative `explore_url`, but the backend returns a configured `default_endpoint` verbatim. A user-configured endpoint such as `/superset/dashboard/custom` is therefore indistinguishable from a legacy dashboard URL and is silently changed to `/dashboard/custom`, redirecting the dataset to a different destination. Restrict this rewrite to URLs known to represent the legacy dashboard route, or preserve explicitly configured endpoints separately from generated dashboard URLs.
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%2F43190&comment_hash=5366b45012a70d603a609a5d2c6da30d231aadb4859ff0bcbcfdfe78a6da0ad1&reaction=like'>👍</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F43190&comment_hash=5366b45012a70d603a609a5d2c6da30d231aadb4859ff0bcbcfdfe78a6da0ad1&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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(dataset): preserve legacy default dashboard URLs [superset]
Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #43190:
URL: https://github.com/apache/superset/pull/43190#issuecomment-5299065513
## [Codecov](https://app.codecov.io/gh/apache/superset/pull/43190?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:white_check_mark: All modified and coverable lines are covered by tests.
:white_check_mark: Project coverage is 66.65%. Comparing base ([`f458721`](https://app.codecov.io/gh/apache/superset/commit/f4587218dd19d046c3e4d00063e7d27f8a2ed354?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`cd5e702`](https://app.codecov.io/gh/apache/superset/commit/cd5e7027ac117dfac06f9871958fbb678961bcc4?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
:warning: Report is 1 commits behind head on master.
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## master #43190 +/- ##
=======================================
Coverage 66.64% 66.65%
=======================================
Files 2873 2873
Lines 163625 163627 +2
Branches 37767 37767
=======================================
+ Hits 109056 109058 +2
Misses 52440 52440
Partials 2129 2129
```
| [Flag](https://app.codecov.io/gh/apache/superset/pull/43190/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [javascript](https://app.codecov.io/gh/apache/superset/pull/43190/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `73.76% <100.00%> (+<0.01%)` | :arrow_up: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#carryforward-flags-in-the-pull-request-comment) to find out more.
</details>
[:umbrella: View full report in Codecov by Harness](https://app.codecov.io/gh/apache/superset/pull/43190?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
<details><summary> :rocket: New features to boost your workflow: </summary>
- :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
- :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
</details>
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(dataset): preserve legacy default dashboard URLs [superset]
Posted by "gabotorresruiz (via GitHub)" <gi...@apache.org>.
gabotorresruiz commented on code in PR #43190:
URL: https://github.com/apache/superset/pull/43190#discussion_r3806865445
##########
superset-frontend/src/pages/DatasetList/DatasetList.subdirectory.test.tsx:
##########
@@ -115,6 +123,27 @@ test('explore link is single-prefixed under a subdirectory deployment', async ()
expect(exploreLink.getAttribute('href')).not.toContain('/superset/superset');
});
+test('legacy dashboard default URL uses the router basename once', async () => {
+ const dataset = {
+ ...mockDatasets[0],
+ explore_url: '/superset/dashboard/123/?standalone=1#section',
Review Comment:
Just a small NIT, not a blocker: this test passes even without the fix. I verified it on this branch with `index.tsx` reverted to master: `stripAppRoot` already removes `/superset` here because the mocked app root is also `/superset`, so `normalizeLegacyDashboardUrl` never sees the legacy prefix and the assertion stays green. It still pins the single-prefix invariant, which is valuable, but it does not cover the new regex under a subdirectory deployment.
If you change the input to the double-prefixed form a subdirectory user would actually have saved by pasting the full browser path, the test exercises the fix and fails without it (I ran both directions):
```ts
explore_url: '/superset/superset/dashboard/123/?standalone=1#section',
```
Same expected href, `stripAppRoot` eats the app root and the new normalization eats the legacy prefix.
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(dataset): preserve legacy default dashboard URLs [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #43190:
URL: https://github.com/apache/superset/pull/43190#issuecomment-5332445081
<!-- Bito Reply -->
The observation is accurate. In the current test setup, `stripAppRoot` removes the `/superset` prefix because the mocked `APP_ROOT` matches the prefix in the URL, causing `normalizeLegacyDashboardUrl` to receive a string that no longer starts with `/superset`. Consequently, the regex does not match, and the test passes without exercising the new normalization logic.
Using a double-prefixed URL like `/superset/superset/dashboard/123/...` would indeed force the test to exercise the `normalizeLegacyDashboardUrl` logic, as `stripAppRoot` would leave one `/superset` prefix remaining for the regex to catch.
**superset-frontend/src/pages/DatasetList/index.tsx**
```
// Keep saved Default URLs compatible with the prefix-free SPA route.
const normalizeLegacyDashboardUrl = (url: string) =>
url.replace(/^\/superset(?=\/dashboard(?:\/|$))/, '');
```
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(dataset): preserve legacy default dashboard URLs [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #43190:
URL: https://github.com/apache/superset/pull/43190#issuecomment-5298239535
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(dataset): preserve legacy default dashboard URLs [superset]
Posted by "netlify[bot] (via GitHub)" <gi...@apache.org>.
netlify[bot] commented on PR #43190:
URL: https://github.com/apache/superset/pull/43190#issuecomment-5332573029
### <span aria-hidden="true">✅</span> Deploy Preview for *superset-docs-preview* ready!
| Name | Link |
|:-:|------------------------|
|<span aria-hidden="true">🔨</span> Latest commit | afc8841ff387fa2fa4b5008f0ea3e7412e6a1aff |
|<span aria-hidden="true">🔍</span> Latest deploy log | https://app.netlify.com/projects/superset-docs-preview/deploys/6a84a7a0c37e8c000876f9ca |
|<span aria-hidden="true">😎</span> Deploy Preview | [https://deploy-preview-43190--superset-docs-preview.netlify.app](https://deploy-preview-43190--superset-docs-preview.netlify.app) |
|<span aria-hidden="true">📱</span> Preview on mobile | <details><summary> Toggle QR Code... </summary><br /><br /><br /><br />_Use your smartphone camera to open QR code link._</details> |
|<span aria-hidden="true">🤖</span> Make changes | [Run an agent on this branch](https://app.netlify.com/projects/superset-docs-preview/agent-runs#DctBCsMgEAXQu8y6EqFFTW_QY_w6YxRMFGMgofTucfc270e1lbV2ehM96Nuw-TiMNW1LBEvbS0xTSKdidOzSFUvAkbs6Wh6DpeZyfXgcA_eChfZPK85rrZ01Yfag_w0) |
---
<!-- [superset-docs-preview Preview](https://deploy-preview-43190--superset-docs-preview.netlify.app) -->
_To edit notification comments on pull requests, go to your [Netlify project configuration](https://app.netlify.com/projects/superset-docs-preview/configuration/notifications#deploy-notifications)._
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(dataset): preserve legacy default dashboard URLs [superset]
Posted by "aminghadersohi (via GitHub)" <gi...@apache.org>.
aminghadersohi merged PR #43190:
URL: https://github.com/apache/superset/pull/43190
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(dataset): preserve legacy default dashboard URLs [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #43190:
URL: https://github.com/apache/superset/pull/43190#issuecomment-5334447488
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
<details>
<summary>
Bito Usage Guide
</summary>
**Commands**
Type the following command in the pull request comment and save the comment.
- `/review` - Manually triggers a full AI review.
- `/pause` - Pauses automatic reviews on this pull request.
- `/resume` - Resumes automatic reviews.
- `/resolve` - Marks all Bito-posted review comments as resolved.
- `/abort` - Cancels all in-progress reviews.
Refer to the <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/available-commands" target="_blank" rel="noopener noreferrer">documentation</a> for additional commands.
**Configuration**
This repository uses `Superset` You can customize the agent settings <a href="https://alpha.bito.ai/home/ai-agents/code-review-agent" target="_blank" rel="noopener noreferrer">here</a> or contact your Bito workspace admin at evan@preset.io.
**Documentation & Help**
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/getting-started/install-run-using-bito-cloud/create-or-customize-an-agent-instance" target="_blank" rel="noopener noreferrer">Customize agent settings</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/implementing-custom-code-review-rules" target="_blank" rel="noopener noreferrer">Review rules</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent" target="_blank" rel="noopener noreferrer">General documentation</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/faqs" target="_blank" rel="noopener noreferrer">FAQ</a>
</details>
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org