This is an automated email from the ASF dual-hosted git repository. diegopucci pushed a commit to branch enxdev/fix/sql-lab-persist-table-state in repository https://gitbox.apache.org/repos/asf/superset.git
commit 4c6124a5f953851afc28915be65944fcba362585 Author: Diego Pucci <[email protected]> AuthorDate: Mon Dec 29 16:28:20 2025 +0100 fix: SqlLab error when collapsing the left panel preview --- superset-frontend/src/SqlLab/actions/sqlLab.js | 6 +- .../src/SqlLab/actions/sqlLab.test.js | 195 ++++++++++++++++++++- 2 files changed, 193 insertions(+), 8 deletions(-) diff --git a/superset-frontend/src/SqlLab/actions/sqlLab.js b/superset-frontend/src/SqlLab/actions/sqlLab.js index fd23cb5753..13cd3fc21e 100644 --- a/superset-frontend/src/SqlLab/actions/sqlLab.js +++ b/superset-frontend/src/SqlLab/actions/sqlLab.js @@ -1098,7 +1098,8 @@ export function reFetchQueryResults(query) { export function expandTable(table) { return function (dispatch) { - const sync = isFeatureEnabled(FeatureFlag.SqllabBackendPersistence) + const sync = isFeatureEnabled(FeatureFlag.SqllabBackendPersistence) && + table.initialized ? SupersetClient.post({ endpoint: encodeURI(`/tableschemaview/${table.id}/expanded`), postPayload: { expanded: true }, @@ -1122,7 +1123,8 @@ export function expandTable(table) { export function collapseTable(table) { return function (dispatch) { - const sync = isFeatureEnabled(FeatureFlag.SqllabBackendPersistence) + const sync = isFeatureEnabled(FeatureFlag.SqllabBackendPersistence) && + table.initialized ? SupersetClient.post({ endpoint: encodeURI(`/tableschemaview/${table.id}/expanded`), postPayload: { expanded: false }, diff --git a/superset-frontend/src/SqlLab/actions/sqlLab.test.js b/superset-frontend/src/SqlLab/actions/sqlLab.test.js index a90355b876..ac6263d72d 100644 --- a/superset-frontend/src/SqlLab/actions/sqlLab.test.js +++ b/superset-frontend/src/SqlLab/actions/sqlLab.test.js @@ -934,6 +934,10 @@ describe('async actions', () => { fetchMock.delete(updateTableSchemaEndpoint, {}); fetchMock.post(updateTableSchemaEndpoint, JSON.stringify({ id: 1 })); + const updateTableSchemaExpandedEndpoint = + 'glob:**/tableschemaview/*/expanded'; + fetchMock.post(updateTableSchemaExpandedEndpoint, {}); + const getTableMetadataEndpoint = 'glob:**/api/v1/database/*/table_metadata/*'; fetchMock.get(getTableMetadataEndpoint, {}); @@ -1411,10 +1415,10 @@ describe('async actions', () => { // eslint-disable-next-line no-restricted-globals -- TODO: Migrate from describe blocks describe('expandTable', () => { - test('updates the table schema state in the backend', () => { + test('updates the table schema state in the backend when initialized', () => { expect.assertions(2); - const table = { id: 1 }; + const table = { id: 1, initialized: true }; const store = mockStore({}); const expectedActions = [ { @@ -1424,17 +1428,108 @@ describe('async actions', () => { ]; return store.dispatch(actions.expandTable(table)).then(() => { expect(store.getActions()).toEqual(expectedActions); - expect(fetchMock.calls(updateTableSchemaEndpoint)).toHaveLength(1); + const expandedCalls = fetchMock + .calls() + .filter( + call => + call[0] && + call[0].includes('/tableschemaview/') && + call[0].includes('/expanded'), + ); + expect(expandedCalls).toHaveLength(1); + }); + }); + + test('does not call backend when table is not initialized', () => { + expect.assertions(2); + + const table = { id: 'yVJPtuSackF', initialized: false }; + const store = mockStore({}); + const expectedActions = [ + { + type: actions.EXPAND_TABLE, + table, + }, + ]; + return store.dispatch(actions.expandTable(table)).then(() => { + expect(store.getActions()).toEqual(expectedActions); + // Check all POST calls to find the expanded endpoint + const expandedCalls = fetchMock + .calls() + .filter( + call => + call[0] && + call[0].includes('/tableschemaview/') && + call[0].includes('/expanded'), + ); + expect(expandedCalls).toHaveLength(0); + }); + }); + + test('does not call backend when initialized is undefined', () => { + expect.assertions(2); + + const table = { id: 'yVJPtuSackF' }; + const store = mockStore({}); + const expectedActions = [ + { + type: actions.EXPAND_TABLE, + table, + }, + ]; + return store.dispatch(actions.expandTable(table)).then(() => { + expect(store.getActions()).toEqual(expectedActions); + // Check all POST calls to find the expanded endpoint + const expandedCalls = fetchMock + .calls() + .filter( + call => + call[0] && + call[0].includes('/tableschemaview/') && + call[0].includes('/expanded'), + ); + expect(expandedCalls).toHaveLength(0); + }); + }); + + test('does not call backend when feature flag is off', () => { + expect.assertions(2); + + isFeatureEnabled.mockImplementation( + feature => !(feature === 'SQLLAB_BACKEND_PERSISTENCE'), + ); + + const table = { id: 1, initialized: true }; + const store = mockStore({}); + const expectedActions = [ + { + type: actions.EXPAND_TABLE, + table, + }, + ]; + return store.dispatch(actions.expandTable(table)).then(() => { + expect(store.getActions()).toEqual(expectedActions); + // Check all POST calls to find the expanded endpoint + const expandedCalls = fetchMock + .calls() + .filter( + call => + call[0] && + call[0].includes('/tableschemaview/') && + call[0].includes('/expanded'), + ); + expect(expandedCalls).toHaveLength(0); + isFeatureEnabled.mockRestore(); }); }); }); // eslint-disable-next-line no-restricted-globals -- TODO: Migrate from describe blocks describe('collapseTable', () => { - test('updates the table schema state in the backend', () => { + test('updates the table schema state in the backend when initialized', () => { expect.assertions(2); - const table = { id: 1 }; + const table = { id: 1, initialized: true }; const store = mockStore({}); const expectedActions = [ { @@ -1444,7 +1539,95 @@ describe('async actions', () => { ]; return store.dispatch(actions.collapseTable(table)).then(() => { expect(store.getActions()).toEqual(expectedActions); - expect(fetchMock.calls(updateTableSchemaEndpoint)).toHaveLength(1); + const expandedCalls = fetchMock + .calls() + .filter( + call => + call[0] && + call[0].includes('/tableschemaview/') && + call[0].includes('/expanded'), + ); + expect(expandedCalls).toHaveLength(1); + }); + }); + + test('does not call backend when table is not initialized', () => { + expect.assertions(2); + + const table = { id: 'yVJPtuSackF', initialized: false }; + const store = mockStore({}); + const expectedActions = [ + { + type: actions.COLLAPSE_TABLE, + table, + }, + ]; + return store.dispatch(actions.collapseTable(table)).then(() => { + expect(store.getActions()).toEqual(expectedActions); + const expandedCalls = fetchMock + .calls() + .filter( + call => + call[0] && + call[0].includes('/tableschemaview/') && + call[0].includes('/expanded'), + ); + expect(expandedCalls).toHaveLength(0); + }); + }); + + test('does not call backend when initialized is undefined', () => { + expect.assertions(2); + + const table = { id: 'yVJPtuSackF' }; + const store = mockStore({}); + const expectedActions = [ + { + type: actions.COLLAPSE_TABLE, + table, + }, + ]; + return store.dispatch(actions.collapseTable(table)).then(() => { + expect(store.getActions()).toEqual(expectedActions); + const expandedCalls = fetchMock + .calls() + .filter( + call => + call[0] && + call[0].includes('/tableschemaview/') && + call[0].includes('/expanded'), + ); + expect(expandedCalls).toHaveLength(0); + }); + }); + + test('does not call backend when feature flag is off', () => { + expect.assertions(2); + + isFeatureEnabled.mockImplementation( + feature => !(feature === 'SQLLAB_BACKEND_PERSISTENCE'), + ); + + const table = { id: 1, initialized: true }; + const store = mockStore({}); + const expectedActions = [ + { + type: actions.COLLAPSE_TABLE, + table, + }, + ]; + return store.dispatch(actions.collapseTable(table)).then(() => { + expect(store.getActions()).toEqual(expectedActions); + const expandedCalls = fetchMock + .calls() + .filter( + call => + call[0] && + call[0].includes('/tableschemaview/') && + call[0].includes('/expanded'), + ); + expect(expandedCalls).toHaveLength(0); + isFeatureEnabled.mockRestore(); }); }); });
