diff --git a/superset-frontend/src/SqlLab/actions/sqlLab.js b/superset-frontend/src/SqlLab/actions/sqlLab.js index 9a672f594ea7..3ea6781cc2d0 100644 --- a/superset-frontend/src/SqlLab/actions/sqlLab.js +++ b/superset-frontend/src/SqlLab/actions/sqlLab.js @@ -950,7 +950,7 @@ 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 +964,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..3469036da4db 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 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', () => {