aminghadersohi commented on code in PR #39922:
URL: https://github.com/apache/superset/pull/39922#discussion_r3482746701
##########
superset/mcp_service/chart/schemas.py:
##########
@@ -102,7 +105,15 @@ class ChartInfo(BaseModel):
id: int | None = Field(None, description="Chart ID")
slice_name: str | None = Field(None, description="Chart name")
- viz_type: str | None = Field(None, description="Visualization type")
+ viz_type: str | None = Field(None, description="Visualization type
(internal ID)")
+ chart_type_display_name: str | None = Field(
+ None,
+ description=(
+ "User-friendly chart type name (e.g. 'Line Chart', 'Pivot Table').
"
+ "Prefer this field when referring to chart types; "
+ "fall back to viz_type when this field is null."
+ ),
+ )
Review Comment:
`list_charts` fetches rows directly from the database;
`chart_type_display_name` is a computed field (resolved via the plugin
registry, not a DB column) so it cannot appear in the SQL projection. The field
is available in `get_chart_info`, which calls `serialize_chart_object` and
performs the per-chart registry lookup. This is intentional — the list endpoint
avoids per-row registry lookups to stay fast.
##########
superset/mcp_service/chart/schemas.py:
##########
@@ -561,11 +572,27 @@ def serialize_chart_object(chart: ChartLike | None) ->
ChartInfo | None:
# Extract structured filter information
filters_info = extract_filters_from_form_data(chart_form_data)
+ _viz_type = getattr(chart, "viz_type", None)
+ _display_name = None
+ if _viz_type:
+ try:
+ from superset.mcp_service.chart.registry import
display_name_for_viz_type
+ except ImportError:
+ pass
+ else:
+ try:
+ _display_name = display_name_for_viz_type(_viz_type)
+ except Exception as exc: # noqa: BLE001 — third-party plugins may
raise
+ logger.debug(
+ "Failed to resolve display name for viz_type=%r: %s",
_viz_type, exc
+ )
Review Comment:
Fixed in `429e3987df`. `_apply_unsaved_state_override` now recomputes
`result.chart_type_display_name` via `display_name_for_viz_type` immediately
after overriding `result.viz_type` from the cached form_data, keeping both
fields in sync.
--
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]
