fix(sqllab): inconsistent addNewQueryEditor behavior (#21999)

apache · Jan 30, 2023 · 19779d7 · 19779d7
1 parent f2b61fc
commit 19779d7
Show file tree

Hide file tree

Showing 5 changed files with 95 additions and 29 deletions.
diff --git a/superset-frontend/cypress-base/cypress/integration/sqllab/tabs.test.ts b/superset-frontend/cypress-base/cypress/integration/sqllab/tabs.test.ts
@@ -21,9 +21,10 @@ describe('SqlLab query tabs', () => {
     cy.visit('/superset/sqllab');
   });
 
+  const tablistSelector = '[data-test="sql-editor-tabs"] > [role="tablist"]';
+  const tabSelector = `${tablistSelector} [role="tab"]`;
+
   it('allows you to create and close a tab', () => {
-    const tablistSelector = '[data-test="sql-editor-tabs"] > [role="tablist"]';
-    const tabSelector = `${tablistSelector} [role="tab"]`;
     cy.get(tabSelector).then(tabs => {
       const initialTabCount = tabs.length;
       const initialUntitledCount = Math.max(
@@ -58,4 +59,55 @@ describe('SqlLab query tabs', () => {
       cy.get(tabSelector).should('have.length', initialTabCount);
     });
   });
+
+  it('opens a new tab by a button and a shortcut', () => {
+    const editorContent = '#ace-editor .ace_content';
+    const editorInput = '#ace-editor textarea';
+    const queryLimitSelector = '#js-sql-toolbar .limitDropdown';
+    cy.get(tabSelector).then(tabs => {
+      const initialTabCount = tabs.length;
+      const initialUntitledCount = Math.max(
+        0,
+        ...tabs
+          .map(
+            (i, tabItem) =>
+              Number(tabItem.textContent?.match(/Untitled Query (\d+)/)?.[1]) ||
+              0,
+          )
+          .toArray(),
+      );
+
+      // configure some editor settings
+      cy.get(editorInput).type('some random query string', { force: true });
+      cy.get(queryLimitSelector).parent().click({ force: true });
+      cy.get('.ant-dropdown-menu')
+        .last()
+        .find('.ant-dropdown-menu-item')
+        .first()
+        .click({ force: true });
+
+      // open a new tab by a button
+      cy.get('[data-test="add-tab-icon"]:visible:last').click({ force: true });
+      cy.contains('[role="tab"]', `Untitled Query ${initialUntitledCount + 1}`);
+      cy.get(tabSelector).should('have.length', initialTabCount + 1);
+      cy.get(editorContent).contains('SELECT ...');
+      cy.get(queryLimitSelector).contains('10');
+
+      // close the tab
+      cy.get(`${tabSelector}:last [data-test="dropdown-trigger"]`).click({
+        force: true,
+      });
+      cy.get(`${tablistSelector} [aria-label="remove"]:last`).click({
+        force: true,
+      });
+      cy.get(tabSelector).should('have.length', initialTabCount);
+
+      // open a new tab by a shortcut
+      cy.get('body').type('{ctrl}t');
+      cy.get(tabSelector).should('have.length', initialTabCount + 1);
+      cy.contains('[role="tab"]', `Untitled Query ${initialUntitledCount + 1}`);
+      cy.get(editorContent).contains('SELECT ...');
+      cy.get(queryLimitSelector).contains('10');
+    });
+  });
 });
diff --git a/superset-frontend/src/SqlLab/actions/sqlLab.js b/superset-frontend/src/SqlLab/actions/sqlLab.js
@@ -598,16 +598,44 @@ export function addQueryEditor(queryEditor) {
   };
 }
 
-export function addNewQueryEditor(queryEditor) {
+export function addNewQueryEditor() {
   return function (dispatch, getState) {
     const {
-      sqlLab: { queryEditors },
+      sqlLab: {
+        queryEditors,
+        tabHistory,
+        unsavedQueryEditor,
+        defaultDbId,
+        databases,
+      },
+      common,
     } = getState();
+    const activeQueryEditor = queryEditors.find(
+      qe => qe.id === tabHistory[tabHistory.length - 1],
+    );
+    const dbIds = Object.values(databases).map(database => database.id);
+    const firstDbId = dbIds.length > 0 ? Math.min(...dbIds) : undefined;
+    const { dbId, schema, queryLimit, autorun } = {
+      ...queryEditors[0],
+      ...activeQueryEditor,
+      ...(unsavedQueryEditor.id === activeQueryEditor?.id &&
+        unsavedQueryEditor),
+    };
+    const warning = isFeatureEnabled(FeatureFlag.SQLLAB_BACKEND_PERSISTENCE)
+      ? ''
+      : t(
+          '-- Note: Unless you save your query, these tabs will NOT persist if you clear your cookies or change browsers.\n\n',
+        );
+
     const name = newQueryTabName(queryEditors || []);
 
     return dispatch(
       addQueryEditor({
-        ...queryEditor,
+        dbId: dbId || defaultDbId || firstDbId,
+        schema: schema ?? null,
+        autorun: autorun ?? false,
+        sql: `${warning}SELECT ...`,
+        queryLimit: queryLimit || common.conf.DEFAULT_SQLLAB_LIMIT,
         name,
       }),
     );

diff --git a/superset-frontend/src/SqlLab/actions/sqlLab.test.js b/superset-frontend/src/SqlLab/actions/sqlLab.test.js
@@ -433,15 +433,21 @@ describe('async actions', () => {
           {
             type: actions.ADD_QUERY_EDITOR,
             queryEditor: {
-              ...defaultQueryEditor,
               id: 'abcd',
+              sql: expect.stringContaining('SELECT ...'),
               name: `Untitled Query ${
                 store.getState().sqlLab.queryEditors.length + 1
               }`,
+              dbId: defaultQueryEditor.dbId,
+              schema: defaultQueryEditor.schema,
+              autorun: false,
+              queryLimit:
+                defaultQueryEditor.queryLimit ||
+                initialState.common.conf.DEFAULT_SQLLAB_LIMIT,
             },
           },
         ];
-        const request = actions.addNewQueryEditor(defaultQueryEditor);
+        const request = actions.addNewQueryEditor();
         return request(store.dispatch, store.getState).then(() => {
           expect(store.getActions()).toEqual(expectedActions);
         });

diff --git a/superset-frontend/src/SqlLab/components/SqlEditor/index.jsx b/superset-frontend/src/SqlLab/components/SqlEditor/index.jsx
@@ -319,7 +319,7 @@ const SqlEditor = ({
         key: userOS === 'Windows' ? 'ctrl+q' : 'ctrl+t',
         descr: t('New tab'),
         func: () => {
-          dispatch(addNewQueryEditor(queryEditor));
+          dispatch(addNewQueryEditor());
         },
       },
       {

diff --git a/superset-frontend/src/SqlLab/components/TabbedSqlEditors/index.jsx b/superset-frontend/src/SqlLab/components/TabbedSqlEditors/index.jsx
@@ -201,27 +201,7 @@ class TabbedSqlEditors extends React.PureComponent {
   }
 
   newQueryEditor() {
-    const activeQueryEditor = this.activeQueryEditor();
-    const firstDbId = Math.min(
-      ...Object.values(this.props.databases).map(database => database.id),
-    );
-    const warning = isFeatureEnabled(FeatureFlag.SQLLAB_BACKEND_PERSISTENCE)
-      ? ''
-      : t(
-          '-- Note: Unless you save your query, these tabs will NOT persist if you clear your cookies or change browsers.\n\n',
-        );
-
-    const qe = {
-      dbId:
-        activeQueryEditor && activeQueryEditor.dbId
-          ? activeQueryEditor.dbId
-          : this.props.defaultDbId || firstDbId,
-      schema: activeQueryEditor ? activeQueryEditor.schema : null,
-      autorun: false,
-      sql: `${warning}SELECT ...`,
-      queryLimit: this.props.defaultQueryLimit,
-    };
-    this.props.actions.addNewQueryEditor(qe);
+    this.props.actions.addNewQueryEditor();
   }
 
   handleSelect(key) {