bito-code-review[bot] commented on code in PR #41606:
URL: https://github.com/apache/superset/pull/41606#discussion_r3589206379
##########
superset/mcp_service/dashboard/schemas.py:
##########
@@ -1062,6 +1062,312 @@ class UpdateDashboardResponse(BaseModel):
)
+class ManageDashboardOwnersRequest(BaseModel):
+ """Request schema for explicit add/remove dashboard owner management.
+
+ Unlike ``update_dashboard``'s dropped ``owners`` field (a full-replacement
+ list with no safety guard, so an empty or partial list could silently
+ orphan a dashboard), this tool takes explicit add/remove operations and
+ rejects any change that would leave the dashboard with zero owners.
+
+ "Owners" here means USER-type entries in the dashboard's Subject-based
+ ``editors`` list (the ownership model apache/superset#38831 introduced,
+ replacing the legacy ``owners`` relationship). Any ROLE- or GROUP-type
+ editors already on the dashboard are left untouched by this tool.
+ """
+
+ identifier: int | str = Field(
+ ...,
+ description=(
+ "Dashboard ID (integer), UUID, or slug. Same identifier shape "
+ "accepted by ``get_dashboard_info``."
+ ),
+ )
+ add_owner_ids: List[int] = Field(
+ default_factory=list,
+ description=(
+ "User IDs to add as dashboard owners. Discover IDs with
``find_users``."
+ ),
+ )
+ remove_owner_ids: List[int] = Field(
+ default_factory=list,
+ description=(
+ "User IDs to remove from dashboard owners. Rejected if it would "
+ "leave the dashboard with zero owners, or if an ID is not "
+ "currently an owner."
+ ),
+ )
+
+ @model_validator(mode="after")
+ def _validate_operations(self) -> "ManageDashboardOwnersRequest":
+ if not self.add_owner_ids and not self.remove_owner_ids:
+ raise ValueError(
+ "At least one of add_owner_ids or remove_owner_ids is
required."
+ )
+ overlap: list[int] = sorted(
+ set(self.add_owner_ids) & set(self.remove_owner_ids)
+ )
+ if overlap:
+ raise ValueError(
+ "User IDs cannot appear in both add_owner_ids and "
+ f"remove_owner_ids: {overlap}."
+ )
+ return self
+
+
+class ManageDashboardOwnersResponse(BaseModel):
+ """Response schema for ``manage_dashboard_owners``."""
+
+ owners: List[SubjectInfo] = Field(
Review Comment:
<!-- Bito Reply -->
Acknowledged. Since the fix is already present in the codebase and verified,
no further action is required for this review comment.
##########
superset/mcp_service/dashboard/schemas.py:
##########
@@ -1062,6 +1062,312 @@ class UpdateDashboardResponse(BaseModel):
)
+class ManageDashboardOwnersRequest(BaseModel):
+ """Request schema for explicit add/remove dashboard owner management.
+
+ Unlike ``update_dashboard``'s dropped ``owners`` field (a full-replacement
+ list with no safety guard, so an empty or partial list could silently
+ orphan a dashboard), this tool takes explicit add/remove operations and
+ rejects any change that would leave the dashboard with zero owners.
+
+ "Owners" here means USER-type entries in the dashboard's Subject-based
+ ``editors`` list (the ownership model apache/superset#38831 introduced,
+ replacing the legacy ``owners`` relationship). Any ROLE- or GROUP-type
+ editors already on the dashboard are left untouched by this tool.
+ """
+
+ identifier: int | str = Field(
+ ...,
+ description=(
+ "Dashboard ID (integer), UUID, or slug. Same identifier shape "
+ "accepted by ``get_dashboard_info``."
+ ),
+ )
+ add_owner_ids: List[int] = Field(
+ default_factory=list,
+ description=(
+ "User IDs to add as dashboard owners. Discover IDs with
``find_users``."
+ ),
+ )
+ remove_owner_ids: List[int] = Field(
+ default_factory=list,
+ description=(
+ "User IDs to remove from dashboard owners. Rejected if it would "
+ "leave the dashboard with zero owners, or if an ID is not "
+ "currently an owner."
+ ),
+ )
+
+ @model_validator(mode="after")
+ def _validate_operations(self) -> "ManageDashboardOwnersRequest":
+ if not self.add_owner_ids and not self.remove_owner_ids:
+ raise ValueError(
+ "At least one of add_owner_ids or remove_owner_ids is
required."
+ )
+ overlap: list[int] = sorted(
+ set(self.add_owner_ids) & set(self.remove_owner_ids)
+ )
+ if overlap:
+ raise ValueError(
+ "User IDs cannot appear in both add_owner_ids and "
+ f"remove_owner_ids: {overlap}."
+ )
+ return self
+
+
+class ManageDashboardOwnersResponse(BaseModel):
+ """Response schema for ``manage_dashboard_owners``."""
+
+ owners: List[SubjectInfo] = Field(
+ default_factory=list,
+ description=(
+ "Full list of USER-type editor subjects (dashboard owners) "
+ "after the operation. Any ROLE/GROUP-type editors on the "
+ "dashboard are not included here."
+ ),
+ )
+ dashboard_url: str | None = Field(None, description="URL to view the
dashboard")
+ added_owner_ids: List[int] = Field(
+ default_factory=list,
+ description="User IDs actually added as owners by this call.",
+ )
+ removed_owner_ids: List[int] = Field(
+ default_factory=list,
+ description="User IDs actually removed from owners by this call.",
+ )
+ warnings: List[str] = Field(
+ default_factory=list,
+ description=(
+ "Non-fatal advisory messages, e.g. that a non-admin caller was "
+ "automatically re-added as owner after trying to remove "
+ "themselves."
+ ),
+ )
Review Comment:
<!-- Bito Reply -->
Acknowledged. Since the issue has been addressed in a previous commit and
verified against the current state, no further action is required for this
suggestion.
--
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]
