You are viewing a plain text version of this content. The canonical link for it is here.

Posted to notifications@superset.apache.org by "EnxDev (via GitHub)" <gi...@apache.org> on 2025/10/31 15:34:12 UTC

[PR] fix(view-in-sqllab): unable to open virtual dataset after discarding chart edits [superset]

EnxDev opened a new pull request, #35931:
URL: https://github.com/apache/superset/pull/35931

   <!---
   Please write the PR title following the conventions at https://www.conventionalcommits.org/en/v1.0.0/
   Example:
   fix(dashboard): load charts correctly
   -->
   
   ### SUMMARY
   <!--- Describe the change below, including rationale and design decisions -->
   **Problem**
   When modifying a chart and then trying to open the related dataset in SQL Lab without saving, the virtual dataset cannot be opened if you choose to discard the chart edits.
   
   **Solution**
   The navigation hook responsible for handling redirects was only pushing the pathname, causing the component to lose the state (which included the `requestedQuery`).
   The fix ensures that both the pathname and state are preserved when redirecting.
   ### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
   <!--- Skip this if not applicable -->
   - Before
   https://github.com/user-attachments/assets/9be29bea-a1c4-4f8b-8e73-f6d133998c75
   
   - After
   https://github.com/user-attachments/assets/e2d0aae5-1183-49d6-8f1e-6d100ea800d6
   
   ### TESTING INSTRUCTIONS
   
   <!--- Required! What steps can be taken to manually verify the changes? -->
   1) Open chart
   2) Edit chart
   3) click “view in sql lab” from the dataset three dot menu
   4) Warning about losing changes pops up -> click discard
   5) SQL lab opens but the virtual dataset doesn’t load
   
   ### ADDITIONAL INFORMATION
   <!--- Check any relevant boxes with "x" -->
   <!--- HINT: Include "Fixes #nnn" if you are fixing an existing issue -->
   - [ ] Has associated issue:
   - [ ] Required feature flags:
   - [ ] 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(view-in-sqllab): unable to open virtual dataset after discarding chart edits [superset]

Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.

bito-code-review[bot] commented on code in PR #35931:
URL: https://github.com/apache/superset/pull/35931#discussion_r2481848653


##########
superset-frontend/src/hooks/useUnsavedChangesPrompt/index.ts:
##########
@@ -69,15 +70,21 @@ export const useUnsavedChangesPrompt = ({
   }, [onSave]);
 
   const blockCallback = useCallback(
-    ({ pathname }: { pathname: string }) => {
+    ({
+      pathname,
+      state,
+    }: {
+      pathname: Location['state'];

Review Comment:
   
   <div>
   
   
   <div id="suggestion">
   <div id="issue"><b>Incorrect blockCallback parameter types</b></div>
   <div id="fix">
   
   The type annotations for the `blockCallback` parameters are swapped: `pathname` is incorrectly typed as `Location['state']` (which is `any`) and `state` as `Path['pathname']` (which is `string`), but according to the `history` package types, `pathname` should be `string` and `state` should be `any`. This violates TypeScript type safety and the project's rule against improper `any` usage where avoidable, potentially causing runtime type mismatches during navigation blocking. The `history.push(pathname, state)` call downstream in `confirmNavigationRef` would receive incorrectly typed values, breaking navigation functionality. Correcting the types ensures the callback properly handles the location object from `history.block`.
   </div>
   
   <details>
   <summary>
   <b>Code suggestion</b>
   </summary>
   <blockquote>Check the AI-generated fix before applying</blockquote>
   <div id="code">
   
   
   ```
    -  const blockCallback = useCallback(
    -    ({
    -      pathname,
    -      state,
    -    }: {
    -      pathname: Location['state'];
    -      state: Path['pathname'];
    -    }) => {
    +  const blockCallback = useCallback(
    +    ({
    +      pathname,
    -      state,
    +    }: {
    +      pathname: string;
    +      state: any;
    +    }) => {
   ```
   
   </div>
   </details>
   
   
   </div>
   
   <details>
   <summary><b>Citations</b></summary>
   <ul>
   
   <li>
   Rule Violated: <a href="https://github.com/apache/superset/blob/b59ac4d/.cursor/rules/dev-standard.mdc#L16">dev-standard.mdc:16</a>
   </li>
   
   <li>
   Rule Violated: <a href="https://github.com/apache/superset/blob/b59ac4d/AGENTS.md#L57">AGENTS.md:57</a>
   </li>
   
   </ul>
   </details>
   
   
   
   
   <small><i>Code Review Run <a href=https://github.com/apache/superset/pull/35931#issuecomment-3473626599>#29e13f</a></i></small>
   </div>
   
   ---
   Should Bito avoid suggestions like this for future reviews? (<a href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
   - [ ] Yes, avoid them



-- 
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(view-in-sqllab): unable to open virtual dataset after discarding chart edits [superset]

Posted by "korbit-ai[bot] (via GitHub)" <gi...@apache.org>.

korbit-ai[bot] commented on code in PR #35931:
URL: https://github.com/apache/superset/pull/35931#discussion_r2481837251


##########
superset-frontend/src/hooks/useUnsavedChangesPrompt/index.ts:
##########
@@ -69,15 +70,21 @@ export const useUnsavedChangesPrompt = ({
   }, [onSave]);
 
   const blockCallback = useCallback(
-    ({ pathname }: { pathname: string }) => {
+    ({
+      pathname,
+      state,
+    }: {
+      pathname: Location['state'];
+      state: Path['pathname'];

Review Comment:
   ### Swapped type annotations for pathname and state parameters <sub>![category Functionality](https://img.shields.io/badge/Functionality-0284c7)</sub>
   
   <details>
     <summary>Tell me more</summary>
   
   ###### What is the issue?
   The type annotations for pathname and state parameters are swapped - pathname should be Path['pathname'] (string) and state should be Location['state'] (any).
   
   
   ###### Why this matters
   This type mismatch will cause TypeScript compilation errors and runtime issues when the function receives a string pathname but expects Location['state'], and receives state data but expects Path['pathname'].
   
   ###### Suggested change ∙ *Feature Preview*
   Correct the type annotations:
   ```typescript
   ({
     pathname,
     state,
   }: {
     pathname: Path['pathname'];
     state: Location['state'];
   }) => {
   ```
   
   
   ###### Provide feedback to improve future suggestions
   [![Nice Catch](https://img.shields.io/badge/👍%20Nice%20Catch-71BC78)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/a8da02ab-0bce-4982-803d-2e0923bdfa2d/upvote) [![Incorrect](https://img.shields.io/badge/👎%20Incorrect-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/a8da02ab-0bce-4982-803d-2e0923bdfa2d?what_not_true=true)  [![Not in Scope](https://img.shields.io/badge/👎%20Out%20of%20PR%20scope-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/a8da02ab-0bce-4982-803d-2e0923bdfa2d?what_out_of_scope=true) [![Not in coding standard](https://img.shields.io/badge/👎%20Not%20in%20our%20standards-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/a8da02ab-0bce-4982-803d-2e0923bdfa2d?what_not_in_standard=true) [![Other](https://img.shields.io/badge/👎%20Other-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/a8da02ab-0bce-4982-803d-2e0923bdfa2d)
   </details>
   
   <sub>
   
   💬 Looking for more details? Reply to this comment to chat with Korbit.
   </sub>
   
   <!--- korbi internal id:ef487b31-9fd0-4ab0-8063-8cccc59ce7d2 -->
   
   
   [](ef487b31-9fd0-4ab0-8063-8cccc59ce7d2)



-- 
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(view-in-sqllab): unable to open virtual dataset after discarding chart edits [superset]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.

github-actions[bot] commented on PR #35931:
URL: https://github.com/apache/superset/pull/35931#issuecomment-3473832968

   🎪 [Showtime](https://github.com/mistercrunch/superset-showtime) deployed environment on [GHA](https://github.com/apache/superset/actions/runs/18978277634) for [760c1bc](https://github.com/apache/superset/commit/760c1bc)
   
   • **Environment:** http://44.243.242.40:8080 (admin/admin)
   • **Lifetime:** 48h auto-cleanup
   • **Updates:** New commits create fresh environments automatically


-- 
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(view-in-sqllab): unable to open virtual dataset after discarding chart edits [superset]

Posted by "EnxDev (via GitHub)" <gi...@apache.org>.

EnxDev merged PR #35931:
URL: https://github.com/apache/superset/pull/35931


-- 
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(view-in-sqllab): unable to open virtual dataset after discarding chart edits [superset]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.

github-actions[bot] commented on PR #35931:
URL: https://github.com/apache/superset/pull/35931#issuecomment-3473785939

   🎪 [Showtime](https://github.com/mistercrunch/superset-showtime) is building environment on [GHA](https://github.com/apache/superset/actions/runs/18978277634) for [760c1bc](https://github.com/apache/superset/commit/760c1bc)


-- 
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(view-in-sqllab): unable to open virtual dataset after discarding chart edits [superset]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.

github-actions[bot] commented on PR #35931:
URL: https://github.com/apache/superset/pull/35931#issuecomment-3474980716

   🎪 [Showtime](https://github.com/mistercrunch/superset-showtime) deployed environment on [GHA](https://github.com/apache/superset/actions/runs/18985729145) for [48b0bbe](https://github.com/apache/superset/commit/48b0bbe)
   
   • **Environment:** http://52.37.68.90:8080 (admin/admin)
   • **Lifetime:** 48h auto-cleanup
   • **Updates:** New commits create fresh environments automatically


-- 
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(view-in-sqllab): unable to open virtual dataset after discarding chart edits [superset]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.

github-actions[bot] commented on PR #35931:
URL: https://github.com/apache/superset/pull/35931#issuecomment-3474961965

   🎪 [Showtime](https://github.com/mistercrunch/superset-showtime) is building environment on [GHA](https://github.com/apache/superset/actions/runs/18985729145) for [48b0bbe](https://github.com/apache/superset/commit/48b0bbe)


-- 
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(view-in-sqllab): unable to open virtual dataset after discarding chart edits [superset]

Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.

bito-code-review[bot] commented on PR #35931:
URL: https://github.com/apache/superset/pull/35931#issuecomment-3473626599

   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