refactor(sqllab): Remove tableOptions from redux state (apache#23488)

sebastianliebscher · Apr 28, 2023 · 7b10fe1 · 7b10fe1
1 parent 1cd41c5
commit 7b10fe1
Show file tree

Hide file tree

Showing 9 changed files with 12 additions and 92 deletions.
diff --git a/superset-frontend/src/SqlLab/actions/sqlLab.js b/superset-frontend/src/SqlLab/actions/sqlLab.js
@@ -50,7 +50,6 @@ export const EXPAND_TABLE = 'EXPAND_TABLE';
 export const COLLAPSE_TABLE = 'COLLAPSE_TABLE';
 export const QUERY_EDITOR_SETDB = 'QUERY_EDITOR_SETDB';
 export const QUERY_EDITOR_SET_SCHEMA = 'QUERY_EDITOR_SET_SCHEMA';
-export const QUERY_EDITOR_SET_TABLE_OPTIONS = 'QUERY_EDITOR_SET_TABLE_OPTIONS';
 export const QUERY_EDITOR_SET_TITLE = 'QUERY_EDITOR_SET_TITLE';
 export const QUERY_EDITOR_SET_AUTORUN = 'QUERY_EDITOR_SET_AUTORUN';
 export const QUERY_EDITOR_SET_SQL = 'QUERY_EDITOR_SET_SQL';
@@ -957,10 +956,6 @@ export function queryEditorSetSchema(queryEditor, schema) {
   };
 }
 
-export function queryEditorSetTableOptions(queryEditor, options) {
-  return { type: QUERY_EDITOR_SET_TABLE_OPTIONS, queryEditor, options };
-}
-
 export function queryEditorSetAutorun(queryEditor, autorun) {
   return function (dispatch) {
     const sync = isFeatureEnabled(FeatureFlag.SQLLAB_BACKEND_PERSISTENCE)

diff --git a/superset-frontend/src/SqlLab/components/AceEditorWrapper/index.tsx b/superset-frontend/src/SqlLab/components/AceEditorWrapper/index.tsx
@@ -39,7 +39,7 @@ import {
   FullSQLEditor as AceEditor,
 } from 'src/components/AsyncAceEditor';
 import useQueryEditor from 'src/SqlLab/hooks/useQueryEditor';
-import { useSchemas } from 'src/hooks/apiResources';
+import { useSchemas, useTables } from 'src/hooks/apiResources';
 
 type HotKey = {
   key: string;
@@ -96,11 +96,19 @@ const AceEditorWrapper = ({
     'dbId',
     'sql',
     'functionNames',
-    'tableOptions',
     'validationResult',
     'schema',
   ]);
-  const { data: schemaOptions } = useSchemas({ dbId: queryEditor.dbId });
+  const { data: schemaOptions } = useSchemas({
+    ...(autocomplete && { dbId: queryEditor.dbId }),
+  });
+  const { data: tableData } = useTables({
+    ...(autocomplete && {
+      dbId: queryEditor.dbId,
+      schema: queryEditor.schema,
+    }),
+  });
+
   const currentSql = queryEditor.sql ?? '';
   const functionNames = queryEditor.functionNames ?? [];
 
@@ -117,7 +125,7 @@ const AceEditorWrapper = ({
     }),
     [schemaOptions],
   );
-  const tables = queryEditor.tableOptions ?? [];
+  const tables = tableData?.options ?? [];
 
   const [sql, setSql] = useState(currentSql);
   const [words, setWords] = useState<AceCompleterKeyword[]>([]);

diff --git a/superset-frontend/src/SqlLab/components/SaveQuery/index.tsx b/superset-frontend/src/SqlLab/components/SaveQuery/index.tsx
@@ -80,7 +80,6 @@ const SaveQuery = ({
     'schema',
     'selectedText',
     'sql',
-    'tableOptions',
     'templateParams',
   ]);
   const query = useMemo(

diff --git a/superset-frontend/src/SqlLab/components/SqlEditorLeftBar/index.tsx b/superset-frontend/src/SqlLab/components/SqlEditorLeftBar/index.tsx
@@ -35,7 +35,6 @@ import {
   collapseTable,
   expandTable,
   queryEditorSetSchema,
-  queryEditorSetTableOptions,
   setDatabases,
   addDangerToast,
   resetState,
@@ -218,15 +217,6 @@ const SqlEditorLeftBar = ({
     [dispatch, queryEditor],
   );
 
-  const handleTablesLoad = useCallback(
-    (options: Array<any>) => {
-      if (queryEditor) {
-        dispatch(queryEditorSetTableOptions(queryEditor, options));
-      }
-    },
-    [dispatch, queryEditor],
-  );
-
   const handleDbList = useCallback(
     (result: DatabaseObject) => {
       dispatch(setDatabases(result));
@@ -256,7 +246,6 @@ const SqlEditorLeftBar = ({
         onDbChange={onDbChange}
         onSchemaChange={handleSchemaChange}
         onTableSelectChange={onTablesChange}
-        onTablesLoad={handleTablesLoad}
         schema={schema}
         tableValue={selectedTableNames}
         sqlLabMode

diff --git a/superset-frontend/src/SqlLab/fixtures.ts b/superset-frontend/src/SqlLab/fixtures.ts
@@ -185,7 +185,6 @@ export const defaultQueryEditor = {
   name: 'Untitled Query 1',
   schema: 'main',
   remoteId: null,
-  tableOptions: [],
   functionNames: [],
   hideLeftBar: false,
   templateParams: '{}',

diff --git a/superset-frontend/src/SqlLab/reducers/sqlLab.js b/superset-frontend/src/SqlLab/reducers/sqlLab.js
@@ -587,18 +587,6 @@ export default function sqlLabReducer(state = {}, action) {
         ),
       };
     },
-    [actions.QUERY_EDITOR_SET_TABLE_OPTIONS]() {
-      return {
-        ...state,
-        ...alterUnsavedQueryEditorState(
-          state,
-          {
-            tableOptions: action.options,
-          },
-          action.queryEditor.id,
-        ),
-      };
-    },
     [actions.QUERY_EDITOR_SET_TITLE]() {
       return {
         ...state,

diff --git a/superset-frontend/src/SqlLab/types.ts b/superset-frontend/src/SqlLab/types.ts
@@ -39,7 +39,6 @@ export interface QueryEditor {
   autorun: boolean;
   sql: string;
   remoteId: number | null;
-  tableOptions: any[];
   functionNames: string[];
   validationResult?: {
     completed: boolean;

diff --git a/superset-frontend/src/components/TableSelector/TableSelector.test.tsx b/superset-frontend/src/components/TableSelector/TableSelector.test.tsx
@@ -21,7 +21,6 @@ import React from 'react';
 import { render, screen, waitFor, within } from 'spec/helpers/testing-library';
 import { queryClient } from 'src/views/QueryProvider';
 import fetchMock from 'fetch-mock';
-import { act } from 'react-dom/test-utils';
 import userEvent from '@testing-library/user-event';
 import TableSelector, { TableSelectorMultiple } from '.';
 
@@ -36,11 +35,6 @@ const createProps = (props = {}) => ({
   ...props,
 });
 
-const getSchemaMockFunction = () =>
-  ({
-    result: ['schema_a', 'schema_b'],
-  } as any);
-
 const getTableMockFunction = () =>
   ({
     count: 4,
@@ -124,47 +118,6 @@ test('renders disabled without schema', async () => {
   });
 });
 
-test('table options are notified after schema selection', async () => {
-  fetchMock.get(schemaApiRoute, getSchemaMockFunction());
-
-  const callback = jest.fn();
-  const props = createProps({
-    onTablesLoad: callback,
-    schema: undefined,
-  });
-  render(<TableSelector {...props} />, { useRedux: true });
-
-  const schemaSelect = screen.getByRole('combobox', {
-    name: 'Select schema or type to search schemas',
-  });
-  expect(schemaSelect).toBeInTheDocument();
-  expect(callback).not.toHaveBeenCalled();
-
-  userEvent.click(schemaSelect);
-
-  expect(
-    await screen.findByRole('option', { name: 'schema_a' }),
-  ).toBeInTheDocument();
-  expect(
-    await screen.findByRole('option', { name: 'schema_b' }),
-  ).toBeInTheDocument();
-
-  fetchMock.get(tablesApiRoute, getTableMockFunction());
-
-  act(() => {
-    userEvent.click(screen.getAllByText('schema_a')[1]);
-  });
-
-  await waitFor(() => {
-    expect(callback).toHaveBeenCalledWith([
-      { label: 'table_a', value: 'table_a' },
-      { label: 'table_b', value: 'table_b' },
-      { label: 'table_c', value: 'table_c' },
-      { label: 'table_d', value: 'table_d' },
-    ]);
-  });
-});
-
 test('table select retain value if not in SQL Lab mode', async () => {
   fetchMock.get(schemaApiRoute, { result: ['test_schema'] });
   fetchMock.get(tablesApiRoute, getTableMockFunction());

diff --git a/superset-frontend/src/components/TableSelector/index.tsx b/superset-frontend/src/components/TableSelector/index.tsx
@@ -97,7 +97,6 @@ interface TableSelectorProps {
   isDatabaseSelectEnabled?: boolean;
   onDbChange?: (db: DatabaseObject) => void;
   onSchemaChange?: (schema?: string) => void;
-  onTablesLoad?: (options: Array<any>) => void;
   readOnly?: boolean;
   schema?: string;
   onEmptyResults?: (searchText?: string) => void;
@@ -158,7 +157,6 @@ const TableSelector: FunctionComponent<TableSelectorProps> = ({
   isDatabaseSelectEnabled = true,
   onDbChange,
   onSchemaChange,
-  onTablesLoad,
   readOnly = false,
   onEmptyResults,
   schema,
@@ -199,14 +197,6 @@ const TableSelector: FunctionComponent<TableSelectorProps> = ({
     },
   });
 
-  useEffect(() => {
-    // Set the tableOptions in the queryEditor so autocomplete
-    // works on new tabs
-    if (data && isFetched) {
-      onTablesLoad?.(data.options);
-    }
-  }, [data, isFetched, onTablesLoad]);
-
   const tableOptions = useMemo<TableOption[]>(
     () =>
       data