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 9370c8d1dc80a..c7c1ac870812c 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; + normalize_columns?: boolean; + always_filter_main_dttm?: boolean; } export interface ControlPanelState { diff --git a/superset-frontend/src/components/Datasource/DatasourceEditor.jsx b/superset-frontend/src/components/Datasource/DatasourceEditor.jsx index d30b67ad2a751..f3708466ce82e 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 { @@ -694,116 +694,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 +1057,12 @@ class DatasourceEditor extends PureComponent { maxLines={Infinity} readOnly={!this.state.isEditMode} resize="both" + tooltipOptions={{ + title: + 'If changes are made to your SQL query, ' + + 'columns in your dataset will be synced when saving the dataset.', + placement: 'topRight', + }} /> } /> diff --git a/superset-frontend/src/components/Datasource/DatasourceModal.tsx b/superset-frontend/src/components/Datasource/DatasourceModal.tsx index a6f812786f2e4..5370e97a01799 100644 --- a/superset-frontend/src/components/Datasource/DatasourceModal.tsx +++ b/superset-frontend/src/components/Datasource/DatasourceModal.tsx @@ -17,6 +17,7 @@ * 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 { @@ -35,7 +36,15 @@ 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'; const DatasourceEditor = AsyncEsmComponent(() => import('./DatasourceEditor')); @@ -62,6 +71,7 @@ const StyledDatasourceModal = styled(Modal)` interface DatasourceModalProps { addSuccessToast: (msg: string) => void; + addDangerToast: (msg: string) => void; datasource: any; onChange: () => {}; onDatasourceSave: (datasource: object, errors?: Array) => {}; @@ -85,11 +95,13 @@ function buildExtraJsonObject(item: Record) { const DatasourceModal: FunctionComponent = ({ addSuccessToast, + addDangerToast, datasource, onDatasourceSave, onHide, show, }) => { + const dispatch = useDispatch(); const [currentDatasource, setCurrentDatasource] = useState({ ...datasource, metrics: datasource?.metrics?.map((metric: Metric) => ({ @@ -110,124 +122,139 @@ const DatasourceModal: FunctionComponent = ({ const [isEditing, setIsEditing] = useState(false); const dialog = useRef(null); const [modal, contextHolder] = Modal.useModal(); - - const onConfirmSave = () => { + const buildPayload = (datasource: Record) => ({ + 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: Record) => { + 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: Record) => ({ + 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) => 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) => { - 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) => ({ - 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) => 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: ( - - ), - }); + 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()); + const { error: clientError, statusText } = + await getClientErrorObject(error); + addDangerToast( + clientError || statusText || t('An error has occurred'), + ); + } + 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: ( + + ), }); + } }; const onDatasourceChange = (data: Record, err: Array) => { diff --git a/superset-frontend/src/components/Datasource/utils.js b/superset-frontend/src/components/Datasource/utils.js index ccdb1b414a092..a87573c1e7b21 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, t } from '@superset-ui/core'; +import rison from 'rison'; export function recurseReactClone(children, type, propExtender) { /** @@ -40,3 +43,86 @@ 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( + t('Modified columns: %s', columnChanges.modified.join(', ')), + ); + } + if (columnChanges.removed.length) { + addSuccessToast(t('Removed columns: %s', columnChanges.removed.join(', '))); + } + if (columnChanges.added.length) { + addSuccessToast(t('New columns added: %s', columnChanges.added.join(', '))); + } + 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 dd1db852a8f83..3d15fd620f432 100644 --- a/superset-frontend/src/components/Tooltip/index.tsx +++ b/superset-frontend/src/components/Tooltip/index.tsx @@ -16,12 +16,10 @@ * specific language governing permissions and limitations * under the License. */ -import { useTheme } from '@superset-ui/core'; +import { useTheme, css } from '@superset-ui/core'; import { Tooltip as AntdTooltip } from 'antd'; -import { - TooltipProps, - TooltipPlacement as AntdTooltipPlacement, -} from 'antd/lib/tooltip'; +import { Global } from '@emotion/react'; +import { TooltipProps, TooltipPlacement } from 'antd/lib/tooltip'; export { TooltipProps, TooltipPlacement }; @@ -29,6 +27,18 @@ export const Tooltip = (props: TooltipProps) => { const theme = useTheme(); return ( <> + {/* Safari hack to hide browser default tooltips */} +