This is an automated email from the ASF dual-hosted git repository. michaelsmolina pushed a commit to branch 5.0 in repository https://gitbox.apache.org/repos/asf/superset.git
commit 8ad24fabc9ea7507add52c6f56e0f9dd098f9841 Author: Jack <[email protected]> AuthorDate: Fri Feb 7 11:16:44 2025 -0600 fix(virtual dataset sync): Sync virtual dataset columns when changing the SQL query (#30903) Co-authored-by: Kamil Gabryjelski <[email protected]> (cherry picked from commit f3e7c64de636a2fd287abdc04e6a7bec0fa12f53) --- .../cypress/e2e/explore/control.test.ts | 4 +- superset-frontend/jest.config.js | 1 + .../superset-ui-chart-controls/src/types.ts | 6 + .../superset-ui-core/src/query/types/Metric.ts | 4 +- .../test/BigNumber/transformProps.test.ts | 2 +- .../src/components/Datasource/DatasourceEditor.jsx | 136 +--- .../src/components/Datasource/DatasourceModal.tsx | 277 ++++---- .../src/components/Datasource/utils.js | 102 +++ superset-frontend/src/components/Tooltip/index.tsx | 8 +- .../src/explore/actions/exploreActions.ts | 16 + .../components/controls/TextAreaControl.jsx | 61 +- .../src/explore/reducers/exploreReducer.js | 18 + .../DatabaseConnectionForm/EncryptedField.tsx | 14 +- .../databases/DatabaseModal/index.test.tsx | 701 ++++++++++----------- .../src/features/databases/DatabaseModal/index.tsx | 4 +- .../UploadDataModel/UploadDataModal.test.tsx | 6 +- superset-frontend/src/features/datasets/types.ts | 24 +- 17 files changed, 734 insertions(+), 650 deletions(-) diff --git a/superset-frontend/cypress-base/cypress/e2e/explore/control.test.ts b/superset-frontend/cypress-base/cypress/e2e/explore/control.test.ts index b68d828ba8..c792a310ef 100644 --- a/superset-frontend/cypress-base/cypress/e2e/explore/control.test.ts +++ b/superset-frontend/cypress-base/cypress/e2e/explore/control.test.ts @@ -51,8 +51,8 @@ describe('Datasource control', () => { ) .first() .focus(); - cy.focused().clear(); - cy.focused().type(`${newMetricName}{enter}`); + cy.focused().clear({ force: true }); + cy.focused().type(`${newMetricName}{enter}`, { force: true }); cy.get('[data-test="datasource-modal-save"]').click(); cy.get('.antd5-modal-confirm-btns button').contains('OK').click(); diff --git a/superset-frontend/jest.config.js b/superset-frontend/jest.config.js index 07553d889e..13d07aa198 100644 --- a/superset-frontend/jest.config.js +++ b/superset-frontend/jest.config.js @@ -75,4 +75,5 @@ module.exports = { }, ], ], + testTimeout: 10000, }; diff --git a/superset-frontend/packages/superset-ui-chart-controls/src/types.ts b/superset-frontend/packages/superset-ui-chart-controls/src/types.ts index a53a45de35..62ac944688 100644 --- a/superset-frontend/packages/superset-ui-chart-controls/src/types.ts +++ b/superset-frontend/packages/superset-ui-chart-controls/src/types.ts @@ -84,6 +84,12 @@ export interface Dataset { filter_select?: boolean; filter_select_enabled?: boolean; column_names?: string[]; + catalog?: string; + schema?: string; + table_name?: string; + database?: Record<string, unknown>; + normalize_columns?: boolean; + always_filter_main_dttm?: boolean; } export interface ControlPanelState { diff --git a/superset-frontend/packages/superset-ui-core/src/query/types/Metric.ts b/superset-frontend/packages/superset-ui-core/src/query/types/Metric.ts index 227ca6e71d..229852373a 100644 --- a/superset-frontend/packages/superset-ui-core/src/query/types/Metric.ts +++ b/superset-frontend/packages/superset-ui-core/src/query/types/Metric.ts @@ -17,7 +17,7 @@ * specific language governing permissions and limitations * under the License. */ -import { Maybe, QueryFormMetric } from '../../types'; +import { Currency, Maybe, QueryFormMetric } from '../../types'; import { Column } from './Column'; export type Aggregate = @@ -65,7 +65,7 @@ export interface Metric { certification_details?: Maybe<string>; certified_by?: Maybe<string>; d3format?: Maybe<string>; - currency?: Maybe<string>; + currency?: Maybe<Currency>; description?: Maybe<string>; is_certified?: boolean; verbose_name?: string; diff --git a/superset-frontend/plugins/plugin-chart-echarts/test/BigNumber/transformProps.test.ts b/superset-frontend/plugins/plugin-chart-echarts/test/BigNumber/transformProps.test.ts index 8c9ee5621c..8b0bf35525 100644 --- a/superset-frontend/plugins/plugin-chart-echarts/test/BigNumber/transformProps.test.ts +++ b/superset-frontend/plugins/plugin-chart-echarts/test/BigNumber/transformProps.test.ts @@ -173,7 +173,7 @@ describe('BigNumberWithTrendline', () => { label: 'value', metric_name: 'value', d3format: '.2f', - currency: `{symbol: 'USD', symbolPosition: 'prefix' }`, + currency: { symbol: 'USD', symbolPosition: 'prefix' }, }, ], }, diff --git a/superset-frontend/src/components/Datasource/DatasourceEditor.jsx b/superset-frontend/src/components/Datasource/DatasourceEditor.jsx index d30b67ad2a..fae0270828 100644 --- a/superset-frontend/src/components/Datasource/DatasourceEditor.jsx +++ b/superset-frontend/src/components/Datasource/DatasourceEditor.jsx @@ -23,7 +23,6 @@ import { Radio } from 'src/components/Radio'; import Card from 'src/components/Card'; import Alert from 'src/components/Alert'; import Badge from 'src/components/Badge'; -import { nanoid } from 'nanoid'; import { css, isFeatureEnabled, @@ -57,6 +56,7 @@ import CurrencyControl from 'src/explore/components/controls/CurrencyControl'; import CollectionTable from './CollectionTable'; import Fieldset from './Fieldset'; import Field from './Field'; +import { fetchSyncedColumns, updateColumns } from './utils'; const DatasourceContainer = styled.div` .change-warning { @@ -140,6 +140,14 @@ const StyledButtonWrapper = styled.span` `} `; +const sqlTooltipOptions = { + placement: 'topRight', + title: t( + 'If changes are made to your SQL query, ' + + 'columns in your dataset will be synced when saving the dataset.', + ), +}; + const checkboxGenerator = (d, onChange) => ( <CheckboxControl value={d} onChange={onChange} /> ); @@ -694,116 +702,27 @@ class DatasourceEditor extends PureComponent { }); } - updateColumns(cols) { - // cols: Array<{column_name: string; is_dttm: boolean; type: string;}> - const { databaseColumns } = this.state; - const databaseColumnNames = cols.map(col => col.column_name); - const currentCols = databaseColumns.reduce( - (agg, col) => ({ - ...agg, - [col.column_name]: col, - }), - {}, - ); - const finalColumns = []; - const results = { - added: [], - modified: [], - removed: databaseColumns - .map(col => col.column_name) - .filter(col => !databaseColumnNames.includes(col)), - }; - cols.forEach(col => { - const currentCol = currentCols[col.column_name]; - if (!currentCol) { - // new column - finalColumns.push({ - id: nanoid(), - column_name: col.column_name, - type: col.type, - groupby: true, - filterable: true, - is_dttm: col.is_dttm, - }); - results.added.push(col.column_name); - } else if ( - currentCol.type !== col.type || - (!currentCol.is_dttm && col.is_dttm) - ) { - // modified column - finalColumns.push({ - ...currentCol, - type: col.type, - is_dttm: currentCol.is_dttm || col.is_dttm, - }); - results.modified.push(col.column_name); - } else { - // unchanged - finalColumns.push(currentCol); - } - }); - if ( - results.added.length || - results.modified.length || - results.removed.length - ) { - this.setColumns({ databaseColumns: finalColumns }); - } - return results; - } - - syncMetadata() { + async syncMetadata() { const { datasource } = this.state; - const params = { - datasource_type: datasource.type || datasource.datasource_type, - database_name: - datasource.database.database_name || datasource.database.name, - catalog_name: datasource.catalog, - schema_name: datasource.schema, - table_name: datasource.table_name, - normalize_columns: datasource.normalize_columns, - always_filter_main_dttm: datasource.always_filter_main_dttm, - }; - Object.entries(params).forEach(([key, value]) => { - // rison can't encode the undefined value - if (value === undefined) { - params[key] = null; - } - }); - const endpoint = `/datasource/external_metadata_by_name/?q=${rison.encode_uri( - params, - )}`; this.setState({ metadataLoading: true }); - - SupersetClient.get({ endpoint }) - .then(({ json }) => { - const results = this.updateColumns(json); - if (results.modified.length) { - this.props.addSuccessToast( - t('Modified columns: %s', results.modified.join(', ')), - ); - } - if (results.removed.length) { - this.props.addSuccessToast( - t('Removed columns: %s', results.removed.join(', ')), - ); - } - if (results.added.length) { - this.props.addSuccessToast( - t('New columns added: %s', results.added.join(', ')), - ); - } - this.props.addSuccessToast(t('Metadata has been synced')); - this.setState({ metadataLoading: false }); - }) - .catch(response => - getClientErrorObject(response).then(({ error, statusText }) => { - this.props.addDangerToast( - error || statusText || t('An error has occurred'), - ); - this.setState({ metadataLoading: false }); - }), + try { + const newCols = await fetchSyncedColumns(datasource); + const columnChanges = updateColumns( + datasource.columns, + newCols, + this.props.addSuccessToast, ); + this.setColumns({ databaseColumns: columnChanges.finalColumns }); + this.props.addSuccessToast(t('Metadata has been synced')); + this.setState({ metadataLoading: false }); + } catch (error) { + const { error: clientError, statusText } = + await getClientErrorObject(error); + this.props.addDangerToast( + clientError || statusText || t('An error has occurred'), + ); + this.setState({ metadataLoading: false }); + } } findDuplicates(arr, accessor) { @@ -1146,6 +1065,7 @@ class DatasourceEditor extends PureComponent { maxLines={Infinity} readOnly={!this.state.isEditMode} resize="both" + tooltipOptions={sqlTooltipOptions} /> } /> diff --git a/superset-frontend/src/components/Datasource/DatasourceModal.tsx b/superset-frontend/src/components/Datasource/DatasourceModal.tsx index 78483771d3..33cd820677 100644 --- a/superset-frontend/src/components/Datasource/DatasourceModal.tsx +++ b/superset-frontend/src/components/Datasource/DatasourceModal.tsx @@ -17,11 +17,11 @@ * under the License. */ import { FunctionComponent, useState, useRef } from 'react'; +import { useDispatch, useSelector } from 'react-redux'; import Alert from 'src/components/Alert'; import Button from 'src/components/Button'; import { isDefined, - Metric, styled, SupersetClient, getClientErrorObject, @@ -33,7 +33,16 @@ import Modal from 'src/components/Modal'; import AsyncEsmComponent from 'src/components/AsyncEsmComponent'; import ErrorMessageWithStackTrace from 'src/components/ErrorMessage/ErrorMessageWithStackTrace'; import withToasts from 'src/components/MessageToasts/withToasts'; -import { useSelector } from 'react-redux'; +import { + startMetaDataLoading, + stopMetaDataLoading, + syncDatasourceMetadata, +} from 'src/explore/actions/exploreActions'; +import { + fetchSyncedColumns, + updateColumns, +} from 'src/components/Datasource/utils'; +import { DatasetObject } from '../../features/datasets/types'; const DatasourceEditor = AsyncEsmComponent(() => import('./DatasourceEditor')); @@ -60,14 +69,17 @@ const StyledDatasourceModal = styled(Modal)` interface DatasourceModalProps { addSuccessToast: (msg: string) => void; - datasource: any; + addDangerToast: (msg: string) => void; + datasource: DatasetObject; onChange: () => {}; onDatasourceSave: (datasource: object, errors?: Array<any>) => {}; onHide: () => {}; show: boolean; } -function buildExtraJsonObject(item: Record<string, unknown>) { +function buildExtraJsonObject( + item: DatasetObject['metrics'][0] | DatasetObject['columns'][0], +) { const certification = item?.certified_by || item?.certification_details ? { @@ -83,18 +95,14 @@ function buildExtraJsonObject(item: Record<string, unknown>) { const DatasourceModal: FunctionComponent<DatasourceModalProps> = ({ addSuccessToast, + addDangerToast, datasource, onDatasourceSave, onHide, show, }) => { - const [currentDatasource, setCurrentDatasource] = useState({ - ...datasource, - metrics: datasource?.metrics?.map((metric: Metric) => ({ - ...metric, - currency: JSON.parse(metric.currency || 'null'), - })), - }); + const dispatch = useDispatch(); + const [currentDatasource, setCurrentDatasource] = useState(datasource); const currencies = useSelector< { common: { @@ -108,130 +116,145 @@ const DatasourceModal: FunctionComponent<DatasourceModalProps> = ({ const [isEditing, setIsEditing] = useState<boolean>(false); const dialog = useRef<any>(null); const [modal, contextHolder] = Modal.useModal(); - - const onConfirmSave = () => { + const buildPayload = (datasource: Record<string, any>) => ({ + table_name: datasource.table_name, + database_id: datasource.database?.id, + sql: datasource.sql, + filter_select_enabled: datasource.filter_select_enabled, + fetch_values_predicate: datasource.fetch_values_predicate, + schema: + datasource.tableSelector?.schema || + datasource.databaseSelector?.schema || + datasource.schema, + description: datasource.description, + main_dttm_col: datasource.main_dttm_col, + normalize_columns: datasource.normalize_columns, + always_filter_main_dttm: datasource.always_filter_main_dttm, + offset: datasource.offset, + default_endpoint: datasource.default_endpoint, + cache_timeout: + datasource.cache_timeout === '' ? null : datasource.cache_timeout, + is_sqllab_view: datasource.is_sqllab_view, + template_params: datasource.template_params, + extra: datasource.extra, + is_managed_externally: datasource.is_managed_externally, + external_url: datasource.external_url, + metrics: datasource?.metrics?.map((metric: DatasetObject['metrics'][0]) => { + const metricBody: any = { + expression: metric.expression, + description: metric.description, + metric_name: metric.metric_name, + metric_type: metric.metric_type, + d3format: metric.d3format || null, + currency: !isDefined(metric.currency) + ? null + : JSON.stringify(metric.currency), + verbose_name: metric.verbose_name, + warning_text: metric.warning_text, + uuid: metric.uuid, + extra: buildExtraJsonObject(metric), + }; + if (!Number.isNaN(Number(metric.id))) { + metricBody.id = metric.id; + } + return metricBody; + }), + columns: datasource?.columns?.map( + (column: DatasetObject['columns'][0]) => ({ + id: typeof column.id === 'number' ? column.id : undefined, + column_name: column.column_name, + type: column.type, + advanced_data_type: column.advanced_data_type, + verbose_name: column.verbose_name, + description: column.description, + expression: column.expression, + filterable: column.filterable, + groupby: column.groupby, + is_active: column.is_active, + is_dttm: column.is_dttm, + python_date_format: column.python_date_format || null, + uuid: column.uuid, + extra: buildExtraJsonObject(column), + }), + ), + owners: datasource.owners.map( + (o: Record<string, number>) => o.value || o.id, + ), + }); + const onConfirmSave = async () => { // Pull out extra fields into the extra object - const schema = - currentDatasource.tableSelector?.schema || - currentDatasource.databaseSelector?.schema || - currentDatasource.schema; - setIsSaving(true); - SupersetClient.put({ - endpoint: `/api/v1/dataset/${currentDatasource.id}`, - jsonPayload: { - table_name: currentDatasource.table_name, - database_id: currentDatasource.database?.id, - sql: currentDatasource.sql, - filter_select_enabled: currentDatasource.filter_select_enabled, - fetch_values_predicate: currentDatasource.fetch_values_predicate, - schema, - description: currentDatasource.description, - main_dttm_col: currentDatasource.main_dttm_col, - normalize_columns: currentDatasource.normalize_columns, - always_filter_main_dttm: currentDatasource.always_filter_main_dttm, - offset: currentDatasource.offset, - default_endpoint: currentDatasource.default_endpoint, - cache_timeout: - currentDatasource.cache_timeout === '' - ? null - : currentDatasource.cache_timeout, - is_sqllab_view: currentDatasource.is_sqllab_view, - template_params: currentDatasource.template_params, - extra: currentDatasource.extra, - is_managed_externally: currentDatasource.is_managed_externally, - external_url: currentDatasource.external_url, - metrics: currentDatasource?.metrics?.map( - (metric: Record<string, unknown>) => { - const metricBody: any = { - expression: metric.expression, - description: metric.description, - metric_name: metric.metric_name, - metric_type: metric.metric_type, - d3format: metric.d3format || null, - currency: !isDefined(metric.currency) - ? null - : JSON.stringify(metric.currency), - verbose_name: metric.verbose_name, - warning_text: metric.warning_text, - uuid: metric.uuid, - extra: buildExtraJsonObject(metric), - }; - if (!Number.isNaN(Number(metric.id))) { - metricBody.id = metric.id; - } - return metricBody; - }, - ), - columns: currentDatasource?.columns?.map( - (column: Record<string, unknown>) => ({ - id: typeof column.id === 'number' ? column.id : undefined, - column_name: column.column_name, - type: column.type, - advanced_data_type: column.advanced_data_type, - verbose_name: column.verbose_name, - description: column.description, - expression: column.expression, - filterable: column.filterable, - groupby: column.groupby, - is_active: column.is_active, - is_dttm: column.is_dttm, - python_date_format: column.python_date_format || null, - uuid: column.uuid, - extra: buildExtraJsonObject(column), - }), - ), - owners: currentDatasource.owners.map( - (o: Record<string, number>) => o.value || o.id, - ), - }, - }) - .then(() => { - addSuccessToast(t('The dataset has been saved')); - return SupersetClient.get({ - endpoint: `/api/v1/dataset/${currentDatasource?.id}`, - }); - }) - .then(({ json }) => { - // eslint-disable-next-line no-param-reassign - json.result.type = 'table'; - onDatasourceSave({ - ...json.result, - owners: currentDatasource.owners, - }); - onHide(); - }) - .catch(response => { - setIsSaving(false); - getClientErrorObject(response).then(error => { - let errorResponse: SupersetError | undefined; - let errorText: string | undefined; - // sip-40 error response - if (error?.errors?.length) { - errorResponse = error.errors[0]; - } else if (typeof error.error === 'string') { - // backward compatible with old error messages - errorText = error.error; - } - modal.error({ - title: t('Error saving dataset'), - okButtonProps: { danger: true, className: 'btn-danger' }, - content: ( - <ErrorMessageWithStackTrace - error={errorResponse} - source="crud" - fallback={errorText} - /> - ), - }); + try { + await SupersetClient.put({ + endpoint: `/api/v1/dataset/${currentDatasource.id}`, + jsonPayload: buildPayload(currentDatasource), + }); + if (datasource.sql !== currentDatasource.sql) { + // if sql has changed, save a second time with synced columns + dispatch(startMetaDataLoading()); + try { + const columnJson = await fetchSyncedColumns(currentDatasource); + const columnChanges = updateColumns( + currentDatasource.columns, + columnJson, + addSuccessToast, + ); + currentDatasource.columns = columnChanges.finalColumns; + dispatch(syncDatasourceMetadata(currentDatasource)); + dispatch(stopMetaDataLoading()); + addSuccessToast(t('Metadata has been synced')); + } catch (error) { + dispatch(stopMetaDataLoading()); + addDangerToast( + t('An error has occurred while syncing virtual dataset columns'), + ); + } + await SupersetClient.put({ + endpoint: `/api/v1/dataset/${currentDatasource.id}`, + jsonPayload: buildPayload(currentDatasource), }); + } + const { json } = await SupersetClient.get({ + endpoint: `/api/v1/dataset/${currentDatasource?.id}`, + }); + addSuccessToast(t('The dataset has been saved')); + // eslint-disable-next-line no-param-reassign + json.result.type = 'table'; + onDatasourceSave({ + ...json.result, + owners: currentDatasource.owners, + }); + onHide(); + } catch (response) { + setIsSaving(false); + const error = await getClientErrorObject(response); + let errorResponse: SupersetError | undefined; + let errorText: string | undefined; + // sip-40 error response + if (error?.errors?.length) { + errorResponse = error.errors[0]; + } else if (typeof error.error === 'string') { + // backward compatible with old error messages + errorText = error.error; + } + modal.error({ + title: t('Error saving dataset'), + okButtonProps: { danger: true, className: 'btn-danger' }, + content: ( + <ErrorMessageWithStackTrace + error={errorResponse} + source="crud" + fallback={errorText} + /> + ), }); + } }; - const onDatasourceChange = (data: Record<string, any>, err: Array<any>) => { + const onDatasourceChange = (data: DatasetObject, err: Array<any>) => { setCurrentDatasource({ ...data, - metrics: data?.metrics.map((metric: Record<string, unknown>) => ({ + metrics: data?.metrics.map((metric: DatasetObject['metrics'][0]) => ({ ...metric, is_certified: metric?.certified_by || metric?.certification_details, })), diff --git a/superset-frontend/src/components/Datasource/utils.js b/superset-frontend/src/components/Datasource/utils.js index ccdb1b414a..001a1a30b7 100644 --- a/superset-frontend/src/components/Datasource/utils.js +++ b/superset-frontend/src/components/Datasource/utils.js @@ -17,6 +17,9 @@ * under the License. */ import { Children, cloneElement } from 'react'; +import { nanoid } from 'nanoid'; +import { SupersetClient, tn } from '@superset-ui/core'; +import rison from 'rison'; export function recurseReactClone(children, type, propExtender) { /** @@ -40,3 +43,102 @@ export function recurseReactClone(children, type, propExtender) { return newChild; }); } + +export function updateColumns(prevCols, newCols, addSuccessToast) { + // cols: Array<{column_name: string; is_dttm: boolean; type: string;}> + const databaseColumnNames = newCols.map(col => col.column_name); + const currentCols = prevCols.reduce((agg, col) => { + // eslint-disable-next-line no-param-reassign + agg[col.column_name] = col; + return agg; + }, {}); + const columnChanges = { + added: [], + modified: [], + removed: prevCols + .map(col => col.column_name) + .filter(col => !databaseColumnNames.includes(col)), + finalColumns: [], + }; + newCols.forEach(col => { + const currentCol = currentCols[col.column_name]; + if (!currentCol) { + // new column + columnChanges.finalColumns.push({ + id: nanoid(), + column_name: col.column_name, + type: col.type, + groupby: true, + filterable: true, + is_dttm: col.is_dttm, + }); + columnChanges.added.push(col.column_name); + } else if ( + currentCol.type !== col.type || + currentCol.is_dttm !== col.is_dttm + ) { + // modified column + columnChanges.finalColumns.push({ + ...currentCol, + type: col.type, + is_dttm: currentCol.is_dttm || col.is_dttm, + }); + columnChanges.modified.push(col.column_name); + } else { + // unchanged + columnChanges.finalColumns.push(currentCol); + } + }); + if (columnChanges.modified.length) { + addSuccessToast( + tn( + 'Modified 1 column in the virtual dataset', + 'Modified %s columns in the virtual dataset', + columnChanges.modified.length, + ), + ); + } + if (columnChanges.removed.length) { + addSuccessToast( + tn( + 'Removed 1 column from the virtual dataset', + 'Removed %s columns from the virtual dataset', + columnChanges.removed.length, + ), + ); + } + if (columnChanges.added.length) { + addSuccessToast( + tn( + 'Added 1 new column to the virtual dataset', + 'Added %s new columns to the virtual dataset', + columnChanges.added.length, + ), + ); + } + return columnChanges; +} + +export async function fetchSyncedColumns(datasource) { + const params = { + datasource_type: datasource.type, + database_name: + datasource.database?.database_name || datasource.database?.name, + catalog_name: datasource.catalog, + schema_name: datasource.schema, + table_name: datasource.table_name, + normalize_columns: datasource.normalize_columns, + always_filter_main_dttm: datasource.always_filter_main_dttm, + }; + Object.entries(params).forEach(([key, value]) => { + // rison can't encode the undefined value + if (value === undefined) { + params[key] = null; + } + }); + const endpoint = `/datasource/external_metadata_by_name/?q=${rison.encode_uri( + params, + )}`; + const { json } = await SupersetClient.get({ endpoint }); + return json; +} diff --git a/superset-frontend/src/components/Tooltip/index.tsx b/superset-frontend/src/components/Tooltip/index.tsx index de5eef4b40..252aac3402 100644 --- a/superset-frontend/src/components/Tooltip/index.tsx +++ b/superset-frontend/src/components/Tooltip/index.tsx @@ -18,13 +18,9 @@ */ import { supersetTheme } from '@superset-ui/core'; import { Tooltip as AntdTooltip } from 'antd-v5'; -import { - TooltipProps as AntdTooltipProps, - TooltipPlacement as AntdTooltipPlacement, -} from 'antd-v5/lib/tooltip'; +import { TooltipProps, TooltipPlacement } from 'antd-v5/lib/tooltip'; -export type TooltipPlacement = AntdTooltipPlacement; -export type TooltipProps = AntdTooltipProps; +export { TooltipProps, TooltipPlacement }; export const Tooltip = ({ overlayStyle, ...props }: TooltipProps) => ( <> diff --git a/superset-frontend/src/explore/actions/exploreActions.ts b/superset-frontend/src/explore/actions/exploreActions.ts index da702ac16f..25b03ec9f2 100644 --- a/superset-frontend/src/explore/actions/exploreActions.ts +++ b/superset-frontend/src/explore/actions/exploreActions.ts @@ -164,6 +164,21 @@ export function setStashFormData( }; } +export const START_METADATA_LOADING = 'START_METADATA_LOADING'; +export function startMetaDataLoading() { + return { type: START_METADATA_LOADING }; +} + +export const STOP_METADATA_LOADING = 'STOP_METADATA_LOADING'; +export function stopMetaDataLoading() { + return { type: STOP_METADATA_LOADING }; +} + +export const SYNC_DATASOURCE_METADATA = 'SYNC_DATASOURCE_METADATA'; +export function syncDatasourceMetadata(datasource: Dataset) { + return { type: SYNC_DATASOURCE_METADATA, datasource }; +} + export const exploreActions = { ...toastActions, fetchDatasourcesStarted, @@ -178,6 +193,7 @@ export const exploreActions = { createNewSlice, sliceUpdated, setForceQuery, + syncDatasourceMetadata, }; export type ExploreActions = typeof exploreActions; diff --git a/superset-frontend/src/explore/components/controls/TextAreaControl.jsx b/superset-frontend/src/explore/components/controls/TextAreaControl.jsx index fc0545d8c4..e8f165c8e1 100644 --- a/superset-frontend/src/explore/components/controls/TextAreaControl.jsx +++ b/superset-frontend/src/explore/components/controls/TextAreaControl.jsx @@ -19,6 +19,10 @@ import { Component } from 'react'; import PropTypes from 'prop-types'; import { TextArea } from 'src/components/Input'; +import { + Tooltip, + TooltipProps as TooltipOptions, +} from 'src/components/Tooltip'; import { t, withTheme } from '@superset-ui/core'; import Button from 'src/components/Button'; @@ -55,6 +59,7 @@ const propTypes = { 'vertical', ]), textAreaStyles: PropTypes.object, + tooltipOptions: PropTypes.oneOf([null, TooltipOptions]), }; const defaultProps = { @@ -67,6 +72,7 @@ const defaultProps = { readOnly: false, resize: null, textAreaStyles: {}, + tooltipOptions: {}, }; class TextAreaControl extends Component { @@ -94,31 +100,44 @@ class TextAreaControl extends Component { if (this.props.readOnly) { style.backgroundColor = '#f2f2f2'; } + const codeEditor = ( + <div> + <TextAreaEditor + mode={this.props.language} + style={style} + minLines={minLines} + maxLines={inModal ? 1000 : this.props.maxLines} + editorProps={{ $blockScrolling: true }} + defaultValue={this.props.initialValue} + readOnly={this.props.readOnly} + key={this.props.name} + {...this.props} + onChange={this.onAreaEditorChange.bind(this)} + /> + </div> + ); + + if (this.props.tooltipOptions) { + return <Tooltip {...this.props.tooltipOptions}>{codeEditor}</Tooltip>; + } + return codeEditor; + } - return ( - <TextAreaEditor - mode={this.props.language} - style={style} - minLines={minLines} - maxLines={inModal ? 1000 : this.props.maxLines} - editorProps={{ $blockScrolling: true }} + const textArea = ( + <div> + <TextArea + placeholder={t('textarea')} + onChange={this.onControlChange.bind(this)} defaultValue={this.props.initialValue} - readOnly={this.props.readOnly} - key={this.props.name} - {...this.props} - onChange={this.onAreaEditorChange.bind(this)} + disabled={this.props.readOnly} + style={{ height: this.props.height }} /> - ); - } - return ( - <TextArea - placeholder={t('textarea')} - onChange={this.onControlChange.bind(this)} - defaultValue={this.props.initialValue} - disabled={this.props.readOnly} - style={{ height: this.props.height }} - /> + </div> ); + if (this.props.tooltipOptions) { + return <Tooltip {...this.props.tooltipOptions}>{textArea}</Tooltip>; + } + return textArea; } renderModalBody() { diff --git a/superset-frontend/src/explore/reducers/exploreReducer.js b/superset-frontend/src/explore/reducers/exploreReducer.js index 377a66d5b5..2677c1e3ff 100644 --- a/superset-frontend/src/explore/reducers/exploreReducer.js +++ b/superset-frontend/src/explore/reducers/exploreReducer.js @@ -50,6 +50,24 @@ export default function exploreReducer(state = {}, action) { isDatasourceMetaLoading: true, }; }, + [actions.START_METADATA_LOADING]() { + return { + ...state, + isDatasourceMetaLoading: true, + }; + }, + [actions.STOP_METADATA_LOADING]() { + return { + ...state, + isDatasourceMetaLoading: false, + }; + }, + [actions.SYNC_DATASOURCE_METADATA]() { + return { + ...state, + datasource: action.datasource, + }; + }, [actions.UPDATE_FORM_DATA_BY_DATASOURCE]() { const newFormData = { ...state.form_data }; const { prevDatasource, newDatasource } = action; diff --git a/superset-frontend/src/features/databases/DatabaseModal/DatabaseConnectionForm/EncryptedField.tsx b/superset-frontend/src/features/databases/DatabaseModal/DatabaseConnectionForm/EncryptedField.tsx index 147b983c45..ad80699802 100644 --- a/superset-frontend/src/features/databases/DatabaseModal/DatabaseConnectionForm/EncryptedField.tsx +++ b/superset-frontend/src/features/databases/DatabaseModal/DatabaseConnectionForm/EncryptedField.tsx @@ -16,7 +16,7 @@ * specific language governing permissions and limitations * under the License. */ -import { useState } from 'react'; +import { useRef, useState } from 'react'; import { SupersetTheme, t } from '@superset-ui/core'; import { Button, AntdSelect } from 'src/components'; import InfoTooltip from 'src/components/InfoTooltip'; @@ -46,6 +46,7 @@ export const EncryptedField = ({ db, editNewDb, }: FieldPropTypes) => { + const selectedFileInputRef = useRef<HTMLInputElement | null>(null); const [uploadOption, setUploadOption] = useState<number>( CredentialInfoOptions.JsonUpload.valueOf(), ); @@ -152,9 +153,7 @@ export const EncryptedField = ({ {!fileToUpload && ( <Button className="input-upload-btn" - onClick={() => - document?.getElementById('selectedFile')?.click() - } + onClick={() => selectedFileInputRef.current?.click()} > {t('Choose File')} </Button> @@ -178,6 +177,7 @@ export const EncryptedField = ({ )} <input + ref={selectedFileInputRef} id="selectedFile" accept=".json" className="input-upload" @@ -196,9 +196,9 @@ export const EncryptedField = ({ checked: false, }, }); - ( - document.getElementById('selectedFile') as HTMLInputElement - ).value = null as any; + if (selectedFileInputRef.current) { + selectedFileInputRef.current.value = null as any; + } }} /> </div> diff --git a/superset-frontend/src/features/databases/DatabaseModal/index.test.tsx b/superset-frontend/src/features/databases/DatabaseModal/index.test.tsx index b6b3e208bd..5c109a1c4e 100644 --- a/superset-frontend/src/features/databases/DatabaseModal/index.test.tsx +++ b/superset-frontend/src/features/databases/DatabaseModal/index.test.tsx @@ -21,14 +21,7 @@ import fetchMock from 'fetch-mock'; import userEvent from '@testing-library/user-event'; -import { - render, - screen, - within, - cleanup, - act, - waitFor, -} from 'spec/helpers/testing-library'; +import { render, screen, within, waitFor } from 'spec/helpers/testing-library'; import { getExtensionsRegistry } from '@superset-ui/core'; import setupExtensions from 'src/setup/setupExtensions'; import * as hooks from 'src/views/CRUD/hooks'; @@ -37,6 +30,7 @@ import DatabaseModal, { dbReducer, DBReducerActionType, ActionType, + DatabaseModalProps, } from './index'; jest.mock('@superset-ui/core', () => ({ @@ -64,279 +58,281 @@ const AVAILABLE_DB_ENDPOINT = 'glob:*/api/v1/database/available*'; const VALIDATE_PARAMS_ENDPOINT = 'glob:*/api/v1/database/validate_parameters*'; const DATABASE_CONNECT_ENDPOINT = 'glob:*/api/v1/database/'; -fetchMock.post(DATABASE_CONNECT_ENDPOINT, { - id: 10, - result: { - configuration_method: 'sqlalchemy_form', - database_name: 'Other2', - driver: 'apsw', - expose_in_sqllab: true, - extra: '{"allows_virtual_table_explore":true}', - sqlalchemy_uri: 'gsheets://', - }, - json: 'foo', -}); +const databaseFixture: DatabaseObject = { + id: 123, + backend: 'postgres', + configuration_method: ConfigurationMethod.DynamicForm, + database_name: 'Postgres', + name: 'PostgresDB', + is_managed_externally: false, + driver: 'psycopg2', +}; -fetchMock.config.overwriteRoutes = true; -fetchMock.get(DATABASE_FETCH_ENDPOINT, { - result: { - id: 10, - database_name: 'my database', - expose_in_sqllab: false, - allow_ctas: false, - allow_cvas: false, - configuration_method: 'sqlalchemy_form', - }, -}); -fetchMock.mock(AVAILABLE_DB_ENDPOINT, { - databases: [ - { - available_drivers: ['psycopg2'], - default_driver: 'psycopg2', - engine: 'postgresql', - name: 'PostgreSQL', - parameters: { - properties: { - database: { - description: 'Database name', - type: 'string', - }, - encryption: { - description: 'Use an encrypted connection to the database', - type: 'boolean', - }, - host: { - description: 'Hostname or IP address', - type: 'string', - }, - password: { - description: 'Password', - nullable: true, - type: 'string', - }, - port: { - description: 'Database port', - format: 'int32', - maximum: 65536, - minimum: 0, - type: 'integer', - }, - query: { - additionalProperties: {}, - description: 'Additional parameters', - type: 'object', - }, - ssh: { - description: 'Create SSH Tunnel', - type: 'boolean', - }, - username: { - description: 'Username', - nullable: true, - type: 'string', - }, - }, - required: ['database', 'host', 'port', 'username'], - type: 'object', - }, - preferred: true, - sqlalchemy_uri_placeholder: - 'postgresql://user:password@host:port/dbname[?key=value&key=value...]', - engine_information: { - supports_file_upload: true, - disable_ssh_tunneling: false, +describe('DatabaseModal', () => { + beforeEach(() => { + fetchMock.post(DATABASE_CONNECT_ENDPOINT, { + id: 10, + result: { + configuration_method: 'sqlalchemy_form', + database_name: 'Other2', + driver: 'apsw', + expose_in_sqllab: true, + extra: '{"allows_virtual_table_explore":true}', + sqlalchemy_uri: 'gsheets://', }, - }, - { - available_drivers: ['rest'], - engine: 'presto', - name: 'Presto', - preferred: true, - engine_information: { - supports_file_upload: true, - disable_ssh_tunneling: false, + json: 'foo', + }); + + fetchMock.get(DATABASE_FETCH_ENDPOINT, { + result: { + id: 10, + database_name: 'my database', + expose_in_sqllab: false, + allow_ctas: false, + allow_cvas: false, + configuration_method: 'sqlalchemy_form', }, - }, - { - available_drivers: ['mysqldb'], - default_driver: 'mysqldb', - engine: 'mysql', - name: 'MySQL', - parameters: { - properties: { - database: { - description: 'Database name', - type: 'string', - }, - encryption: { - description: 'Use an encrypted connection to the database', - type: 'boolean', - }, - host: { - description: 'Hostname or IP address', - type: 'string', + }); + fetchMock.mock(AVAILABLE_DB_ENDPOINT, { + databases: [ + { + available_drivers: ['psycopg2'], + default_driver: 'psycopg2', + engine: 'postgresql', + name: 'PostgreSQL', + parameters: { + properties: { + database: { + description: 'Database name', + type: 'string', + }, + encryption: { + description: 'Use an encrypted connection to the database', + type: 'boolean', + }, + host: { + description: 'Hostname or IP address', + type: 'string', + }, + password: { + description: 'Password', + nullable: true, + type: 'string', + }, + port: { + description: 'Database port', + format: 'int32', + maximum: 65536, + minimum: 0, + type: 'integer', + }, + query: { + additionalProperties: {}, + description: 'Additional parameters', + type: 'object', + }, + ssh: { + description: 'Create SSH Tunnel', + type: 'boolean', + }, + username: { + description: 'Username', + nullable: true, + type: 'string', + }, + }, + required: ['database', 'host', 'port', 'username'], + type: 'object', }, - password: { - description: 'Password', - nullable: true, - type: 'string', + preferred: true, + sqlalchemy_uri_placeholder: + 'postgresql://user:password@host:port/dbname[?key=value&key=value...]', + engine_information: { + supports_file_upload: true, + disable_ssh_tunneling: false, }, - port: { - description: 'Database port', - format: 'int32', - maximum: 65536, - minimum: 0, - type: 'integer', + }, + { + available_drivers: ['rest'], + engine: 'presto', + name: 'Presto', + preferred: true, + engine_information: { + supports_file_upload: true, + disable_ssh_tunneling: false, }, - query: { - additionalProperties: {}, - description: 'Additional parameters', + }, + { + available_drivers: ['mysqldb'], + default_driver: 'mysqldb', + engine: 'mysql', + name: 'MySQL', + parameters: { + properties: { + database: { + description: 'Database name', + type: 'string', + }, + encryption: { + description: 'Use an encrypted connection to the database', + type: 'boolean', + }, + host: { + description: 'Hostname or IP address', + type: 'string', + }, + password: { + description: 'Password', + nullable: true, + type: 'string', + }, + port: { + description: 'Database port', + format: 'int32', + maximum: 65536, + minimum: 0, + type: 'integer', + }, + query: { + additionalProperties: {}, + description: 'Additional parameters', + type: 'object', + }, + username: { + description: 'Username', + nullable: true, + type: 'string', + }, + }, + required: ['database', 'host', 'port', 'username'], type: 'object', }, - username: { - description: 'Username', - nullable: true, - type: 'string', + preferred: true, + sqlalchemy_uri_placeholder: + 'mysql://user:password@host:port/dbname[?key=value&key=value...]', + engine_information: { + supports_file_upload: true, + disable_ssh_tunneling: false, }, }, - required: ['database', 'host', 'port', 'username'], - type: 'object', - }, - preferred: true, - sqlalchemy_uri_placeholder: - 'mysql://user:password@host:port/dbname[?key=value&key=value...]', - engine_information: { - supports_file_upload: true, - disable_ssh_tunneling: false, - }, - }, - { - available_drivers: ['pysqlite'], - engine: 'sqlite', - name: 'SQLite', - preferred: true, - engine_information: { - supports_file_upload: true, - disable_ssh_tunneling: false, - }, - }, - { - available_drivers: ['rest'], - engine: 'druid', - name: 'Apache Druid', - preferred: false, - engine_information: { - supports_file_upload: true, - disable_ssh_tunneling: false, - }, - }, - { - available_drivers: ['bigquery'], - default_driver: 'bigquery', - engine: 'bigquery', - name: 'Google BigQuery', - parameters: { - properties: { - credentials_info: { - description: 'Contents of BigQuery JSON credentials.', - type: 'string', - 'x-encrypted-extra': true, - }, - query: { - type: 'object', + { + available_drivers: ['pysqlite'], + engine: 'sqlite', + name: 'SQLite', + preferred: true, + engine_information: { + supports_file_upload: true, + disable_ssh_tunneling: false, }, }, - type: 'object', - }, - preferred: false, - sqlalchemy_uri_placeholder: 'bigquery://{project_id}', - engine_information: { - supports_file_upload: true, - disable_ssh_tunneling: true, - }, - }, - { - available_drivers: ['rest'], - default_driver: 'apsw', - engine: 'gsheets', - name: 'Google Sheets', - preferred: false, - engine_information: { - supports_file_upload: false, - disable_ssh_tunneling: true, - }, - }, - { - available_drivers: ['connector'], - default_driver: 'connector', - engine: 'databricks', - name: 'Databricks', - parameters: { - properties: { - access_token: { - type: 'string', + { + available_drivers: ['rest'], + engine: 'druid', + name: 'Apache Druid', + preferred: false, + engine_information: { + supports_file_upload: true, + disable_ssh_tunneling: false, }, - database: { - type: 'string', + }, + { + available_drivers: ['bigquery'], + default_driver: 'bigquery', + engine: 'bigquery', + name: 'Google BigQuery', + parameters: { + properties: { + credentials_info: { + description: 'Contents of BigQuery JSON credentials.', + type: 'string', + 'x-encrypted-extra': true, + }, + query: { + type: 'object', + }, + }, + type: 'object', }, - host: { - type: 'string', + preferred: false, + sqlalchemy_uri_placeholder: 'bigquery://{project_id}', + engine_information: { + supports_file_upload: true, + disable_ssh_tunneling: true, }, - http_path: { - type: 'string', + }, + { + available_drivers: ['rest'], + default_driver: 'apsw', + engine: 'gsheets', + name: 'Google Sheets', + preferred: false, + engine_information: { + supports_file_upload: false, + disable_ssh_tunneling: true, }, - port: { - format: 'int32', - type: 'integer', + }, + { + available_drivers: ['connector'], + default_driver: 'connector', + engine: 'databricks', + name: 'Databricks', + parameters: { + properties: { + access_token: { + type: 'string', + }, + database: { + type: 'string', + }, + host: { + type: 'string', + }, + http_path: { + type: 'string', + }, + port: { + format: 'int32', + type: 'integer', + }, + }, + required: ['access_token', 'database', 'host', 'http_path', 'port'], + type: 'object', }, + preferred: true, + sqlalchemy_uri_placeholder: + 'databricks+connector://token:{access_token}@{host}:{port}/{database_name}', }, - required: ['access_token', 'database', 'host', 'http_path', 'port'], - type: 'object', - }, - preferred: true, - sqlalchemy_uri_placeholder: - 'databricks+connector://token:{access_token}@{host}:{port}/{database_name}', - }, - ], -}); -fetchMock.post(VALIDATE_PARAMS_ENDPOINT, { - message: 'OK', -}); - -const databaseFixture: DatabaseObject = { - id: 123, - backend: 'postgres', - configuration_method: ConfigurationMethod.DynamicForm, - database_name: 'Postgres', - name: 'PostgresDB', - is_managed_externally: false, - driver: 'psycopg2', -}; - -describe('DatabaseModal', () => { - const renderAndWait = async () => - waitFor(() => - render(<DatabaseModal {...dbProps} />, { - useRedux: true, - }), - ); + ], + }); + fetchMock.post(VALIDATE_PARAMS_ENDPOINT, { + message: 'OK', + }); + }); - beforeEach(async () => { - await renderAndWait(); + beforeEach(() => { + jest.clearAllMocks(); + }); + afterEach(() => { + fetchMock.restore(); }); - afterEach(cleanup); + const setup = (propsOverwrite: Partial<DatabaseModalProps> = {}) => + render(<DatabaseModal {...dbProps} {...propsOverwrite} />, { + useRedux: true, + }); describe('Visual: New database connection', () => { - test('renders the initial load of Step 1 correctly', () => { + test('renders the initial load of Step 1 correctly', async () => { + setup(); + // ---------- Components ---------- // <TabHeader> - AntD header - const closeButton = screen.getByLabelText('Close'); + const closeButton = await screen.findByLabelText('Close'); const step1Header = screen.getByRole('heading', { name: /connect a database/i, }); // <ModalHeader> - Connection header - const step1Helper = screen.getByText(/step 1 of 3/i); + const step1Helper = await screen.findByText(/step 1 of 3/i); const selectDbHeader = screen.getByRole('heading', { name: /select a database to connect/i, }); @@ -376,7 +372,8 @@ describe('DatabaseModal', () => { hidden: true, }); - const footer = document.getElementsByClassName('ant-modal-footer'); + const modal = screen.getByRole('dialog'); + const footer = modal.querySelector('.ant-modal-footer'); // ---------- TODO (lyndsiWilliams): Selector options, can't seem to get these to render properly. // renderAvailableSelector() => <Alert> - Supported databases alert @@ -415,13 +412,15 @@ describe('DatabaseModal', () => { expect(component).toBeInTheDocument(); }); // there should be a footer but it should not have any buttons in it - expect(footer[0]).toBeEmptyDOMElement(); + expect(footer).toBeEmptyDOMElement(); }); test('renders the "Basic" tab of SQL Alchemy form (step 2 of 2) correctly', async () => { + setup(); + // On step 1, click dbButton to access SQL Alchemy form userEvent.click( - screen.getByRole('button', { + await screen.findByRole('button', { name: /sqlite/i, }), ); @@ -429,7 +428,7 @@ describe('DatabaseModal', () => { // ---------- Components ---------- // <TabHeader> - AntD header - const closeButton = screen.getByRole('button', { name: 'Close' }); + const closeButton = await screen.findByRole('button', { name: 'Close' }); const basicHeader = screen.getByRole('heading', { name: /connect a database/i, @@ -462,7 +461,7 @@ describe('DatabaseModal', () => { // <SSHTunnelForm> - Basic tab's SSH Tunnel Form const SSHTunnelingToggle = screen.getByTestId('ssh-tunnel-switch'); userEvent.click(SSHTunnelingToggle); - const SSHTunnelServerAddressInput = screen.getByTestId( + const SSHTunnelServerAddressInput = await screen.findByTestId( 'ssh-tunnel-server_address-input', ); const SSHTunnelServerPortInput = screen.getByTestId( @@ -527,9 +526,11 @@ describe('DatabaseModal', () => { }); test('renders the unexpanded "Advanced" tab correctly', async () => { + setup(); + // On step 1, click dbButton to access step 2 userEvent.click( - screen.getByRole('button', { + await screen.findByRole('button', { name: /sqlite/i, }), ); @@ -626,18 +627,20 @@ describe('DatabaseModal', () => { }); test('renders the "Advanced" - SQL LAB tab correctly (unexpanded)', async () => { + setup(); + // ---------- Components ---------- // On step 1, click dbButton to access step 2 userEvent.click( - screen.getByRole('button', { + await screen.findByRole('button', { name: /sqlite/i, }), ); // Click the "Advanced" tab - userEvent.click(screen.getByRole('tab', { name: /advanced/i })); + userEvent.click(await screen.findByRole('tab', { name: /advanced/i })); // Click the "SQL Lab" tab userEvent.click( - screen.getByRole('tab', { + await screen.findByRole('tab', { name: /right sql lab adjust how this database will interact with sql lab\./i, }), ); @@ -645,7 +648,7 @@ describe('DatabaseModal', () => { // ----- BEGIN STEP 2 (ADVANCED - SQL LAB) // <TabHeader> - AntD header - const closeButton = screen.getByRole('button', { name: /close/i }); + const closeButton = await screen.findByRole('button', { name: /close/i }); const advancedHeader = screen.getByRole('heading', { name: /connect a database/i, }); @@ -788,10 +791,12 @@ describe('DatabaseModal', () => { }); test('renders the "Advanced" - PERFORMANCE tab correctly', async () => { + setup(); + // ---------- Components ---------- // On step 1, click dbButton to access step 2 userEvent.click( - screen.getByRole('button', { + await screen.findByRole('button', { name: /sqlite/i, }), ); @@ -851,10 +856,12 @@ describe('DatabaseModal', () => { }); test('renders the "Advanced" - SECURITY tab correctly', async () => { + setup(); + // ---------- Components ---------- // On step 1, click dbButton to access step 2 userEvent.click( - screen.getByRole('button', { + await screen.findByRole('button', { name: /sqlite/i, }), ); @@ -935,10 +942,12 @@ describe('DatabaseModal', () => { }); it('renders the "Advanced" - SECURITY tab correctly after selecting Allow file uploads', async () => { + setup(); + // ---------- Components ---------- // On step 1, click dbButton to access step 2 userEvent.click( - screen.getByRole('button', { + await screen.findByRole('button', { name: /sqlite/i, }), ); @@ -1021,10 +1030,12 @@ describe('DatabaseModal', () => { }); test('renders the "Advanced" - OTHER tab correctly', async () => { + setup(); + // ---------- Components ---------- // On step 1, click dbButton to access step 2 userEvent.click( - screen.getByRole('button', { + await screen.findByRole('button', { name: /sqlite/i, }), ); @@ -1040,7 +1051,7 @@ describe('DatabaseModal', () => { // ----- BEGIN STEP 2 (ADVANCED - OTHER) // <TabHeader> - AntD header - const closeButton = screen.getByRole('button', { name: /close/i }); + const closeButton = await screen.findByRole('button', { name: /close/i }); const advancedHeader = screen.getByRole('heading', { name: /connect a database/i, }); @@ -1092,10 +1103,12 @@ describe('DatabaseModal', () => { }); test('Dynamic form', async () => { + setup(); + // ---------- Components ---------- // On step 1, click dbButton to access step 2 userEvent.click( - screen.getByRole('button', { + await screen.findByRole('button', { name: /postgresql/i, }), ); @@ -1107,9 +1120,11 @@ describe('DatabaseModal', () => { describe('Functional: Create new database', () => { test('directs databases to the appropriate form (dynamic vs. SQL Alchemy)', async () => { + setup(); + // ---------- Dynamic example (3-step form) // Click the PostgreSQL button to enter the dynamic form - const postgreSQLButton = screen.getByRole('button', { + const postgreSQLButton = await screen.findByRole('button', { name: /postgresql/i, }); userEvent.click(postgreSQLButton); @@ -1139,8 +1154,10 @@ describe('DatabaseModal', () => { describe('SQL Alchemy form flow', () => { test('enters step 2 of 2 when proper database is selected', async () => { + setup(); + userEvent.click( - screen.getByRole('button', { + await screen.findByRole('button', { name: /sqlite/i, }), ); @@ -1167,8 +1184,10 @@ describe('DatabaseModal', () => { describe('step 2 component interaction', () => { test('properly interacts with textboxes', async () => { + setup(); + userEvent.click( - screen.getByRole('button', { + await screen.findByRole('button', { name: /sqlite/i, }), ); @@ -1211,15 +1230,17 @@ describe('DatabaseModal', () => { describe('SSH Tunnel Form interaction', () => { test('properly interacts with SSH Tunnel form textboxes for dynamic form', async () => { + setup(); + userEvent.click( - screen.getByRole('button', { + await screen.findByRole('button', { name: /postgresql/i, }), ); expect(await screen.findByText(/step 2 of 3/i)).toBeInTheDocument(); const SSHTunnelingToggle = screen.getByTestId('ssh-tunnel-switch'); userEvent.click(SSHTunnelingToggle); - const SSHTunnelServerAddressInput = screen.getByTestId( + const SSHTunnelServerAddressInput = await screen.findByTestId( 'ssh-tunnel-server_address-input', ); expect(SSHTunnelServerAddressInput).toHaveValue(''); @@ -1246,8 +1267,10 @@ describe('DatabaseModal', () => { }); test('properly interacts with SSH Tunnel form textboxes', async () => { + setup(); + userEvent.click( - screen.getByRole('button', { + await screen.findByRole('button', { name: /sqlite/i, }), ); @@ -1255,7 +1278,7 @@ describe('DatabaseModal', () => { expect(await screen.findByText(/step 2 of 2/i)).toBeInTheDocument(); const SSHTunnelingToggle = screen.getByTestId('ssh-tunnel-switch'); userEvent.click(SSHTunnelingToggle); - const SSHTunnelServerAddressInput = screen.getByTestId( + const SSHTunnelServerAddressInput = await screen.findByTestId( 'ssh-tunnel-server_address-input', ); expect(SSHTunnelServerAddressInput).toHaveValue(''); @@ -1282,8 +1305,10 @@ describe('DatabaseModal', () => { }); test('if the SSH Tunneling toggle is not true, no inputs are displayed', async () => { + setup(); + userEvent.click( - screen.getByRole('button', { + await screen.findByRole('button', { name: /sqlite/i, }), ); @@ -1310,8 +1335,10 @@ describe('DatabaseModal', () => { }); test('If user changes the login method, the inputs change', async () => { + setup(); + userEvent.click( - screen.getByRole('button', { + await screen.findByRole('button', { name: /sqlite/i, }), ); @@ -1319,7 +1346,7 @@ describe('DatabaseModal', () => { expect(await screen.findByText(/step 2 of 2/i)).toBeInTheDocument(); const SSHTunnelingToggle = screen.getByTestId('ssh-tunnel-switch'); userEvent.click(SSHTunnelingToggle); - const SSHTunnelUsePasswordInput = screen.getByTestId( + const SSHTunnelUsePasswordInput = await screen.findByTestId( 'ssh-tunnel-use_password-radio', ); expect(SSHTunnelUsePasswordInput).toBeInTheDocument(); @@ -1348,6 +1375,8 @@ describe('DatabaseModal', () => { describe('Dynamic form flow', () => { test('enters step 2 of 3 when proper database is selected', async () => { + setup(); + expect(await screen.findByText(/step 1 of 3/i)).toBeInTheDocument(); userEvent.click( screen.getByRole('button', { @@ -1355,14 +1384,13 @@ describe('DatabaseModal', () => { }), ); expect(await screen.findByText(/step 2 of 3/i)).toBeInTheDocument(); - - const step2of3text = screen.getByText(/step 2 of 3/i); - expect(step2of3text).toBeVisible(); }); test('enters form credentials and runs fetchResource when "Connect" is clicked', async () => { + setup(); + userEvent.click( - screen.getByRole('button', { + await screen.findByRole('button', { name: /postgresql/i, }), ); @@ -1403,9 +1431,11 @@ describe('DatabaseModal', () => { describe('Import database flow', () => { test('imports a file', async () => { - const importDbButton = screen.getByTestId( + setup(); + + const importDbButton = (await screen.findByTestId( 'import-database-btn', - ) as HTMLInputElement; + )) as HTMLInputElement; importDbButton.type = 'file'; importDbButton.files = {} as FileList; expect(importDbButton).toBeInTheDocument(); @@ -1423,54 +1453,25 @@ describe('DatabaseModal', () => { }); describe('DatabaseModal w/ Deeplinking Engine', () => { - const renderAndWait = async () => { - const mounted = act(async () => { - render(<DatabaseModal {...dbProps} dbEngine="PostgreSQL" />, { - useRedux: true, - }); - }); - - return mounted; - }; - - beforeEach(async () => { - await renderAndWait(); - }); - - test('enters step 2 of 3 when proper database is selected', () => { - const step2of3text = screen.getByText(/step 2 of 3/i); + test('enters step 2 of 3 when proper database is selected', async () => { + setup({ dbEngine: 'PostgreSQL' }); + const step2of3text = await screen.findByText(/step 2 of 3/i); expect(step2of3text).toBeInTheDocument(); }); }); describe('DatabaseModal w/ GSheet Engine', () => { - const renderAndWait = async () => { - const dbProps = { - show: true, - database_name: 'my database', - sqlalchemy_uri: 'gsheets://', - }; - const mounted = act(async () => { - render(<DatabaseModal {...dbProps} dbEngine="Google Sheets" />, { - useRedux: true, - }); - }); - - return mounted; - }; - - beforeEach(async () => { - await renderAndWait(); - }); - - it('enters step 2 of 2 when proper database is selected', () => { - const step2of2text = screen.getByText(/step 2 of 2/i); + it('enters step 2 of 2 when proper database is selected', async () => { + setup({ dbEngine: 'Google Sheets' }); + const step2of2text = await screen.findByText(/step 2 of 2/i); expect(step2of2text).toBeInTheDocument(); }); it('renders the "Advanced" - SECURITY tab without Allow File Upload Checkbox', async () => { + setup({ dbEngine: 'Google Sheets' }); + // Click the "Advanced" tab - userEvent.click(screen.getByRole('tab', { name: /advanced/i })); + userEvent.click(await screen.findByRole('tab', { name: /advanced/i })); // Click the "Security" tab userEvent.click( screen.getByRole('tab', { @@ -1509,6 +1510,8 @@ describe('DatabaseModal', () => { }); it('if the SSH Tunneling toggle is not displayed, nothing should get displayed', async () => { + setup({ dbEngine: 'Google Sheets' }); + const SSHTunnelingToggle = screen.queryByTestId('ssh-tunnel-switch'); expect(SSHTunnelingToggle).not.toBeInTheDocument(); const SSHTunnelServerAddressInput = screen.queryByTestId( @@ -1536,22 +1539,9 @@ describe('DatabaseModal', () => { useSingleViewResource: jest.fn(), })); - const renderAndWait = async () => { - const mounted = act(async () => { - render(<DatabaseModal {...dbProps} dbEngine="PostgreSQL" />, { - useRedux: true, - }); - }); - - return mounted; - }; - - beforeEach(async () => { - await renderAndWait(); - }); - test('Error displays when it is an object', async () => { - const step2of3text = screen.getByText(/step 2 of 3/i); + setup({ dbEngine: 'PostgreSQL' }); + const step2of3text = await screen.findByText(/step 2 of 3/i); const errorSection = screen.getByText(/Database Creation Error/i); expect(step2of3text).toBeInTheDocument(); expect(errorSection).toBeInTheDocument(); @@ -1581,22 +1571,10 @@ describe('DatabaseModal', () => { setResource: jest.fn(), }); - const renderAndWait = async () => { - const mounted = act(async () => { - render(<DatabaseModal {...dbProps} dbEngine="PostgreSQL" />, { - useRedux: true, - }); - }); - - return mounted; - }; - - beforeEach(async () => { - await renderAndWait(); - }); - test('Error displays when it is a string', async () => { - const step2of3text = screen.getByText(/step 2 of 3/i); + setup({ dbEngine: 'PostgreSQL' }); + + const step2of3text = await screen.findByText(/step 2 of 3/i); const errorTitleMessage = screen.getByText(/Database Creation Error/i); const button = screen.getByText('See more'); userEvent.click(button); @@ -1608,7 +1586,7 @@ describe('DatabaseModal', () => { }); describe('DatabaseModal w Extensions', () => { - const renderAndWait = async () => { + beforeAll(() => { const extensionsRegistry = getExtensionsRegistry(); extensionsRegistry.set('ssh_tunnel.form.switch', () => ( @@ -1616,23 +1594,12 @@ describe('DatabaseModal', () => { )); setupExtensions(); - - const mounted = act(async () => { - render(<DatabaseModal {...dbProps} dbEngine="SQLite" />, { - useRedux: true, - }); - }); - - return mounted; - }; - - beforeEach(async () => { - await renderAndWait(); }); - test('should render an extension component if one is supplied', () => { + test('should render an extension component if one is supplied', async () => { + setup({ dbEngine: 'SQLite' }); expect( - screen.getByText('ssh_tunnel.form.switch extension component'), + await screen.findByText('ssh_tunnel.form.switch extension component'), ).toBeInTheDocument(); }); }); diff --git a/superset-frontend/src/features/databases/DatabaseModal/index.tsx b/superset-frontend/src/features/databases/DatabaseModal/index.tsx index 10ae7a82cc..3a7eb0d21d 100644 --- a/superset-frontend/src/features/databases/DatabaseModal/index.tsx +++ b/superset-frontend/src/features/databases/DatabaseModal/index.tsx @@ -139,7 +139,7 @@ const SSHTunnelContainer = styled.div` `}; `; -interface DatabaseModalProps { +export interface DatabaseModalProps { addDangerToast: (msg: string) => void; addSuccessToast: (msg: string) => void; onDatabaseAdd?: (database?: DatabaseObject) => void; @@ -1335,7 +1335,7 @@ const DatabaseModal: FunctionComponent<DatabaseModalProps> = ({ useEffect(() => { if (importingModal) { document - .getElementsByClassName('ant-upload-list-item-name')[0] + ?.getElementsByClassName('ant-upload-list-item-name')[0] .scrollIntoView(); } }, [importingModal]); diff --git a/superset-frontend/src/features/databases/UploadDataModel/UploadDataModal.test.tsx b/superset-frontend/src/features/databases/UploadDataModel/UploadDataModal.test.tsx index 5a3958e00a..34bebac917 100644 --- a/superset-frontend/src/features/databases/UploadDataModel/UploadDataModal.test.tsx +++ b/superset-frontend/src/features/databases/UploadDataModel/UploadDataModal.test.tsx @@ -622,7 +622,7 @@ test('CSV form post', async () => { // Select a file from the file dialog const file = new File(['test'], 'test.csv', { type: 'text' }); - const inputElement = document.querySelector('input[type="file"]'); + const inputElement = screen.getByTestId('model-file-input'); if (inputElement) { userEvent.upload(inputElement as HTMLElement, file); @@ -680,7 +680,7 @@ test('Excel form post', async () => { // Select a file from the file dialog const file = new File(['test'], 'test.xls', { type: 'text' }); - const inputElement = document.querySelector('input[type="file"]'); + const inputElement = screen.getByTestId('model-file-input'); if (inputElement) { userEvent.upload(inputElement as HTMLElement, file); @@ -738,7 +738,7 @@ test('Columnar form post', async () => { // Select a file from the file dialog const file = new File(['test'], 'test.parquet', { type: 'text' }); - const inputElement = document.querySelector('input[type="file"]'); + const inputElement = screen.getByTestId('model-file-input'); if (inputElement) { userEvent.upload(inputElement as HTMLElement, file); diff --git a/superset-frontend/src/features/datasets/types.ts b/superset-frontend/src/features/datasets/types.ts index 6ac19a7649..e0afb07670 100644 --- a/superset-frontend/src/features/datasets/types.ts +++ b/superset-frontend/src/features/datasets/types.ts @@ -1,4 +1,5 @@ -import { Currency } from '@superset-ui/core'; +import { Currency, type DatasourceType } from '@superset-ui/core'; +import { Owner } from '@superset-ui/chart-controls'; /** * Licensed to the Apache Software Foundation (ASF) under one @@ -32,37 +33,52 @@ export type ColumnObject = { python_date_format?: string; uuid?: string; extra?: string; + certified_by?: string; + certification_details?: string; + warning_markdown?: string; + advanced_data_type?: string; }; type MetricObject = { id: number; + uuid: number; expression?: string; description?: string; metric_name: string; + verbose_name?: string; metric_type: string; d3format?: string; currency?: Currency; warning_text?: string; + certified_by?: string; + certification_details?: string; + warning_markdown?: string; }; export type DatasetObject = { + id: number; table_name?: string; sql?: string; filter_select_enabled?: boolean; fetch_values_predicate?: string; schema?: string; - description?: string; - main_dttm_col?: string; + description: string | null; + main_dttm_col: string; offset?: number; default_endpoint?: string; cache_timeout?: number; is_sqllab_view?: boolean; template_params?: string; - owners: number[]; + owners: Owner[]; columns: ColumnObject[]; metrics: MetricObject[]; extra?: string; is_managed_externally: boolean; normalize_columns: boolean; always_filter_main_dttm: boolean; + type: DatasourceType; + column_formats: Record<string, string>; + currency_formats: Record<string, Currency>; + datasource_name: string | null; + verbose_map: Record<string, string>; };
