Skip to content

fix(sqllab): preserve query history after tab migration#41403

Draft
sadpandajoe wants to merge 2 commits into
masterfrom
fix/sqllab-query-history-tab-migration
Draft

fix(sqllab): preserve query history after tab migration#41403
sadpandajoe wants to merge 2 commits into
masterfrom
fix/sqllab-query-history-tab-migration

Conversation

@sadpandajoe

Copy link
Copy Markdown
Member

SUMMARY

Fixes the SQL Lab query history pane becoming empty after running a query in a new tab from Query History.

When a new SQL Lab tab is created locally and then persisted to the backend, the editor id changes from the temporary local id to the backend tab id. syncQueryEditor only migrated queries that were marked as inLocalStorage, so a just-started query from an autorun cloned tab could stay attached to the old local editor id. The Query History pane then filtered by the backend tab id and no longer found the query.

This updates tab sync to migrate all non-preview queries attached to the local editor id, and adds regression coverage for a running query that is not marked inLocalStorage.

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

N/A. Behavior-only SQL Lab state fix.

TESTING INSTRUCTIONS

  1. Enable SQL Lab backend persistence.
  2. Open SQL > Query History.
  3. Click Run query in a new tab for an existing query.
  4. Confirm the new SQL Lab tab keeps showing the query in the Query History pane after the tab is saved to the backend.

Automated/local checks run:

  • npm run test -- src/SqlLab/actions/sqlLab.test.ts --runTestsByPath
  • npm run lint -- src/SqlLab/actions/sqlLab.ts src/SqlLab/actions/sqlLab.test.ts
  • pre-commit run --files superset-frontend/src/SqlLab/actions/sqlLab.ts superset-frontend/src/SqlLab/actions/sqlLab.test.ts
  • git diff --check

Note: pre-commit run --all-files was run before pushing, but this local worktree did not complete green because of unrelated baseline/environment blockers outside this diff, including existing mypy/ruff failures in unrelated Python files and missing local all-files tooling/build prerequisites (yarn, helm-docs, and package build artifacts before local package build).

ADDITIONAL INFORMATION

  • Has associated issue: Shortcut SC-105345 / PPR-1271
  • Required feature flags: SQLLAB_BACKEND_PERSISTENCE
  • 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

@netlify

netlify Bot commented Jun 25, 2026

Copy link
Copy Markdown

Deploy Preview for superset-docs-preview ready!

Name Link
🔨 Latest commit a51f627
🔍 Latest deploy log https://app.netlify.com/projects/superset-docs-preview/deploys/6a3c76d8eb472b0008fe9858
😎 Deploy Preview https://deploy-preview-41403--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.

@codecov

codecov Bot commented Jun 25, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 64.37%. Comparing base (d6ede99) to head (a2241b1).
⚠️ Report is 53 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master   #41403      +/-   ##
==========================================
- Coverage   64.37%   64.37%   -0.01%     
==========================================
  Files        2653     2653              
  Lines      145152   145152              
  Branches    33491    33491              
==========================================
- Hits        93446    93440       -6     
- Misses      50014    50020       +6     
  Partials     1692     1692              
Flag Coverage Δ
javascript 68.66% <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.

Copilot AI 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.

Pull request overview

Fixes a SQL Lab state migration edge case where Query History could appear empty after running a query in a newly-cloned tab that transitions from a temporary local editor id to a persisted backend tab id.

Changes:

  • Update syncQueryEditor to migrate all non-preview queries associated with the local editor id (not just those marked inLocalStorage).
  • Add a regression test case covering a running query that is not marked inLocalStorage, ensuring it gets migrated during tab persistence.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
superset-frontend/src/SqlLab/actions/sqlLab.ts Expands query migration criteria during tab persistence to include non-inLocalStorage non-preview queries tied to the local editor id.
superset-frontend/src/SqlLab/actions/sqlLab.test.ts Extends syncQueryEditor thunk coverage to include migrating a running query that is not marked inLocalStorage.

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

Copy link
Copy Markdown
Contributor

The suggestion to update the syncQueryEditor test is appropriate. Currently, the test does not explicitly model preview queries with isDataPreview: true, which means the new exclusion logic (!query.isDataPreview) is not being verified against actual preview query objects. Updating the test to include a mock preview query (with isDataPreview: true and sqlEditorId: null) would ensure that preview queries are correctly excluded from migration, while still verifying that standard running queries are migrated as expected.

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

{
            ...query,
            id: 'previewQuery',
            sqlEditorId: null,
            isDataPreview: true,
          },

@pull-request-size pull-request-size Bot added size/M and removed size/S labels Jun 25, 2026
@sadpandajoe sadpandajoe requested a review from Copilot June 25, 2026 18:44

Copilot AI 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.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants