From 75f6ad7461bf7922226a5b6eb26c6a92d72e2e40 Mon Sep 17 00:00:00 2001 From: cccs-Dustin <96579982+cccs-Dustin@users.noreply.github.com> Date: Thu, 24 Feb 2022 13:39:21 -0500 Subject: [PATCH 1/6] 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 --- .../components/TabbedSqlEditors/index.jsx | 31 ++++++++++++++++--- 1 file changed, 26 insertions(+), 5 deletions(-) diff --git a/superset-frontend/src/SqlLab/components/TabbedSqlEditors/index.jsx b/superset-frontend/src/SqlLab/components/TabbedSqlEditors/index.jsx index ea400b8b91c73..c0191ea91c7f2 100644 --- a/superset-frontend/src/SqlLab/components/TabbedSqlEditors/index.jsx +++ b/superset-frontend/src/SqlLab/components/TabbedSqlEditors/index.jsx @@ -57,8 +57,6 @@ const defaultProps = { scheduleQueryWarning: null, }; -let queryCount = 1; - const TabTitleWrapper = styled.div` display: flex; align-items: center; @@ -228,7 +226,6 @@ class TabbedSqlEditors extends React.PureComponent { } popNewTab() { - queryCount += 1; // Clean the url in browser history window.history.replaceState({}, document.title, this.state.sqlLabUrl); } @@ -250,7 +247,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), @@ -260,8 +256,33 @@ 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; + + if (this.props.queryEditors.length > 0) { + const untitled_query_names = this.props.queryEditors + .filter(x => x.title.includes('Untitled Query')) + .map(x => x.title); + if (untitled_query_names.length > 0) { + //When there are editors open, and at least one is called "Untitled Query #" + const largest_Number = Math.max.apply( + null, + untitled_query_names.map(x => + x.replace('Untitled Query ', ''), + ), + ); + newTitle = t('Untitled Query %s', largest_Number + 1); + } else { + //When there are editors open but none of them are called "Untitled Query #" + newTitle = 'Untitled Query 1'; + } + } else { + //When there are no editors currently open + newTitle = 'Untitled Query 1'; + } + const qe = { - title: t('Untitled Query %s', queryCount), + title: newTitle, dbId: activeQueryEditor && activeQueryEditor.dbId ? activeQueryEditor.dbId From 8073173d62f5003b135590a05a736896a9e33867 Mon Sep 17 00:00:00 2001 From: cccs-Dustin <96579982+cccs-Dustin@users.noreply.github.com> Date: Fri, 25 Feb 2022 12:08:15 -0500 Subject: [PATCH 2/6] 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. --- .../components/TabbedSqlEditors/index.jsx | 25 +++++++++---------- 1 file changed, 12 insertions(+), 13 deletions(-) diff --git a/superset-frontend/src/SqlLab/components/TabbedSqlEditors/index.jsx b/superset-frontend/src/SqlLab/components/TabbedSqlEditors/index.jsx index c0191ea91c7f2..f223157cc6a33 100644 --- a/superset-frontend/src/SqlLab/components/TabbedSqlEditors/index.jsx +++ b/superset-frontend/src/SqlLab/components/TabbedSqlEditors/index.jsx @@ -260,24 +260,23 @@ class TabbedSqlEditors extends React.PureComponent { let newTitle; if (this.props.queryEditors.length > 0) { - const untitled_query_names = this.props.queryEditors - .filter(x => x.title.includes('Untitled Query')) - .map(x => x.title); - if (untitled_query_names.length > 0) { - //When there are editors open, and at least one is called "Untitled Query #" - const largest_Number = Math.max.apply( - null, - untitled_query_names.map(x => - x.replace('Untitled Query ', ''), - ), - ); + const untitled_query_numbers = this.props.queryEditors + .filter(x => x.title.includes('Untitled Query ')) + .map(x => x.title).map(x => + x.replace('Untitled Query ', ''), + ).filter(x => !isNaN(x)); + if (untitled_query_numbers.length > 0) { + //When there are query tabs open, and at least one is called "Untitled Query #" + //Where # is a valid number + const largest_Number = Math.max.apply(null, untitled_query_numbers); newTitle = t('Untitled Query %s', largest_Number + 1); } else { - //When there are editors open but none of them are called "Untitled Query #" + //When there are query tabs open but none of them are called "Untitled Query #" + //Where # is a valid number newTitle = 'Untitled Query 1'; } } else { - //When there are no editors currently open + //When there are no query tabs currently open newTitle = 'Untitled Query 1'; } From b8f0aea8125bfcfc84165b7a9f38507600de976e Mon Sep 17 00:00:00 2001 From: cccs-Dustin <96579982+cccs-Dustin@users.noreply.github.com> Date: Fri, 25 Feb 2022 12:14:13 -0500 Subject: [PATCH 3/6] Refactored part of the mapping code, to make it shorter and easier to read/understand --- .../src/SqlLab/components/TabbedSqlEditors/index.jsx | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/superset-frontend/src/SqlLab/components/TabbedSqlEditors/index.jsx b/superset-frontend/src/SqlLab/components/TabbedSqlEditors/index.jsx index f223157cc6a33..53b672e827a4e 100644 --- a/superset-frontend/src/SqlLab/components/TabbedSqlEditors/index.jsx +++ b/superset-frontend/src/SqlLab/components/TabbedSqlEditors/index.jsx @@ -262,9 +262,8 @@ class TabbedSqlEditors extends React.PureComponent { if (this.props.queryEditors.length > 0) { const untitled_query_numbers = this.props.queryEditors .filter(x => x.title.includes('Untitled Query ')) - .map(x => x.title).map(x => - x.replace('Untitled Query ', ''), - ).filter(x => !isNaN(x)); + .map(x => x.title.replace('Untitled Query ', '')) + .filter(x => !isNaN(x)); if (untitled_query_numbers.length > 0) { //When there are query tabs open, and at least one is called "Untitled Query #" //Where # is a valid number From 3fcd0529f458fbf8dc6139fd36fe923dff9a2edd Mon Sep 17 00:00:00 2001 From: cccs-Dustin <96579982+cccs-Dustin@users.noreply.github.com> Date: Fri, 25 Feb 2022 13:54:19 -0500 Subject: [PATCH 4/6] Fixed issues in the code that were causing some of the CI tests to fail --- .../components/TabbedSqlEditors/index.jsx | 20 +++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/superset-frontend/src/SqlLab/components/TabbedSqlEditors/index.jsx b/superset-frontend/src/SqlLab/components/TabbedSqlEditors/index.jsx index 102a5e562b35e..12a044f328ae3 100644 --- a/superset-frontend/src/SqlLab/components/TabbedSqlEditors/index.jsx +++ b/superset-frontend/src/SqlLab/components/TabbedSqlEditors/index.jsx @@ -260,22 +260,22 @@ class TabbedSqlEditors extends React.PureComponent { let newTitle; if (this.props.queryEditors.length > 0) { - const untitled_query_numbers = this.props.queryEditors + const untitledQueryNumbers = this.props.queryEditors .filter(x => x.title.includes('Untitled Query ')) .map(x => x.title.replace('Untitled Query ', '')) - .filter(x => !isNaN(x)); - if (untitled_query_numbers.length > 0) { - //When there are query tabs open, and at least one is called "Untitled Query #" - //Where # is a valid number - const largest_Number = Math.max.apply(null, untitled_query_numbers); - newTitle = t('Untitled Query %s', largest_Number + 1); + .filter(x => !Number.isNaN(Number(x))); + 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); } else { - //When there are query tabs open but none of them are called "Untitled Query #" - //Where # is a valid number + // When there are query tabs open but none of them are called "Untitled Query #" + // Where # is a valid number newTitle = 'Untitled Query 1'; } } else { - //When there are no query tabs currently open + // When there are no query tabs currently open newTitle = 'Untitled Query 1'; } From c49fb7e79f259e017f4aa81a8dd739cf8aa18ede Mon Sep 17 00:00:00 2001 From: cccs-Dustin <96579982+cccs-Dustin@users.noreply.github.com> Date: Wed, 2 Mar 2022 09:09:51 -0500 Subject: [PATCH 5/6] 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 --- .../src/SqlLab/actions/sqlLab.test.js | 2 +- .../TabbedSqlEditors/TabbedSqlEditors.test.jsx | 9 +++++++++ .../SqlLab/components/TabbedSqlEditors/index.jsx | 14 +++----------- superset-frontend/src/SqlLab/fixtures.ts | 2 +- 4 files changed, 14 insertions(+), 13 deletions(-) diff --git a/superset-frontend/src/SqlLab/actions/sqlLab.test.js b/superset-frontend/src/SqlLab/actions/sqlLab.test.js index 7e1326336f351..d04d8b90ab1a8 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 7acef14cf06c7..3d1877dee1ff1 100644 --- a/superset-frontend/src/SqlLab/components/TabbedSqlEditors/TabbedSqlEditors.test.jsx +++ b/superset-frontend/src/SqlLab/components/TabbedSqlEditors/TabbedSqlEditors.test.jsx @@ -179,6 +179,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 12a044f328ae3..4327ad48df148 100644 --- a/superset-frontend/src/SqlLab/components/TabbedSqlEditors/index.jsx +++ b/superset-frontend/src/SqlLab/components/TabbedSqlEditors/index.jsx @@ -257,26 +257,18 @@ 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', ); - let newTitle; + let newTitle = 'Untitled Query 1'; if (this.props.queryEditors.length > 0) { const untitledQueryNumbers = this.props.queryEditors - .filter(x => x.title.includes('Untitled Query ')) - .map(x => x.title.replace('Untitled Query ', '')) - .filter(x => !Number.isNaN(Number(x))); + .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); - } else { - // When there are query tabs open but none of them are called "Untitled Query #" - // Where # is a valid number - newTitle = 'Untitled Query 1'; } - } else { - // When there are no query tabs currently open - newTitle = 'Untitled Query 1'; } const qe = { diff --git a/superset-frontend/src/SqlLab/fixtures.ts b/superset-frontend/src/SqlLab/fixtures.ts index b4b68454fa309..5725bcf75e1f1 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', From 7c6b70fea269fe137690368f37fb8b7f89df2dfb Mon Sep 17 00:00:00 2001 From: cccs-Dustin <96579982+cccs-Dustin@users.noreply.github.com> Date: Fri, 4 Mar 2022 10:44:37 -0500 Subject: [PATCH 6/6] Fixed the failing cypress test in tabs.test.js --- .../cypress-base/cypress/integration/sqllab/tabs.test.js | 4 ++-- 1 file changed, 2 insertions(+), 2 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 b7544ed8bf52f..24dd074992b02 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}`); }); });