From 9fbfcf0ccd217ebfdbea71cfad424e44e97f568b Mon Sep 17 00:00:00 2001 From: Richard Fogaca Nienkotter <63572350+richardfogaca@users.noreply.github.com> Date: Wed, 12 Nov 2025 06:48:48 -0300 Subject: [PATCH] fix(sqllab): prevent unwanted tab switching when autocompleting table names on SQL Lab (#35992) --- .../src/SqlLab/actions/sqlLab.js | 10 ++- .../AceEditorWrapper/useKeywords.ts | 1 + .../src/SqlLab/reducers/sqlLab.js | 6 +- .../src/SqlLab/reducers/sqlLab.test.js | 87 +++++++++++++++++++ 4 files changed, 100 insertions(+), 4 deletions(-) diff --git a/superset-frontend/src/SqlLab/actions/sqlLab.js b/superset-frontend/src/SqlLab/actions/sqlLab.js index 9a672f594ea..644d3e33463 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) { +export function addTable( + queryEditor, + tableName, + catalogName, + schemaName, + expanded = true, +) { return function (dispatch, getState) { const { dbId } = getUpToDateQuery(getState(), queryEditor, queryEditor.id); const table = { @@ -964,7 +970,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 548c74e463d..1a8eb7b3684 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 a1e0210d790..3469036da4d 100644 --- a/superset-frontend/src/SqlLab/reducers/sqlLab.js +++ b/superset-frontend/src/SqlLab/reducers/sqlLab.js @@ -222,8 +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)); - 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 c8fd25faf32..fedca09e391 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', () => {