From 5bec1a65ae3bdae91ee3af8ec3d70b1647726d98 Mon Sep 17 00:00:00 2001 From: "JUST.in DO IT" Date: Thu, 20 Apr 2023 15:55:55 -0700 Subject: [PATCH] refactor(sqllab): Remove tableOptions from redux state (#23488) --- .../src/SqlLab/actions/sqlLab.js | 5 -- .../components/AceEditorWrapper/index.tsx | 16 +++++-- .../src/SqlLab/components/SaveQuery/index.tsx | 1 - .../components/SqlEditorLeftBar/index.tsx | 11 ----- superset-frontend/src/SqlLab/fixtures.ts | 1 - .../src/SqlLab/reducers/sqlLab.js | 12 ----- superset-frontend/src/SqlLab/types.ts | 1 - .../TableSelector/TableSelector.test.tsx | 47 ------------------- .../src/components/TableSelector/index.tsx | 10 ---- 9 files changed, 12 insertions(+), 92 deletions(-) diff --git a/superset-frontend/src/SqlLab/actions/sqlLab.js b/superset-frontend/src/SqlLab/actions/sqlLab.js index e4cc78ef0f94c..7ec2e77616a10 100644 --- 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 index 213a8c3239f71..b60faacbe043f 100644 --- 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([]); diff --git a/superset-frontend/src/SqlLab/components/SaveQuery/index.tsx b/superset-frontend/src/SqlLab/components/SaveQuery/index.tsx index b513637b27385..4071b9e2d71d4 100644 --- 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 index 0cacdb86caf0c..1298722d5dd2d 100644 --- 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) => { - 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 index ba88a41b0accc..cfd15bedbe70a 100644 --- 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 index 03b239e56b69c..eafc326aaa0d1 100644 --- 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 index ab63e1c76080a..e209be04be504 100644 --- 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 index 3ab045a8a9454..1e6083e1b46f5 100644 --- 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(, { 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 index 2c51462e6d255..3886a86fd20af 100644 --- 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) => void; readOnly?: boolean; schema?: string; onEmptyResults?: (searchText?: string) => void; @@ -158,7 +157,6 @@ const TableSelector: FunctionComponent = ({ isDatabaseSelectEnabled = true, onDbChange, onSchemaChange, - onTablesLoad, readOnly = false, onEmptyResults, schema, @@ -199,14 +197,6 @@ const TableSelector: FunctionComponent = ({ }, }); - 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( () => data