guilleov commented on code in PR #42002:
URL: https://github.com/apache/superset/pull/42002#discussion_r3578221936


##########
superset/mcp_service/dashboard/tool/update_dashboard.py:
##########
@@ -203,6 +227,14 @@ def _apply_field_updates(dashboard: Any, request: 
UpdateDashboardRequest) -> lis
         update_tags(ObjectType.dashboard, dashboard.id, dashboard.tags, 
request.tags)
         changed.append("tags")
 
+    if request.owners is not None:
+        # Full replacement of owners (empty list clears them). IDs are
+        # validated up front in _validate_update_request, so resolving here
+        # only maps the already-verified IDs to user objects.
+        users, _missing = _resolve_owners(request.owners)

Review Comment:
   Addressed in 6be4201: annotated the unpacking as `owner_users: list[Any]`.



##########
superset/mcp_service/dashboard/tool/update_dashboard.py:
##########
@@ -260,6 +292,20 @@ def _validate_update_request(
                 error_type="DatabaseError",
             )
 
+    # A non-empty owners list must reference existing users. An empty list is
+    # a valid "clear all owners" request and needs no lookup.
+    if request.owners:
+        _users, missing = _resolve_owners(request.owners)

Review Comment:
   Addressed in 6be4201: annotated `missing_owner_ids: list[int]` on the 
validation unpacking.



##########
superset/mcp_service/dashboard/tool/update_dashboard.py:
##########
@@ -260,6 +292,20 @@ def _validate_update_request(
                 error_type="DatabaseError",
             )
 
+    # A non-empty owners list must reference existing users. An empty list is
+    # a valid "clear all owners" request and needs no lookup.
+    if request.owners:
+        _users, missing = _resolve_owners(request.owners)
+        if missing:
+            return DashboardError(
+                error=(
+                    "Unknown owner user IDs: "
+                    + ", ".join(str(m) for m in missing)
+                    + ". Find valid IDs with list_users."
+                ),
+                error_type="OwnersNotFound",
+            )

Review Comment:
   Addressed in 6be4201: owner resolution in validation is now wrapped in 
`except SQLAlchemyError`, returning a structured `DatabaseError` like the tag 
validation path.



-- 
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]

Reply via email to