From f40cb4ed21cfd656c5a309b10fca97358fbb9a10 Mon Sep 17 00:00:00 2001 From: justin-park Date: Sat, 25 Mar 2023 16:45:25 -0700 Subject: [PATCH 1/5] refactor(sqllab): Remove tableOptions from redux state --- .../src/SqlLab/components/AceEditorWrapper/index.tsx | 10 +++++++--- .../src/SqlLab/components/SaveQuery/index.tsx | 1 - .../src/SqlLab/components/SqlEditorLeftBar/index.tsx | 10 ---------- superset-frontend/src/SqlLab/fixtures.ts | 1 - superset-frontend/src/SqlLab/reducers/sqlLab.js | 12 ------------ superset-frontend/src/SqlLab/types.ts | 1 - .../components/TableSelector/TableSelector.test.tsx | 1 - .../src/components/TableSelector/index.tsx | 10 ---------- 8 files changed, 7 insertions(+), 39 deletions(-) diff --git a/superset-frontend/src/SqlLab/components/AceEditorWrapper/index.tsx b/superset-frontend/src/SqlLab/components/AceEditorWrapper/index.tsx index d14d532dcd81b..9122189d40e12 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; @@ -99,11 +99,15 @@ const AceEditorWrapper = ({ 'dbId', 'sql', 'functionNames', - 'tableOptions', 'validationResult', 'schema', ]); const { data: schemaOptions } = useSchemas({ dbId: queryEditor.dbId }); + const { data: tableData } = useTables({ + dbId: queryEditor.dbId, + schema: queryEditor.schema, + }); + const currentSql = queryEditor.sql ?? ''; const functionNames = queryEditor.functionNames ?? []; @@ -120,7 +124,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..4d6b8982b5eed 100644 --- a/superset-frontend/src/SqlLab/components/SqlEditorLeftBar/index.tsx +++ b/superset-frontend/src/SqlLab/components/SqlEditorLeftBar/index.tsx @@ -218,15 +218,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 +247,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 ff2cb340bbd08..114f5e4ea5b7d 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..c537c71465be7 100644 --- a/superset-frontend/src/components/TableSelector/TableSelector.test.tsx +++ b/superset-frontend/src/components/TableSelector/TableSelector.test.tsx @@ -129,7 +129,6 @@ test('table options are notified after schema selection', async () => { const callback = jest.fn(); const props = createProps({ - onTablesLoad: callback, schema: undefined, }); render(, { useRedux: true }); 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 From c48a5b05a4d43ae68a2d7cb71e2cfb588e2938e9 Mon Sep 17 00:00:00 2001 From: justin-park Date: Sun, 26 Mar 2023 14:03:59 -0700 Subject: [PATCH 2/5] remove queryEditorSetTableOptions --- superset-frontend/src/SqlLab/actions/sqlLab.js | 5 ----- .../src/SqlLab/components/SqlEditorLeftBar/index.tsx | 1 - 2 files changed, 6 deletions(-) diff --git a/superset-frontend/src/SqlLab/actions/sqlLab.js b/superset-frontend/src/SqlLab/actions/sqlLab.js index 260f9944f9b04..e5696c42f0083 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'; @@ -952,10 +951,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/SqlEditorLeftBar/index.tsx b/superset-frontend/src/SqlLab/components/SqlEditorLeftBar/index.tsx index 4d6b8982b5eed..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, From 38d93a1bed0c38c93f963ed33e2708a39cb79192 Mon Sep 17 00:00:00 2001 From: justin-park Date: Mon, 27 Mar 2023 09:43:54 -0700 Subject: [PATCH 3/5] remove unnecessary test --- .../TableSelector/TableSelector.test.tsx | 40 ------------------- 1 file changed, 40 deletions(-) diff --git a/superset-frontend/src/components/TableSelector/TableSelector.test.tsx b/superset-frontend/src/components/TableSelector/TableSelector.test.tsx index c537c71465be7..338508bb074b4 100644 --- a/superset-frontend/src/components/TableSelector/TableSelector.test.tsx +++ b/superset-frontend/src/components/TableSelector/TableSelector.test.tsx @@ -124,46 +124,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({ - 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()); From 0d2737e291631a916237e69f0917515668d3589c Mon Sep 17 00:00:00 2001 From: justin-park Date: Mon, 27 Mar 2023 10:05:28 -0700 Subject: [PATCH 4/5] remove unused ref --- .../src/components/TableSelector/TableSelector.test.tsx | 6 ------ 1 file changed, 6 deletions(-) diff --git a/superset-frontend/src/components/TableSelector/TableSelector.test.tsx b/superset-frontend/src/components/TableSelector/TableSelector.test.tsx index 338508bb074b4..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, From eab204a4ec928ace18102b0e3a4d3f7169dd4f3a Mon Sep 17 00:00:00 2001 From: justin-park Date: Wed, 12 Apr 2023 13:36:34 -0700 Subject: [PATCH 5/5] get autocomplete words only needed --- .../src/SqlLab/components/AceEditorWrapper/index.tsx | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/superset-frontend/src/SqlLab/components/AceEditorWrapper/index.tsx b/superset-frontend/src/SqlLab/components/AceEditorWrapper/index.tsx index 9122189d40e12..3b51c6b02d876 100644 --- a/superset-frontend/src/SqlLab/components/AceEditorWrapper/index.tsx +++ b/superset-frontend/src/SqlLab/components/AceEditorWrapper/index.tsx @@ -102,10 +102,14 @@ const AceEditorWrapper = ({ 'validationResult', 'schema', ]); - const { data: schemaOptions } = useSchemas({ dbId: queryEditor.dbId }); + const { data: schemaOptions } = useSchemas({ + ...(autocomplete && { dbId: queryEditor.dbId }), + }); const { data: tableData } = useTables({ - dbId: queryEditor.dbId, - schema: queryEditor.schema, + ...(autocomplete && { + dbId: queryEditor.dbId, + schema: queryEditor.schema, + }), }); const currentSql = queryEditor.sql ?? '';