Skip to content

Commit

Permalink
fix(SQL Editor): names new query tabs correctly (#18951)
Browse files Browse the repository at this point in the history
* 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 5a5ff99)
  • Loading branch information
cccs-Dustin authored and villebro committed Apr 3, 2022
1 parent ea534e2 commit 00a53da
Show file tree
Hide file tree
Showing 5 changed files with 29 additions and 9 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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}`);
});
});

Expand Down
2 changes: 1 addition & 1 deletion superset-frontend/src/SqlLab/actions/sqlLab.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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' }],
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand Down
21 changes: 16 additions & 5 deletions superset-frontend/src/SqlLab/components/TabbedSqlEditors/index.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -58,8 +58,6 @@ const defaultProps = {
scheduleQueryWarning: null,
};

let queryCount = 1;

const TabTitleWrapper = styled.div`
display: flex;
align-items: center;
Expand Down Expand Up @@ -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);
}
Expand All @@ -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),
Expand All @@ -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
Expand Down
2 changes: 1 addition & 1 deletion superset-frontend/src/SqlLab/fixtures.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down

0 comments on commit 00a53da

Please sign in to comment.