From f3bf3ec2edab6274554024653703b0594ec3ed27 Mon Sep 17 00:00:00 2001 From: EugeneTorap Date: Mon, 5 Dec 2022 14:12:52 +0300 Subject: [PATCH] chore: Remove actions prop and refactor code in SQL Lab (#22231) --- .../superset-ui-core/src/query/types/Query.ts | 1 + .../QueryHistory/QueryHistory.test.tsx | 8 -- .../SqlLab/components/QueryHistory/index.tsx | 9 -- .../QuerySearch/QuerySearch.test.jsx | 1 - .../SqlLab/components/QuerySearch/index.tsx | 29 +++--- .../components/QueryTable/QueryTable.test.jsx | 2 - .../SqlLab/components/QueryTable/index.tsx | 51 ++++------- .../SaveDatasetActionButton/index.tsx | 10 ++- .../components/SouthPane/SouthPane.test.jsx | 88 +++++++------------ .../src/SqlLab/components/SouthPane/index.tsx | 80 ++++++++++------- .../src/SqlLab/components/SouthPane/state.ts | 61 ------------- .../components/SqlEditor/SqlEditor.test.jsx | 18 +--- .../src/SqlLab/components/SqlEditor/index.jsx | 7 +- .../components/TabbedSqlEditors/index.jsx | 1 - superset-frontend/src/SqlLab/types.ts | 4 +- 15 files changed, 125 insertions(+), 245 deletions(-) delete mode 100644 superset-frontend/src/SqlLab/components/SouthPane/state.ts diff --git a/superset-frontend/packages/superset-ui-core/src/query/types/Query.ts b/superset-frontend/packages/superset-ui-core/src/query/types/Query.ts index dd6b6c8b0147c..8a7e403703715 100644 --- a/superset-frontend/packages/superset-ui-core/src/query/types/Query.ts +++ b/superset-frontend/packages/superset-ui-core/src/query/types/Query.ts @@ -296,6 +296,7 @@ export type Query = { errorMessage: string | null; extra: { progress: string | null; + errors?: SupersetError[]; }; id: string; isDataPreview: boolean; diff --git a/superset-frontend/src/SqlLab/components/QueryHistory/QueryHistory.test.tsx b/superset-frontend/src/SqlLab/components/QueryHistory/QueryHistory.test.tsx index 8d25fca910124..6fd84a0d2a290 100644 --- a/superset-frontend/src/SqlLab/components/QueryHistory/QueryHistory.test.tsx +++ b/superset-frontend/src/SqlLab/components/QueryHistory/QueryHistory.test.tsx @@ -20,16 +20,8 @@ import React from 'react'; import { render, screen } from 'spec/helpers/testing-library'; import QueryHistory from 'src/SqlLab/components/QueryHistory'; -const NOOP = () => {}; const mockedProps = { queries: [], - actions: { - queryEditorSetAndSaveSql: NOOP, - cloneQueryToNewTab: NOOP, - fetchQueryResults: NOOP, - clearQueryResults: NOOP, - removeQuery: NOOP, - }, displayLimit: 1000, latestQueryId: 'yhMUZCGb', }; diff --git a/superset-frontend/src/SqlLab/components/QueryHistory/index.tsx b/superset-frontend/src/SqlLab/components/QueryHistory/index.tsx index 86f28069209da..cab1160144a3d 100644 --- a/superset-frontend/src/SqlLab/components/QueryHistory/index.tsx +++ b/superset-frontend/src/SqlLab/components/QueryHistory/index.tsx @@ -23,13 +23,6 @@ import QueryTable from 'src/SqlLab/components/QueryTable'; interface QueryHistoryProps { queries: QueryResponse[]; - actions: { - queryEditorSetAndSaveSql: Function; - cloneQueryToNewTab: Function; - fetchQueryResults: Function; - clearQueryResults: Function; - removeQuery: Function; - }; displayLimit: number; latestQueryId: string | undefined; } @@ -47,7 +40,6 @@ const StyledEmptyStateWrapper = styled.div` const QueryHistory = ({ queries, - actions, displayLimit, latestQueryId, }: QueryHistoryProps) => @@ -64,7 +56,6 @@ const QueryHistory = ({ 'actions', ]} queries={queries} - actions={actions} displayLimit={displayLimit} latestQueryId={latestQueryId} /> diff --git a/superset-frontend/src/SqlLab/components/QuerySearch/QuerySearch.test.jsx b/superset-frontend/src/SqlLab/components/QuerySearch/QuerySearch.test.jsx index a1efdc06c02e6..2a891d34af84c 100644 --- a/superset-frontend/src/SqlLab/components/QuerySearch/QuerySearch.test.jsx +++ b/superset-frontend/src/SqlLab/components/QuerySearch/QuerySearch.test.jsx @@ -43,7 +43,6 @@ fetchMock.get(DATABASE_ENDPOINT, []); describe('QuerySearch', () => { const mockedProps = { - actions: { addDangerToast: jest.fn() }, displayLimit: 50, }; diff --git a/superset-frontend/src/SqlLab/components/QuerySearch/index.tsx b/superset-frontend/src/SqlLab/components/QuerySearch/index.tsx index c36ab374784cc..3018ff1924586 100644 --- a/superset-frontend/src/SqlLab/components/QuerySearch/index.tsx +++ b/superset-frontend/src/SqlLab/components/QuerySearch/index.tsx @@ -17,6 +17,9 @@ * under the License. */ import React, { useState, useEffect } from 'react'; +import { useDispatch } from 'react-redux'; + +import { setDatabases, addDangerToast } from 'src/SqlLab/actions/sqlLab'; import Button from 'src/components/Button'; import Select from 'src/components/DeprecatedSelect'; import { styled, t, SupersetClient, QueryResponse } from '@superset-ui/core'; @@ -33,15 +36,6 @@ import { STATUS_OPTIONS, TIME_OPTIONS } from 'src/SqlLab/constants'; import QueryTable from '../QueryTable'; interface QuerySearchProps { - actions: { - addDangerToast: (msg: string) => void; - setDatabases: (data: Record) => Record; - queryEditorSetAndSaveSql: Function; - cloneQueryToNewTab: Function; - fetchQueryResults: Function; - clearQueryResults: Function; - removeQuery: Function; - }; displayLimit: number; } @@ -77,7 +71,10 @@ const TableStyles = styled.div` const StyledTableStylesContainer = styled.div` overflow: auto; `; -function QuerySearch({ actions, displayLimit }: QuerySearchProps) { + +const QuerySearch = ({ displayLimit }: QuerySearchProps) => { + const dispatch = useDispatch(); + const [databaseId, setDatabaseId] = useState(''); const [userId, setUserId] = useState(''); const [searchText, setSearchText] = useState(''); @@ -133,7 +130,7 @@ function QuerySearch({ actions, displayLimit }: QuerySearchProps) { const queries = Object.values(response.json); setQueriesArray(queries); } catch (err) { - actions.addDangerToast(t('An error occurred when refreshing queries')); + dispatch(addDangerToast(t('An error occurred when refreshing queries'))); } finally { setQueriesLoading(false); } @@ -178,10 +175,10 @@ function QuerySearch({ actions, displayLimit }: QuerySearchProps) { value: id, label: database_name, })); - actions.setDatabases(result); + dispatch(setDatabases(result)); if (result.length === 0) { - actions.addDangerToast( - t("It seems you don't have access to any database"), + dispatch( + addDangerToast(t("It seems you don't have access to any database")), ); } return options; @@ -280,7 +277,6 @@ function QuerySearch({ actions, displayLimit }: QuerySearchProps) { onUserClicked={onUserClicked} onDbClicked={onDbClicked} queries={queriesArray} - actions={actions} displayLimit={displayLimit} /> @@ -288,5 +284,6 @@ function QuerySearch({ actions, displayLimit }: QuerySearchProps) { ); -} +}; + export default QuerySearch; diff --git a/superset-frontend/src/SqlLab/components/QueryTable/QueryTable.test.jsx b/superset-frontend/src/SqlLab/components/QueryTable/QueryTable.test.jsx index f77e631ae2f5c..76784695a8140 100644 --- a/superset-frontend/src/SqlLab/components/QueryTable/QueryTable.test.jsx +++ b/superset-frontend/src/SqlLab/components/QueryTable/QueryTable.test.jsx @@ -25,13 +25,11 @@ import TableView from 'src/components/TableView'; import TableCollection from 'src/components/TableCollection'; import { Provider } from 'react-redux'; import { queries, user } from 'src/SqlLab/fixtures'; -import * as actions from 'src/SqlLab/actions/sqlLab'; describe('QueryTable', () => { const mockedProps = { queries, displayLimit: 100, - actions, latestQueryId: 'ryhMUZCGb', }; it('is valid', () => { diff --git a/superset-frontend/src/SqlLab/components/QueryTable/index.tsx b/superset-frontend/src/SqlLab/components/QueryTable/index.tsx index d7ef5ed51c028..26c395f26a926 100644 --- a/superset-frontend/src/SqlLab/components/QueryTable/index.tsx +++ b/superset-frontend/src/SqlLab/components/QueryTable/index.tsx @@ -22,7 +22,15 @@ import Card from 'src/components/Card'; import ProgressBar from 'src/components/ProgressBar'; import Label from 'src/components/Label'; import { t, useTheme, QueryResponse } from '@superset-ui/core'; -import { useSelector } from 'react-redux'; +import { useDispatch, useSelector } from 'react-redux'; + +import { + queryEditorSetAndSaveSql, + cloneQueryToNewTab, + fetchQueryResults, + clearQueryResults, + removeQuery, +} from 'src/SqlLab/actions/sqlLab'; import TableView from 'src/components/TableView'; import Button from 'src/components/Button'; import { fDuration } from 'src/utils/dates'; @@ -45,13 +53,6 @@ interface QueryTableQuery interface QueryTableProps { columns?: string[]; - actions: { - queryEditorSetAndSaveSql: Function; - cloneQueryToNewTab: Function; - fetchQueryResults: Function; - clearQueryResults: Function; - removeQuery: Function; - }; queries?: QueryResponse[]; onUserClicked?: Function; onDbClicked?: Function; @@ -66,7 +67,6 @@ const openQuery = (id: number) => { const QueryTable = ({ columns = ['started', 'duration', 'rows'], - actions, queries = [], onUserClicked = () => undefined, onDbClicked = () => undefined, @@ -74,6 +74,7 @@ const QueryTable = ({ latestQueryId, }: QueryTableProps) => { const theme = useTheme(); + const dispatch = useDispatch(); const setHeaders = (column: string) => { if (column === 'sql') { @@ -93,25 +94,17 @@ const QueryTable = ({ const user = useSelector(state => state.sqlLab.user); - const { - queryEditorSetAndSaveSql, - cloneQueryToNewTab, - fetchQueryResults, - clearQueryResults, - removeQuery, - } = actions; - const data = useMemo(() => { const restoreSql = (query: QueryResponse) => { - queryEditorSetAndSaveSql({ id: query.sqlEditorId }, query.sql); + dispatch(queryEditorSetAndSaveSql({ id: query.sqlEditorId }, query.sql)); }; const openQueryInNewTab = (query: QueryResponse) => { - cloneQueryToNewTab(query, true); + dispatch(cloneQueryToNewTab(query, true)); }; const openAsyncResults = (query: QueryResponse, displayLimit: number) => { - fetchQueryResults(query, displayLimit); + dispatch(fetchQueryResults(query, displayLimit)); }; const statusAttributes = { @@ -239,7 +232,7 @@ const QueryTable = ({ } modalTitle={t('Data preview')} beforeOpen={() => openAsyncResults(query, displayLimit)} - onExit={() => clearQueryResults(query)} + onExit={() => dispatch(clearQueryResults(query))} modalBody={ removeQuery(query)} + onClick={() => dispatch(removeQuery(query))} > @@ -303,19 +296,7 @@ const QueryTable = ({ return q; }) .reverse(); - }, [ - queries, - onUserClicked, - onDbClicked, - user, - displayLimit, - actions, - clearQueryResults, - cloneQueryToNewTab, - fetchQueryResults, - queryEditorSetAndSaveSql, - removeQuery, - ]); + }, [queries, onUserClicked, onDbClicked, user, displayLimit]); return (
diff --git a/superset-frontend/src/SqlLab/components/SaveDatasetActionButton/index.tsx b/superset-frontend/src/SqlLab/components/SaveDatasetActionButton/index.tsx index 73a85e2ce24fc..dbb25b138a58e 100644 --- a/superset-frontend/src/SqlLab/components/SaveDatasetActionButton/index.tsx +++ b/superset-frontend/src/SqlLab/components/SaveDatasetActionButton/index.tsx @@ -23,15 +23,15 @@ import { DropdownButton } from 'src/components/DropdownButton'; import Button from 'src/components/Button'; import { DropdownButtonProps } from 'antd/lib/dropdown'; -interface Props { +interface SaveDatasetActionButtonProps { setShowSave: (arg0: boolean) => void; overlayMenu: JSX.Element | null; } -export default function SaveDatasetActionButton({ +const SaveDatasetActionButton = ({ setShowSave, overlayMenu, -}: Props) { +}: SaveDatasetActionButtonProps) => { const theme = useTheme(); const StyledDropdownButton = styled( @@ -80,4 +80,6 @@ export default function SaveDatasetActionButton({ {t('Save')} ); -} +}; + +export default SaveDatasetActionButton; diff --git a/superset-frontend/src/SqlLab/components/SouthPane/SouthPane.test.jsx b/superset-frontend/src/SqlLab/components/SouthPane/SouthPane.test.jsx index 5a1e0d339cf94..519e729c41847 100644 --- a/superset-frontend/src/SqlLab/components/SouthPane/SouthPane.test.jsx +++ b/superset-frontend/src/SqlLab/components/SouthPane/SouthPane.test.jsx @@ -19,47 +19,30 @@ import React from 'react'; import configureStore from 'redux-mock-store'; import thunk from 'redux-thunk'; -import { styledShallow as shallow } from 'spec/helpers/theming'; -import { render, screen, act } from 'spec/helpers/testing-library'; -import SouthPaneContainer from 'src/SqlLab/components/SouthPane/state'; -import ResultSet from 'src/SqlLab/components/ResultSet'; +import { render, screen, waitFor } from 'spec/helpers/testing-library'; +import SouthPane from 'src/SqlLab/components/SouthPane'; import '@testing-library/jest-dom/extend-expect'; import { STATUS_OPTIONS } from 'src/SqlLab/constants'; import { initialState, table, defaultQueryEditor } from 'src/SqlLab/fixtures'; -import { UserWithPermissionsAndRoles } from 'src/types/bootstrapTypes'; - -const NOOP = () => {}; const mockedProps = { queryEditorId: defaultQueryEditor.id, latestQueryId: 'LCly_kkIN', - actions: {}, - activeSouthPaneTab: '', height: 1, displayLimit: 1, - databases: {}, defaultQueryLimit: 100, }; const mockedEmptyProps = { queryEditorId: 'random_id', latestQueryId: '', - actions: { - queryEditorSetAndSaveSql: NOOP, - cloneQueryToNewTab: NOOP, - fetchQueryResults: NOOP, - clearQueryResults: NOOP, - removeQuery: NOOP, - setActiveSouthPaneTab: NOOP, - }, - activeSouthPaneTab: '', height: 100, - databases: '', displayLimit: 100, - user: UserWithPermissionsAndRoles, defaultQueryLimit: 100, }; +const latestQueryProgressMsg = 'LATEST QUERY MESSAGE - LCly_kkIN'; + const middlewares = [thunk]; const mockStore = configureStore(middlewares); const store = mockStore({ @@ -84,6 +67,7 @@ const store = mockStore({ id: 'LCly_kkIN', startDttm: Date.now(), sqlEditorId: defaultQueryEditor.id, + extra: { progress: latestQueryProgressMsg }, }, lXJa7F9_r: { cached: false, @@ -115,48 +99,40 @@ const store = mockStore({ }, }, }); -const setup = (overrides = {}) => ( - -); - -describe('SouthPane - Enzyme', () => { - const getWrapper = () => shallow(setup()).dive(); +const setup = (props, store) => + render(, { + useRedux: true, + ...(store && { store }), + }); - let wrapper; +describe('SouthPane', () => { + const renderAndWait = (props, store) => + waitFor(async () => setup(props, store)); - it('should render offline when the state is offline', () => { - wrapper = getWrapper().dive(); - wrapper.setProps({ offline: true }); - expect(wrapper.childAt(0).text()).toBe(STATUS_OPTIONS.offline); + it('Renders an empty state for results', async () => { + await renderAndWait(mockedEmptyProps, store); + const emptyStateText = screen.getByText(/run a query to display results/i); + expect(emptyStateText).toBeVisible(); }); - it('should pass latest query down to ResultSet component', () => { - wrapper = getWrapper().dive(); - expect(wrapper.find(ResultSet)).toExist(); - // for editorQueries - expect(wrapper.find(ResultSet).first().props().query.id).toEqual( - mockedProps.latestQueryId, + it('should render offline when the state is offline', async () => { + await renderAndWait( + mockedEmptyProps, + mockStore({ + ...initialState, + sqlLab: { + ...initialState.sqlLab, + offline: true, + }, + }), ); - // for dataPreviewQueries - expect(wrapper.find(ResultSet).last().props().query.id).toEqual( - '2g2_iRFMl', - ); - }); -}); - -describe('SouthPane - RTL', () => { - const renderAndWait = overrides => { - const mounted = act(async () => { - render(setup(overrides)); - }); - return mounted; - }; - it('Renders an empty state for results', async () => { - await renderAndWait(mockedEmptyProps); + expect(screen.getByText(STATUS_OPTIONS.offline)).toBeVisible(); + }); - const emptyStateText = screen.getByText(/run a query to display results/i); + it('should pass latest query down to ResultSet component', async () => { + await renderAndWait(mockedProps, store); - expect(emptyStateText).toBeVisible(); + expect(screen.getByText(latestQueryProgressMsg)).toBeVisible(); }); }); diff --git a/superset-frontend/src/SqlLab/components/SouthPane/index.tsx b/superset-frontend/src/SqlLab/components/SouthPane/index.tsx index eb1f6c5805ab2..d8b2d5a0cfba8 100644 --- a/superset-frontend/src/SqlLab/components/SouthPane/index.tsx +++ b/superset-frontend/src/SqlLab/components/SouthPane/index.tsx @@ -17,16 +17,18 @@ * under the License. */ import React, { createRef } from 'react'; +import { useDispatch, useSelector } from 'react-redux'; import shortid from 'shortid'; import Alert from 'src/components/Alert'; import Tabs from 'src/components/Tabs'; import { EmptyStateMedium } from 'src/components/EmptyState'; import { t, styled } from '@superset-ui/core'; +import { setActiveSouthPaneTab } from 'src/SqlLab/actions/sqlLab'; import { isFeatureEnabled, FeatureFlag } from 'src/featureFlags'; import Label from 'src/components/Label'; -import { UserWithPermissionsAndRoles } from 'src/types/bootstrapTypes'; +import { SqlLabRootState } from 'src/SqlLab/types'; import QueryHistory from '../QueryHistory'; import ResultSet from '../ResultSet'; import { @@ -39,27 +41,13 @@ const TAB_HEIGHT = 140; /* editorQueries are queries executed by users passed from SqlEditor component - dataPrebiewQueries are all queries executed for preview of table data (from SqlEditorLeft) + dataPreviewQueries are all queries executed for preview of table data (from SqlEditorLeft) */ -export interface SouthPanePropTypes { +export interface SouthPaneProps { queryEditorId: string; - editorQueries: any[]; latestQueryId?: string; - dataPreviewQueries: any[]; - actions: { - queryEditorSetAndSaveSql: Function; - cloneQueryToNewTab: Function; - fetchQueryResults: Function; - clearQueryResults: Function; - removeQuery: Function; - setActiveSouthPaneTab: Function; - }; - activeSouthPaneTab?: string; height: number; - databases: Record; - offline?: boolean; displayLimit: number; - user: UserWithPermissionsAndRoles; defaultQueryLimit: number; } @@ -111,23 +99,49 @@ const StyledEmptyStateWrapper = styled.div` } `; -export default function SouthPane({ - editorQueries, +const SouthPane = ({ + queryEditorId, latestQueryId, - dataPreviewQueries, - actions, - activeSouthPaneTab = 'Results', height, - databases, - offline = false, displayLimit, - user, defaultQueryLimit, -}: SouthPanePropTypes) { +}: SouthPaneProps) => { + const dispatch = useDispatch(); + + const { editorQueries, dataPreviewQueries, databases, offline, user } = + useSelector(({ sqlLab }: SqlLabRootState) => { + const { databases, offline, user, queries, tables } = sqlLab; + const dataPreviewQueries = tables + .filter( + ({ dataPreviewQueryId, queryEditorId: qeId }) => + dataPreviewQueryId && + queryEditorId === qeId && + queries[dataPreviewQueryId], + ) + .map(({ name, dataPreviewQueryId }) => ({ + ...queries[dataPreviewQueryId], + tableName: name, + })); + const editorQueries = Object.values(queries).filter( + ({ sqlEditorId }) => sqlEditorId === queryEditorId, + ); + return { + editorQueries, + dataPreviewQueries, + databases, + offline: offline ?? false, + user, + }; + }); + + const activeSouthPaneTab = + useSelector( + state => state.sqlLab.activeSouthPaneTab as string, + ) ?? 'Results'; const innerTabContentHeight = height - TAB_HEIGHT; const southPaneRef = createRef(); const switchTab = (id: string) => { - actions.setActiveSouthPaneTab(id); + dispatch(setActiveSouthPaneTab(id)); }; const renderOfflineStatus = () => (
- xt.queryEditorId === qe.id)} queryEditor={qe} - actions={this.props.actions} defaultQueryLimit={this.props.defaultQueryLimit} maxRow={this.props.maxRow} displayLimit={this.props.displayLimit} diff --git a/superset-frontend/src/SqlLab/types.ts b/superset-frontend/src/SqlLab/types.ts index 18c6773ae6ed1..7317ef0789d5a 100644 --- a/superset-frontend/src/SqlLab/types.ts +++ b/superset-frontend/src/SqlLab/types.ts @@ -16,7 +16,7 @@ * specific language governing permissions and limitations * under the License. */ -import { JsonObject, Query, QueryResponse } from '@superset-ui/core'; +import { JsonObject, QueryResponse } from '@superset-ui/core'; import { SupersetError } from 'src/components/ErrorMessage/types'; import { UserWithPermissionsAndRoles } from 'src/types/bootstrapTypes'; import { ToastType } from 'src/components/MessageToasts/types'; @@ -69,7 +69,7 @@ export type SqlLabRootState = { databases: Record; dbConnect: boolean; offline: boolean; - queries: Record; + queries: Record; queryEditors: QueryEditor[]; tabHistory: string[]; // default is activeTab ? [activeTab.id.toString()] : [] tables: Record[];