From c7ee0080e7a163b2818d4c3438fdb579252dd606 Mon Sep 17 00:00:00 2001 From: lyndsiWilliams Date: Mon, 25 Jul 2022 12:21:40 -0500 Subject: [PATCH 01/12] Moving entire split save btn PR --- .../src/SqlLab/actions/sqlLab.js | 28 +- .../src/SqlLab/actions/sqlLab.test.js | 16 +- .../src/SqlLab/components/ResultSet/index.tsx | 43 +- .../components/RunQueryActionButton/index.tsx | 12 +- .../SaveDatasetActionButton.test.tsx | 62 ++ .../SaveDatasetActionButton/index.tsx | 82 ++ .../SaveDatasetModal.test.tsx | 856 +++++++++++++++++- .../components/SaveDatasetModal/index.tsx | 15 +- .../components/SaveQuery/SaveQuery.test.jsx | 193 ++-- .../src/SqlLab/components/SaveQuery/index.tsx | 72 +- .../ShareSqlLabQuery.test.jsx | 2 +- .../components/ShareSqlLabQuery/index.tsx | 4 +- .../src/SqlLab/components/SqlEditor/index.jsx | 11 +- .../TabbedSqlEditors.test.jsx | 6 +- .../components/TabbedSqlEditors/index.jsx | 6 +- superset-frontend/src/SqlLab/fixtures.ts | 4 +- .../src/SqlLab/reducers/getInitialState.js | 6 +- .../src/SqlLab/reducers/sqlLab.js | 6 +- .../src/SqlLab/reducers/sqlLab.test.js | 4 +- superset-frontend/src/SqlLab/types.ts | 6 +- .../src/SqlLab/utils/newQueryTabName.test.ts | 4 +- .../src/SqlLab/utils/newQueryTabName.ts | 4 +- .../src/components/Chart/Chart.jsx | 62 +- .../components/Chart/ChartErrorMessage.tsx | 1 - .../src/components/DropdownButton/index.tsx | 3 +- 25 files changed, 1297 insertions(+), 211 deletions(-) create mode 100644 superset-frontend/src/SqlLab/components/SaveDatasetActionButton/SaveDatasetActionButton.test.tsx create mode 100644 superset-frontend/src/SqlLab/components/SaveDatasetActionButton/index.tsx diff --git a/superset-frontend/src/SqlLab/actions/sqlLab.js b/superset-frontend/src/SqlLab/actions/sqlLab.js index 6a43821b5cbcc..1435e0d796e21 100644 --- a/superset-frontend/src/SqlLab/actions/sqlLab.js +++ b/superset-frontend/src/SqlLab/actions/sqlLab.js @@ -112,7 +112,7 @@ const queryClientMapping = { id: 'remoteId', db_id: 'dbId', client_id: 'id', - label: 'title', + label: 'name', }; const queryServerMapping = invert(queryClientMapping); @@ -541,7 +541,7 @@ export function cloneQueryToNewTab(query, autorun) { qe => qe.id === tabHistory[tabHistory.length - 1], ); const queryEditor = { - title: t('Copy of %s', sourceQueryEditor.title), + name: t('Copy of %s', sourceQueryEditor.name), dbId: query.dbId ? query.dbId : null, schema: query.schema ? query.schema : null, autorun, @@ -629,7 +629,7 @@ export function switchQueryEditor(queryEditor, displayLimit) { const loadedQueryEditor = { id: json.id.toString(), loaded: true, - title: json.label, + name: json.label, sql: json.sql, selectedText: null, latestQueryId: json.latest_query?.id, @@ -834,24 +834,22 @@ export function queryEditorSetAutorun(queryEditor, autorun) { }; } -export function queryEditorSetTitle(queryEditor, title) { +export function queryEditorSetTitle(queryEditor, name) { return function (dispatch) { const sync = isFeatureEnabled(FeatureFlag.SQLLAB_BACKEND_PERSISTENCE) ? SupersetClient.put({ endpoint: encodeURI(`/tabstateview/${queryEditor.id}`), - postPayload: { label: title }, + postPayload: { label: name }, }) : Promise.resolve(); return sync - .then(() => - dispatch({ type: QUERY_EDITOR_SET_TITLE, queryEditor, title }), - ) + .then(() => dispatch({ type: QUERY_EDITOR_SET_TITLE, queryEditor, name })) .catch(() => dispatch( addDangerToast( t( - 'An error occurred while setting the tab title. Please contact your administrator.', + 'An error occurred while setting the tab name. Please contact your administrator.', ), ), ), @@ -873,7 +871,7 @@ export function saveQuery(query) { query, result: savedQuery, }); - dispatch(queryEditorSetTitle(query, query.title)); + dispatch(queryEditorSetTitle(query, query.name)); return savedQuery; }) .catch(() => @@ -908,7 +906,7 @@ export function updateSavedQuery(query) { }) .then(() => { dispatch(addSuccessToast(t('Your query was updated'))); - dispatch(queryEditorSetTitle(query, query.title)); + dispatch(queryEditorSetTitle(query, query.name)); }) .catch(() => dispatch(addDangerToast(t('Your query could not be updated'))), @@ -965,7 +963,7 @@ export function queryEditorSetQueryLimit(queryEditor, queryLimit) { dispatch( addDangerToast( t( - 'An error occurred while setting the tab title. Please contact your administrator.', + 'An error occurred while setting the tab name. Please contact your administrator.', ), ), ), @@ -1264,7 +1262,7 @@ export function popStoredQuery(urlId) { .then(({ json }) => dispatch( addQueryEditor({ - title: json.title ? json.title : t('Shared query'), + name: json.name ? json.name : t('Shared query'), dbId: json.dbId ? parseInt(json.dbId, 10) : null, schema: json.schema ? json.schema : null, autorun: json.autorun ? json.autorun : false, @@ -1302,7 +1300,7 @@ export function popQuery(queryId) { dbId: queryData.database.id, schema: queryData.schema, sql: queryData.sql, - title: `Copy of ${queryData.tab_name}`, + name: `Copy of ${queryData.tab_name}`, autorun: false, }; return dispatch(addQueryEditor(queryEditorProps)); @@ -1318,7 +1316,7 @@ export function popDatasourceQuery(datasourceKey, sql) { .then(({ json }) => dispatch( addQueryEditor({ - title: `Query ${json.name}`, + name: `Query ${json.name}`, dbId: json.database.id, schema: json.schema, autorun: sql !== undefined, diff --git a/superset-frontend/src/SqlLab/actions/sqlLab.test.js b/superset-frontend/src/SqlLab/actions/sqlLab.test.js index f9e972d2a453e..1c4509b3a1ad4 100644 --- a/superset-frontend/src/SqlLab/actions/sqlLab.test.js +++ b/superset-frontend/src/SqlLab/actions/sqlLab.test.js @@ -38,8 +38,8 @@ describe('async actions', () => { latestQueryId: null, selectedText: null, sql: 'SELECT *\nFROM\nWHERE', - title: 'Untitled Query 1', - schemaOptions: [{ value: 'main', label: 'main', title: 'main' }], + name: 'Untitled Query 1', + schemaOptions: [{ value: 'main', label: 'main', name: 'main' }], }; let dispatch; @@ -290,7 +290,7 @@ describe('async actions', () => { const state = { sqlLab: { tabHistory: [id], - queryEditors: [{ id, title: 'Dummy query editor' }], + queryEditors: [{ id, name: 'Dummy query editor' }], }, }; const store = mockStore(state); @@ -350,7 +350,7 @@ describe('async actions', () => { const state = { sqlLab: { tabHistory: [id], - queryEditors: [{ id, title: 'Dummy query editor' }], + queryEditors: [{ id, name: 'Dummy query editor' }], }, }; const store = mockStore(state); @@ -358,7 +358,7 @@ describe('async actions', () => { { type: actions.ADD_QUERY_EDITOR, queryEditor: { - title: 'Copy of Dummy query editor', + name: 'Copy of Dummy query editor', dbId: 1, schema: null, autorun: true, @@ -617,17 +617,17 @@ describe('async actions', () => { it('updates the tab state in the backend', () => { expect.assertions(2); - const title = 'title'; + const name = 'name'; const store = mockStore({}); const expectedActions = [ { type: actions.QUERY_EDITOR_SET_TITLE, queryEditor, - title, + name, }, ]; return store - .dispatch(actions.queryEditorSetTitle(queryEditor, title)) + .dispatch(actions.queryEditorSetTitle(queryEditor, name)) .then(() => { expect(store.getActions()).toEqual(expectedActions); expect(fetchMock.calls(updateTabStateEndpoint)).toHaveLength(1); diff --git a/superset-frontend/src/SqlLab/components/ResultSet/index.tsx b/superset-frontend/src/SqlLab/components/ResultSet/index.tsx index 6e0e0d4317800..8ab8f1426b3ac 100644 --- a/superset-frontend/src/SqlLab/components/ResultSet/index.tsx +++ b/superset-frontend/src/SqlLab/components/ResultSet/index.tsx @@ -29,19 +29,14 @@ import { SaveDatasetModal, } from 'src/SqlLab/components/SaveDatasetModal'; import { UserWithPermissionsAndRoles } from 'src/types/bootstrapTypes'; -import { EXPLORE_CHART_DEFAULT } from 'src/SqlLab/types'; -import { mountExploreUrl } from 'src/explore/exploreUtils'; -import { postFormData } from 'src/explore/exploreUtils/formData'; import ProgressBar from 'src/components/ProgressBar'; import Loading from 'src/components/Loading'; import FilterableTable, { MAX_COLUMNS_FOR_TABLE, } from 'src/components/FilterableTable'; import CopyToClipboard from 'src/components/CopyToClipboard'; -import { addDangerToast } from 'src/components/MessageToasts/actions'; import { prepareCopyToClipboardTabularData } from 'src/utils/common'; import { CtasEnum } from 'src/SqlLab/actions/sqlLab'; -import { URL_PARAMS } from 'src/constants'; import ExploreCtasResultsButton from '../ExploreCtasResultsButton'; import ExploreResultsButton from '../ExploreResultsButton'; import HighlightedSql from '../HighlightedSql'; @@ -188,7 +183,7 @@ export default class ResultSet extends React.PureComponent< popSelectStar(tempSchema: string | null, tempTable: string) { const qe = { id: shortid.generate(), - title: tempTable, + name: tempTable, autorun: false, dbId: this.props.query.dbId, sql: `SELECT * FROM ${tempSchema ? `${tempSchema}.` : ''}${tempTable}`, @@ -224,26 +219,6 @@ export default class ResultSet extends React.PureComponent< } } - createExploreResultsOnClick = async () => { - const { results } = this.props.query; - - if (results?.query_id) { - const key = await postFormData(results.query_id, 'query', { - ...EXPLORE_CHART_DEFAULT, - datasource: `${results.query_id}__query`, - ...{ - all_columns: results.columns.map(column => column.name), - }, - }); - const url = mountExploreUrl(null, { - [URL_PARAMS.formDataKey.name]: key, - }); - window.open(url, '_blank', 'noreferrer'); - } else { - addDangerToast(t('Unable to create chart without a query id.')); - } - }; - renderControls() { if (this.props.search || this.props.visualize || this.props.csv) { let { data } = this.props.query.results; @@ -281,11 +256,19 @@ export default class ResultSet extends React.PureComponent< this.props.database?.allows_virtual_table_explore && ( this.setState({ showSaveDatasetModal: true })} + onClick={() => { + // There is currently redux / state issue where sometimes a query will have serverId + // and other times it will not. We need this attribute consistently for this to work + // const qid = this.props?.query?.results?.query_id; + // if (qid) { + // // This will open explore using the query as datasource + // window.location.href = `/explore/?dataset_type=query&dataset_id=${qid}`; + // } else { + // this.setState({ showSaveDatasetModal: true }); + // } + this.setState({ showSaveDatasetModal: true }); + }} /> - // In order to use the new workflow for a query powered chart, replace the - // above function with: - // onClick={this.createExploreResultsOnClick} )} {this.props.csv && ( + ) : ( + + } + trigger={['click']} + > + {t('Save')} + + ); +} diff --git a/superset-frontend/src/SqlLab/components/SaveDatasetModal/SaveDatasetModal.test.tsx b/superset-frontend/src/SqlLab/components/SaveDatasetModal/SaveDatasetModal.test.tsx index b7f3ad8a8a42d..5520855d87768 100644 --- a/superset-frontend/src/SqlLab/components/SaveDatasetModal/SaveDatasetModal.test.tsx +++ b/superset-frontend/src/SqlLab/components/SaveDatasetModal/SaveDatasetModal.test.tsx @@ -21,8 +21,16 @@ import { ISaveableDatasource, SaveDatasetModal, } from 'src/SqlLab/components/SaveDatasetModal'; -import { render, screen } from 'spec/helpers/testing-library'; +import { Select } from 'src/components'; +import { + render, + screen, + act, + waitFor, + within, +} from 'spec/helpers/testing-library'; import { DatasourceType } from '@superset-ui/core'; +import userEvent from '@testing-library/user-event'; const testQuery: ISaveableDatasource = { name: 'unimportant', @@ -55,9 +63,732 @@ const mockedProps = { datasource: testQuery, }; -describe('SaveDatasetModal RTL', () => { - it('renders a "Save as new" field', () => { - render(, { useRedux: true }); +const testDatasetList = { + count: 25, + description_columns: {}, + ids: [ + 48, 42, 36, 47, 46, 45, 44, 43, 41, 40, 39, 38, 37, 35, 34, 33, 32, 31, 30, + 29, + ], + label_columns: { + 'changed_by.first_name': 'Changed By First Name', + 'changed_by.username': 'Changed By Username', + changed_by_name: 'Changed By Name', + changed_by_url: 'Changed By Url', + changed_on_delta_humanized: 'Changed On Delta Humanized', + changed_on_utc: 'Changed On Utc', + 'database.database_name': 'Database Database Name', + 'database.id': 'Database Id', + datasource_type: 'Datasource Type', + default_endpoint: 'Default Endpoint', + description: 'Description', + explore_url: 'Explore Url', + extra: 'Extra', + id: 'Id', + kind: 'Kind', + 'owners.first_name': 'Owners First Name', + 'owners.id': 'Owners Id', + 'owners.last_name': 'Owners Last Name', + 'owners.username': 'Owners Username', + schema: 'Schema', + sql: 'Sql', + table_name: 'Table Name', + }, + list_columns: [ + 'id', + 'database.id', + 'database.database_name', + 'changed_by_name', + 'changed_by_url', + 'changed_by.first_name', + 'changed_by.username', + 'changed_on_utc', + 'changed_on_delta_humanized', + 'default_endpoint', + 'description', + 'datasource_type', + 'explore_url', + 'extra', + 'kind', + 'owners.id', + 'owners.username', + 'owners.first_name', + 'owners.last_name', + 'schema', + 'sql', + 'table_name', + ], + list_title: 'List Sqla Table', + order_columns: [ + 'table_name', + 'schema', + 'changed_by.first_name', + 'changed_on_delta_humanized', + 'database.database_name', + ], + result: [ + { + changed_by: { + first_name: 'Admin I.', + username: 'admin', + }, + changed_by_name: 'Admin I. Strator', + changed_by_url: '/superset/profile/admin', + changed_on_delta_humanized: '20 hours ago', + changed_on_utc: '2022-07-12T22:41:31.297778+0000', + database: { + database_name: 'Google Sheets', + id: 2, + }, + datasource_type: 'table', + default_endpoint: null, + description: null, + explore_url: '/explore/?dataset_type=table&dataset_id=48', + extra: null, + id: 48, + kind: 'virtual', + owners: [ + { + first_name: 'Admin I.', + id: 1, + last_name: 'Strator', + username: 'admin', + }, + ], + schema: null, + sql: 'SELECT * FROM Sheet1 ', + table_name: 'Garfield query 07/12/2022 14:30:54', + }, + { + changed_by: { + first_name: 'Admin I.', + username: 'admin', + }, + changed_by_name: 'Admin I. Strator', + changed_by_url: '/superset/profile/admin', + changed_on_delta_humanized: '20 hours ago', + changed_on_utc: '2022-07-12T22:24:58.698853+0000', + database: { + database_name: 'Google Sheets', + id: 2, + }, + datasource_type: 'table', + default_endpoint: null, + description: null, + explore_url: '/explore/?dataset_type=table&dataset_id=42', + extra: null, + id: 42, + kind: 'virtual', + owners: [ + { + first_name: 'Admin I.', + id: 1, + last_name: 'Strator', + username: 'admin', + }, + ], + schema: 'main', + sql: 'SELECT * FROM Sheet1 ', + table_name: 'undefined 06/27/2022 18:24:39', + }, + { + changed_by: { + first_name: 'Admin I.', + username: 'admin', + }, + changed_by_name: 'Admin I. Strator', + changed_by_url: '/superset/profile/admin', + changed_on_delta_humanized: 'a day ago', + changed_on_utc: '2022-07-12T16:32:36.599554+0000', + database: { + database_name: 'Google Sheets', + id: 2, + }, + datasource_type: 'table', + default_endpoint: null, + description: null, + explore_url: '/explore/?dataset_type=table&dataset_id=36', + extra: null, + id: 36, + kind: 'virtual', + owners: [ + { + first_name: 'Admin I.', + id: 1, + last_name: 'Strator', + username: 'admin', + }, + ], + schema: 'main', + sql: 'SELECT * FROM Sheet1 ', + table_name: 'Query 05/19/2022 18:56:50', + }, + { + changed_by: { + first_name: 'Admin I.', + username: 'admin', + }, + changed_by_name: 'Admin I. Strator', + changed_by_url: '/superset/profile/admin', + changed_on_delta_humanized: '8 days ago', + changed_on_utc: '2022-07-05T17:38:12.502830+0000', + database: { + database_name: 'Google Sheets', + id: 2, + }, + datasource_type: 'table', + default_endpoint: null, + description: null, + explore_url: '/explore/?dataset_type=table&dataset_id=47', + extra: null, + id: 47, + kind: 'virtual', + owners: [ + { + first_name: 'Admin I.', + id: 1, + last_name: 'Strator', + username: 'admin', + }, + ], + schema: 'main', + sql: 'SELECT * FROM Sheet1', + table_name: 'Untitled Query 1 07/05/2022 12:38:08', + }, + { + changed_by: { + first_name: 'Admin I.', + username: 'admin', + }, + changed_by_name: 'Admin I. Strator', + changed_by_url: '/superset/profile/admin', + changed_on_delta_humanized: '10 days ago', + changed_on_utc: '2022-07-03T02:32:59.161912+0000', + database: { + database_name: 'Google Sheets', + id: 2, + }, + datasource_type: 'table', + default_endpoint: null, + description: null, + explore_url: '/explore/?dataset_type=table&dataset_id=46', + extra: null, + id: 46, + kind: 'virtual', + owners: [ + { + first_name: 'Admin I.', + id: 1, + last_name: 'Strator', + username: 'admin', + }, + ], + schema: 'main', + sql: 'SELECT * FROM Sheet1', + table_name: 'Untitled Query 1 07/02/2022 21:28:53', + }, + { + changed_by: { + first_name: 'Admin I.', + username: 'admin', + }, + changed_by_name: 'Admin I. Strator', + changed_by_url: '/superset/profile/admin', + changed_on_delta_humanized: '13 days ago', + changed_on_utc: '2022-06-30T18:43:02.757895+0000', + database: { + database_name: 'Google Sheets', + id: 2, + }, + datasource_type: 'table', + default_endpoint: null, + description: null, + explore_url: '/explore/?dataset_type=table&dataset_id=45', + extra: null, + id: 45, + kind: 'virtual', + owners: [ + { + first_name: 'Admin I.', + id: 1, + last_name: 'Strator', + username: 'admin', + }, + ], + schema: null, + sql: 'SELECT * FROM Sheet1', + table_name: 'Untitled 06/30/2022 13:43:00', + }, + { + changed_by: { + first_name: 'Admin I.', + username: 'admin', + }, + changed_by_name: 'Admin I. Strator', + changed_by_url: '/superset/profile/admin', + changed_on_delta_humanized: '13 days ago', + changed_on_utc: '2022-06-30T17:32:38.088415+0000', + database: { + database_name: 'Google Sheets', + id: 2, + }, + datasource_type: 'table', + default_endpoint: null, + description: null, + explore_url: '/explore/?dataset_type=table&dataset_id=44', + extra: null, + id: 44, + kind: 'virtual', + owners: [ + { + first_name: 'Admin I.', + id: 1, + last_name: 'Strator', + username: 'admin', + }, + ], + schema: 'main', + sql: 'SELECT * FROM Sheet1', + table_name: 'Untitled Query 1 06/30/2022 12:30:59', + }, + { + changed_by: { + first_name: 'Admin I.', + username: 'admin', + }, + changed_by_name: 'Admin I. Strator', + changed_by_url: '/superset/profile/admin', + changed_on_delta_humanized: '13 days ago', + changed_on_utc: '2022-06-30T17:30:59.136026+0000', + database: { + database_name: 'Google Sheets', + id: 2, + }, + datasource_type: 'table', + default_endpoint: null, + description: null, + explore_url: '/explore/?dataset_type=table&dataset_id=43', + extra: null, + id: 43, + kind: 'virtual', + owners: [ + { + first_name: 'Admin I.', + id: 1, + last_name: 'Strator', + username: 'admin', + }, + ], + schema: 'main', + sql: 'SELECT * FROM Sheet1', + table_name: 'Untitled Query 1 06/30/2022 12:30:44', + }, + { + changed_by: { + first_name: 'Admin I.', + username: 'admin', + }, + changed_by_name: 'Admin I. Strator', + changed_by_url: '/superset/profile/admin', + changed_on_delta_humanized: '20 days ago', + changed_on_utc: '2022-06-22T19:53:04.308253+0000', + database: { + database_name: 'examples', + id: 1, + }, + datasource_type: 'table', + default_endpoint: null, + description: null, + explore_url: '/explore/?dataset_type=table&dataset_id=41', + extra: null, + id: 41, + kind: 'virtual', + owners: [ + { + first_name: 'Admin I.', + id: 1, + last_name: 'Strator', + username: 'admin', + }, + ], + schema: 'main', + sql: 'SELECT * FROM "FCC 2018 Survey"', + table_name: 'Untitled Query 1 06/22/2022 14:46:56', + }, + { + changed_by: { + first_name: 'Admin I.', + username: 'admin', + }, + changed_by_name: 'Admin I. Strator', + changed_by_url: '/superset/profile/admin', + changed_on_delta_humanized: '20 days ago', + changed_on_utc: '2022-06-22T19:46:56.758263+0000', + database: { + database_name: 'examples', + id: 1, + }, + datasource_type: 'table', + default_endpoint: null, + description: null, + explore_url: '/explore/?dataset_type=table&dataset_id=40', + extra: null, + id: 40, + kind: 'virtual', + owners: [ + { + first_name: 'Admin I.', + id: 1, + last_name: 'Strator', + username: 'admin', + }, + ], + schema: 'main', + sql: 'SELECT * FROM "FCC 2018 Survey"', + table_name: 'Untitled Query 1 06/22/2022 14:46:05', + }, + { + changed_by: { + first_name: 'Admin I.', + username: 'admin', + }, + changed_by_name: 'Admin I. Strator', + changed_by_url: '/superset/profile/admin', + changed_on_delta_humanized: '20 days ago', + changed_on_utc: '2022-06-22T19:13:10.106631+0000', + database: { + database_name: 'examples', + id: 1, + }, + datasource_type: 'table', + default_endpoint: null, + description: null, + explore_url: '/explore/?dataset_type=table&dataset_id=39', + extra: null, + id: 39, + kind: 'virtual', + owners: [ + { + first_name: 'Admin I.', + id: 1, + last_name: 'Strator', + username: 'admin', + }, + ], + schema: null, + sql: 'SELECT * FROM "FCC 2018 Survey"', + table_name: 'Untitled Dataset 06/22/2022 14:13:00', + }, + { + changed_by: { + first_name: 'Admin I.', + username: 'admin', + }, + changed_by_name: 'Admin I. Strator', + changed_by_url: '/superset/profile/admin', + changed_on_delta_humanized: '21 days ago', + changed_on_utc: '2022-06-21T19:43:58.510913+0000', + database: { + database_name: 'examples', + id: 1, + }, + datasource_type: 'table', + default_endpoint: null, + description: null, + explore_url: '/explore/?dataset_type=table&dataset_id=38', + extra: null, + id: 38, + kind: 'physical', + owners: [ + { + first_name: 'Admin I.', + id: 1, + last_name: 'Strator', + username: 'admin', + }, + ], + schema: null, + sql: '', + table_name: 'Untitled Dataset 06/21/2022 14:43:54', + }, + { + changed_by: { + first_name: 'Admin I.', + username: 'admin', + }, + changed_by_name: 'Admin I. Strator', + changed_by_url: '/superset/profile/admin', + changed_on_delta_humanized: '21 days ago', + changed_on_utc: '2022-06-21T19:42:46.112798+0000', + database: { + database_name: 'examples', + id: 1, + }, + datasource_type: 'table', + default_endpoint: null, + description: null, + explore_url: '/explore/?dataset_type=table&dataset_id=37', + extra: null, + id: 37, + kind: 'physical', + owners: [ + { + first_name: 'Admin I.', + id: 1, + last_name: 'Strator', + username: 'admin', + }, + ], + schema: null, + sql: '', + table_name: 'Untitled Dataset 06/21/2022 14:42:36', + }, + { + changed_by: { + first_name: 'Admin I.', + username: 'admin', + }, + changed_by_name: 'Admin I. Strator', + changed_by_url: '/superset/profile/admin', + changed_on_delta_humanized: 'a month ago', + changed_on_utc: '2022-05-17T14:25:41.210001+0000', + database: { + database_name: 'examples', + id: 1, + }, + datasource_type: 'table', + default_endpoint: null, + description: null, + explore_url: '/explore/?dataset_type=table&dataset_id=35', + extra: null, + id: 35, + kind: 'virtual', + owners: [ + { + first_name: 'Admin I.', + id: 1, + last_name: 'Strator', + username: 'admin', + }, + ], + schema: 'main', + sql: 'SELECT * FROM "FCC 2018 Survey"', + table_name: 'Untitled Query 1 05/16/2022 16:31:04', + }, + { + changed_by: { + first_name: 'Admin I.', + username: 'admin', + }, + changed_by_name: 'Admin I. Strator', + changed_by_url: '/superset/profile/admin', + changed_on_delta_humanized: 'a month ago', + changed_on_utc: '2022-05-16T21:02:33.825514+0000', + database: { + database_name: 'examples', + id: 1, + }, + datasource_type: 'table', + default_endpoint: null, + description: null, + explore_url: '/explore/?dataset_type=table&dataset_id=34', + extra: null, + id: 34, + kind: 'virtual', + owners: [ + { + first_name: 'Admin I.', + id: 1, + last_name: 'Strator', + username: 'admin', + }, + ], + schema: 'main', + sql: 'SELECT * FROM "FCC 2018 Survey"', + table_name: 'Untitled Query 1 05/16/2022 16:02:04', + }, + { + changed_by: { + first_name: 'Admin I.', + username: 'admin', + }, + changed_by_name: 'Admin I. Strator', + changed_by_url: '/superset/profile/admin', + changed_on_delta_humanized: 'a month ago', + changed_on_utc: '2022-05-16T21:02:04.789763+0000', + database: { + database_name: 'examples', + id: 1, + }, + datasource_type: 'table', + default_endpoint: null, + description: null, + explore_url: '/explore/?dataset_type=table&dataset_id=33', + extra: null, + id: 33, + kind: 'virtual', + owners: [ + { + first_name: 'Admin I.', + id: 1, + last_name: 'Strator', + username: 'admin', + }, + ], + schema: 'main', + sql: 'SELECT * FROM "FCC 2018 Survey"', + table_name: 'Untitled Query 1 05/16/2022 16:02:00', + }, + { + changed_by: { + first_name: 'Admin I.', + username: 'admin', + }, + changed_by_name: 'Admin I. Strator', + changed_by_url: '/superset/profile/admin', + changed_on_delta_humanized: 'a month ago', + changed_on_utc: '2022-05-16T20:14:23.034596+0000', + database: { + database_name: 'examples', + id: 1, + }, + datasource_type: 'table', + default_endpoint: null, + description: null, + explore_url: '/explore/?dataset_type=table&dataset_id=32', + extra: null, + id: 32, + kind: 'virtual', + owners: [ + { + first_name: 'Admin I.', + id: 1, + last_name: 'Strator', + username: 'admin', + }, + ], + schema: 'main', + sql: 'SELECT * FROM "FCC 2018 Survey"', + table_name: 'Untitled Query 1 05/16/2022 15:14:18', + }, + { + changed_by: { + first_name: 'Admin I.', + username: 'admin', + }, + changed_by_name: 'Admin I. Strator', + changed_by_url: '/superset/profile/admin', + changed_on_delta_humanized: 'a month ago', + changed_on_utc: '2022-05-16T18:27:26.167253+0000', + database: { + database_name: 'examples', + id: 1, + }, + datasource_type: 'table', + default_endpoint: null, + description: null, + explore_url: '/explore/?dataset_type=table&dataset_id=31', + extra: null, + id: 31, + kind: 'virtual', + owners: [ + { + first_name: 'Admin I.', + id: 1, + last_name: 'Strator', + username: 'admin', + }, + ], + schema: 'main', + sql: 'SELECT * FROM "FCC 2018 Survey"', + table_name: 'Untitled Query 1 05/16/2022 13:27:20', + }, + { + changed_by: { + first_name: 'Admin I.', + username: 'admin', + }, + changed_by_name: 'Admin I. Strator', + changed_by_url: '/superset/profile/admin', + changed_on_delta_humanized: '2 months ago', + changed_on_utc: '2022-05-13T17:32:43.420029+0000', + database: { + database_name: 'examples', + id: 1, + }, + datasource_type: 'table', + default_endpoint: null, + description: null, + explore_url: '/explore/?dataset_type=table&dataset_id=30', + extra: null, + id: 30, + kind: 'virtual', + owners: [ + { + first_name: 'Admin I.', + id: 1, + last_name: 'Strator', + username: 'admin', + }, + ], + schema: 'main', + sql: 'SELECT * FROM "FCC 2018 Survey"', + table_name: 'Untitled Query 1 05/13/2022 12:27:46', + }, + { + changed_by: { + first_name: 'Admin I.', + username: 'admin', + }, + changed_by_name: 'Admin I. Strator', + changed_by_url: '/superset/profile/admin', + changed_on_delta_humanized: '2 months ago', + changed_on_utc: '2022-05-13T17:27:46.998589+0000', + database: { + database_name: 'examples', + id: 1, + }, + datasource_type: 'table', + default_endpoint: null, + description: null, + explore_url: '/explore/?dataset_type=table&dataset_id=29', + extra: null, + id: 29, + kind: 'virtual', + owners: [ + { + first_name: 'Admin I.', + id: 1, + last_name: 'Strator', + username: 'admin', + }, + ], + schema: 'main', + sql: 'SELECT * FROM "FCC 2018 Survey"', + table_name: 'Untitled Query 1 05/13/2022 12:27:43', + }, + ], +}; + +const mockDatasetNames = testDatasetList.result.map( + dataset => dataset.table_name, +); + +const getElementByClassName = (className: string) => + document.querySelector(className)! as HTMLElement; + +const findSelectValue = () => + waitFor(() => getElementByClassName('.ant-select-selection-item')); +const getElementsByClassName = (className: string) => + document.querySelectorAll(className)! as NodeListOf; +const findAllSelectOptions = () => + waitFor(() => getElementsByClassName('.ant-select-item-option-content')); + +describe('SaveDatasetModal', () => { + it('renders a "Save as new" field', async () => { + await act(async () => { + render(, { useRedux: true }); + }); const saveRadioBtn = screen.getByRole('radio', { name: /save as new unimportant/i, @@ -73,8 +804,10 @@ describe('SaveDatasetModal RTL', () => { expect(inputFieldText).toBeVisible(); }); - it('renders an "Overwrite existing" field', () => { - render(, { useRedux: true }); + it('renders an "Overwrite existing" field', async () => { + await act(async () => { + render(, { useRedux: true }); + }); const overwriteRadioBtn = screen.getByRole('radio', { name: /overwrite existing/i, @@ -89,15 +822,116 @@ describe('SaveDatasetModal RTL', () => { expect(placeholderText).toBeVisible(); }); - it('renders a save button', () => { - render(, { useRedux: true }); + it('renders a close button', async () => { + await act(async () => { + render(, { useRedux: true }); + }); + + expect(screen.getByRole('button', { name: /close/i })).toBeVisible(); + }); + + it('renders a save button when "Save as new" is selected', async () => { + await act(async () => { + render(, { useRedux: true }); + }); + // "Save as new" is selected when the modal opens by default expect(screen.getByRole('button', { name: /save/i })).toBeVisible(); }); - it('renders a close button', () => { - render(, { useRedux: true }); + it('renders a back and overwrite button when "Overwrite existing" is selected', async () => { + await act(async () => { + render(, { useRedux: true }); + }); - expect(screen.getByRole('button', { name: /close/i })).toBeVisible(); + const overwriteRadioBtn = screen.getByRole('radio', { + name: /overwrite existing/i, + }); + + // Click the overwrite radio button to reveal the overwrite buttons + userEvent.click(overwriteRadioBtn); + + expect(screen.getByRole('button', { name: /back/i })).toBeVisible(); + expect(screen.getByRole('button', { name: /overwrite/i })).toBeVisible(); + }); + + const OPTIONS = [ + { label: 'John', value: 1, gender: 'Male' }, + { label: 'Liam', value: 2, gender: 'Male' }, + { label: 'Olivia', value: 3, gender: 'Female' }, + { label: 'Emma', value: 4, gender: 'Female' }, + { label: 'Noah', value: 5, gender: 'Male' }, + { label: 'Ava', value: 6, gender: 'Female' }, + { label: 'Oliver', value: 7, gender: 'Male' }, + { label: 'ElijahH', value: 8, gender: 'Male' }, + { label: 'Charlotte', value: 9, gender: 'Female' }, + { label: 'Giovanni', value: 10, gender: 'Male' }, + { label: 'Franco', value: 11, gender: 'Male' }, + { label: 'Sandro', value: 12, gender: 'Male' }, + { label: 'Alehandro', value: 13, gender: 'Male' }, + { label: 'Johnny', value: 14, gender: 'Male' }, + { label: 'Nikole', value: 15, gender: 'Female' }, + { label: 'Igor', value: 16, gender: 'Male' }, + { label: 'Guilherme', value: 17, gender: 'Male' }, + { label: 'Irfan', value: 18, gender: 'Male' }, + { label: 'George', value: 19, gender: 'Male' }, + { label: 'Ashfaq', value: 20, gender: 'Male' }, + { label: 'Herme', value: 21, gender: 'Male' }, + { label: 'Cher', value: 22, gender: 'Female' }, + { label: 'Her', value: 23, gender: 'Male' }, + ].sort((option1, option2) => option1.label.localeCompare(option2.label)); + const defaultSelectProps = { + allowClear: true, + ariaLabel: 'Test', + labelInValue: true, + options: OPTIONS, + pageSize: 10, + showSearch: true, + }; + + it('renders the overwrite button as disabled until an existing dataset is selected', async () => { + await act(async () => { + render( + + - , - { useRedux: true }, - ); - }); + useSelectorMock.mockReturnValue({ ...user }); + render(, { useRedux: true }); - // expect.assertions(2); + // Click the overwrite radio button const overwriteRadioBtn = screen.getByRole('radio', { name: /overwrite existing/i, }); - - await act(async () => { + await waitFor(async () => { userEvent.click(overwriteRadioBtn); }); - expect(screen.getByRole('button', { name: /overwrite/i })).toBeDisabled(); - expect(screen.queryByText(/no data/i)).toBe(null); + // Overwrite confirmation button should be disabled at this point + const overwriteConfirmationBtn = screen.getByRole('button', { + name: /overwrite/i, + }); + expect(overwriteConfirmationBtn).toBeDisabled(); + // Click the select component const select = screen.getByRole('combobox', { name: /existing dataset/i }); + await waitFor(async () => userEvent.click(select)); - await act(async () => { - userEvent.clear(select); - userEvent.click(select); - userEvent.type(select, 'e'); - const options = await findAllSelectOptions(); - console.log(options); - // userEvent.click(await findAllSelectOptions()[0]); + // Select the first "existing dataset" from the listbox + await waitFor(async () => { + const listbox = screen.getByRole('listbox'); + userEvent.selectOptions(listbox, 'coolest table 0'); }); - screen.logTestingPlaygroundURL(); - - // expect(screen.queryByText(/no data/i)?.innerHTML).toBe('No Data'); - // expect(screen.queryByText(/no data/i)).toBeInTheDocument(); - - // await act(async () => { - // userEvent.click(screen.getByText(/no data/i)); - // }); - // const options = await findSelectValue(); - // screen.debug(options); - // console.log(options); + // Overwrite button should now be enabled + expect(overwriteConfirmationBtn).toBeEnabled(); }); }); From c415fe40039f4292292704f1c52e82be1bc2a9d3 Mon Sep 17 00:00:00 2001 From: lyndsiWilliams Date: Wed, 27 Jul 2022 13:44:47 -0500 Subject: [PATCH 08/12] SaveDatasetModal tests all passing --- .../SaveDatasetModal/SaveDatasetModal.test.tsx | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/superset-frontend/src/SqlLab/components/SaveDatasetModal/SaveDatasetModal.test.tsx b/superset-frontend/src/SqlLab/components/SaveDatasetModal/SaveDatasetModal.test.tsx index 14b229f26c169..5c8ea6b04b1b1 100644 --- a/superset-frontend/src/SqlLab/components/SaveDatasetModal/SaveDatasetModal.test.tsx +++ b/superset-frontend/src/SqlLab/components/SaveDatasetModal/SaveDatasetModal.test.tsx @@ -22,7 +22,7 @@ import { ISaveableDatasource, SaveDatasetModal, } from 'src/SqlLab/components/SaveDatasetModal'; -import { render, screen, waitFor } from 'spec/helpers/testing-library'; +import { render, screen, waitFor, within } from 'spec/helpers/testing-library'; import { DatasourceType } from '@superset-ui/core'; import userEvent from '@testing-library/user-event'; import fetchMock from 'fetch-mock'; @@ -182,10 +182,10 @@ describe('SaveDatasetModal', () => { await waitFor(async () => userEvent.click(select)); // Select the first "existing dataset" from the listbox - await waitFor(async () => { - const listbox = screen.getByRole('listbox'); - userEvent.selectOptions(listbox, 'coolest table 0'); - }); + const option = within( + document.querySelector('.rc-virtual-list')!, + ).getByText('coolest table 0')!; + userEvent.click(option); // Overwrite button should now be enabled expect(overwriteConfirmationBtn).toBeEnabled(); From 3fe37413a7735aeea6583d1cb816f26f92137413 Mon Sep 17 00:00:00 2001 From: lyndsiWilliams Date: Wed, 27 Jul 2022 22:54:51 -0500 Subject: [PATCH 09/12] Clean SaveDatasetModal test --- .../SaveDatasetModal.test.tsx | 64 +------------------ superset-frontend/src/SqlLab/fixtures.ts | 38 +++++++++++ 2 files changed, 41 insertions(+), 61 deletions(-) diff --git a/superset-frontend/src/SqlLab/components/SaveDatasetModal/SaveDatasetModal.test.tsx b/superset-frontend/src/SqlLab/components/SaveDatasetModal/SaveDatasetModal.test.tsx index 5c8ea6b04b1b1..94017525f08d6 100644 --- a/superset-frontend/src/SqlLab/components/SaveDatasetModal/SaveDatasetModal.test.tsx +++ b/superset-frontend/src/SqlLab/components/SaveDatasetModal/SaveDatasetModal.test.tsx @@ -18,69 +18,11 @@ */ import React from 'react'; import * as reactRedux from 'react-redux'; -import { - ISaveableDatasource, - SaveDatasetModal, -} from 'src/SqlLab/components/SaveDatasetModal'; import { render, screen, waitFor, within } from 'spec/helpers/testing-library'; -import { DatasourceType } from '@superset-ui/core'; import userEvent from '@testing-library/user-event'; import fetchMock from 'fetch-mock'; - -const user = { - createdOn: '2021-04-27T18:12:38.952304', - email: 'admin', - firstName: 'admin', - isActive: true, - lastName: 'admin', - permissions: {}, - roles: { - Admin: [ - ['can_sqllab', 'Superset'], - ['can_write', 'Dashboard'], - ['can_write', 'Chart'], - ], - }, - userId: 1, - username: 'admin', -}; - -const testQuery: ISaveableDatasource = { - name: 'unimportant', - dbId: 1, - sql: 'SELECT *', - columns: [ - { - name: 'Column 1', - type: DatasourceType.Query, - is_dttm: false, - }, - { - name: 'Column 3', - type: DatasourceType.Query, - is_dttm: false, - }, - { - name: 'Column 2', - type: DatasourceType.Query, - is_dttm: true, - }, - ], -}; - -const mockdatasets = [...new Array(3)].map((_, i) => ({ - changed_by_name: 'user', - kind: i === 0 ? 'virtual' : 'physical', // ensure there is 1 virtual - changed_by_url: 'changed_by_url', - changed_by: 'user', - changed_on: new Date().toISOString(), - database_name: `db ${i}`, - explore_url: `/explore/?dataset_type=table&dataset_id=${i}`, - id: i, - schema: `schema ${i}`, - table_name: `coolest table ${i}`, - owners: [{ username: 'admin', userId: 1 }], -})); +import { SaveDatasetModal } from 'src/SqlLab/components/SaveDatasetModal'; +import { user, testQuery, mockdatasets } from 'src/SqlLab/fixtures'; const mockedProps = { visible: true, @@ -178,7 +120,7 @@ describe('SaveDatasetModal', () => { expect(overwriteConfirmationBtn).toBeDisabled(); // Click the select component - const select = screen.getByRole('combobox', { name: /existing dataset/i }); + const select = screen.getByRole('combobox', { name: /existing dataset/i })!; await waitFor(async () => userEvent.click(select)); // Select the first "existing dataset" from the listbox diff --git a/superset-frontend/src/SqlLab/fixtures.ts b/superset-frontend/src/SqlLab/fixtures.ts index d74544d267d84..ea0fbd1bb8d1e 100644 --- a/superset-frontend/src/SqlLab/fixtures.ts +++ b/superset-frontend/src/SqlLab/fixtures.ts @@ -20,6 +20,7 @@ import sinon from 'sinon'; import * as actions from 'src/SqlLab/actions/sqlLab'; import { ColumnKeyTypeType } from 'src/SqlLab/components/ColumnElement'; import { DatasourceType, QueryResponse, QueryState } from '@superset-ui/core'; +import { ISaveableDatasource } from 'src/SqlLab/components/SaveDatasetModal'; export const mockedActions = sinon.stub({ ...actions }); @@ -670,3 +671,40 @@ export const query = { ctas: false, cached: false, }; + +export const testQuery: ISaveableDatasource = { + name: 'unimportant', + dbId: 1, + sql: 'SELECT *', + columns: [ + { + name: 'Column 1', + type: DatasourceType.Query, + is_dttm: false, + }, + { + name: 'Column 3', + type: DatasourceType.Query, + is_dttm: false, + }, + { + name: 'Column 2', + type: DatasourceType.Query, + is_dttm: true, + }, + ], +}; + +export const mockdatasets = [...new Array(3)].map((_, i) => ({ + changed_by_name: 'user', + kind: i === 0 ? 'virtual' : 'physical', // ensure there is 1 virtual + changed_by_url: 'changed_by_url', + changed_by: 'user', + changed_on: new Date().toISOString(), + database_name: `db ${i}`, + explore_url: `/explore/?dataset_type=table&dataset_id=${i}`, + id: i, + schema: `schema ${i}`, + table_name: `coolest table ${i}`, + owners: [{ username: 'admin', userId: 1 }], +})); From da9032074bc27887cfa6d59e9dc94078647c9f41 Mon Sep 17 00:00:00 2001 From: lyndsiWilliams Date: Fri, 29 Jul 2022 16:44:42 -0500 Subject: [PATCH 10/12] Fix create chart button and SaveDatasetModal text in SQL Lab --- superset-frontend/src/SqlLab/components/ResultSet/index.tsx | 5 +---- superset-frontend/src/SqlLab/components/SaveQuery/index.tsx | 4 ++-- 2 files changed, 3 insertions(+), 6 deletions(-) diff --git a/superset-frontend/src/SqlLab/components/ResultSet/index.tsx b/superset-frontend/src/SqlLab/components/ResultSet/index.tsx index fe29aa7240f0d..ebb8e4d3f030c 100644 --- a/superset-frontend/src/SqlLab/components/ResultSet/index.tsx +++ b/superset-frontend/src/SqlLab/components/ResultSet/index.tsx @@ -281,11 +281,8 @@ export default class ResultSet extends React.PureComponent< this.props.database?.allows_virtual_table_explore && ( this.setState({ showSaveDatasetModal: true })} + onClick={this.createExploreResultsOnClick} /> - // In order to use the new workflow for a query powered chart, replace the - // above function with: - // onClick={this.createExploreResultsOnClick} )} {this.props.csv && (