sadpandajoe commented on code in PR #44148:
URL: https://github.com/apache/superset/pull/44148#discussion_r4191551064
##########
superset/mcp_service/chart/chart_utils.py:
##########
@@ -2630,3 +2648,128 @@ def preserve_previous_adhoc_filters(
merged_filters.append(generated_filter)
new_form_data["adhoc_filters"] = merged_filters
+
+
+def _apply_geographic_temporal_update(
+ form_data: dict[str, Any], config: ChartConfig
+) -> None:
+ """Apply explicit time changes after native controls have been merged."""
+ if "temporal_column" not in config.model_fields_set:
+ return
+ # Native granularity takes precedence over the requested adhoc binding.
+ form_data.pop("granularity_sqla", None)
+ if config.temporal_column is None:
+ form_data["adhoc_filters"] =
_without_generated_dashboard_time_filter(form_data)
+ form_data.pop(MCP_DASHBOARD_TIME_FILTER_SUBJECT, None)
+
+
+def merge_geographic_update_form_data( # noqa: C901
+ existing: dict[str, Any],
+ mapped: dict[str, Any],
+ config: ChartConfig,
+ *,
+ dataset_rebind: bool,
+) -> dict[str, Any]:
+ """Preserve omitted native controls; clear explicit nullable roles and
filters.
+
+ Rebinding drops old dataset references and Jinja/query state. All optional
+ roles are remapped from the complete target-dataset config.
+ """
+ if dataset_rebind:
+ existing = {
+ key: value
+ for key, value in existing.items()
+ if key
+ in {
+ "linear_color_scheme",
+ "number_format",
+ "max_bubble_size",
+ "point_unit",
+ "color_by",
+ "color_picker",
+ "color_scheme",
+ "y_axis_format",
+ "map_renderer",
+ "maplibre_style",
+ "mapbox_style",
+ "min_radius",
+ "max_radius",
+ "multiplier",
+ }
+ }
+ field_map = {
+ "country": "select_country",
+ "region_format": "region_format",
+ "country_format": "country_fieldtype",
+ "entity": "entity",
+ "metric": "metric",
+ "secondary_metric": "secondary_metric",
+ "dimension": "dimension",
+ "show_bubbles": "show_bubbles",
+ "max_bubble_size": "max_bubble_size",
+ "sort_by_metric": "sort_by_metric",
+ "linear_color_scheme": "linear_color_scheme",
+ "number_format": "number_format",
+ "row_limit": "row_limit",
+ "time_range": "time_range",
+ "point_unit": "point_unit",
+ }
+ fields = config.model_fields_set
+ patch = dict(mapped)
+ if "temporal_column" in fields and config.temporal_column is None:
+ patch["adhoc_filters"] =
_without_generated_dashboard_time_filter(patch)
+ elif "temporal_column" not in fields and not dataset_rebind:
+ # Omission keeps the saved dashboard-time binding instead of remapping
+ # it to the dataset default the mapper generated for this update.
+ # An absent marker also preserves an explicitly cleared binding.
+ saved_binding = [
+ filter_
+ for filter_ in existing.get("adhoc_filters") or []
+ if filter_ not in
_without_generated_dashboard_time_filter(existing)
+ ]
+ patch["adhoc_filters"] = [
+ *saved_binding,
+ *_without_generated_dashboard_time_filter(patch),
+ ]
+ if binding := existing.get(MCP_DASHBOARD_TIME_FILTER_SUBJECT):
+ patch[MCP_DASHBOARD_TIME_FILTER_SUBJECT] = binding
+ else:
+ patch.pop(MCP_DASHBOARD_TIME_FILTER_SUBJECT, None)
+ # Native UI-only presentation controls are not part of this public config.
+ for key in (
+ "color_by",
+ "color_picker",
+ "color_scheme",
+ "y_axis_format",
+ "map_renderer",
+ "maplibre_style",
+ "mapbox_style",
+ "viewport",
+ "autozoom",
+ "min_radius",
+ "max_radius",
+ "multiplier",
+ ):
+ if key in existing:
+ patch.pop(key, None)
+ for source, target in field_map.items():
+ if source not in fields and target in existing:
+ patch.pop(target, None)
+ if not {"radius", "radius_metric"} & fields and "point_radius_fixed" in
existing:
+ patch.pop("point_radius_fixed", None)
+ if "filters" not in fields:
+ source_form = dict(existing)
+ if "temporal_column" in fields:
+ source_form["adhoc_filters"] =
_without_generated_dashboard_time_filter(
+ existing
+ )
+ preserve_previous_adhoc_filters(patch, source_form)
+ merged = {**existing, **patch}
Review Comment:
An update that omits `dimension` keeps the saved one in the merged form
data, and the coordinate-collision check only runs on a `dimension` the caller
sends. So a saved Scatter with latitude `lat`, longitude `lon`, dimension
`category`, updated with latitude `category`, compiles and saves, but the
frontend strips that coordinate from the feature properties and the categorical
filter then drops every point. Should the merged Scatter roles be revalidated
here (or in a Scatter merged-form-data check) so an inherited dimension can't
alias a replacement coordinate?
--
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]
