aminghadersohi commented on code in PR #37970:
URL: https://github.com/apache/superset/pull/37970#discussion_r2835376613
##########
superset/mcp_service/dashboard/tool/generate_dashboard.py:
##########
@@ -39,13 +40,24 @@
logger = logging.getLogger(__name__)
+# Match frontend defaults from
superset-frontend/src/dashboard/util/constants.ts
+GRID_DEFAULT_CHART_WIDTH = 4
Review Comment:
Good call! Extracted `GRID_DEFAULT_CHART_WIDTH`, `GRID_COLUMN_COUNT`, and
`generate_id()` into `superset/mcp_service/dashboard/constants.py` and both
tools now import from there.
##########
superset/mcp_service/dashboard/tool/add_chart_to_existing_dashboard.py:
##########
@@ -40,38 +45,89 @@
logger = logging.getLogger(__name__)
-def _find_next_row_position(layout: Dict[str, Any]) -> int:
+def _find_next_row_position(layout: Dict[str, Any]) -> str:
"""
- Find the next available row position in the dashboard layout.
+ Generate a unique ROW ID for a new row in the dashboard layout.
+
+ Uses UUID-based IDs (e.g. ``ROW-a1b2c3d4``) instead of numeric indices
+ so that the IDs are compatible with real dashboard layouts that use
+ nanoid-style identifiers.
Returns:
- Row index for the new chart
+ A new unique ROW ID string.
+ """
+ row_key = generate_id("ROW")
+ # Ensure uniqueness (extremely unlikely collision, but safe)
+ while row_key in layout:
+ row_key = generate_id("ROW")
+ return row_key
+
+
+def _find_tab_insert_target(layout: Dict[str, Any]) -> str | None:
"""
- # Find existing rows
- row_indices = []
- for key in layout.keys():
- if key.startswith("ROW-") and key[4:].isdigit():
- row_indices.append(int(key[4:]))
+ Detect if the dashboard uses tabs and return the first tab's ID.
- # Return next available row index
- return max(row_indices) + 1 if row_indices else 0
+ If ``GRID_ID`` has children that are ``TABS`` components, this walks
+ into the first ``TAB`` child so that new rows are placed inside the
+ active tab rather than directly under GRID_ID.
+
+ Returns:
+ The ID of the first TAB component, or ``None`` if the dashboard
+ does not use top-level tabs.
+ """
+ grid = layout.get("GRID_ID")
+ if not grid:
+ return None
+
+ for child_id in grid.get("children", []):
+ child = layout.get(child_id)
+ if child and child.get("type") == "TABS":
+ # Found a TABS component; use its first TAB child
+ tabs_children = child.get("children", [])
+ if tabs_children:
+ first_tab_id = tabs_children[0]
+ first_tab = layout.get(first_tab_id)
+ if first_tab and first_tab.get("type") == "TAB":
+ return first_tab_id
+ return None
def _add_chart_to_layout(
- layout: Dict[str, Any], chart: Any, chart_id: int, row_index: int
-) -> tuple[str, str]:
+ layout: Dict[str, Any],
+ chart: Any,
+ chart_id: int,
+ row_key: str,
+ parent_id: str,
+) -> tuple[str, str, str]:
"""
- Add chart and row components to the dashboard layout.
+ Add chart, column, and row components to the dashboard layout.
+
+ Creates the proper ``ROW > COLUMN > CHART`` hierarchy that the
+ frontend expects for rendering.
+
+ Args:
+ layout: The mutable layout dict to update.
+ chart: The chart ORM object.
+ chart_id: The chart's integer ID.
+ row_key: The pre-generated ROW component ID.
+ parent_id: The parent container ID (GRID_ID or a TAB ID).
Returns:
- Tuple of (chart_key, row_key)
+ Tuple of ``(chart_key, column_key, row_key)``.
"""
chart_key = f"CHART-{chart_id}"
- row_key = f"ROW-{row_index}"
- chart_width = 5 # Balanced width for good proportions
+ column_key = generate_id("COLUMN")
+ chart_width = GRID_DEFAULT_CHART_WIDTH
chart_height = 50 # Good height for most chart types
- # Add chart to layout using proper Superset structure
+ # Build the parents chain up to the parent container
+ parent_component = layout.get(parent_id, {})
+ parent_parents = parent_component.get("parents", [])
Review Comment:
Good catch. Fixed - when `GRID_ID` doesn't exist yet (empty layout), we now
default `parent_parents` to `["ROOT_ID"]` so the parents chain is consistent
with what `_ensure_layout_structure` creates. Added test assertion to verify.
--
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]
