From f61f76cd2442e5e512d70e2a963ff88fc560c9fc Mon Sep 17 00:00:00 2001 From: "JUST.in DO IT" Date: Mon, 30 Jan 2023 15:54:03 -0800 Subject: [PATCH] fix(sqllab): inconsistent addNewQueryEditor behavior (#21999) --- .../cypress/integration/sqllab/tabs.test.ts | 56 ++++++++++++++++++- .../src/SqlLab/actions/sqlLab.js | 34 ++++++++++- .../src/SqlLab/actions/sqlLab.test.js | 10 +++- .../src/SqlLab/components/SqlEditor/index.jsx | 2 +- .../components/TabbedSqlEditors/index.jsx | 22 +------- 5 files changed, 95 insertions(+), 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 index 34f8844d49a0..b2c7a180ad83 100644 --- 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 index cd63a464b140..40aea6630121 100644 --- 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 index b743f11d29cf..63711550b248 100644 --- 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 index 99e7ddb8f251..d7626c8cbf84 100644 --- 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 index 43259a37eedb..1b3e3a999d77 100644 --- 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) {