- migrate RunQuery button using useQueryEditor

- set disable for comments only
apache · Sep 30, 2022 · 9ef8499 · 9ef8499
1 parent e694b97
commit 9ef8499
Show file tree

Hide file tree

Showing 3 changed files with 109 additions and 110 deletions.
diff --git 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<Props>, 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
@@ -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<Props> = ({
   allowAsync = false,
-  queryEditor,
+  queryEditorId,
   queryState,
   overlayCreateAsMenu,
   runQuery,
   stopQuery,
-}: Props) => {
+}) => {
   const theme = useTheme();
   const userOS = detectOS();
-  const { selectedText, sql } = useSelector<
-    SqlLabRootState,
-    Pick<QueryEditor, 'selectedText' | 'sql'>
-  >(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
@@ -535,7 +535,7 @@ const SqlEditor = ({
           <span>
             <RunQueryActionButton
               allowAsync={database ? database.allow_run_async : false}
-              queryEditor={queryEditor}
+              queryEditorId={queryEditor.id}
               queryState={latestQuery?.state}
               runQuery={startQuery}
               stopQuery={stopQuery}