fix(SqlLab): Hit tableschemaview with a valid queryEditorId (#35341)

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 } });
 

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();
+});

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);