sadpandajoe commented on code in PR #43575:
URL: https://github.com/apache/superset/pull/43575#discussion_r3904241531
##########
superset/models/dashboard.py:
##########
@@ -378,43 +378,111 @@ 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":
+ # A tab that cannot be keyed by a string is unusable: it
+ # could not be selected, and it would reach the API as a
+ # tree entry with no ``value``. Keep it out of the tree as
+ # well as out of ``all_tabs``.
+ if child.get("type") == "TAB" and not isinstance(
+ child.get("id"), str
+ ):
+ logger.warning(
+ "Dashboard %s: skipping tab node with no usable id
"
+ "in the layout",
+ self.id,
+ )
+ continue
# if TABS add create a new list and append children to it
# new_children.append(child)
children.append(child)
Review Comment:
An untyped child of a `TABS` node is appended here before the queued node is
rejected at `build_tab_tree`'s type guard. The tabs response then contains a
tree entry without a selectable `value` or `title`, even though malformed
layouts are meant to degrade safely. Should the child be validated before it is
added to `tab_tree`?
--
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]
