Skip to content

fix(sqllab): Fall back to empty editor tab when linked query no longer exists#42241

Merged
alexandrusoare merged 8 commits into
masterfrom
alexandrusoare/fix/sqllab-delete-tab
Jul 22, 2026
Merged

fix(sqllab): Fall back to empty editor tab when linked query no longer exists#42241
alexandrusoare merged 8 commits into
masterfrom
alexandrusoare/fix/sqllab-delete-tab

Conversation

@alexandrusoare

Copy link
Copy Markdown
Contributor

SUMMARY

When a user opens a SQL Lab link pointing to a deleted resource (/sqllab/?savedQueryId=<id>, permalink, stored query, query, or datasource), the fetch returns 404 but the UI never exits its loading state — the page is stuck on a skeleton indefinitely.

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

TESTING INSTRUCTIONS

ADDITIONAL INFORMATION

  • Has associated issue:
  • Required feature flags:
  • Changes UI
  • Includes DB Migration (follow approval process in SIP-59)
    • 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

@dosubot dosubot Bot added change:frontend Requires changing the frontend sqllab Namespace | Anything related to the SQL Lab labels Jul 20, 2026
@bito-code-review

bito-code-review Bot commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

Code Review Agent Run #8d2ada

Actionable Suggestions - 0
Review Details
  • Files reviewed - 2 · Commit Range: 2abf5fd..2abf5fd
    • superset-frontend/src/SqlLab/actions/sqlLab.test.ts
    • superset-frontend/src/SqlLab/actions/sqlLab.ts
  • Files skipped - 0
  • Tools
    • Whispers (Secret Scanner) - ✔︎ Successful
    • Detect-secrets (Secret Scanner) - ✔︎ Successful
    • Eslint (Linter) - ✔︎ Successful

Bito Usage Guide

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 documentation for additional commands.

Configuration

This repository uses Superset You can customize the agent settings here or contact your Bito workspace admin at evan@preset.io.

Documentation & Help

AI Code Review powered by Bito Logo

Comment thread superset-frontend/src/SqlLab/actions/sqlLab.test.ts
@bito-code-review

Copy link
Copy Markdown
Contributor

The flagged issue is correct. Because the store is defined in the describe scope, actions dispatched in previous tests persist, leading to potential false positives in assertions. To resolve this, you should recreate the store for each test using beforeEach.

Here is the corrected implementation for superset-frontend/src/SqlLab/actions/sqlLab.test.ts:

  describe('popSavedQuery', () => {
    const supersetClientGetSpy = jest.spyOn(SupersetClient, 'get');
    let store: any;

    beforeEach(() => {
      store = mockStore(initialState);
    });

There are no other comments on this PR to address. Would you like me to review any other parts of the code?

superset-frontend/src/SqlLab/actions/sqlLab.test.ts

const supersetClientGetSpy = jest.spyOn(SupersetClient, 'get');
    let store: any;

    beforeEach(() => {
      store = mockStore(initialState);
    });

@codecov

codecov Bot commented Jul 20, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 65.25%. Comparing base (bafe5ad) to head (6586682).

Additional details and impacted files
@@            Coverage Diff             @@
##           master   #42241      +/-   ##
==========================================
- Coverage   65.27%   65.25%   -0.03%     
==========================================
  Files        2769     2768       -1     
  Lines      156339   156201     -138     
  Branches    35787    35771      -16     
==========================================
- Hits       102055   101933     -122     
+ Misses      52317    52308       -9     
+ Partials     1967     1960       -7     
Flag Coverage Δ
javascript 70.94% <100.00%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Comment thread superset-frontend/src/SqlLab/actions/sqlLab.ts Outdated
Comment thread superset-frontend/src/SqlLab/actions/sqlLab.ts Outdated
Comment thread superset-frontend/src/SqlLab/actions/sqlLab.ts Outdated
Comment thread superset-frontend/src/SqlLab/actions/sqlLab.ts Outdated
Comment thread superset-frontend/src/SqlLab/actions/sqlLab.ts Outdated
Comment thread superset-frontend/src/SqlLab/actions/sqlLab.ts Outdated
Comment thread superset-frontend/src/SqlLab/actions/sqlLab.ts Outdated
describe('popSavedQuery', () => {
const supersetClientGetSpy = jest.spyOn(SupersetClient, 'get');
const store = mockStore({});
const store = mockStore(initialState);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only popSavedQuery has an error-path test. the other four (popPermalink, popStoredQuery, popQuery, popDatasourceQuery) get the same fallback but no assertion that addNewQueryEditor fires on failure. could we add coverage for those too, or a shared table-driven case, so the contract is locked for all five?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point, took care of it

@netlify

netlify Bot commented Jul 21, 2026

Copy link
Copy Markdown

Deploy Preview for superset-docs-preview ready!

Name Link
🔨 Latest commit f0dd1d7
🔍 Latest deploy log https://app.netlify.com/projects/superset-docs-preview/deploys/6a608ab303c83400094165b7
😎 Deploy Preview https://deploy-preview-42241--superset-docs-preview.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.
🤖 Make changes Run an agent on this branch

To edit notification comments on pull requests, go to your Netlify project configuration.

Comment thread superset-frontend/src/SqlLab/actions/sqlLab.ts Outdated
@bito-code-review

bito-code-review Bot commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

Code Review Agent Run #76e2b7

Actionable Suggestions - 0
Additional Suggestions - 1
  • superset-frontend/src/SqlLab/actions/sqlLab.test.ts - 1
    • Missing spy cleanup in test.each · Line 869-870
      Add an `afterEach(() => jest.restoreAllMocks())` after the `test.each` block to restore the spy created by `jest.spyOn` and prevent leakage into other tests.
Review Details
  • Files reviewed - 3 · Commit Range: 2abf5fd..6586682
    • superset-frontend/src/SqlLab/actions/sqlLab.test.ts
    • superset-frontend/src/SqlLab/actions/sqlLab.ts
    • superset-frontend/src/features/home/SavedQueries.test.tsx
  • Files skipped - 0
  • Tools
    • Whispers (Secret Scanner) - ✔︎ Successful
    • Detect-secrets (Secret Scanner) - ✔︎ Successful
    • Eslint (Linter) - ✔︎ Successful

Bito Usage Guide

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 documentation for additional commands.

Configuration

This repository uses Superset You can customize the agent settings here or contact your Bito workspace admin at evan@preset.io.

Documentation & Help

AI Code Review powered by Bito Logo

@pull-request-size pull-request-size Bot added size/L and removed size/M labels Jul 22, 2026
Comment thread superset-frontend/src/SqlLab/components/PopEditorTab/index.tsx
@bito-code-review

bito-code-review Bot commented Jul 22, 2026

Copy link
Copy Markdown
Contributor

Code Review Agent Run #6fd4f3

Actionable Suggestions - 0
Additional Suggestions - 2
  • superset-frontend/src/SqlLab/components/PopEditorTab/PopEditorTab.test.tsx - 1
    • Incomplete test assertion · Line 140-160
      Test name 'should clear loading state when saved query fetch fails' describes a specific behavior but the assertion only checks rendering. Add `expect(fetchMock.callHistory.calls('glob:*/api/v1/saved_query/999')).toHaveLength(1)` to verify the API call was made, consistent with other tests in this file.
  • superset-frontend/src/SqlLab/actions/sqlLab.test.ts - 1
    • Test name accuracy · Line 828-828
      The test name now accurately reflects the actual behavior. Verified against `sqlLab.ts` lines 1533, 1557, 1585, 1605, 1636 — error handlers dispatch only `addDangerToast`, not `ADD_QUERY_EDITOR`. No functional changes.
Review Details
  • Files reviewed - 4 · Commit Range: 6586682..6ed254f
    • superset-frontend/src/SqlLab/actions/sqlLab.test.ts
    • superset-frontend/src/SqlLab/actions/sqlLab.ts
    • superset-frontend/src/SqlLab/components/PopEditorTab/index.tsx
    • superset-frontend/src/SqlLab/components/PopEditorTab/PopEditorTab.test.tsx
  • Files skipped - 0
  • Tools
    • Whispers (Secret Scanner) - ✔︎ Successful
    • Detect-secrets (Secret Scanner) - ✔︎ Successful
    • Eslint (Linter) - ✔︎ Successful

Bito Usage Guide

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 documentation for additional commands.

Configuration

This repository uses Superset You can customize the agent settings here or contact your Bito workspace admin at evan@preset.io.

Documentation & Help

AI Code Review powered by Bito Logo

@EnxDev EnxDev left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@alexandrusoare
alexandrusoare merged commit b8f3b76 into master Jul 22, 2026
62 checks passed
@alexandrusoare
alexandrusoare deleted the alexandrusoare/fix/sqllab-delete-tab branch July 22, 2026 12:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

change:frontend Requires changing the frontend size/L sqllab Namespace | Anything related to the SQL Lab

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants