From 4a7f1a977cb4e5f9729e478b94517ed316c72d12 Mon Sep 17 00:00:00 2001 From: EugeneTorap Date: Thu, 29 Sep 2022 19:04:06 +0300 Subject: [PATCH 1/8] chore: Use queryEditorId in child components of SqlEditor --- .../EstimateQueryCostButton.test.tsx | 14 +- .../EstimateQueryCostButton/index.tsx | 23 +-- .../QueryLimitSelect.test.tsx | 13 +- .../components/QueryLimitSelect/index.tsx | 27 ++-- .../RunQueryActionButton.test.tsx | 2 +- .../components/RunQueryActionButton/index.tsx | 28 ++-- .../components/SaveQuery/SaveQuery.test.jsx | 21 +-- .../src/SqlLab/components/SaveQuery/index.tsx | 18 +-- .../ShareSqlLabQuery.test.jsx | 17 +-- .../components/ShareSqlLabQuery/index.tsx | 32 ++-- .../src/SqlLab/components/SqlEditor/index.jsx | 15 +- .../SqlEditorLeftBar.test.jsx | 60 +++----- .../components/SqlEditorLeftBar/index.tsx | 137 ++++++++---------- .../TableElement/TableElement.test.jsx | 27 +++- .../SqlLab/components/TableElement/index.tsx | 14 +- .../TemplateParamsEditor.test.tsx | 2 +- .../components/TemplateParamsEditor/index.tsx | 20 +-- superset-frontend/src/SqlLab/fixtures.ts | 3 + 18 files changed, 224 insertions(+), 249 deletions(-) diff --git a/superset-frontend/src/SqlLab/components/EstimateQueryCostButton/EstimateQueryCostButton.test.tsx b/superset-frontend/src/SqlLab/components/EstimateQueryCostButton/EstimateQueryCostButton.test.tsx index 59a6a5a118c1..5b2cae174166 100644 --- a/superset-frontend/src/SqlLab/components/EstimateQueryCostButton/EstimateQueryCostButton.test.tsx +++ b/superset-frontend/src/SqlLab/components/EstimateQueryCostButton/EstimateQueryCostButton.test.tsx @@ -21,7 +21,11 @@ import configureStore from 'redux-mock-store'; import thunk from 'redux-thunk'; import { render } from 'spec/helpers/testing-library'; import { Store } from 'redux'; -import { initialState, defaultQueryEditor } from 'src/SqlLab/fixtures'; +import { + initialState, + defaultQueryEditor, + extraQueryEditor1, +} from 'src/SqlLab/fixtures'; import EstimateQueryCostButton, { EstimateQueryCostButtonProps, @@ -43,7 +47,7 @@ jest.mock('src/components/Select/AsyncSelect', () => () => ( const setup = (props: Partial, store?: Store) => render( , @@ -61,12 +65,8 @@ describe('EstimateQueryCostButton', () => { }); it('renders label for selected query', async () => { - const queryEditorWithSelectedText = { - ...defaultQueryEditor, - selectedText: 'SELECT', - }; const { queryByText } = setup( - { queryEditor: queryEditorWithSelectedText }, + { queryEditorId: extraQueryEditor1.id }, mockStore(initialState), ); diff --git a/superset-frontend/src/SqlLab/components/EstimateQueryCostButton/index.tsx b/superset-frontend/src/SqlLab/components/EstimateQueryCostButton/index.tsx index dc2c23c40626..323f7cb8c43a 100644 --- a/superset-frontend/src/SqlLab/components/EstimateQueryCostButton/index.tsx +++ b/superset-frontend/src/SqlLab/components/EstimateQueryCostButton/index.tsx @@ -17,8 +17,10 @@ * under the License. */ import React, { useMemo } from 'react'; -import Alert from 'src/components/Alert'; +import { useSelector } from 'react-redux'; import { t } from '@superset-ui/core'; + +import Alert from 'src/components/Alert'; import TableView from 'src/components/TableView'; import Button from 'src/components/Button'; import Loading from 'src/components/Loading'; @@ -29,31 +31,32 @@ import { QueryCostEstimate, QueryEditor, } from 'src/SqlLab/types'; -import { getUpToDateQuery } from 'src/SqlLab/actions/sqlLab'; -import { useSelector } from 'react-redux'; export interface EstimateQueryCostButtonProps { getEstimate: Function; - queryEditor: QueryEditor; + queryEditorId: string; tooltip?: string; disabled?: boolean; } const EstimateQueryCostButton = ({ getEstimate, - queryEditor, + queryEditorId, tooltip = '', disabled = false, }: EstimateQueryCostButtonProps) => { const queryCostEstimate = useSelector< SqlLabRootState, QueryCostEstimate | undefined - >(state => state.sqlLab.queryCostEstimates?.[queryEditor.id]); - const selectedText = useSelector( - rootState => - (getUpToDateQuery(rootState, queryEditor) as unknown as QueryEditor) - .selectedText, + >(state => state.sqlLab.queryCostEstimates?.[queryEditorId]); + + const queryEditor = useSelector>( + ({ sqlLab: { unsavedQueryEditor, queryEditors } }) => ({ + ...queryEditors.find(({ id }) => id === queryEditorId), + ...(queryEditorId === unsavedQueryEditor.id && unsavedQueryEditor), + }), ); + const { selectedText } = queryEditor; const { cost } = queryCostEstimate || {}; const tableData = useMemo(() => (Array.isArray(cost) ? cost : []), [cost]); const columns = useMemo( diff --git a/superset-frontend/src/SqlLab/components/QueryLimitSelect/QueryLimitSelect.test.tsx b/superset-frontend/src/SqlLab/components/QueryLimitSelect/QueryLimitSelect.test.tsx index c640d5a9dfed..ec226776ef43 100644 --- a/superset-frontend/src/SqlLab/components/QueryLimitSelect/QueryLimitSelect.test.tsx +++ b/superset-frontend/src/SqlLab/components/QueryLimitSelect/QueryLimitSelect.test.tsx @@ -23,7 +23,11 @@ import { Store } from 'redux'; import { render, fireEvent, waitFor } from 'spec/helpers/testing-library'; import userEvent from '@testing-library/user-event'; -import { initialState, defaultQueryEditor } from 'src/SqlLab/fixtures'; +import { + initialState, + defaultQueryEditor, + extraQueryEditor1, +} from 'src/SqlLab/fixtures'; import QueryLimitSelect, { LIMIT_DROPDOWN, QueryLimitSelectProps, @@ -51,7 +55,7 @@ const defaultQueryLimit = 100; const setup = (props?: Partial, store?: Store) => render( { const queryLimit = 10; const { getByText } = setup( { - queryEditor: { - ...defaultQueryEditor, - queryLimit, - }, + queryEditorId: extraQueryEditor1.id, }, mockStore(initialState), ); diff --git a/superset-frontend/src/SqlLab/components/QueryLimitSelect/index.tsx b/superset-frontend/src/SqlLab/components/QueryLimitSelect/index.tsx index f438ebc59edc..07b5e689eccd 100644 --- a/superset-frontend/src/SqlLab/components/QueryLimitSelect/index.tsx +++ b/superset-frontend/src/SqlLab/components/QueryLimitSelect/index.tsx @@ -19,14 +19,15 @@ import React from 'react'; import { useSelector, useDispatch } from 'react-redux'; import { styled, useTheme } from '@superset-ui/core'; + import { AntdDropdown } from 'src/components'; import { Menu } from 'src/components/Menu'; import Icons from 'src/components/Icons'; -import { SqlLabRootState, QueryEditor } from 'src/SqlLab/types'; +import { QueryEditor, SqlLabRootState } from 'src/SqlLab/types'; import { queryEditorSetQueryLimit } from 'src/SqlLab/actions/sqlLab'; export interface QueryLimitSelectProps { - queryEditor: QueryEditor; + queryEditorId: string; maxRow: number; defaultQueryLimit: number; } @@ -79,23 +80,23 @@ function renderQueryLimit( } const QueryLimitSelect = ({ - queryEditor, + queryEditorId, maxRow, defaultQueryLimit, }: QueryLimitSelectProps) => { - const queryLimit = useSelector( - ({ sqlLab: { unsavedQueryEditor } }) => { - const updatedQueryEditor = { - ...queryEditor, - ...(unsavedQueryEditor.id === queryEditor.id && unsavedQueryEditor), - }; - return updatedQueryEditor.queryLimit || defaultQueryLimit; - }, - ); + const theme = useTheme(); const dispatch = useDispatch(); + + const queryEditor = useSelector>( + ({ sqlLab: { unsavedQueryEditor, queryEditors } }) => ({ + ...queryEditors.find(({ id }) => id === queryEditorId), + ...(queryEditorId === unsavedQueryEditor.id && unsavedQueryEditor), + }), + ); + const queryLimit = queryEditor.queryLimit ?? defaultQueryLimit; + const setQueryLimit = (updatedQueryLimit: number) => dispatch(queryEditorSetQueryLimit(queryEditor, updatedQueryLimit)); - const theme = useTheme(); return ( diff --git a/superset-frontend/src/SqlLab/components/RunQueryActionButton/RunQueryActionButton.test.tsx b/superset-frontend/src/SqlLab/components/RunQueryActionButton/RunQueryActionButton.test.tsx index 189a5fd2f7ab..eb22f8ae7769 100644 --- a/superset-frontend/src/SqlLab/components/RunQueryActionButton/RunQueryActionButton.test.tsx +++ b/superset-frontend/src/SqlLab/components/RunQueryActionButton/RunQueryActionButton.test.tsx @@ -41,7 +41,7 @@ jest.mock('src/components/Select/AsyncSelect', () => () => ( )); const defaultProps = { - queryEditor: defaultQueryEditor, + queryEditorId: defaultQueryEditor.id, allowAsync: false, dbId: 1, queryState: 'ready', diff --git a/superset-frontend/src/SqlLab/components/RunQueryActionButton/index.tsx b/superset-frontend/src/SqlLab/components/RunQueryActionButton/index.tsx index 5cc453f5ee93..6bc84e6ceabc 100644 --- a/superset-frontend/src/SqlLab/components/RunQueryActionButton/index.tsx +++ b/superset-frontend/src/SqlLab/components/RunQueryActionButton/index.tsx @@ -17,6 +17,7 @@ * under the License. */ import React, { useMemo } from 'react'; +import { useSelector } from 'react-redux'; import { t, styled, useTheme } from '@superset-ui/core'; import { Menu } from 'src/components/Menu'; @@ -24,16 +25,14 @@ import Button from 'src/components/Button'; import Icons from 'src/components/Icons'; import { DropdownButton } from 'src/components/DropdownButton'; import { detectOS } from 'src/utils/common'; -import { shallowEqual, useSelector } from 'react-redux'; import { QueryEditor, SqlLabRootState, QueryButtonProps, } from 'src/SqlLab/types'; -import { getUpToDateQuery } from 'src/SqlLab/actions/sqlLab'; export interface Props { - queryEditor: QueryEditor; + queryEditorId: string; allowAsync: boolean; queryState?: string; runQuery: (c?: boolean) => void; @@ -88,7 +87,7 @@ const StyledButton = styled.span` const RunQueryActionButton = ({ allowAsync = false, - queryEditor, + queryEditorId, queryState, overlayCreateAsMenu, runQuery, @@ -96,19 +95,14 @@ const RunQueryActionButton = ({ }: Props) => { const theme = useTheme(); const userOS = detectOS(); - const { selectedText, sql } = useSelector< - SqlLabRootState, - Pick - >(rootState => { - const currentQueryEditor = getUpToDateQuery( - rootState, - queryEditor, - ) as unknown as QueryEditor; - return { - selectedText: currentQueryEditor.selectedText, - sql: currentQueryEditor.sql, - }; - }, shallowEqual); + + const queryEditor = useSelector>( + ({ sqlLab: { unsavedQueryEditor, queryEditors } }) => ({ + ...queryEditors.find(({ id }) => id === queryEditorId), + ...(queryEditorId === unsavedQueryEditor.id && unsavedQueryEditor), + }), + ); + const { selectedText, sql } = queryEditor; const shouldShowStopBtn = !!queryState && ['running', 'pending'].indexOf(queryState) > -1; diff --git a/superset-frontend/src/SqlLab/components/SaveQuery/SaveQuery.test.jsx b/superset-frontend/src/SqlLab/components/SaveQuery/SaveQuery.test.jsx index 2a5fcf3eb79f..171ac527efbe 100644 --- a/superset-frontend/src/SqlLab/components/SaveQuery/SaveQuery.test.jsx +++ b/superset-frontend/src/SqlLab/components/SaveQuery/SaveQuery.test.jsx @@ -22,14 +22,14 @@ import thunk from 'redux-thunk'; import { render, screen, waitFor } from 'spec/helpers/testing-library'; import userEvent from '@testing-library/user-event'; import SaveQuery from 'src/SqlLab/components/SaveQuery'; -import { initialState, databases } from 'src/SqlLab/fixtures'; +import { + initialState, + extraQueryEditor1, + databases, +} from 'src/SqlLab/fixtures'; const mockedProps = { - queryEditor: { - dbId: 1, - schema: 'main', - sql: 'SELECT * FROM t', - }, + queryEditorId: extraQueryEditor1.id, animation: false, database: databases.result[0], onUpdate: () => {}, @@ -111,14 +111,7 @@ describe('SavedQuery', () => { }); it('renders a "save as new" and "update" button if query already exists', () => { - const props = { - ...mockedProps, - queryEditor: { - ...mockedProps.query, - remoteId: '42', - }, - }; - render(, { + render(, { useRedux: true, store: mockStore(initialState), }); diff --git a/superset-frontend/src/SqlLab/components/SaveQuery/index.tsx b/superset-frontend/src/SqlLab/components/SaveQuery/index.tsx index 38cd5625ecd6..9f9042ab38a2 100644 --- a/superset-frontend/src/SqlLab/components/SaveQuery/index.tsx +++ b/superset-frontend/src/SqlLab/components/SaveQuery/index.tsx @@ -17,7 +17,7 @@ * under the License. */ import React, { useState, useEffect } from 'react'; -import { useSelector, shallowEqual } from 'react-redux'; +import { useSelector } from 'react-redux'; import { Row, Col } from 'src/components'; import { Input, TextArea } from 'src/components/Input'; import { t, styled } from '@superset-ui/core'; @@ -34,7 +34,7 @@ import { getDatasourceAsSaveableDataset } from 'src/utils/datasourceUtils'; import { QueryEditor, SqlLabRootState } from 'src/SqlLab/types'; interface SaveQueryProps { - queryEditor: QueryEditor; + queryEditorId: string; columns: ISaveableDatasource['columns']; onSave: (arg0: QueryPayload) => void; onUpdate: (arg0: QueryPayload) => void; @@ -81,21 +81,21 @@ const Styles = styled.span` `; export default function SaveQuery({ - queryEditor, + queryEditorId, onSave = () => {}, onUpdate, saveQueryWarning = null, database, columns, }: SaveQueryProps) { - const query = useSelector( - ({ sqlLab: { unsavedQueryEditor } }) => ({ - ...queryEditor, - ...(queryEditor.id === unsavedQueryEditor.id && unsavedQueryEditor), - columns, + const queryEditor = useSelector>( + ({ sqlLab: { unsavedQueryEditor, queryEditors } }) => ({ + ...queryEditors.find(({ id }) => id === queryEditorId), + ...(queryEditorId === unsavedQueryEditor.id && unsavedQueryEditor), }), - shallowEqual, ); + + const query = { ...queryEditor, columns }; const defaultLabel = query.name || query.description || t('Undefined'); const [description, setDescription] = useState( query.description || '', diff --git a/superset-frontend/src/SqlLab/components/ShareSqlLabQuery/ShareSqlLabQuery.test.jsx b/superset-frontend/src/SqlLab/components/ShareSqlLabQuery/ShareSqlLabQuery.test.jsx index 22913894fbc3..a3fc13993a89 100644 --- a/superset-frontend/src/SqlLab/components/ShareSqlLabQuery/ShareSqlLabQuery.test.jsx +++ b/superset-frontend/src/SqlLab/components/ShareSqlLabQuery/ShareSqlLabQuery.test.jsx @@ -28,7 +28,7 @@ import '@testing-library/jest-dom/extend-expect'; import userEvent from '@testing-library/user-event'; import * as utils from 'src/utils/common'; import ShareSqlLabQuery from 'src/SqlLab/components/ShareSqlLabQuery'; -import { initialState } from 'src/SqlLab/fixtures'; +import { initialState, extraQueryEditor1 } from 'src/SqlLab/fixtures'; const mockStore = configureStore([thunk]); const store = mockStore(initialState); @@ -41,20 +41,12 @@ const standardProvider = ({ children }) => ( ); const defaultProps = { - queryEditor: { - id: 'qe1', - dbId: 0, - name: 'query title', - schema: 'query_schema', - autorun: false, - sql: 'SELECT * FROM ...', - remoteId: 999, - }, + queryEditorId: extraQueryEditor1.id, addDangerToast: jest.fn(), }; const unsavedQueryEditor = { - id: defaultProps.queryEditor.id, + id: defaultProps.queryEditorId, dbId: 9888, name: 'query title changed', schema: 'query_schema_updated', @@ -110,7 +102,8 @@ describe('ShareSqlLabQuery', () => { }); }); const button = screen.getByRole('button'); - const { id, remoteId, ...expected } = defaultProps.queryEditor; + const { autorun, dbId, name, schema, sql } = extraQueryEditor1; + const expected = { autorun, dbId, name, schema, sql }; const storeQuerySpy = jest.spyOn(utils, 'storeQuery'); userEvent.click(button); expect(storeQuerySpy.mock.calls).toHaveLength(1); diff --git a/superset-frontend/src/SqlLab/components/ShareSqlLabQuery/index.tsx b/superset-frontend/src/SqlLab/components/ShareSqlLabQuery/index.tsx index 37481bdee924..ba339f0bc77d 100644 --- a/superset-frontend/src/SqlLab/components/ShareSqlLabQuery/index.tsx +++ b/superset-frontend/src/SqlLab/components/ShareSqlLabQuery/index.tsx @@ -17,7 +17,7 @@ * under the License. */ import React from 'react'; -import { shallowEqual, useSelector } from 'react-redux'; +import { useSelector } from 'react-redux'; import { t, useTheme, styled } from '@superset-ui/core'; import Button from 'src/components/Button'; import Icons from 'src/components/Icons'; @@ -28,8 +28,8 @@ import { getClientErrorObject } from 'src/utils/getClientErrorObject'; import { FeatureFlag, isFeatureEnabled } from 'src/featureFlags'; import { QueryEditor, SqlLabRootState } from 'src/SqlLab/types'; -interface ShareSqlLabQueryPropTypes { - queryEditor: QueryEditor; +interface ShareSqlLabQueryProps { + queryEditorId: string; addDangerToast: (msg: string) => void; } @@ -43,22 +43,20 @@ const StyledIcon = styled(Icons.Link)` } `; -function ShareSqlLabQuery({ - queryEditor, +const ShareSqlLabQuery = ({ + queryEditorId, addDangerToast, -}: ShareSqlLabQueryPropTypes) { +}: ShareSqlLabQueryProps) => { const theme = useTheme(); - const { dbId, name, schema, autorun, sql, remoteId } = useSelector< - SqlLabRootState, - Partial - >(({ sqlLab: { unsavedQueryEditor } }) => { - const { dbId, name, schema, autorun, sql, remoteId } = { - ...queryEditor, - ...(unsavedQueryEditor.id === queryEditor.id && unsavedQueryEditor), - }; - return { dbId, name, schema, autorun, sql, remoteId }; - }, shallowEqual); + const queryEditor = useSelector>( + ({ sqlLab: { unsavedQueryEditor, queryEditors } }) => ({ + ...queryEditors.find(({ id }) => id === queryEditorId), + ...(queryEditorId === unsavedQueryEditor.id && unsavedQueryEditor), + }), + ); + + const { dbId, name, schema, autorun, sql, remoteId } = queryEditor; const getCopyUrlForKvStore = (callback: Function) => { const sharedQuery = { dbId, name, schema, autorun, sql }; @@ -127,6 +125,6 @@ function ShareSqlLabQuery({ )} ); -} +}; export default withToasts(ShareSqlLabQuery); diff --git a/superset-frontend/src/SqlLab/components/SqlEditor/index.jsx b/superset-frontend/src/SqlLab/components/SqlEditor/index.jsx index 3e8ee84f6c50..15e4b589880d 100644 --- a/superset-frontend/src/SqlLab/components/SqlEditor/index.jsx +++ b/superset-frontend/src/SqlLab/components/SqlEditor/index.jsx @@ -476,7 +476,7 @@ const SqlEditor = ({ onChange={params => { dispatch(queryEditorSetTemplateParams(qe, params)); }} - queryEditor={qe} + queryEditorId={qe.id} /> )} @@ -540,7 +540,7 @@ const SqlEditor = ({ )} @@ -576,7 +576,7 @@ const SqlEditor = ({
dispatch(updateSavedQuery(query))} @@ -585,7 +585,7 @@ const SqlEditor = ({ /> - + @@ -665,9 +665,8 @@ const SqlEditor = ({ > setShowEmptyState(bool)} /> diff --git a/superset-frontend/src/SqlLab/components/SqlEditorLeftBar/SqlEditorLeftBar.test.jsx b/superset-frontend/src/SqlLab/components/SqlEditorLeftBar/SqlEditorLeftBar.test.jsx index 5f9b140c12fa..77b4613a0c26 100644 --- a/superset-frontend/src/SqlLab/components/SqlEditorLeftBar/SqlEditorLeftBar.test.jsx +++ b/superset-frontend/src/SqlLab/components/SqlEditorLeftBar/SqlEditorLeftBar.test.jsx @@ -19,23 +19,17 @@ import React from 'react'; import configureStore from 'redux-mock-store'; import fetchMock from 'fetch-mock'; -import { render, screen, act } from 'spec/helpers/testing-library'; +import { render, screen, waitFor } from 'spec/helpers/testing-library'; import userEvent from '@testing-library/user-event'; import { Provider } from 'react-redux'; import '@testing-library/jest-dom/extend-expect'; import thunk from 'redux-thunk'; import SqlEditorLeftBar from 'src/SqlLab/components/SqlEditorLeftBar'; -import { - table, - initialState, - defaultQueryEditor, - mockedActions, -} from 'src/SqlLab/fixtures'; +import { table, initialState, defaultQueryEditor } from 'src/SqlLab/fixtures'; const mockedProps = { - actions: mockedActions, tables: [table], - queryEditor: defaultQueryEditor, + queryEditorId: defaultQueryEditor.id, database: { id: 1, database_name: 'main', @@ -58,7 +52,16 @@ fetchMock.get('glob:*/superset/tables/**', { tableLength: 1, }); +const setup = (props, store) => + render(, { + useRedux: true, + ...(store && { store }), + }); + describe('Left Panel Expansion', () => { + const renderAndWait = (props, store) => + waitFor(async () => setup(props, store)); + test('is valid', () => { expect( React.isValidElement( @@ -70,10 +73,7 @@ describe('Left Panel Expansion', () => { }); test('renders a TableElement', async () => { - render(, { - useRedux: true, - initialState, - }); + await renderAndWait(mockedProps, store); expect(await screen.findByText(/Database/i)).toBeInTheDocument(); expect( @@ -82,10 +82,7 @@ describe('Left Panel Expansion', () => { }); test('table should be visible when expanded is true', async () => { - const { container } = render(, { - useRedux: true, - initialState, - }); + const { container } = await renderAndWait(mockedProps, store); const dbSelect = screen.getByRole('combobox', { name: 'Select database or type database name', @@ -96,7 +93,7 @@ describe('Left Panel Expansion', () => { const dropdown = screen.getByText(/Table/i); const abUser = screen.queryAllByText(/ab_user/i); - act(async () => { + waitFor(async () => { expect(await screen.findByText(/Database/i)).toBeInTheDocument(); expect(dbSelect).toBeInTheDocument(); expect(schemaSelect).toBeInTheDocument(); @@ -109,38 +106,29 @@ describe('Left Panel Expansion', () => { }); test('should toggle the table when the header is clicked', async () => { - const collapseMock = jest.fn(); - render( - , - { - useRedux: true, - initialState, - }, - ); + const store = mockStore(initialState); + await renderAndWait(mockedProps, store); expect(await screen.findByText(/ab_user/)).toBeInTheDocument(); const header = screen.getByText(/ab_user/); userEvent.click(header); - expect(collapseMock).toHaveBeenCalled(); + + await waitFor(() => { + expect(store.getActions()).toHaveLength(1); + expect(store.getActions()[0].type).toEqual('COLLAPSE_TABLE'); + }); }); test('When changing database the table list must be updated', async () => { - const { rerender } = render(, { - useRedux: true, - initialState, - }); + const { rerender } = await renderAndWait(mockedProps, store); - await act(async () => { + await waitFor(async () => { expect(await screen.findByText(/main/i)).toBeInTheDocument(); expect(await screen.findByText(/ab_user/i)).toBeInTheDocument(); }); rerender( void; - queryEditorSetFunctionNames: (queryEditor: QueryEditor, dbId: number) => void; - collapseTable: (table: Table) => void; - expandTable: (table: Table) => void; - addTable: (queryEditor: any, database: any, value: any, schema: any) => void; - setDatabases: (arg0: any) => {}; - addDangerToast: (msg: string) => void; - queryEditorSetSchema: (queryEditor: QueryEditor, schema?: string) => void; - queryEditorSetSchemaOptions: ( - queryEditor: QueryEditor, - options: SchemaOption[], - ) => void; - queryEditorSetTableOptions: ( - queryEditor: QueryEditor, - options: Array, - ) => void; - resetState: () => void; -} - interface SqlEditorLeftBarProps { - queryEditor: QueryEditor; + queryEditorId: string; height?: number; tables?: ExtendedTable[]; - actions: actionsTypes & TableElementProps['actions']; database: DatabaseObject; setEmptyState: Dispatch>; } @@ -102,30 +96,30 @@ const collapseStyles = (theme: SupersetTheme) => css` } `; -export default function SqlEditorLeftBar({ - actions, +const SqlEditorLeftBar = ({ database, - queryEditor, + queryEditorId, tables = [], height = 500, setEmptyState, -}: SqlEditorLeftBarProps) { +}: SqlEditorLeftBarProps) => { + const dispatch = useDispatch(); + + const queryEditor = useSelector>( + ({ sqlLab: { unsavedQueryEditor, queryEditors } }) => ({ + ...queryEditors.find(({ id }) => id === queryEditorId), + ...(queryEditorId === unsavedQueryEditor.id && unsavedQueryEditor), + }), + ); + // Ref needed to avoid infinite rerenders on handlers // that require and modify the queryEditor - const queryEditorRef = useRef(queryEditor); + const queryEditorRef = useRef>(queryEditor); const [emptyResultsWithSearch, setEmptyResultsWithSearch] = useState(false); const [userSelectedDb, setUserSelected] = useState( null, ); - const schema = useSelector( - ({ sqlLab: { unsavedQueryEditor } }) => { - const updatedQueryEditor = { - ...queryEditor, - ...(unsavedQueryEditor.id === queryEditor.id && unsavedQueryEditor), - }; - return updatedQueryEditor.schema; - }, - ); + const { schema } = queryEditor; useEffect(() => { const bool = querystring.parse(window.location.search).db; @@ -150,8 +144,8 @@ export default function SqlEditorLeftBar({ const onDbChange = ({ id: dbId }: { id: number }) => { setEmptyState(false); - actions.queryEditorSetDb(queryEditor, dbId); - actions.queryEditorSetFunctionNames(queryEditor, dbId); + dispatch(queryEditorSetDb(queryEditor, dbId)); + dispatch(queryEditorSetFunctionNames(queryEditor, dbId)); }; const selectedTableNames = useMemo( @@ -176,21 +170,21 @@ export default function SqlEditorLeftBar({ }); tablesToAdd.forEach(tableName => - actions.addTable(queryEditor, database, tableName, schemaName), + dispatch(addTable(queryEditor, database, tableName, schemaName)), ); - actions.removeTables(currentTables); + dispatch(removeTables(currentTables)); }; const onToggleTable = (updatedTables: string[]) => { tables.forEach((table: ExtendedTable) => { if (!updatedTables.includes(table.id.toString()) && table.expanded) { - actions.collapseTable(table); + dispatch(collapseTable(table)); } else if ( updatedTables.includes(table.id.toString()) && !table.expanded ) { - actions.expandTable(table); + dispatch(expandTable(table)); } }); }; @@ -233,41 +227,32 @@ export default function SqlEditorLeftBar({ } /> ); - const handleSchemaChange = useCallback( - (schema: string) => { - if (queryEditorRef.current) { - actions.queryEditorSetSchema(queryEditorRef.current, schema); - } - }, - [actions], - ); + const handleSchemaChange = useCallback((schema: string) => { + if (queryEditorRef.current) { + dispatch(queryEditorSetSchema(queryEditorRef.current, schema)); + } + }, []); - const handleTablesLoad = React.useCallback( - (options: Array) => { - if (queryEditorRef.current) { - actions.queryEditorSetTableOptions(queryEditorRef.current, options); - } - }, - [actions], - ); + const handleTablesLoad = useCallback((options: Array) => { + if (queryEditorRef.current) { + dispatch(queryEditorSetTableOptions(queryEditorRef.current, options)); + } + }, []); - const handleSchemasLoad = React.useCallback( - (options: Array) => { - if (queryEditorRef.current) { - actions.queryEditorSetSchemaOptions(queryEditorRef.current, options); - } - }, - [actions], - ); + const handleSchemasLoad = useCallback((options: Array) => { + if (queryEditorRef.current) { + dispatch(queryEditorSetSchemaOptions(queryEditorRef.current, options)); + } + }, []); return ( -
+
dispatch(setDatabases(result))} + handleError={message => dispatch(addDangerToast(message))} onDbChange={onDbChange} onSchemaChange={handleSchemaChange} onSchemasLoad={handleSchemasLoad} @@ -295,7 +280,7 @@ export default function SqlEditorLeftBar({ expandIcon={renderExpandIconWithTooltip} > {tables.map(table => ( - + ))}
@@ -304,11 +289,13 @@ export default function SqlEditorLeftBar({ )}
); -} +}; + +export default SqlEditorLeftBar; diff --git a/superset-frontend/src/SqlLab/components/TableElement/TableElement.test.jsx b/superset-frontend/src/SqlLab/components/TableElement/TableElement.test.jsx index 91bddc57fb2b..60efa3a59677 100644 --- a/superset-frontend/src/SqlLab/components/TableElement/TableElement.test.jsx +++ b/superset-frontend/src/SqlLab/components/TableElement/TableElement.test.jsx @@ -17,22 +17,24 @@ * under the License. */ import React from 'react'; -import { mount, shallow } from 'enzyme'; +import { mount } from 'enzyme'; import { Provider } from 'react-redux'; import configureStore from 'redux-mock-store'; +import thunk from 'redux-thunk'; import { supersetTheme, ThemeProvider } from '@superset-ui/core'; import Collapse from 'src/components/Collapse'; import { IconTooltip } from 'src/components/IconTooltip'; import TableElement from 'src/SqlLab/components/TableElement'; import ColumnElement from 'src/SqlLab/components/ColumnElement'; import waitForComponentToPaint from 'spec/helpers/waitForComponentToPaint'; -import { mockedActions, table } from 'src/SqlLab/fixtures'; +import { initialState, table } from 'src/SqlLab/fixtures'; + +const middlewares = [thunk]; +const mockStore = configureStore(middlewares); describe('TableElement', () => { - const mockStore = configureStore([]); - const store = mockStore({}); + const store = mockStore(initialState); const mockedProps = { - actions: mockedActions, table, timeout: 0, }; @@ -57,7 +59,17 @@ describe('TableElement', () => { expect(wrapper.find(IconTooltip)).toHaveLength(4); }); it('has 14 columns', () => { - const wrapper = shallow(); + const wrapper = mount( + + + , + { + wrappingComponent: ThemeProvider, + wrappingComponentProps: { + theme: supersetTheme, + }, + }, + ); expect(wrapper.find(ColumnElement)).toHaveLength(14); }); it('mounts', () => { @@ -143,6 +155,7 @@ describe('TableElement', () => { }, ); wrapper.find('.table-remove').hostNodes().simulate('click'); - expect(mockedActions.removeDataPreview.called).toBe(true); + expect(store.getActions()).toHaveLength(1); + expect(store.getActions()[0].type).toEqual('REMOVE_DATA_PREVIEW'); }); }); diff --git a/superset-frontend/src/SqlLab/components/TableElement/index.tsx b/superset-frontend/src/SqlLab/components/TableElement/index.tsx index 1ecc7822203f..44fbe6e1cc0c 100644 --- a/superset-frontend/src/SqlLab/components/TableElement/index.tsx +++ b/superset-frontend/src/SqlLab/components/TableElement/index.tsx @@ -17,12 +17,14 @@ * under the License. */ import React, { useState, useRef } from 'react'; +import { useDispatch } from 'react-redux'; import Collapse from 'src/components/Collapse'; import Card from 'src/components/Card'; import ButtonGroup from 'src/components/ButtonGroup'; import { t, styled } from '@superset-ui/core'; import { debounce } from 'lodash'; +import { removeDataPreview, removeTables } from 'src/SqlLab/actions/sqlLab'; import { Tooltip } from 'src/components/Tooltip'; import CopyToClipboard from 'src/components/CopyToClipboard'; import { IconTooltip } from 'src/components/IconTooltip'; @@ -55,10 +57,6 @@ export interface Table { export interface TableElementProps { table: Table; - actions: { - removeDataPreview: (table: Table) => void; - removeTables: (tables: Table[]) => void; - }; } const StyledSpan = styled.span` @@ -74,7 +72,9 @@ const Fade = styled.div` opacity: ${(props: { hovered: boolean }) => (props.hovered ? 1 : 0)}; `; -const TableElement = ({ table, actions, ...props }: TableElementProps) => { +const TableElement = ({ table, ...props }: TableElementProps) => { + const dispatch = useDispatch(); + const [sortColumns, setSortColumns] = useState(false); const [hovered, setHovered] = useState(false); const tableNameRef = useRef(null); @@ -84,8 +84,8 @@ const TableElement = ({ table, actions, ...props }: TableElementProps) => { }; const removeTable = () => { - actions.removeDataPreview(table); - actions.removeTables([table]); + dispatch(removeDataPreview(table)); + dispatch(removeTables([table])); }; const toggleSortColumns = () => { diff --git a/superset-frontend/src/SqlLab/components/TemplateParamsEditor/TemplateParamsEditor.test.tsx b/superset-frontend/src/SqlLab/components/TemplateParamsEditor/TemplateParamsEditor.test.tsx index 886edb7afc12..30c1b74319d4 100644 --- a/superset-frontend/src/SqlLab/components/TemplateParamsEditor/TemplateParamsEditor.test.tsx +++ b/superset-frontend/src/SqlLab/components/TemplateParamsEditor/TemplateParamsEditor.test.tsx @@ -55,7 +55,7 @@ const setup = (otherProps: Partial = {}, store?: Store) => {}} - queryEditor={defaultQueryEditor} + queryEditorId={defaultQueryEditor.id} {...otherProps} />, { diff --git a/superset-frontend/src/SqlLab/components/TemplateParamsEditor/index.tsx b/superset-frontend/src/SqlLab/components/TemplateParamsEditor/index.tsx index 4eea10da05fe..7ca92e810aa3 100644 --- a/superset-frontend/src/SqlLab/components/TemplateParamsEditor/index.tsx +++ b/superset-frontend/src/SqlLab/components/TemplateParamsEditor/index.tsx @@ -17,18 +17,17 @@ * under the License. */ import React, { useState, useEffect } from 'react'; -import Badge from 'src/components/Badge'; +import { useSelector } from 'react-redux'; import { t, styled } from '@superset-ui/core'; import { InfoTooltipWithTrigger } from '@superset-ui/chart-controls'; import { debounce } from 'lodash'; +import Badge from 'src/components/Badge'; import ModalTrigger from 'src/components/ModalTrigger'; import { ConfigEditor } from 'src/components/AsyncAceEditor'; import { FAST_DEBOUNCE } from 'src/constants'; import { Tooltip } from 'src/components/Tooltip'; -import { useSelector } from 'react-redux'; import { QueryEditor, SqlLabRootState } from 'src/SqlLab/types'; -import { getUpToDateQuery } from 'src/SqlLab/actions/sqlLab'; const StyledConfigEditor = styled(ConfigEditor)` &.ace_editor { @@ -37,23 +36,26 @@ const StyledConfigEditor = styled(ConfigEditor)` `; export type Props = { - queryEditor: QueryEditor; + queryEditorId: string; language: 'yaml' | 'json'; onChange: () => void; }; function TemplateParamsEditor({ - queryEditor, + queryEditorId, language, onChange = () => {}, }: Props) { const [parsedJSON, setParsedJSON] = useState({}); const [isValid, setIsValid] = useState(true); - const code = useSelector( - rootState => - (getUpToDateQuery(rootState, queryEditor) as unknown as QueryEditor) - .templateParams || '{}', + + const queryEditor = useSelector>( + ({ sqlLab: { unsavedQueryEditor, queryEditors } }) => ({ + ...queryEditors.find(({ id }) => id === queryEditorId), + ...(queryEditorId === unsavedQueryEditor.id && unsavedQueryEditor), + }), ); + const code = queryEditor.templateParams ?? '{}'; useEffect(() => { try { diff --git a/superset-frontend/src/SqlLab/fixtures.ts b/superset-frontend/src/SqlLab/fixtures.ts index 72c1c0d50f34..6ac027585dd0 100644 --- a/superset-frontend/src/SqlLab/fixtures.ts +++ b/superset-frontend/src/SqlLab/fixtures.ts @@ -203,6 +203,9 @@ export const extraQueryEditor1 = { id: 'diekd23', sql: 'SELECT *\nFROM\nWHERE\nLIMIT', name: 'Untitled Query 2', + selectedText: 'SELECT', + queryLimit: 10, + remoteId: '42', }; export const extraQueryEditor2 = { From f84d3312acb30b818736aefa698188cd3b0a269e Mon Sep 17 00:00:00 2001 From: EugeneTorap Date: Thu, 29 Sep 2022 19:11:46 +0300 Subject: [PATCH 2/8] Refactoring --- .../RunQueryActionButton/RunQueryActionButton.test.tsx | 4 ++-- .../src/SqlLab/components/RunQueryActionButton/index.tsx | 4 ++-- .../src/SqlLab/components/SaveQuery/index.tsx | 8 +++++--- .../TemplateParamsEditor/TemplateParamsEditor.test.tsx | 7 +++++-- .../src/SqlLab/components/TemplateParamsEditor/index.tsx | 6 +++--- 5 files changed, 17 insertions(+), 12 deletions(-) diff --git a/superset-frontend/src/SqlLab/components/RunQueryActionButton/RunQueryActionButton.test.tsx b/superset-frontend/src/SqlLab/components/RunQueryActionButton/RunQueryActionButton.test.tsx index eb22f8ae7769..6c54b9938205 100644 --- a/superset-frontend/src/SqlLab/components/RunQueryActionButton/RunQueryActionButton.test.tsx +++ b/superset-frontend/src/SqlLab/components/RunQueryActionButton/RunQueryActionButton.test.tsx @@ -24,7 +24,7 @@ import { Store } from 'redux'; import { render, fireEvent, waitFor } from 'spec/helpers/testing-library'; import { initialState, defaultQueryEditor } from 'src/SqlLab/fixtures'; import RunQueryActionButton, { - Props, + RunQueryActionButtonProps, } from 'src/SqlLab/components/RunQueryActionButton'; const middlewares = [thunk]; @@ -51,7 +51,7 @@ const defaultProps = { overlayCreateAsMenu: null, }; -const setup = (props?: Partial, store?: Store) => +const setup = (props?: Partial, store?: Store) => render(, { useRedux: true, ...(store && { store }), diff --git a/superset-frontend/src/SqlLab/components/RunQueryActionButton/index.tsx b/superset-frontend/src/SqlLab/components/RunQueryActionButton/index.tsx index 6bc84e6ceabc..bd27e4436ee8 100644 --- a/superset-frontend/src/SqlLab/components/RunQueryActionButton/index.tsx +++ b/superset-frontend/src/SqlLab/components/RunQueryActionButton/index.tsx @@ -31,7 +31,7 @@ import { QueryButtonProps, } from 'src/SqlLab/types'; -export interface Props { +export interface RunQueryActionButtonProps { queryEditorId: string; allowAsync: boolean; queryState?: string; @@ -92,7 +92,7 @@ const RunQueryActionButton = ({ overlayCreateAsMenu, runQuery, stopQuery, -}: Props) => { +}: RunQueryActionButtonProps) => { const theme = useTheme(); const userOS = detectOS(); diff --git a/superset-frontend/src/SqlLab/components/SaveQuery/index.tsx b/superset-frontend/src/SqlLab/components/SaveQuery/index.tsx index 9f9042ab38a2..8fe5444cc9cd 100644 --- a/superset-frontend/src/SqlLab/components/SaveQuery/index.tsx +++ b/superset-frontend/src/SqlLab/components/SaveQuery/index.tsx @@ -80,14 +80,14 @@ const Styles = styled.span` } `; -export default function SaveQuery({ +const SaveQuery = ({ queryEditorId, onSave = () => {}, onUpdate, saveQueryWarning = null, database, columns, -}: SaveQueryProps) { +}: SaveQueryProps) => { const queryEditor = useSelector>( ({ sqlLab: { unsavedQueryEditor, queryEditors } }) => ({ ...queryEditors.find(({ id }) => id === queryEditorId), @@ -233,4 +233,6 @@ export default function SaveQuery({ ); -} +}; + +export default SaveQuery; diff --git a/superset-frontend/src/SqlLab/components/TemplateParamsEditor/TemplateParamsEditor.test.tsx b/superset-frontend/src/SqlLab/components/TemplateParamsEditor/TemplateParamsEditor.test.tsx index 30c1b74319d4..fdf8fd3b53f5 100644 --- a/superset-frontend/src/SqlLab/components/TemplateParamsEditor/TemplateParamsEditor.test.tsx +++ b/superset-frontend/src/SqlLab/components/TemplateParamsEditor/TemplateParamsEditor.test.tsx @@ -30,7 +30,7 @@ import { import { initialState, defaultQueryEditor } from 'src/SqlLab/fixtures'; import TemplateParamsEditor, { - Props, + TemplateParamsEditorProps, } from 'src/SqlLab/components/TemplateParamsEditor'; jest.mock('src/components/DeprecatedSelect', () => () => ( @@ -50,7 +50,10 @@ jest.mock('src/components/AsyncAceEditor', () => ({ const middlewares = [thunk]; const mockStore = configureStore(middlewares); -const setup = (otherProps: Partial = {}, store?: Store) => +const setup = ( + otherProps: Partial = {}, + store?: Store, +) => render( void; }; -function TemplateParamsEditor({ +const TemplateParamsEditor = ({ queryEditorId, language, onChange = () => {}, -}: Props) { +}: TemplateParamsEditorProps) => { const [parsedJSON, setParsedJSON] = useState({}); const [isValid, setIsValid] = useState(true); From 2aacc10af4a609c686f9cdf9d89c76773756d184 Mon Sep 17 00:00:00 2001 From: EugeneTorap Date: Wed, 5 Oct 2022 21:37:43 +0300 Subject: [PATCH 3/8] Refactoring --- .../components/EstimateQueryCostButton/index.tsx | 15 +++------------ .../SqlLab/components/SqlEditorLeftBar/index.tsx | 7 ++++--- .../components/TemplateParamsEditor/index.tsx | 14 ++++---------- superset-frontend/src/SqlLab/fixtures.ts | 2 -- 4 files changed, 11 insertions(+), 27 deletions(-) diff --git a/superset-frontend/src/SqlLab/components/EstimateQueryCostButton/index.tsx b/superset-frontend/src/SqlLab/components/EstimateQueryCostButton/index.tsx index 323f7cb8c43a..c0c92e3a5549 100644 --- a/superset-frontend/src/SqlLab/components/EstimateQueryCostButton/index.tsx +++ b/superset-frontend/src/SqlLab/components/EstimateQueryCostButton/index.tsx @@ -26,11 +26,8 @@ import Button from 'src/components/Button'; import Loading from 'src/components/Loading'; import ModalTrigger from 'src/components/ModalTrigger'; import { EmptyWrapperType } from 'src/components/TableView/TableView'; -import { - SqlLabRootState, - QueryCostEstimate, - QueryEditor, -} from 'src/SqlLab/types'; +import useQueryEditor from 'src/SqlLab/hooks/useQueryEditor'; +import { SqlLabRootState, QueryCostEstimate } from 'src/SqlLab/types'; export interface EstimateQueryCostButtonProps { getEstimate: Function; @@ -50,13 +47,7 @@ const EstimateQueryCostButton = ({ QueryCostEstimate | undefined >(state => state.sqlLab.queryCostEstimates?.[queryEditorId]); - const queryEditor = useSelector>( - ({ sqlLab: { unsavedQueryEditor, queryEditors } }) => ({ - ...queryEditors.find(({ id }) => id === queryEditorId), - ...(queryEditorId === unsavedQueryEditor.id && unsavedQueryEditor), - }), - ); - const { selectedText } = queryEditor; + const { selectedText } = useQueryEditor(queryEditorId, ['selectedText']); const { cost } = queryCostEstimate || {}; const tableData = useMemo(() => (Array.isArray(cost) ? cost : []), [cost]); const columns = useMemo( diff --git a/superset-frontend/src/SqlLab/components/SqlEditorLeftBar/index.tsx b/superset-frontend/src/SqlLab/components/SqlEditorLeftBar/index.tsx index 76f0a1d0a2ad..1ad183b8dc9b 100644 --- a/superset-frontend/src/SqlLab/components/SqlEditorLeftBar/index.tsx +++ b/superset-frontend/src/SqlLab/components/SqlEditorLeftBar/index.tsx @@ -25,6 +25,7 @@ import React, { Dispatch, SetStateAction, } from 'react'; +import { useDispatch } from 'react-redux'; import querystring from 'query-string'; import { @@ -47,7 +48,6 @@ import Collapse from 'src/components/Collapse'; import Icons from 'src/components/Icons'; import { TableSelectorMultiple } from 'src/components/TableSelector'; import { IconTooltip } from 'src/components/IconTooltip'; -import { QueryEditor, SchemaOption } from 'src/SqlLab/types'; import useQueryEditor from 'src/SqlLab/hooks/useQueryEditor'; import { DatabaseObject } from 'src/components/DatabaseSelector'; import { EmptyStateSmall } from 'src/components/EmptyState'; @@ -104,15 +104,16 @@ const SqlEditorLeftBar = ({ setEmptyState, }: SqlEditorLeftBarProps) => { const dispatch = useDispatch(); + const queryEditor = useQueryEditor(queryEditorId, ['schema']); // Ref needed to avoid infinite rerenders on handlers // that require and modify the queryEditor - const queryEditorRef = useRef(queryEditor); + const queryEditorRef = useRef(queryEditor); const [emptyResultsWithSearch, setEmptyResultsWithSearch] = useState(false); const [userSelectedDb, setUserSelected] = useState( null, ); - const { schema } = useQueryEditor(queryEditor.id, ['schema']); + const { schema } = queryEditor; useEffect(() => { const bool = querystring.parse(window.location.search).db; diff --git a/superset-frontend/src/SqlLab/components/TemplateParamsEditor/index.tsx b/superset-frontend/src/SqlLab/components/TemplateParamsEditor/index.tsx index a34f084b817c..8943c2b8d5d4 100644 --- a/superset-frontend/src/SqlLab/components/TemplateParamsEditor/index.tsx +++ b/superset-frontend/src/SqlLab/components/TemplateParamsEditor/index.tsx @@ -17,7 +17,6 @@ * under the License. */ import React, { useState, useEffect } from 'react'; -import { useSelector } from 'react-redux'; import { t, styled } from '@superset-ui/core'; import { InfoTooltipWithTrigger } from '@superset-ui/chart-controls'; import { debounce } from 'lodash'; @@ -27,7 +26,7 @@ import ModalTrigger from 'src/components/ModalTrigger'; import { ConfigEditor } from 'src/components/AsyncAceEditor'; import { FAST_DEBOUNCE } from 'src/constants'; import { Tooltip } from 'src/components/Tooltip'; -import { QueryEditor, SqlLabRootState } from 'src/SqlLab/types'; +import useQueryEditor from 'src/SqlLab/hooks/useQueryEditor'; const StyledConfigEditor = styled(ConfigEditor)` &.ace_editor { @@ -49,13 +48,8 @@ const TemplateParamsEditor = ({ const [parsedJSON, setParsedJSON] = useState({}); const [isValid, setIsValid] = useState(true); - const queryEditor = useSelector>( - ({ sqlLab: { unsavedQueryEditor, queryEditors } }) => ({ - ...queryEditors.find(({ id }) => id === queryEditorId), - ...(queryEditorId === unsavedQueryEditor.id && unsavedQueryEditor), - }), - ); - const code = queryEditor.templateParams ?? '{}'; + const { templateParams } = useQueryEditor(queryEditorId, ['templateParams']); + const code = templateParams ?? '{}'; useEffect(() => { try { @@ -127,6 +121,6 @@ const TemplateParamsEditor = ({ modalBody={modalBody} /> ); -} +}; export default TemplateParamsEditor; diff --git a/superset-frontend/src/SqlLab/fixtures.ts b/superset-frontend/src/SqlLab/fixtures.ts index 6ac027585dd0..c58613a46671 100644 --- a/superset-frontend/src/SqlLab/fixtures.ts +++ b/superset-frontend/src/SqlLab/fixtures.ts @@ -204,8 +204,6 @@ export const extraQueryEditor1 = { sql: 'SELECT *\nFROM\nWHERE\nLIMIT', name: 'Untitled Query 2', selectedText: 'SELECT', - queryLimit: 10, - remoteId: '42', }; export const extraQueryEditor2 = { From 6e626a7e27c9af21ad48ad4809f1a34e435c55e3 Mon Sep 17 00:00:00 2001 From: EugeneTorap Date: Thu, 6 Oct 2022 12:33:19 +0300 Subject: [PATCH 4/8] Refactoring and bug fixing --- .../components/SqlEditorLeftBar/index.tsx | 39 +++++++++++-------- .../src/components/DatabaseSelector/index.tsx | 2 +- .../src/components/TableSelector/index.tsx | 2 +- 3 files changed, 24 insertions(+), 19 deletions(-) diff --git a/superset-frontend/src/SqlLab/components/SqlEditorLeftBar/index.tsx b/superset-frontend/src/SqlLab/components/SqlEditorLeftBar/index.tsx index 1ad183b8dc9b..a644d9e80d19 100644 --- a/superset-frontend/src/SqlLab/components/SqlEditorLeftBar/index.tsx +++ b/superset-frontend/src/SqlLab/components/SqlEditorLeftBar/index.tsx @@ -18,7 +18,6 @@ */ import React, { useEffect, - useRef, useCallback, useMemo, useState, @@ -106,9 +105,6 @@ const SqlEditorLeftBar = ({ const dispatch = useDispatch(); const queryEditor = useQueryEditor(queryEditorId, ['schema']); - // Ref needed to avoid infinite rerenders on handlers - // that require and modify the queryEditor - const queryEditorRef = useRef(queryEditor); const [emptyResultsWithSearch, setEmptyResultsWithSearch] = useState(false); const [userSelectedDb, setUserSelected] = useState( null, @@ -128,10 +124,6 @@ const SqlEditorLeftBar = ({ } else setUserSelected(database); }, [database]); - useEffect(() => { - queryEditorRef.current = queryEditor; - }, [queryEditor, database]); - const onEmptyResults = (searchText?: string) => { setEmptyResultsWithSearch(!!searchText); }; @@ -221,32 +213,45 @@ const SqlEditorLeftBar = ({ } /> ); + const handleSchemaChange = useCallback((schema: string) => { - if (queryEditorRef.current) { - dispatch(queryEditorSetSchema(queryEditorRef.current, schema)); + if (queryEditor) { + dispatch(queryEditorSetSchema(queryEditor, schema)); } }, []); const handleTablesLoad = useCallback((options: Array) => { - if (queryEditorRef.current) { - dispatch(queryEditorSetTableOptions(queryEditorRef.current, options)); + if (queryEditor) { + dispatch(queryEditorSetTableOptions(queryEditor, options)); } }, []); const handleSchemasLoad = useCallback((options: Array) => { - if (queryEditorRef.current) { - dispatch(queryEditorSetSchemaOptions(queryEditorRef.current, options)); + if (queryEditor) { + dispatch(queryEditorSetSchemaOptions(queryEditor, options)); } }, []); + const handleDbList = useCallback((result: DatabaseObject) => { + dispatch(setDatabases(result)); + }, []); + + const handleError = useCallback((message: string) => { + dispatch(addDangerToast(message)); + }, []); + + const handleResetState = useCallback(() => { + dispatch(resetState()); + }, []); + return (
dispatch(setDatabases(result))} - handleError={message => dispatch(addDangerToast(message))} + getDbList={handleDbList} + handleError={handleError} onDbChange={onDbChange} onSchemaChange={handleSchemaChange} onSchemasLoad={handleSchemasLoad} @@ -283,7 +288,7 @@ const SqlEditorLeftBar = ({ diff --git a/superset-frontend/src/components/DatabaseSelector/index.tsx b/superset-frontend/src/components/DatabaseSelector/index.tsx index c1b13f6f70d8..f15261481935 100644 --- a/superset-frontend/src/components/DatabaseSelector/index.tsx +++ b/superset-frontend/src/components/DatabaseSelector/index.tsx @@ -88,7 +88,7 @@ export interface DatabaseSelectorProps { db?: DatabaseObject | null; emptyState?: ReactNode; formMode?: boolean; - getDbList?: (arg0: any) => {}; + getDbList?: (arg0: any) => void; handleError: (msg: string) => void; isDatabaseSelectEnabled?: boolean; onDbChange?: (db: DatabaseObject) => void; diff --git a/superset-frontend/src/components/TableSelector/index.tsx b/superset-frontend/src/components/TableSelector/index.tsx index 06e42c3badfe..6f085936771d 100644 --- a/superset-frontend/src/components/TableSelector/index.tsx +++ b/superset-frontend/src/components/TableSelector/index.tsx @@ -89,7 +89,7 @@ interface TableSelectorProps { database?: DatabaseObject | null; emptyState?: ReactNode; formMode?: boolean; - getDbList?: (arg0: any) => {}; + getDbList?: (arg0: any) => void; handleError: (msg: string) => void; isDatabaseSelectEnabled?: boolean; onDbChange?: (db: DatabaseObject) => void; From 2956f9633c0718475d64ac294c694f9fc914f6b6 Mon Sep 17 00:00:00 2001 From: EugeneTorap Date: Sun, 9 Oct 2022 16:12:09 +0300 Subject: [PATCH 5/8] Refactoring SqlEditorLeftBar.test.jsx --- .../SqlEditorLeftBar.test.jsx | 154 +++++++++--------- 1 file changed, 74 insertions(+), 80 deletions(-) diff --git a/superset-frontend/src/SqlLab/components/SqlEditorLeftBar/SqlEditorLeftBar.test.jsx b/superset-frontend/src/SqlLab/components/SqlEditorLeftBar/SqlEditorLeftBar.test.jsx index 77b4613a0c26..c2cf1b5a785f 100644 --- a/superset-frontend/src/SqlLab/components/SqlEditorLeftBar/SqlEditorLeftBar.test.jsx +++ b/superset-frontend/src/SqlLab/components/SqlEditorLeftBar/SqlEditorLeftBar.test.jsx @@ -52,97 +52,91 @@ fetchMock.get('glob:*/superset/tables/**', { tableLength: 1, }); -const setup = (props, store) => - render(, { - useRedux: true, - ...(store && { store }), - }); +const renderAndWait = (props, store) => + waitFor(() => + render(, { + useRedux: true, + ...(store && { store }), + }), + ); -describe('Left Panel Expansion', () => { - const renderAndWait = (props, store) => - waitFor(async () => setup(props, store)); +test('is valid', () => { + expect( + React.isValidElement( + + + , + ), + ).toBe(true); +}); - test('is valid', () => { - expect( - React.isValidElement( - - - , - ), - ).toBe(true); - }); +test('renders a TableElement', async () => { + await renderAndWait(mockedProps, store); + expect(await screen.findByText(/Database/i)).toBeInTheDocument(); + const tableElement = screen.getAllByTestId('table-element'); + expect(tableElement.length).toBeGreaterThanOrEqual(1); +}); - test('renders a TableElement', async () => { - await renderAndWait(mockedProps, store); +test('table should be visible when expanded is true', async () => { + const { container } = await renderAndWait(mockedProps, store); - expect(await screen.findByText(/Database/i)).toBeInTheDocument(); - expect( - screen.queryAllByTestId('table-element').length, - ).toBeGreaterThanOrEqual(1); + const dbSelect = screen.getByRole('combobox', { + name: 'Select database or type database name', }); + const schemaSelect = screen.getByRole('combobox', { + name: 'Select schema or type schema name', + }); + const dropdown = screen.getByText(/Table/i); + const abUser = screen.queryAllByText(/ab_user/i); - test('table should be visible when expanded is true', async () => { - const { container } = await renderAndWait(mockedProps, store); - - const dbSelect = screen.getByRole('combobox', { - name: 'Select database or type database name', - }); - const schemaSelect = screen.getByRole('combobox', { - name: 'Select schema or type schema name', - }); - const dropdown = screen.getByText(/Table/i); - const abUser = screen.queryAllByText(/ab_user/i); - - waitFor(async () => { - expect(await screen.findByText(/Database/i)).toBeInTheDocument(); - expect(dbSelect).toBeInTheDocument(); - expect(schemaSelect).toBeInTheDocument(); - expect(dropdown).toBeInTheDocument(); - expect(abUser).toHaveLength(2); - expect( - container.querySelector('.ant-collapse-content-active'), - ).toBeInTheDocument(); - }); + await waitFor(() => { + expect(screen.getByText(/Database/i)).toBeInTheDocument(); + expect(dbSelect).toBeInTheDocument(); + expect(schemaSelect).toBeInTheDocument(); + expect(dropdown).toBeInTheDocument(); + expect(abUser).toHaveLength(2); + expect( + container.querySelector('.ant-collapse-content-active'), + ).toBeInTheDocument(); }); +}); - test('should toggle the table when the header is clicked', async () => { - const store = mockStore(initialState); - await renderAndWait(mockedProps, store); +test('should toggle the table when the header is clicked', async () => { + const store = mockStore(initialState); + await renderAndWait(mockedProps, store); - expect(await screen.findByText(/ab_user/)).toBeInTheDocument(); - const header = screen.getByText(/ab_user/); - userEvent.click(header); + const header = (await screen.findAllByText(/ab_user/))[1]; + expect(header).toBeInTheDocument(); + userEvent.click(header); - await waitFor(() => { - expect(store.getActions()).toHaveLength(1); - expect(store.getActions()[0].type).toEqual('COLLAPSE_TABLE'); - }); + await waitFor(() => { + expect(store.getActions()).toHaveLength(2); + expect(store.getActions()[1].type).toEqual('COLLAPSE_TABLE'); }); +}); - test('When changing database the table list must be updated', async () => { - const { rerender } = await renderAndWait(mockedProps, store); +test('When changing database the table list must be updated', async () => { + const { rerender } = await renderAndWait(mockedProps, store); - await waitFor(async () => { - expect(await screen.findByText(/main/i)).toBeInTheDocument(); - expect(await screen.findByText(/ab_user/i)).toBeInTheDocument(); - }); - rerender( - , - { - useRedux: true, - initialState, - }, - ); - expect(await screen.findByText(/new_db/i)).toBeInTheDocument(); - expect(await screen.findByText(/new_table/i)).toBeInTheDocument(); - }); + expect(screen.getAllByText(/main/i)[0]).toBeInTheDocument(); + expect(screen.getAllByText(/ab_user/i)[0]).toBeInTheDocument(); + + rerender( + , + { + useRedux: true, + initialState, + }, + ); + expect(await screen.findByText(/new_db/i)).toBeInTheDocument(); + expect(await screen.findByText(/new_table/i)).toBeInTheDocument(); }); From 87aec3bf21b5b4d9a4d08e69562a8b854c855aa6 Mon Sep 17 00:00:00 2001 From: EugeneTorap Date: Mon, 7 Nov 2022 17:29:37 +0300 Subject: [PATCH 6/8] Add the correct dependencies to useCallback functions --- .../components/SqlEditorLeftBar/index.tsx | 59 ++++++++++++------- 1 file changed, 37 insertions(+), 22 deletions(-) diff --git a/superset-frontend/src/SqlLab/components/SqlEditorLeftBar/index.tsx b/superset-frontend/src/SqlLab/components/SqlEditorLeftBar/index.tsx index a644d9e80d19..1112bed63acd 100644 --- a/superset-frontend/src/SqlLab/components/SqlEditorLeftBar/index.tsx +++ b/superset-frontend/src/SqlLab/components/SqlEditorLeftBar/index.tsx @@ -214,35 +214,50 @@ const SqlEditorLeftBar = ({ /> ); - const handleSchemaChange = useCallback((schema: string) => { - if (queryEditor) { - dispatch(queryEditorSetSchema(queryEditor, schema)); - } - }, []); + const handleSchemaChange = useCallback( + (schema: string) => { + if (queryEditor) { + dispatch(queryEditorSetSchema(queryEditor, schema)); + } + }, + [dispatch, queryEditor], + ); - const handleTablesLoad = useCallback((options: Array) => { - if (queryEditor) { - dispatch(queryEditorSetTableOptions(queryEditor, options)); - } - }, []); + const handleTablesLoad = useCallback( + (options: Array) => { + if (queryEditor) { + dispatch(queryEditorSetTableOptions(queryEditor, options)); + } + }, + [dispatch, queryEditor], + ); - const handleSchemasLoad = useCallback((options: Array) => { - if (queryEditor) { - dispatch(queryEditorSetSchemaOptions(queryEditor, options)); - } - }, []); + const handleSchemasLoad = useCallback( + (options: Array) => { + if (queryEditor) { + dispatch(queryEditorSetSchemaOptions(queryEditor, options)); + } + }, + [dispatch, queryEditor], + ); - const handleDbList = useCallback((result: DatabaseObject) => { - dispatch(setDatabases(result)); - }, []); + const handleDbList = useCallback( + (result: DatabaseObject) => { + dispatch(setDatabases(result)); + }, + [dispatch], + ); - const handleError = useCallback((message: string) => { - dispatch(addDangerToast(message)); - }, []); + const handleError = useCallback( + (message: string) => { + dispatch(addDangerToast(message)); + }, + [dispatch], + ); const handleResetState = useCallback(() => { dispatch(resetState()); - }, []); + }, [dispatch]); return (
From 74f0c53b8d4caf19f91dd84c38a8c1916ed91c6a Mon Sep 17 00:00:00 2001 From: EugeneTorap Date: Mon, 14 Nov 2022 23:07:27 +0300 Subject: [PATCH 7/8] Fix infinite http requests --- .../components/SqlEditorLeftBar/index.tsx | 59 +++++++------------ 1 file changed, 22 insertions(+), 37 deletions(-) diff --git a/superset-frontend/src/SqlLab/components/SqlEditorLeftBar/index.tsx b/superset-frontend/src/SqlLab/components/SqlEditorLeftBar/index.tsx index 1112bed63acd..a644d9e80d19 100644 --- a/superset-frontend/src/SqlLab/components/SqlEditorLeftBar/index.tsx +++ b/superset-frontend/src/SqlLab/components/SqlEditorLeftBar/index.tsx @@ -214,50 +214,35 @@ const SqlEditorLeftBar = ({ /> ); - const handleSchemaChange = useCallback( - (schema: string) => { - if (queryEditor) { - dispatch(queryEditorSetSchema(queryEditor, schema)); - } - }, - [dispatch, queryEditor], - ); + const handleSchemaChange = useCallback((schema: string) => { + if (queryEditor) { + dispatch(queryEditorSetSchema(queryEditor, schema)); + } + }, []); - const handleTablesLoad = useCallback( - (options: Array) => { - if (queryEditor) { - dispatch(queryEditorSetTableOptions(queryEditor, options)); - } - }, - [dispatch, queryEditor], - ); + const handleTablesLoad = useCallback((options: Array) => { + if (queryEditor) { + dispatch(queryEditorSetTableOptions(queryEditor, options)); + } + }, []); - const handleSchemasLoad = useCallback( - (options: Array) => { - if (queryEditor) { - dispatch(queryEditorSetSchemaOptions(queryEditor, options)); - } - }, - [dispatch, queryEditor], - ); + const handleSchemasLoad = useCallback((options: Array) => { + if (queryEditor) { + dispatch(queryEditorSetSchemaOptions(queryEditor, options)); + } + }, []); - const handleDbList = useCallback( - (result: DatabaseObject) => { - dispatch(setDatabases(result)); - }, - [dispatch], - ); + const handleDbList = useCallback((result: DatabaseObject) => { + dispatch(setDatabases(result)); + }, []); - const handleError = useCallback( - (message: string) => { - dispatch(addDangerToast(message)); - }, - [dispatch], - ); + const handleError = useCallback((message: string) => { + dispatch(addDangerToast(message)); + }, []); const handleResetState = useCallback(() => { dispatch(resetState()); - }, [dispatch]); + }, []); return (
From 8e68df5fbd0af1e44f1cb528e151714b69362f20 Mon Sep 17 00:00:00 2001 From: EugeneTorap Date: Mon, 14 Nov 2022 23:54:23 +0300 Subject: [PATCH 8/8] Fix dbId is undefined --- .../src/SqlLab/components/SqlEditorLeftBar/index.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/superset-frontend/src/SqlLab/components/SqlEditorLeftBar/index.tsx b/superset-frontend/src/SqlLab/components/SqlEditorLeftBar/index.tsx index a644d9e80d19..4b73cffff986 100644 --- a/superset-frontend/src/SqlLab/components/SqlEditorLeftBar/index.tsx +++ b/superset-frontend/src/SqlLab/components/SqlEditorLeftBar/index.tsx @@ -103,7 +103,7 @@ const SqlEditorLeftBar = ({ setEmptyState, }: SqlEditorLeftBarProps) => { const dispatch = useDispatch(); - const queryEditor = useQueryEditor(queryEditorId, ['schema']); + const queryEditor = useQueryEditor(queryEditorId, ['dbId', 'schema']); const [emptyResultsWithSearch, setEmptyResultsWithSearch] = useState(false); const [userSelectedDb, setUserSelected] = useState(