sadpandajoe commented on code in PR #43575:
URL: https://github.com/apache/superset/pull/43575#discussion_r3876697601
##########
superset/models/dashboard.py:
##########
@@ -378,43 +378,98 @@ def position(self) -> dict[str, Any]:
return {}
@property
- def tabs(self) -> dict[str, Any]:
+ def tabs(self) -> dict[str, Any]: # noqa: C901
+ if not isinstance(self.position, dict):
+ logger.warning("Dashboard %s: layout is not a mapping", self.id)
+ return {}
if self.position == {}:
return {}
- def get_node(node_id: str) -> dict[str, Any]:
+ def get_node(node_id: str) -> Optional[dict[str, Any]]:
"""
Helper function for getting a node from the position_data
"""
- return self.position[node_id]
+ return self.position.get(node_id)
+
+ def register_tab(node: dict[str, Any]) -> None:
+ """
+ Helper function for titling a TAB node and adding it to all_tabs
+ """
+ meta = node.get("meta")
+ title = meta.get("text") if isinstance(meta, dict) else None
+ if not isinstance(title, str):
+ logger.warning(
+ "Dashboard %s: tab node %s has no title in the layout",
+ self.id,
+ node.get("id"),
+ )
+ title = ""
+ node["title"] = title
+ node_id = node.get("id")
+ if not isinstance(node_id, str):
+ logger.warning(
+ "Dashboard %s: skipping tab node with no usable id in the
layout",
+ self.id,
+ )
+ return
+ node["value"] = node_id
+ all_tabs[node_id] = title
def build_tab_tree(
node: dict[str, Any], children: list[dict[str, Any]]
) -> None:
"""
Function for building the tab tree structure and list of all tabs
"""
-
+ if "type" not in node:
+ logger.warning(
+ "Dashboard %s: skipping untyped layout node %s",
+ self.id,
+ node.get("id"),
+ )
+ return
+
+ # A node whose type is not one of the four below is walked through
+ # without contributing to the tree, exactly as an untabbed layout
+ # element always has been.
+ node_type = node["type"]
+ child_ids = node.get("children", [])
+ if not isinstance(child_ids, list):
+ logger.warning(
+ "Dashboard %s: layout node %s has malformed children",
+ self.id,
+ node.get("id"),
+ )
+ child_ids = []
new_children: list[dict[str, Any]] = []
# new children to overwrite parent's children
- for child_id in node.get("children", []):
- child = get_node(child_id)
- if node["type"] == "TABS":
+ for child_id in child_ids:
+ child = get_node(child_id) if isinstance(child_id, str) else
None
+ if not isinstance(child, dict):
+ logger.warning(
+ "Dashboard %s: skipping layout node %s, missing or
malformed",
+ self.id,
+ child_id,
+ )
+ continue
+ if node_type == "TABS":
# if TABS add create a new list and append children to it
# new_children.append(child)
children.append(child)
Review Comment:
The non-string-ID guard still puts the node into `tab_tree` before
`register_tab` rejects it. A stored TAB with `id: []` therefore produces an
entry with no string `value`, so `/tabs` violates `TabSchema` and the tab
picker cannot select it. Should invalid tabs be excluded from the tree as well?
--
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]
