aminghadersohi commented on code in PR #40473:
URL: https://github.com/apache/superset/pull/40473#discussion_r3328082870
##########
superset/mcp_service/dashboard/schemas.py:
##########
@@ -288,6 +288,33 @@ def validate_search_and_filters(self) ->
"ListDashboardsRequest":
return self
+DEFAULT_GET_DASHBOARD_INFO_COLUMNS: List[str] = [
+ "id",
+ "dashboard_title",
+ "slug",
+ "description",
+ "certified_by",
+ "certification_details",
+ "published",
+ "is_managed_externally",
+ "external_url",
+ "created_on",
+ "changed_on",
+ "uuid",
+ "url",
+ "created_on_humanized",
+ "changed_on_humanized",
+ "chart_count",
+ "tags",
+ "charts",
+ "native_filters",
+ "cross_filters_enabled",
+ "is_permalink_state",
+ "permalink_key",
+ "filter_state",
Review Comment:
Fixed in the follow-up commit 3c917c7f99. The unsaved-chart path now calls
`result.model_dump(mode="json", context={"select_columns":
request.select_columns})` — same pattern as the saved-chart path.
##########
tests/unit_tests/mcp_service/chart/tool/test_get_chart_info.py:
##########
@@ -365,7 +365,8 @@ async def
test_restricted_user_redacts_saved_chart_data_model_fields(
assert result["datasource_name"] is None
assert result["datasource_type"] is None
assert result["filters"] is None
- assert result["form_data"] is None
+ # form_data is excluded from default select_columns, so it won't be in
result
+ assert "form_data" not in result
Review Comment:
Added a new test `test_unsaved_chart_select_columns_filters_response` in
3c917c7f99 that verifies: (1) default select_columns excludes form_data from
the unsaved-chart response, and (2) explicitly passing
`select_columns=["viz_type", "form_data"]` includes it.
##########
superset/mcp_service/dashboard/schemas.py:
##########
@@ -288,6 +288,33 @@ def validate_search_and_filters(self) ->
"ListDashboardsRequest":
return self
+DEFAULT_GET_DASHBOARD_INFO_COLUMNS: List[str] = [
+ "id",
+ "dashboard_title",
+ "slug",
+ "description",
+ "certified_by",
+ "certification_details",
+ "published",
+ "is_managed_externally",
+ "external_url",
+ "created_on",
+ "changed_on",
+ "uuid",
+ "url",
+ "created_on_humanized",
+ "changed_on_humanized",
+ "chart_count",
+ "tags",
+ "charts",
+ "native_filters",
+ "cross_filters_enabled",
+ "is_permalink_state",
+ "permalink_key",
+ "filter_state",
Review Comment:
The `filter_state` and `permalink_key` fields are not included in the
current `DEFAULT_GET_DASHBOARD_INFO_COLUMNS` list — the docstring and defaults
are consistent. The copilot reviewed an earlier version of the diff; the
current HEAD already has the correct lean set.
--
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]
