From 9ef84990782e986bd5a96d3261bd921e4eeac4ca Mon Sep 17 00:00:00 2001 From: justin-park Date: Fri, 30 Sep 2022 10:03:55 -0700 Subject: [PATCH] - migrate RunQuery button using useQueryEditor - set disable for comments only --- .../RunQueryActionButton.test.tsx | 177 +++++++++--------- .../components/RunQueryActionButton/index.tsx | 40 ++-- .../src/SqlLab/components/SqlEditor/index.jsx | 2 +- 3 files changed, 109 insertions(+), 110 deletions(-) diff --git a/superset-frontend/src/SqlLab/components/RunQueryActionButton/RunQueryActionButton.test.tsx b/superset-frontend/src/SqlLab/components/RunQueryActionButton/RunQueryActionButton.test.tsx index 189a5fd2f7ab9..7062ad694a2e2 100644 --- a/superset-frontend/src/SqlLab/components/RunQueryActionButton/RunQueryActionButton.test.tsx +++ b/superset-frontend/src/SqlLab/components/RunQueryActionButton/RunQueryActionButton.test.tsx @@ -41,13 +41,13 @@ jest.mock('src/components/Select/AsyncSelect', () => () => ( )); const defaultProps = { - queryEditor: defaultQueryEditor, + queryEditorId: defaultQueryEditor.id, allowAsync: false, dbId: 1, queryState: 'ready', - runQuery: jest.fn(), + runQuery: () => {}, selectedText: null, - stopQuery: jest.fn(), + stopQuery: () => {}, overlayCreateAsMenu: null, }; @@ -57,95 +57,104 @@ const setup = (props?: Partial, store?: Store) => ...(store && { store }), }); -describe('RunQueryActionButton', () => { - beforeEach(() => { - defaultProps.runQuery.mockReset(); - defaultProps.stopQuery.mockReset(); - }); +it('renders a single Button', () => { + const { getByRole } = setup({}, mockStore(initialState)); + expect(getByRole('button')).toBeInTheDocument(); +}); - it('renders a single Button', () => { - const { getByRole } = setup({}, mockStore(initialState)); - expect(getByRole('button')).toBeInTheDocument(); - }); +it('renders a label for Run Query', () => { + const { getByText } = setup({}, mockStore(initialState)); + expect(getByText('Run')).toBeInTheDocument(); +}); - it('renders a label for Run Query', () => { - const { getByText } = setup({}, mockStore(initialState)); - expect(getByText('Run')).toBeInTheDocument(); - }); +it('renders a label for Selected Query', () => { + const { getByText } = setup( + {}, + mockStore({ + ...initialState, + sqlLab: { + ...initialState.sqlLab, + unsavedQueryEditor: { + id: defaultQueryEditor.id, + selectedText: 'select * from\n-- this is comment\nwhere', + }, + }, + }), + ); + expect(getByText('Run selection')).toBeInTheDocument(); +}); - it('renders a label for Selected Query', () => { - const { getByText } = setup( - {}, - mockStore({ - ...initialState, - sqlLab: { - ...initialState.sqlLab, - unsavedQueryEditor: { - id: defaultQueryEditor.id, - selectedText: 'FROM', - }, +it('disable button when sql from unsaved changes is empty', () => { + const { getByRole } = setup( + {}, + mockStore({ + ...initialState, + sqlLab: { + ...initialState.sqlLab, + unsavedQueryEditor: { + id: defaultQueryEditor.id, + sql: '', }, - }), - ); - expect(getByText('Run selection')).toBeInTheDocument(); - }); + }, + }), + ); + const button = getByRole('button'); + expect(button).toBeDisabled(); +}); - it('disable button when sql from unsaved changes is empty', () => { - const { getByRole } = setup( - {}, - mockStore({ - ...initialState, - sqlLab: { - ...initialState.sqlLab, - unsavedQueryEditor: { - id: defaultQueryEditor.id, - sql: '', - }, +it('disable button when selectedText only contains blank contents', () => { + const { getByRole } = setup( + {}, + mockStore({ + ...initialState, + sqlLab: { + ...initialState.sqlLab, + unsavedQueryEditor: { + id: defaultQueryEditor.id, + selectedText: '-- this is comment\n\n \t', }, - }), - ); - const button = getByRole('button'); - expect(button).toBeDisabled(); - }); + }, + }), + ); + const button = getByRole('button'); + expect(button).toBeDisabled(); +}); - it('enable default button for unrelated unsaved changes', () => { - const { getByRole } = setup( - {}, - mockStore({ - ...initialState, - sqlLab: { - ...initialState.sqlLab, - unsavedQueryEditor: { - id: `${defaultQueryEditor.id}-other`, - sql: '', - }, +it('enable default button for unrelated unsaved changes', () => { + const { getByRole } = setup( + {}, + mockStore({ + ...initialState, + sqlLab: { + ...initialState.sqlLab, + unsavedQueryEditor: { + id: `${defaultQueryEditor.id}-other`, + sql: '', }, - }), - ); - const button = getByRole('button'); - expect(button).toBeEnabled(); - }); + }, + }), + ); + const button = getByRole('button'); + expect(button).toBeEnabled(); +}); - it('dispatch runQuery on click', async () => { - const { getByRole } = setup({}, mockStore(initialState)); - const button = getByRole('button'); - expect(defaultProps.runQuery).toHaveBeenCalledTimes(0); - fireEvent.click(button); - await waitFor(() => expect(defaultProps.runQuery).toHaveBeenCalledTimes(1)); - }); +it('dispatch runQuery on click', async () => { + const runQuery = jest.fn(); + const { getByRole } = setup({ runQuery }, mockStore(initialState)); + const button = getByRole('button'); + expect(runQuery).toHaveBeenCalledTimes(0); + fireEvent.click(button); + await waitFor(() => expect(runQuery).toHaveBeenCalledTimes(1)); +}); - describe('on running state', () => { - it('dispatch stopQuery on click', async () => { - const { getByRole } = setup( - { queryState: 'running' }, - mockStore(initialState), - ); - const button = getByRole('button'); - expect(defaultProps.stopQuery).toHaveBeenCalledTimes(0); - fireEvent.click(button); - await waitFor(() => - expect(defaultProps.stopQuery).toHaveBeenCalledTimes(1), - ); - }); - }); +it('dispatch stopQuery on click while running state', async () => { + const stopQuery = jest.fn(); + const { getByRole } = setup( + { queryState: 'running', stopQuery }, + mockStore(initialState), + ); + const button = getByRole('button'); + expect(stopQuery).toHaveBeenCalledTimes(0); + fireEvent.click(button); + await waitFor(() => expect(stopQuery).toHaveBeenCalledTimes(1)); }); diff --git a/superset-frontend/src/SqlLab/components/RunQueryActionButton/index.tsx b/superset-frontend/src/SqlLab/components/RunQueryActionButton/index.tsx index 5cc453f5ee93e..a85b4b6b7bb88 100644 --- a/superset-frontend/src/SqlLab/components/RunQueryActionButton/index.tsx +++ b/superset-frontend/src/SqlLab/components/RunQueryActionButton/index.tsx @@ -24,16 +24,11 @@ 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'; +import { QueryButtonProps } from 'src/SqlLab/types'; +import useQueryEditor from 'src/SqlLab/hooks/useQueryEditor'; export interface Props { - queryEditor: QueryEditor; + queryEditorId: string; allowAsync: boolean; queryState?: string; runQuery: (c?: boolean) => void; @@ -86,29 +81,21 @@ const StyledButton = styled.span` } `; -const RunQueryActionButton = ({ +const RunQueryActionButton: React.FC = ({ allowAsync = false, - queryEditor, + queryEditorId, queryState, overlayCreateAsMenu, runQuery, stopQuery, -}: 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 { selectedText, sql } = useQueryEditor(queryEditorId, [ + 'selectedText', + 'sql', + ]); const shouldShowStopBtn = !!queryState && ['running', 'pending'].indexOf(queryState) > -1; @@ -117,7 +104,10 @@ const RunQueryActionButton = ({ ? (DropdownButton as React.FC) : Button; - const isDisabled = !sql || !sql.trim(); + const sqlContent = selectedText || sql || ''; + const isDisabled = + !sqlContent || + !sqlContent.replace(/(\/\*[^*]*\*\/)|(\/\/[^*]*)|(--[^.].*)/gm, '').trim(); const stopButtonTooltipText = useMemo( () => diff --git a/superset-frontend/src/SqlLab/components/SqlEditor/index.jsx b/superset-frontend/src/SqlLab/components/SqlEditor/index.jsx index 71aa0f4e70419..bc4cf013d5cfb 100644 --- a/superset-frontend/src/SqlLab/components/SqlEditor/index.jsx +++ b/superset-frontend/src/SqlLab/components/SqlEditor/index.jsx @@ -535,7 +535,7 @@ const SqlEditor = ({