chore(sqllab): refactor addQueryEditor for new tab (#21711)

apache · Oct 7, 2022 · 0c46149 · 0c46149
1 parent a5ff094
commit 0c46149
Show file tree

Hide file tree

Showing 5 changed files with 73 additions and 32 deletions.
diff --git a/superset-frontend/src/SqlLab/actions/sqlLab.js b/superset-frontend/src/SqlLab/actions/sqlLab.js
@@ -32,6 +32,7 @@ import {
 } from 'src/components/MessageToasts/actions';
 import { getClientErrorObject } from 'src/utils/getClientErrorObject';
 import COMMON_ERR_MESSAGES from 'src/utils/errorMessages';
+import { newQueryTabName } from '../utils/newQueryTabName';
 
 export const RESET_STATE = 'RESET_STATE';
 export const ADD_QUERY_EDITOR = 'ADD_QUERY_EDITOR';
@@ -590,6 +591,22 @@ export function addQueryEditor(queryEditor) {
   };
 }
 
+export function addNewQueryEditor(queryEditor) {
+  return function (dispatch, getState) {
+    const {
+      sqlLab: { queryEditors },
+    } = getState();
+    const name = newQueryTabName(queryEditors || []);
+
+    return dispatch(
+      addQueryEditor({
+        ...queryEditor,
+        name,
+      }),
+    );
+  };
+}
+
 export function cloneQueryToNewTab(query, autorun) {
   return function (dispatch, getState) {
     const state = getState();

diff --git a/superset-frontend/src/SqlLab/actions/sqlLab.test.js b/superset-frontend/src/SqlLab/actions/sqlLab.test.js
@@ -418,6 +418,28 @@ describe('async actions', () => {
         expect(store.getActions()).toEqual(expectedActions);
       });
     });
+
+    describe('addNewQueryEditor', () => {
+      it('creates new query editor with new tab name', () => {
+        const store = mockStore(initialState);
+        const expectedActions = [
+          {
+            type: actions.ADD_QUERY_EDITOR,
+            queryEditor: {
+              ...defaultQueryEditor,
+              id: 'abcd',
+              name: `Untitled Query ${
+                store.getState().sqlLab.queryEditors.length + 1
+              }`,
+            },
+          },
+        ];
+        const request = actions.addNewQueryEditor(defaultQueryEditor);
+        return request(store.dispatch, store.getState).then(() => {
+          expect(store.getActions()).toEqual(expectedActions);
+        });
+      });
+    });
   });
 
   describe('backend sync', () => {

diff --git a/superset-frontend/src/SqlLab/components/SqlEditor/index.jsx b/superset-frontend/src/SqlLab/components/SqlEditor/index.jsx
@@ -37,7 +37,7 @@ import { Menu } from 'src/components/Menu';
 import Icons from 'src/components/Icons';
 import { detectOS } from 'src/utils/common';
 import {
-  addQueryEditor,
+  addNewQueryEditor,
   CtasEnum,
   estimateQueryCost,
   persistEditorHeight,
@@ -84,7 +84,6 @@ import ShareSqlLabQuery from '../ShareSqlLabQuery';
 import SqlEditorLeftBar from '../SqlEditorLeftBar';
 import AceEditorWrapper from '../AceEditorWrapper';
 import RunQueryActionButton from '../RunQueryActionButton';
-import { newQueryTabName } from '../../utils/newQueryTabName';
 import QueryLimitSelect from '../QueryLimitSelect';
 
 const appContainer = document.getElementById('app');
@@ -179,8 +178,6 @@ const SqlEditor = ({
     },
   );
 
-  const queryEditors = useSelector(({ sqlLab }) => sqlLab.queryEditors);
-
   const [height, setHeight] = useState(0);
   const [autorun, setAutorun] = useState(queryEditor.autorun);
   const [ctas, setCtas] = useState('');
@@ -274,13 +271,7 @@ const SqlEditor = ({
         key: userOS === 'Windows' ? 'ctrl+q' : 'ctrl+t',
         descr: t('New tab'),
         func: () => {
-          const name = newQueryTabName(queryEditors || []);
-          dispatch(
-            addQueryEditor({
-              ...queryEditor,
-              name,
-            }),
-          );
+          dispatch(addNewQueryEditor(queryEditor));
         },
       },
       {

diff --git a/superset-frontend/src/SqlLab/components/TabbedSqlEditors/TabbedSqlEditors.test.jsx b/superset-frontend/src/SqlLab/components/TabbedSqlEditors/TabbedSqlEditors.test.jsx
@@ -22,6 +22,7 @@ import thunk from 'redux-thunk';
 import URI from 'urijs';
 import { Provider } from 'react-redux';
 import { shallow, mount } from 'enzyme';
+import { fireEvent, render, waitFor } from 'spec/helpers/testing-library';
 import sinon from 'sinon';
 import { act } from 'react-dom/test-utils';
 import fetchMock from 'fetch-mock';
@@ -101,6 +102,11 @@ describe('TabbedSqlEditors', () => {
         },
       );
     });
+  const setup = (props = {}, overridesStore) =>
+    render(<TabbedSqlEditors {...props} />, {
+      useRedux: true,
+      store: overridesStore || store,
+    });
 
   let wrapper;
   it('is valid', () => {
@@ -163,23 +169,32 @@ describe('TabbedSqlEditors', () => {
       wrapper.instance().props.actions.removeQueryEditor.getCall(0).args[0],
     ).toBe(queryEditors[0]);
   });
-  it('should add new query editor', () => {
-    wrapper = getWrapper();
-    sinon.stub(wrapper.instance().props.actions, 'addQueryEditor');
-
-    wrapper.instance().newQueryEditor();
-    expect(
-      wrapper.instance().props.actions.addQueryEditor.getCall(0).args[0].name,
-    ).toContain('Untitled Query');
+  it('should add new query editor', async () => {
+    const { getAllByLabelText } = setup(mockedProps);
+    fireEvent.click(getAllByLabelText('Add tab')[0]);
+    const actions = store.getActions();
+    await waitFor(() =>
+      expect(actions).toContainEqual({
+        type: 'ADD_QUERY_EDITOR',
+        queryEditor: expect.objectContaining({
+          name: expect.stringMatching(/Untitled Query (\d+)+/),
+        }),
+      }),
+    );
   });
-  it('should properly increment query tab name', () => {
-    wrapper = getWrapper();
-    sinon.stub(wrapper.instance().props.actions, 'addQueryEditor');
-    const newTitle = newQueryTabName(wrapper.instance().props.queryEditors);
-    wrapper.instance().newQueryEditor();
-    expect(
-      wrapper.instance().props.actions.addQueryEditor.getCall(0).args[0].name,
-    ).toContain(newTitle);
+  it('should properly increment query tab name', async () => {
+    const { getAllByLabelText } = setup(mockedProps);
+    const newTitle = newQueryTabName(store.getState().sqlLab.queryEditors);
+    fireEvent.click(getAllByLabelText('Add tab')[0]);
+    const actions = store.getActions();
+    await waitFor(() =>
+      expect(actions).toContainEqual({
+        type: 'ADD_QUERY_EDITOR',
+        queryEditor: expect.objectContaining({
+          name: newTitle,
+        }),
+      }),
+    );
   });
   it('should duplicate query editor', () => {
     wrapper = getWrapper();

diff --git a/superset-frontend/src/SqlLab/components/TabbedSqlEditors/index.jsx b/superset-frontend/src/SqlLab/components/TabbedSqlEditors/index.jsx
@@ -29,7 +29,6 @@ import { Tooltip } from 'src/components/Tooltip';
 import { detectOS } from 'src/utils/common';
 import * as Actions from 'src/SqlLab/actions/sqlLab';
 import { EmptyStateBig } from 'src/components/EmptyState';
-import { newQueryTabName } from '../../utils/newQueryTabName';
 import SqlEditor from '../SqlEditor';
 import SqlEditorTabHeader from '../SqlEditorTabHeader';
 
@@ -242,10 +241,7 @@ class TabbedSqlEditors extends React.PureComponent {
           '-- Note: Unless you save your query, these tabs will NOT persist if you clear your cookies or change browsers.\n\n',
         );
 
-    const newTitle = newQueryTabName(this.props.queryEditors || []);
-
     const qe = {
-      name: newTitle,
       dbId:
         activeQueryEditor && activeQueryEditor.dbId
           ? activeQueryEditor.dbId
@@ -255,7 +251,7 @@ class TabbedSqlEditors extends React.PureComponent {
       sql: `${warning}SELECT ...`,
       queryLimit: this.props.defaultQueryLimit,
     };
-    this.props.actions.addQueryEditor(qe);
+    this.props.actions.addNewQueryEditor(qe);
   }
 
   handleSelect(key) {