This is an automated email from the ASF dual-hosted git repository.
diegopucci pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/superset.git
The following commit(s) were added to refs/heads/master by this push:
new a66c230058 fix(SqlLab): Hit tableschemaview with a valid queryEditorId
(#35341)
a66c230058 is described below
commit a66c230058d44dd87d9c7b23c6ed9d50e1d6031f
Author: GeidÅ <[email protected]>
AuthorDate: Wed Oct 1 14:39:02 2025 +0300
fix(SqlLab): Hit tableschemaview with a valid queryEditorId (#35341)
---
superset-frontend/src/SqlLab/actions/sqlLab.js | 5 +-
.../components/TableElement/TableElement.test.tsx | 232 +++++++++++++++++++++
.../src/SqlLab/components/TableElement/index.tsx | 38 +++-
3 files changed, 263 insertions(+), 12 deletions(-)
diff --git a/superset-frontend/src/SqlLab/actions/sqlLab.js
b/superset-frontend/src/SqlLab/actions/sqlLab.js
index 2c6a6e47b5..d7bf0bffbf 100644
--- a/superset-frontend/src/SqlLab/actions/sqlLab.js
+++ b/superset-frontend/src/SqlLab/actions/sqlLab.js
@@ -1017,12 +1017,13 @@ export function runTablePreviewQuery(newTable,
runPreviewOnly) {
};
}
-export function syncTable(table, tableMetadata) {
+export function syncTable(table, tableMetadata, finalQueryEditorId) {
return function (dispatch) {
+ const finalTable = { ...table, queryEditorId: finalQueryEditorId };
const sync = isFeatureEnabled(FeatureFlag.SqllabBackendPersistence)
? SupersetClient.post({
endpoint: encodeURI('/tableschemaview/'),
- postPayload: { table: { ...tableMetadata, ...table } },
+ postPayload: { table: { ...tableMetadata, ...finalTable } },
})
: Promise.resolve({ json: { id: table.id } });
diff --git
a/superset-frontend/src/SqlLab/components/TableElement/TableElement.test.tsx
b/superset-frontend/src/SqlLab/components/TableElement/TableElement.test.tsx
index 96f68b6666..3412c5869a 100644
--- a/superset-frontend/src/SqlLab/components/TableElement/TableElement.test.tsx
+++ b/superset-frontend/src/SqlLab/components/TableElement/TableElement.test.tsx
@@ -22,6 +22,8 @@ import { FeatureFlag, isFeatureEnabled } from
'@superset-ui/core';
import TableElement, { Column } from 'src/SqlLab/components/TableElement';
import { table, initialState } from 'src/SqlLab/fixtures';
import { render, waitFor, fireEvent } from 'spec/helpers/testing-library';
+import * as sqlLabActions from 'src/SqlLab/actions/sqlLab';
+import { QueryEditor } from 'src/SqlLab/types';
jest.mock('@superset-ui/core', () => ({
...jest.requireActual('@superset-ui/core'),
@@ -79,6 +81,27 @@ const mockedProps = {
activeKey: [table.id],
};
+const createStateWithQueryEditor = (queryEditor: Partial<QueryEditor>) => ({
+ ...initialState,
+ sqlLab: {
+ ...initialState.sqlLab,
+ queryEditors: [queryEditor],
+ },
+});
+
+const setupSyncTableTest = () => {
+ const spy = jest.spyOn(sqlLabActions, 'syncTable');
+ mockedIsFeatureEnabled.mockImplementation(
+ featureFlag => featureFlag === FeatureFlag.SqllabBackendPersistence,
+ );
+ fetchMock.post(
+ updateTableSchemaEndpoint,
+ { id: 100 },
+ { overwriteRoutes: true },
+ );
+ return spy;
+};
+
test('renders', () => {
expect(isValidElement(<TableElement table={table} />)).toBe(true);
});
@@ -207,3 +230,212 @@ test('refreshes table metadata when triggered', async ()
=> {
expect(fetchMock.calls(updateTableSchemaEndpoint)).toHaveLength(1),
);
});
+
+test('calls syncTable with valid backend ID when query editor has tabViewId',
async () => {
+ const syncTableSpy = setupSyncTableTest();
+ const testTable = {
+ ...table,
+ initialized: false,
+ queryEditorId: 'temp-id-123',
+ };
+
+ const state = createStateWithQueryEditor({
+ id: 'temp-id-123',
+ tabViewId: '42',
+ inLocalStorage: false,
+ name: 'Test Editor',
+ });
+
+ render(<TableElement table={testTable} activeKey={[testTable.id]} />, {
+ useRedux: true,
+ initialState: state,
+ });
+
+ await waitFor(() => {
+ expect(syncTableSpy).toHaveBeenCalledWith(
+ expect.any(Object),
+ expect.any(Object),
+ '42', // finalQueryEditorId
+ );
+ });
+
+ syncTableSpy.mockRestore();
+});
+
+test('does not call syncTable when query editor is in localStorage', async ()
=> {
+ const syncTableSpy = setupSyncTableTest();
+ const testTable = {
+ ...table,
+ initialized: false,
+ queryEditorId: 'local-id',
+ };
+
+ const state = createStateWithQueryEditor({
+ id: 'local-id',
+ tabViewId: undefined,
+ inLocalStorage: true,
+ name: 'Local Editor',
+ });
+
+ render(<TableElement table={testTable} activeKey={[testTable.id]} />, {
+ useRedux: true,
+ initialState: state,
+ });
+
+ await waitFor(() => {
+ expect(fetchMock.calls(getTableMetadataEndpoint)).toHaveLength(1);
+ });
+
+ await new Promise(resolve => setTimeout(resolve, 100));
+ expect(syncTableSpy).not.toHaveBeenCalled();
+
+ syncTableSpy.mockRestore();
+});
+
+test('does not call syncTable with non-numeric queryEditorId', async () => {
+ const syncTableSpy = setupSyncTableTest();
+ const testTable = {
+ ...table,
+ initialized: false,
+ queryEditorId: 'not-a-number',
+ };
+
+ const state = createStateWithQueryEditor({
+ id: 'not-a-number',
+ tabViewId: 'also-not-a-number',
+ inLocalStorage: false,
+ name: 'Invalid Editor',
+ });
+
+ render(<TableElement table={testTable} activeKey={[testTable.id]} />, {
+ useRedux: true,
+ initialState: state,
+ });
+
+ await waitFor(() => {
+ expect(fetchMock.calls(getTableMetadataEndpoint)).toHaveLength(1);
+ });
+
+ await new Promise(resolve => setTimeout(resolve, 100));
+ expect(syncTableSpy).not.toHaveBeenCalled();
+
+ syncTableSpy.mockRestore();
+});
+
+test('does not call syncTable for already initialized tables', async () => {
+ const syncTableSpy = setupSyncTableTest();
+ const testTable = {
+ ...table,
+ initialized: true, // Already initialized
+ queryEditorId: '789',
+ };
+
+ const state = createStateWithQueryEditor({
+ id: '789',
+ tabViewId: '789',
+ inLocalStorage: false,
+ name: 'Initialized Editor',
+ });
+
+ render(<TableElement table={testTable} activeKey={[testTable.id]} />, {
+ useRedux: true,
+ initialState: state,
+ });
+
+ await waitFor(() => {
+ expect(fetchMock.calls(getTableMetadataEndpoint)).toHaveLength(1);
+ });
+
+ await new Promise(resolve => setTimeout(resolve, 100));
+ expect(syncTableSpy).not.toHaveBeenCalled();
+
+ syncTableSpy.mockRestore();
+});
+
+test('calls syncTable after query editor is migrated from localStorage', async
() => {
+ const syncTableSpy = setupSyncTableTest();
+ const testTable = {
+ ...table,
+ initialized: false,
+ queryEditorId: 'temp-editor-id',
+ };
+
+ // Start with editor in localStorage
+ const localState = createStateWithQueryEditor({
+ id: 'temp-editor-id',
+ tabViewId: undefined,
+ inLocalStorage: true,
+ name: 'Temp Editor',
+ });
+
+ const { rerender } = render(
+ <TableElement table={testTable} activeKey={[testTable.id]} />,
+ {
+ useRedux: true,
+ initialState: localState,
+ },
+ );
+
+ await new Promise(resolve => setTimeout(resolve, 100));
+ expect(syncTableSpy).not.toHaveBeenCalled();
+
+ const migratedState = createStateWithQueryEditor({
+ id: 'temp-editor-id',
+ tabViewId: '999',
+ inLocalStorage: false,
+ name: 'Temp Editor',
+ });
+
+ rerender(<TableElement table={testTable} activeKey={[testTable.id]} />);
+
+ const { unmount } = render(
+ <TableElement table={testTable} activeKey={[testTable.id]} />,
+ {
+ useRedux: true,
+ initialState: migratedState,
+ },
+ );
+
+ await waitFor(() => {
+ expect(syncTableSpy).toHaveBeenCalledWith(
+ expect.any(Object),
+ expect.any(Object),
+ '999',
+ );
+ });
+
+ unmount();
+ syncTableSpy.mockRestore();
+});
+
+test('passes numeric queryEditorId validation', async () => {
+ const syncTableSpy = setupSyncTableTest();
+ const testTable = {
+ ...table,
+ initialized: false,
+ queryEditorId: 'editor-123',
+ };
+
+ const state = createStateWithQueryEditor({
+ id: 'editor-123',
+ tabViewId: '456',
+ inLocalStorage: false,
+ name: 'Valid Editor',
+ });
+
+ render(<TableElement table={testTable} activeKey={[testTable.id]} />, {
+ useRedux: true,
+ initialState: state,
+ });
+
+ await waitFor(() => {
+ expect(syncTableSpy).toHaveBeenCalled();
+ const [, , finalQueryEditorId] = syncTableSpy.mock.calls[0];
+ // Verify it's a valid numeric string
+ expect(Number.isNaN(Number(finalQueryEditorId))).toBe(false);
+ expect(typeof finalQueryEditorId).toBe('string');
+ expect(finalQueryEditorId).toMatch(/^\d+$/);
+ });
+
+ syncTableSpy.mockRestore();
+});
diff --git a/superset-frontend/src/SqlLab/components/TableElement/index.tsx
b/superset-frontend/src/SqlLab/components/TableElement/index.tsx
index 1bef2037d9..309c74d8b9 100644
--- a/superset-frontend/src/SqlLab/components/TableElement/index.tsx
+++ b/superset-frontend/src/SqlLab/components/TableElement/index.tsx
@@ -17,8 +17,8 @@
* under the License.
*/
import { useState, useRef, useEffect } from 'react';
-import { useDispatch } from 'react-redux';
-import type { Table } from 'src/SqlLab/types';
+import { useDispatch, useSelector } from 'react-redux';
+import type { QueryEditor, SqlLabRootState, Table } from 'src/SqlLab/types';
import {
ButtonGroup,
Card,
@@ -103,6 +103,19 @@ const TableElement = ({ table, ...props }:
TableElementProps) => {
},
{ skip: !expanded },
);
+ const tableData = {
+ ...tableMetadata,
+ ...tableExtendedMetadata,
+ };
+ const queryEditors = useSelector<SqlLabRootState, QueryEditor[]>(
+ state => state.sqlLab.queryEditors,
+ );
+ const currentTable = { ...tableData, ...table };
+ const { queryEditorId } = currentTable;
+ const queryEditor = queryEditors.find(
+ qe => qe.id === queryEditorId || qe.tabViewId === queryEditorId,
+ );
+ const currentQueryEditorId = queryEditor?.tabViewId || queryEditorId;
useEffect(() => {
if (hasMetadataError || hasExtendedMetadataError) {
@@ -112,16 +125,16 @@ const TableElement = ({ table, ...props }:
TableElementProps) => {
}
}, [hasMetadataError, hasExtendedMetadataError, dispatch]);
- const tableData = {
- ...tableMetadata,
- ...tableExtendedMetadata,
- };
-
// TODO: migrate syncTable logic by SIP-93
const syncTableMetadata = useEffectEvent(() => {
const { initialized } = table;
- if (!initialized) {
- dispatch(syncTable(table, tableData));
+ // if not a valid number, wait for backend to assign one
+ const hasFinalQueryEditorId =
+ currentQueryEditorId &&
+ !Number.isNaN(Number(currentQueryEditorId)) &&
+ currentTable.queryEditorId !== currentQueryEditorId;
+ if (!initialized && hasFinalQueryEditorId) {
+ dispatch(syncTable(currentTable, tableData, currentQueryEditorId));
}
});
@@ -129,7 +142,12 @@ const TableElement = ({ table, ...props }:
TableElementProps) => {
if (isMetadataSuccess && isExtraMetadataSuccess) {
syncTableMetadata();
}
- }, [isMetadataSuccess, isExtraMetadataSuccess, syncTableMetadata]);
+ }, [
+ isMetadataSuccess,
+ isExtraMetadataSuccess,
+ currentQueryEditorId,
+ syncTableMetadata,
+ ]);
const [sortColumns, setSortColumns] = useState(false);
const [hovered, setHovered] = useState(false);
