aminghadersohi commented on code in PR #40957:
URL: https://github.com/apache/superset/pull/40957#discussion_r3493750819
##########
superset/mcp_service/dashboard/schemas.py:
##########
@@ -858,6 +889,20 @@ def sanitize_dashboard_title(cls, v: str | None) -> str |
None:
v, "Dashboard title", max_length=500, allow_empty=True
)
+ @field_validator("slug")
+ @classmethod
+ def normalize_slug(cls, v: str | None) -> str | None:
+ """Normalize the slug to match the REST DashboardPutSchema contract.
+
+ Mirrors ``BaseDashboardSchema.post_load``: strip, replace spaces with
+ hyphens, and drop characters outside ``[\\w-]`` so the tool cannot
+ persist slugs the REST update path would have cleaned.
+ """
+ if not v:
+ return v
+ v = v.strip().replace(" ", "-")
+ return re.sub(r"[^\w\-]+", "", v)
+
Review Comment:
Fixed in a2c740a336. Added a strip-before-check: whitespace-only inputs
(e.g. `" "`) now return `""` (clear slug) instead of raising, matching the REST
`BaseDashboardSchema` behavior. Non-whitespace inputs that normalize to empty
(e.g. `"!!!"`) still raise to prevent silent slot clearing.
##########
superset/mcp_service/dashboard/tool/update_dashboard.py:
##########
@@ -152,57 +189,117 @@ def _apply_field_updates(dashboard: Any, request:
UpdateDashboardRequest) -> lis
dashboard.position_json = json.dumps(request.position_json)
changed.append("position_json")
- if request.json_metadata_overrides is not None:
- dashboard.json_metadata = _merge_json_metadata(
- dashboard, request.json_metadata_overrides
- )
+ metadata_overrides = _collect_metadata_overrides(request)
+ if metadata_overrides:
+ dashboard.json_metadata = _merge_json_metadata(dashboard,
metadata_overrides)
changed.append("json_metadata")
if request.css is not None:
dashboard.css = request.css or None
changed.append("css")
+ if request.tags is not None:
+ # Reuse the same helper the REST UpdateDashboardCommand uses so tag
+ # association semantics (custom-tag full replacement) stay identical.
+ from superset.commands.utils import update_tags
+ from superset.tags.models import ObjectType
+
+ update_tags(ObjectType.dashboard, dashboard.id, dashboard.tags,
request.tags)
+ changed.append("tags")
+
return changed
+def _validate_update_request(
+ dashboard: Any, request: UpdateDashboardRequest
+) -> DashboardError | None:
+ """Pre-flight validation mirroring the REST update path.
+
+ Runs before any mutation so the tool rejects the same payloads the REST
+ ``DashboardPutSchema`` / ``UpdateDashboardCommand`` would — invalid CSS,
+ conflicting metadata keys, and unauthorized or unknown tag IDs — returning
a
+ structured error instead of failing deep inside the commit.
+ """
+ from marshmallow import ValidationError as MarshmallowValidationError
+
+ from superset.commands.exceptions import (
+ TagForbiddenError,
+ TagNotFoundValidationError,
+ )
+ from superset.commands.utils import validate_tags
+ from superset.dashboards.schemas import validate_css
+ from superset.tags.models import ObjectType
+
+ # Empty string clears CSS (no validation needed); only validate real
content.
+ if request.css:
+ try:
+ validate_css(request.css)
+ except MarshmallowValidationError as ex:
+ detail = (
+ "; ".join(str(m) for m in ex.messages)
+ if isinstance(ex.messages, list)
+ else str(ex.messages)
+ )
+ return DashboardError(
+ error=f"Dashboard CSS is invalid: {detail}",
+ error_type="InvalidCSS",
+ )
+
+ try:
+ _collect_metadata_overrides(request)
+ except ValueError as ex:
+ return DashboardError(error=str(ex), error_type="InvalidRequest")
+
+ if request.tags is not None:
+ try:
+ validate_tags(ObjectType.dashboard, dashboard.tags, request.tags)
+ except TagForbiddenError as ex:
+ return DashboardError(error=str(ex), error_type="TagForbidden")
+ except TagNotFoundValidationError as ex:
+ return DashboardError(error=str(ex), error_type="TagNotFound")
Review Comment:
Already handled: `validate_tags` runs inside its own `except
SQLAlchemyError` block at lines 259–264 of `_validate_update_request`, which
catches any DB error from tag lookups and returns a structured `DashboardError`
rather than propagating.
--
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]
