From c267f365ef2305a1a711aad67667145e79d358df Mon Sep 17 00:00:00 2001 From: richardfn Date: Tue, 4 Nov 2025 21:20:45 -0300 Subject: [PATCH 1/4] fix(sqllab): prevent unwanted tab switching when autocompleting table names --- superset-frontend/src/SqlLab/actions/sqlLab.js | 7 ++++--- .../src/SqlLab/components/AceEditorWrapper/useKeywords.ts | 1 + superset-frontend/src/SqlLab/reducers/sqlLab.js | 4 +++- 3 files changed, 8 insertions(+), 4 deletions(-) diff --git a/superset-frontend/src/SqlLab/actions/sqlLab.js b/superset-frontend/src/SqlLab/actions/sqlLab.js index 9a672f594ea7..8cd99036848e 100644 --- a/superset-frontend/src/SqlLab/actions/sqlLab.js +++ b/superset-frontend/src/SqlLab/actions/sqlLab.js @@ -950,12 +950,13 @@ export function mergeTable(table, query, prepend) { return { type: MERGE_TABLE, table, query, prepend }; } -export function addTable(queryEditor, tableName, catalogName, schemaName) { +export function addTable(queryEditor, tableName, catalogName, schemaName, expanded = true) { return function (dispatch, getState) { const { dbId } = getUpToDateQuery(getState(), queryEditor, queryEditor.id); + const queryEditorId = queryEditor.tabViewId ?? queryEditor.id; const table = { dbId, - queryEditorId: queryEditor.tabViewId ?? queryEditor.id, + queryEditorId, catalog: catalogName, schema: schemaName, name: tableName, @@ -964,7 +965,7 @@ export function addTable(queryEditor, tableName, catalogName, schemaName) { mergeTable({ ...table, id: nanoid(11), - expanded: true, + expanded, }), ); }; diff --git a/superset-frontend/src/SqlLab/components/AceEditorWrapper/useKeywords.ts b/superset-frontend/src/SqlLab/components/AceEditorWrapper/useKeywords.ts index 548c74e463d6..1a8eb7b3684e 100644 --- a/superset-frontend/src/SqlLab/components/AceEditorWrapper/useKeywords.ts +++ b/superset-frontend/src/SqlLab/components/AceEditorWrapper/useKeywords.ts @@ -153,6 +153,7 @@ export function useKeywords( data.value, catalog, schema, + false, // Don't auto-expand/switch tabs when adding via autocomplete ), ); } diff --git a/superset-frontend/src/SqlLab/reducers/sqlLab.js b/superset-frontend/src/SqlLab/reducers/sqlLab.js index a1e0210d7900..4be0a5bb4da6 100644 --- a/superset-frontend/src/SqlLab/reducers/sqlLab.js +++ b/superset-frontend/src/SqlLab/reducers/sqlLab.js @@ -223,7 +223,9 @@ export default function sqlLabReducer(state = {}, action) { // for new table, associate Id of query for data preview at.dataPreviewQueryId = null; let newState = addToArr(state, 'tables', at, Boolean(action.prepend)); - newState.activeSouthPaneTab = at.id; + if (at.expanded) { + newState.activeSouthPaneTab = at.id; + } if (action.query) { newState = alterInArr(newState, 'tables', at, { dataPreviewQueryId: action.query.id, From 52eac70be81d2d80846c041d96acef0691254f21 Mon Sep 17 00:00:00 2001 From: richardfn Date: Tue, 4 Nov 2025 21:44:08 -0300 Subject: [PATCH 2/4] chore: rollback line change unrelated to the fix --- superset-frontend/src/SqlLab/actions/sqlLab.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/superset-frontend/src/SqlLab/actions/sqlLab.js b/superset-frontend/src/SqlLab/actions/sqlLab.js index 8cd99036848e..3ea6781cc2d0 100644 --- a/superset-frontend/src/SqlLab/actions/sqlLab.js +++ b/superset-frontend/src/SqlLab/actions/sqlLab.js @@ -953,10 +953,9 @@ export function mergeTable(table, query, prepend) { export function addTable(queryEditor, tableName, catalogName, schemaName, expanded = true) { return function (dispatch, getState) { const { dbId } = getUpToDateQuery(getState(), queryEditor, queryEditor.id); - const queryEditorId = queryEditor.tabViewId ?? queryEditor.id; const table = { dbId, - queryEditorId, + queryEditorId: queryEditor.tabViewId ?? queryEditor.id, catalog: catalogName, schema: schemaName, name: tableName, From d91cecd51317e969e1e9cb73f5ecd5dd5666f8bb Mon Sep 17 00:00:00 2001 From: richardfn Date: Mon, 10 Nov 2025 20:41:31 -0300 Subject: [PATCH 3/4] test(sqllab): add activeSouthPaneTab tests and eliminate code duplication --- .../src/SqlLab/reducers/sqlLab.js | 8 +- .../src/SqlLab/reducers/sqlLab.test.js | 87 +++++++++++++++++++ 2 files changed, 91 insertions(+), 4 deletions(-) diff --git a/superset-frontend/src/SqlLab/reducers/sqlLab.js b/superset-frontend/src/SqlLab/reducers/sqlLab.js index 4be0a5bb4da6..3469036da4db 100644 --- a/superset-frontend/src/SqlLab/reducers/sqlLab.js +++ b/superset-frontend/src/SqlLab/reducers/sqlLab.js @@ -222,10 +222,10 @@ export default function sqlLabReducer(state = {}, action) { } // for new table, associate Id of query for data preview at.dataPreviewQueryId = null; - let newState = addToArr(state, 'tables', at, Boolean(action.prepend)); - if (at.expanded) { - newState.activeSouthPaneTab = at.id; - } + let newState = { + ...addToArr(state, 'tables', at, Boolean(action.prepend)), + ...(at.expanded && { activeSouthPaneTab: at.id }), + }; if (action.query) { newState = alterInArr(newState, 'tables', at, { dataPreviewQueryId: action.query.id, diff --git a/superset-frontend/src/SqlLab/reducers/sqlLab.test.js b/superset-frontend/src/SqlLab/reducers/sqlLab.test.js index c8fd25faf320..fedca09e391d 100644 --- a/superset-frontend/src/SqlLab/reducers/sqlLab.test.js +++ b/superset-frontend/src/SqlLab/reducers/sqlLab.test.js @@ -370,6 +370,93 @@ describe('sqlLabReducer', () => { newState = sqlLabReducer(newState, action); expect(newState.tables).toHaveLength(0); }); + test('should set activeSouthPaneTab when adding expanded table', () => { + const expandedTable = { + ...table, + id: 'expanded_table_id', + name: 'expanded_table', + expanded: true, + }; + const action = { + type: actions.MERGE_TABLE, + table: expandedTable, + }; + newState = sqlLabReducer(initialState, action); + expect(newState.tables).toHaveLength(1); + expect(newState.activeSouthPaneTab).toBe(expandedTable.id); + }); + test('should not set activeSouthPaneTab when adding collapsed table', () => { + const collapsedTable = { + ...table, + id: 'collapsed_table_id', + name: 'collapsed_table', + expanded: false, + }; + const action = { + type: actions.MERGE_TABLE, + table: collapsedTable, + }; + newState = sqlLabReducer(initialState, action); + expect(newState.tables).toHaveLength(1); + expect(newState.activeSouthPaneTab).toBe(initialState.activeSouthPaneTab); + }); + test('should set activeSouthPaneTab when merging existing table with expanded=true', () => { + // First add a table with expanded=false + const collapsedTable = { + ...table, + id: 'existing_table_id', + name: 'existing_table', + expanded: false, + }; + const addAction = { + type: actions.MERGE_TABLE, + table: collapsedTable, + }; + newState = sqlLabReducer(initialState, addAction); + const previousActiveSouthPaneTab = newState.activeSouthPaneTab; + + // Now merge the same table with expanded=true + const expandedTable = { + ...collapsedTable, + expanded: true, + }; + const mergeAction = { + type: actions.MERGE_TABLE, + table: expandedTable, + }; + newState = sqlLabReducer(newState, mergeAction); + expect(newState.tables).toHaveLength(1); + expect(newState.activeSouthPaneTab).toBe(expandedTable.id); + expect(newState.activeSouthPaneTab).not.toBe(previousActiveSouthPaneTab); + }); + test('should not set activeSouthPaneTab when merging existing table with expanded=false', () => { + // First add a table with expanded=true + const expandedTable = { + ...table, + id: 'existing_table_id_2', + name: 'existing_table_2', + expanded: true, + }; + const addAction = { + type: actions.MERGE_TABLE, + table: expandedTable, + }; + newState = sqlLabReducer(initialState, addAction); + expect(newState.activeSouthPaneTab).toBe(expandedTable.id); + + // Now merge the same table with expanded=false + const collapsedTable = { + ...expandedTable, + expanded: false, + }; + const mergeAction = { + type: actions.MERGE_TABLE, + table: collapsedTable, + }; + newState = sqlLabReducer(newState, mergeAction); + expect(newState.tables).toHaveLength(1); + expect(newState.activeSouthPaneTab).toBe(expandedTable.id); + }); }); // eslint-disable-next-line no-restricted-globals -- TODO: Migrate from describe blocks describe('Run Query', () => { From 66676fda8aead923e7b9fac5107bd5ea162a003a Mon Sep 17 00:00:00 2001 From: richardfn Date: Tue, 11 Nov 2025 08:20:34 -0300 Subject: [PATCH 4/4] chore: fix formatting issues on sqlLab.js file --- superset-frontend/src/SqlLab/actions/sqlLab.js | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/superset-frontend/src/SqlLab/actions/sqlLab.js b/superset-frontend/src/SqlLab/actions/sqlLab.js index 3ea6781cc2d0..644d3e33463d 100644 --- a/superset-frontend/src/SqlLab/actions/sqlLab.js +++ b/superset-frontend/src/SqlLab/actions/sqlLab.js @@ -950,7 +950,13 @@ export function mergeTable(table, query, prepend) { return { type: MERGE_TABLE, table, query, prepend }; } -export function addTable(queryEditor, tableName, catalogName, schemaName, expanded = true) { +export function addTable( + queryEditor, + tableName, + catalogName, + schemaName, + expanded = true, +) { return function (dispatch, getState) { const { dbId } = getUpToDateQuery(getState(), queryEditor, queryEditor.id); const table = {