From 00a53dae739a8b82b1c242c2384ceaed9b15f4d6 Mon Sep 17 00:00:00 2001 From: cccs-Dustin <96579982+cccs-Dustin@users.noreply.github.com> Date: Fri, 4 Mar 2022 11:28:30 -0500 Subject: [PATCH] fix(SQL Editor): names new query tabs correctly (#18951) * Added in code changes that now properly increment the Untitled Query SQL Lab tab names. All that is left is to add tests to make sure that the function works correctly * Updated the code so that it adds to the untitled_query_numbers variable only if the character after the string 'Untitled Query ' is a number. This prevents any issues when trying to get the maximum value in the list. * Refactored part of the mapping code, to make it shorter and easier to read/understand * Fixed issues in the code that were causing some of the CI tests to fail * Made code changes based on comments within the PR. Also added a unit test to make sure that the newQueryEditor function in the TabbedSqlEditors component works as intended * Fixed the failing cypress test in tabs.test.js (cherry picked from commit 5a5ff99c372041e14621bc20015edd1b057cb417) --- .../cypress/integration/sqllab/tabs.test.js | 4 ++-- .../src/SqlLab/actions/sqlLab.test.js | 2 +- .../TabbedSqlEditors.test.jsx | 9 ++++++++ .../components/TabbedSqlEditors/index.jsx | 21 ++++++++++++++----- superset-frontend/src/SqlLab/fixtures.ts | 2 +- 5 files changed, 29 insertions(+), 9 deletions(-) diff --git a/superset-frontend/cypress-base/cypress/integration/sqllab/tabs.test.js b/superset-frontend/cypress-base/cypress/integration/sqllab/tabs.test.js index b7544ed8bf5..24dd074992b 100644 --- a/superset-frontend/cypress-base/cypress/integration/sqllab/tabs.test.js +++ b/superset-frontend/cypress-base/cypress/integration/sqllab/tabs.test.js @@ -31,11 +31,11 @@ describe('SqlLab query tabs', () => { cy.get('[data-test="sql-editor-tabs"]') .children() .eq(0) - .contains(`Untitled Query ${initialTabCount + 1}`); + .contains(`Untitled Query ${initialTabCount}`); cy.get('[data-test="sql-editor-tabs"]') .children() .eq(0) - .contains(`Untitled Query ${initialTabCount + 2}`); + .contains(`Untitled Query ${initialTabCount + 1}`); }); }); diff --git a/superset-frontend/src/SqlLab/actions/sqlLab.test.js b/superset-frontend/src/SqlLab/actions/sqlLab.test.js index 7e1326336f3..d04d8b90ab1 100644 --- a/superset-frontend/src/SqlLab/actions/sqlLab.test.js +++ b/superset-frontend/src/SqlLab/actions/sqlLab.test.js @@ -38,7 +38,7 @@ describe('async actions', () => { latestQueryId: null, selectedText: null, sql: 'SELECT *\nFROM\nWHERE', - title: 'Untitled Query', + title: 'Untitled Query 1', schemaOptions: [{ value: 'main', label: 'main', title: 'main' }], }; diff --git a/superset-frontend/src/SqlLab/components/TabbedSqlEditors/TabbedSqlEditors.test.jsx b/superset-frontend/src/SqlLab/components/TabbedSqlEditors/TabbedSqlEditors.test.jsx index 552afe37f8f..9f8e5bcf1ae 100644 --- a/superset-frontend/src/SqlLab/components/TabbedSqlEditors/TabbedSqlEditors.test.jsx +++ b/superset-frontend/src/SqlLab/components/TabbedSqlEditors/TabbedSqlEditors.test.jsx @@ -180,6 +180,15 @@ describe('TabbedSqlEditors', () => { wrapper.instance().props.actions.addQueryEditor.getCall(0).args[0].title, ).toContain('Untitled Query'); }); + it('should properly increment query tab name', () => { + wrapper = getWrapper(); + sinon.stub(wrapper.instance().props.actions, 'addQueryEditor'); + + wrapper.instance().newQueryEditor(); + expect( + wrapper.instance().props.actions.addQueryEditor.getCall(0).args[0].title, + ).toContain('Untitled Query 2'); + }); it('should duplicate query editor', () => { wrapper = getWrapper(); sinon.stub(wrapper.instance().props.actions, 'cloneQueryToNewTab'); diff --git a/superset-frontend/src/SqlLab/components/TabbedSqlEditors/index.jsx b/superset-frontend/src/SqlLab/components/TabbedSqlEditors/index.jsx index 4e486c1c69f..11c6fa8b6c0 100644 --- a/superset-frontend/src/SqlLab/components/TabbedSqlEditors/index.jsx +++ b/superset-frontend/src/SqlLab/components/TabbedSqlEditors/index.jsx @@ -58,8 +58,6 @@ const defaultProps = { scheduleQueryWarning: null, }; -let queryCount = 1; - const TabTitleWrapper = styled.div` display: flex; align-items: center; @@ -233,7 +231,6 @@ class TabbedSqlEditors extends React.PureComponent { } popNewTab() { - queryCount += 1; // Clean the url in browser history window.history.replaceState({}, document.title, this.state.sqlLabUrl); } @@ -255,7 +252,6 @@ class TabbedSqlEditors extends React.PureComponent { } newQueryEditor() { - queryCount += 1; const activeQueryEditor = this.activeQueryEditor(); const firstDbId = Math.min( ...Object.values(this.props.databases).map(database => database.id), @@ -265,8 +261,23 @@ class TabbedSqlEditors extends React.PureComponent { : t( '-- Note: Unless you save your query, these tabs will NOT persist if you clear your cookies or change browsers.\n\n', ); + + let newTitle = 'Untitled Query 1'; + + if (this.props.queryEditors.length > 0) { + const untitledQueryNumbers = this.props.queryEditors + .filter(x => x.title.match(/^Untitled Query (\d+)$/)) + .map(x => x.title.replace('Untitled Query ', '')); + if (untitledQueryNumbers.length > 0) { + // When there are query tabs open, and at least one is called "Untitled Query #" + // Where # is a valid number + const largestNumber = Math.max.apply(null, untitledQueryNumbers); + newTitle = t('Untitled Query %s', largestNumber + 1); + } + } + const qe = { - title: t('Untitled Query %s', queryCount), + title: newTitle, dbId: activeQueryEditor && activeQueryEditor.dbId ? activeQueryEditor.dbId diff --git a/superset-frontend/src/SqlLab/fixtures.ts b/superset-frontend/src/SqlLab/fixtures.ts index b4b68454fa3..5725bcf75e1 100644 --- a/superset-frontend/src/SqlLab/fixtures.ts +++ b/superset-frontend/src/SqlLab/fixtures.ts @@ -180,7 +180,7 @@ export const defaultQueryEditor = { latestQueryId: null, selectedText: null, sql: 'SELECT *\nFROM\nWHERE', - title: 'Untitled Query', + title: 'Untitled Query 1', schemaOptions: [ { value: 'main',