codeant-ai-for-open-source[bot] commented on code in PR #40833:
URL: https://github.com/apache/superset/pull/40833#discussion_r3368070240
##########
superset-frontend/src/utils/navigationUtils.ts:
##########
@@ -30,7 +32,10 @@ export const navigateTo = (
'noopener noreferrer',
);
} else if (options?.assign) {
- window.location.assign(sanitizeUrl(ensureAppRoot(url)));
+ const sanitized = sanitizeUrl(ensureAppRoot(url));
+ if (pendingAssignUrl === sanitized) return;
+ pendingAssignUrl = sanitized;
+ window.location.assign(sanitized);
Review Comment:
**Suggestion:** The dedupe flag is written before navigation is guaranteed
to succeed, and it is never cleared on cancellation/failure. If
`window.location.assign` is blocked (for example by a `beforeunload`
confirmation where the user stays on the page), future clicks to the same URL
become permanent no-ops in that page session. Reset or roll back the pending
value when navigation does not actually complete. [cache]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ❌ New-dashboard button can stop navigating after canceled attempt.
- ⚠️ Same behavior possible for new-chart button on Welcome.
- ⚠️ Dedupe state stays stuck until full page reload.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Observe the navigation helper in
`superset-frontend/src/utils/navigationUtils.ts:24–41`, where `navigateTo()`
handles `{
assign: true }` by computing `sanitized`, checking `pendingAssignUrl`,
storing
`pendingAssignUrl = sanitized`, then calling
`window.location.assign(sanitized)` (lines
35–38).
2. From a caller such as
`superset-frontend/src/features/home/DashboardTable.tsx:204–206`,
click the "+ Dashboard" button, which calls `navigateTo('/dashboard/new', {
assign: true
})`; this sets `pendingAssignUrl` before attempting navigation.
3. In a realistic failure scenario, `window.location.assign` does not
complete
navigation—for example, in a unit/integration test where
`window.location.assign` is
mocked to throw, or in the browser when a `beforeunload` handler blocks
navigation and the
user chooses to stay on the page (the repo already registers `beforeunload`
for unsaved
changes in places like `src/dashboard/components/Dashboard.tsx:11–24` and
via the
`useBeforeUnload` hook in `src/hooks/useBeforeUnload/index.ts:26–13` and
`src/features/themes/ThemeModal.tsx:392–403`).
4. After that failed/canceled navigation, the page remains loaded and
`pendingAssignUrl`
is still set to the sanitized URL, so subsequent clicks to the same callers
(`DashboardTable` at 204–206, `ChartTable` at 203–205, or `DashboardList` at
`src/pages/DashboardList/index.tsx:732–739`) invoke `navigateTo()` again,
hit the `if
(pendingAssignUrl === sanitized) return;` guard, and permanently skip
`window.location.assign` for that URL in this page session.
```
</details>
[Fix in
Cursor](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=785d513cf20f4dbfa397cfa2ecf6e1f2&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
| [Fix in VSCode
Claude](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=785d513cf20f4dbfa397cfa2ecf6e1f2&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/utils/navigationUtils.ts
**Line:** 35:38
**Comment:**
*Cache: The dedupe flag is written before navigation is guaranteed to
succeed, and it is never cleared on cancellation/failure. If
`window.location.assign` is blocked (for example by a `beforeunload`
confirmation where the user stays on the page), future clicks to the same URL
become permanent no-ops in that page session. Reset or roll back the pending
value when navigation does not actually complete.
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%2F40833&comment_hash=68d2c6696d42b33cf6a85a5465ef3e9475d1ea50b384fc6d8896598cd1d9a34d&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40833&comment_hash=68d2c6696d42b33cf6a85a5465ef3e9475d1ea50b384fc6d8896598cd1d9a34d&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]
