bito-code-review[bot] commented on code in PR #38416:
URL: https://github.com/apache/superset/pull/38416#discussion_r2931948374
##########
superset/mcp_service/dashboard/tool/list_dashboards.py:
##########
@@ -108,34 +124,53 @@ def _serialize_dashboard(
"""Serialize dashboard object (field filtering handled by
model_serializer)."""
return serialize_dashboard_object(obj)
- tool = ModelListCore(
- dao_class=DashboardDAO,
- output_schema=DashboardInfo,
- item_serializer=_serialize_dashboard,
- filter_type=DashboardFilter,
- default_columns=DEFAULT_DASHBOARD_COLUMNS,
- search_columns=[
- "dashboard_title",
- "slug",
- "uuid",
- ],
- list_field_name="dashboards",
- output_list_schema=DashboardList,
- all_columns=all_columns,
- sortable_columns=DASHBOARD_SORTABLE_COLUMNS,
- logger=logger,
- )
-
- with event_logger.log_context(action="mcp.list_dashboards.query"):
- result = tool.run_tool(
- filters=request.filters,
- search=request.search,
- select_columns=request.select_columns,
- order_column=request.order_column,
- order_direction=request.order_direction,
- page=max(request.page - 1, 0),
- page_size=request.page_size,
+ # Two-pass approach when sorting by popularity_score
+ if request.order_column == "popularity_score":
+ with
event_logger.log_context(action="mcp.list_dashboards.popularity_sort"):
+ result = _list_dashboards_by_popularity(
+ request, DashboardDAO, _serialize_dashboard, all_columns, ctx
+ )
+ else:
+ list_core = ModelListCore(
+ dao_class=DashboardDAO,
+ output_schema=DashboardInfo,
+ item_serializer=_serialize_dashboard,
+ filter_type=DashboardFilter,
+ default_columns=DEFAULT_DASHBOARD_COLUMNS,
+ search_columns=DASHBOARD_SEARCH_COLUMNS,
+ list_field_name="dashboards",
+ output_list_schema=DashboardList,
+ all_columns=all_columns,
+ sortable_columns=DASHBOARD_SORTABLE_COLUMNS,
+ logger=logger,
)
+
+ # Strip computed fields before passing to DAO query
+ dao_columns = request.select_columns
+ if dao_columns:
+ dao_columns = [c for c in dao_columns if c != "popularity_score"]
+ # Ensure id is loaded when popularity_score was requested
+ # (scores are keyed by id)
+ if "popularity_score" in request.select_columns and "id" not in
dao_columns:
+ dao_columns = ["id"] + dao_columns
+
+ with event_logger.log_context(action="mcp.list_dashboards.query"):
+ result = list_core.run_tool(
+ filters=request.filters,
+ search=request.search,
+ select_columns=dao_columns,
+ order_column=request.order_column,
+ order_direction=request.order_direction,
+ page=max(request.page - 1, 0),
+ page_size=request.page_size,
+ )
+
+ # Attach popularity scores if requested in select_columns
Review Comment:
<!-- Bito Reply -->
Understood. The update ensures that when sorting by popularity_score, the
field is automatically included in columns_requested for the response, even if
not explicitly requested. This lets clients see the sort key. Applied to all
three list tools in the latest commit.
##########
superset/mcp_service/chart/tool/list_charts.py:
##########
@@ -71,7 +92,7 @@ async def list_charts(request: ListChartsRequest, ctx:
Context) -> ChartList:
modified time.
Sortable columns for order_column: id, slice_name, viz_type,
- datasource_name, description, changed_on, created_on
+ datasource_name, description, changed_on, created_on, popularity_score
Review Comment:
<!-- Bito Reply -->
Yes, removing `datasource_name` from the docstring aligns with the updated
code, as it's no longer in the sortable columns list (now
`CHART_SEARCH_COLUMNS`). The change prevents misleading documentation.
**superset/mcp_service/chart/tool/list_charts.py**
```
- "viz_type",
- "description",
+ "viz_type",
+ "datasource_name",
+ "description",
```
--
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]
