geido commented on code in PR #30903:
URL: https://github.com/apache/superset/pull/30903#discussion_r1860896735
##########
superset-frontend/src/components/Datasource/DatasourceModal.tsx:
##########
@@ -110,124 +122,139 @@ 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: 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: datasource?.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: 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());
+ const { error: clientError, statusText } =
+ await getClientErrorObject(error);
+ addDangerToast(
Review Comment:
Do we need to print the actual error to the user? Can we control the error
for a more user friendly output?
##########
superset-frontend/src/components/Datasource/DatasourceEditor.jsx:
##########
@@ -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, ' +
Review Comment:
Let's localize this
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
