fisjac commented on code in PR #30903:
URL: https://github.com/apache/superset/pull/30903#discussion_r1872546306
##########
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>) => ({
Review Comment:
There are some issues with doing this due to a part of the code that checks
for an `is_certified` property on somwhat ambiguous `Metric` `Column` and
`Data` items. I agree that these types should be tightened up, but I don't have
enough context to start tweaking these in this PR.
##########
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>) => ({
Review Comment:
There are some issues with doing this due to a part of the code that checks
for an `is_certified` property on somewhat ambiguous `Metric` `Column` and
`Data` items. I agree that these types should be tightened up, but I don't have
enough context to start tweaking these in this PR.
--
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]
