tien238lnd commented on code in PR #43575:
URL: https://github.com/apache/superset/pull/43575#discussion_r3912317807
##########
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):
Review Comment:
You are right, and this one is worse than a shape inconsistency — it is a
regression I introduced. Before this PR those layouts raised, so the request
failed and the promise rejected. Returning `{}` makes the request succeed with
a payload that has neither key, and `AlertReportModal.tsx:1104` destructures
`tab_tree` and calls `.map` on it with no guard, so the picker throws instead
of showing an empty list. (`SaveModal.tsx:270` checks `Array.isArray` first, so
it was unaffected.)
Every exit now returns `{"all_tabs": {}, "tab_tree": []}`.
One thing worth flagging: I also changed the **empty-layout** exit, which
returned a bare `{}` on master too. It is outside what you asked about, but it
reaches `AlertReportModal` through the same path and would fail the same way,
and leaving one exit returning a different shape from the others seemed worse
than fixing it. Happy to split that back out if you would rather keep this PR
strictly to the paths it introduced.
Tests cover both: the tree no longer accepts a node without `value`/`title`,
and each exit is asserted against the full payload shape. All four new
assertions fail against the previous commit.
--
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]
