codeant-ai-for-open-source[bot] commented on code in PR #40399:
URL: https://github.com/apache/superset/pull/40399#discussion_r3467023754


##########
tests/unit_tests/mcp_service/dashboard/tool/test_update_dashboard.py:
##########
@@ -0,0 +1,311 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+"""Tests for the update_dashboard MCP tool."""
+
+from datetime import datetime
+from unittest.mock import Mock, patch
+
+import pytest
+from fastmcp import Client
+
+from superset.mcp_service.app import mcp
+from superset.utils import json
+
+
[email protected]
+def mcp_server():
+    return mcp
+
+
[email protected](autouse=True)
+def mock_auth():
+    """Mock authentication for all tests in this module."""
+    with patch("superset.mcp_service.auth.get_user_from_request") as 
mock_get_user:
+        with patch("superset.security_manager.raise_for_ownership"):
+            mock_user = Mock()
+            mock_user.id = 1
+            mock_user.username = "admin"
+            mock_get_user.return_value = mock_user
+            yield mock_get_user
+
+
+def _mock_dashboard(
+    id: int = 42,
+    title: str = "Test Dashboard",
+    slug: str | None = "test-slug",
+    published: bool = True,
+    css: str | None = None,
+    json_metadata: str | None = None,
+    position_json: str | None = None,
+):

Review Comment:
   **Suggestion:** Add an explicit return type for this helper function to keep 
new function signatures fully typed. [custom_rule]
   
   **Severity Level:** Minor ⚠️
   <details>
   <summary><b>Why it matters? 🤔 </b></summary>
   
   The helper `_mock_dashboard` has typed parameters but no return type 
annotation. Since this is newly added Python code, it should be fully typed, so 
the suggestion is accurate.
   </details>
   
   [![Fix in 
Cursor](https://new-codeant-butcket.s3.us-west-1.amazonaws.com/badges/fix-in-cursor-flat.svg)](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=00f6acb662c148399a787870acc8c67f&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
 [![Fix in VSCode 
Claude](https://new-codeant-butcket.s3.us-west-1.amazonaws.com/badges/fix-in-vscode-claude-flat.svg)](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=00f6acb662c148399a787870acc8c67f&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
   
   *(Use Cmd/Ctrl + Click for best experience)*
   <details>
   <summary><b>Prompt for AI Agent 🤖 </b></summary>
   
   ```mdx
   This is a comment left during a code review.
   
   **Path:** 
tests/unit_tests/mcp_service/dashboard/tool/test_update_dashboard.py
   **Line:** 47:55
   **Comment:**
        *Custom Rule: Add an explicit return type for this helper function to 
keep new function signatures fully typed.
   
   Validate the correctness of the flagged issue. If correct, How can I resolve 
this? If you propose a fix, implement it and please make it concise.
   Once fix is implemented, also check other comments on the same PR, and ask 
user if the user wants to fix the rest of the comments as well. if said yes, 
then fetch all the comments validate the correctness and implement a minimal fix
   ```
   </details>
   <a 
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40399&comment_hash=09827fb72a022a3d8c9da6147854646dbd7236421255d2a375267ba1da1e3a85&reaction=like'>👍</a>
 | <a 
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40399&comment_hash=09827fb72a022a3d8c9da6147854646dbd7236421255d2a375267ba1da1e3a85&reaction=dislike'>👎</a>



##########
tests/unit_tests/mcp_service/dashboard/test_dashboard_schemas.py:
##########
@@ -625,3 +627,212 @@ def 
test_client_warnings_discarded_even_when_server_also_warns(self) -> None:
         assert len(req.sanitization_warnings) == 1
         assert "dashboard_title" in req.sanitization_warnings[0]
         assert "injected" not in req.sanitization_warnings[0]
+
+
+class TestGenerateDashboardRequestLayoutTheme:
+    """generate_dashboard accepts optional position_json, theme overrides, 
CSS."""
+
+    def test_layout_theme_css_fields_default_to_none(self) -> None:
+        req = GenerateDashboardRequest(chart_ids=[1])
+        assert req.position_json is None
+        assert req.json_metadata_overrides is None
+        assert req.css is None
+        assert req.slug is None
+
+    def test_position_json_accepted(self) -> None:
+        position = {
+            "ROOT_ID": {"children": ["GRID_ID"], "type": "ROOT"},
+            "GRID_ID": {"children": ["ROW-1"], "type": "GRID"},
+        }
+        req = GenerateDashboardRequest(chart_ids=[1], position_json=position)
+        assert req.position_json == position
+
+    def test_json_metadata_overrides_accepted(self) -> None:
+        overrides = {
+            "label_colors": {"Electronics": "#4C78A8"},
+            "cross_filters_enabled": False,
+        }
+        req = GenerateDashboardRequest(chart_ids=[1], 
json_metadata_overrides=overrides)
+        assert req.json_metadata_overrides == overrides
+
+    def test_css_accepted(self) -> None:
+        req = GenerateDashboardRequest(
+            chart_ids=[1], css=".header-controls{display:none}"
+        )
+        assert req.css == ".header-controls{display:none}"
+
+    def test_slug_accepted(self) -> None:
+        req = GenerateDashboardRequest(chart_ids=[1], slug="my-dashboard")
+        assert req.slug == "my-dashboard"
+
+    def test_css_max_length_enforced(self) -> None:
+        with pytest.raises(ValidationError, match="at most 50000"):
+            GenerateDashboardRequest(chart_ids=[1], css="x" * 50001)
+
+    def test_title_alias_accepted(self) -> None:
+        """``title`` is one of the AliasChoices for ``dashboard_title``
+        — JSON callers using either name resolve to the same field."""
+        req = GenerateDashboardRequest(chart_ids=[1], title="Q4 Review")
+        assert req.dashboard_title == "Q4 Review"
+
+    def test_name_alias_accepted(self) -> None:
+        """``name`` is the third AliasChoice for ``dashboard_title``
+        and must resolve identically to ``title`` and ``dashboard_title``."""
+        req = GenerateDashboardRequest(chart_ids=[1], name="Q4 Review")
+        assert req.dashboard_title == "Q4 Review"
+
+    def test_published_defaults_to_false(self) -> None:
+        """``published`` defaults to False — newly generated dashboards
+        are drafts by default to avoid accidentally publishing partial
+        work-in-progress to all users."""
+        req = GenerateDashboardRequest(chart_ids=[1])
+        assert req.published is False
+
+    def test_published_true_accepted(self) -> None:
+        """An explicit ``published=True`` is preserved."""
+        req = GenerateDashboardRequest(chart_ids=[1], published=True)
+        assert req.published is True
+
+
+class TestUpdateDashboardRequest:
+    """Schema validation for update_dashboard's request."""
+
+    def test_identifier_required(self) -> None:

Review Comment:
   **Suggestion:** Add a concise docstring describing the validation scenario 
covered by this new test. [custom_rule]
   
   **Severity Level:** Minor ⚠️
   <details>
   <summary><b>Why it matters? 🤔 </b></summary>
   
   This is a newly introduced test function without a docstring. That directly 
matches the rule requiring docstrings for newly added Python functions.
   </details>
   
   [![Fix in 
Cursor](https://new-codeant-butcket.s3.us-west-1.amazonaws.com/badges/fix-in-cursor-flat.svg)](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=fd63eb5639d242a6af438218ed094bc0&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
 [![Fix in VSCode 
Claude](https://new-codeant-butcket.s3.us-west-1.amazonaws.com/badges/fix-in-vscode-claude-flat.svg)](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=fd63eb5639d242a6af438218ed094bc0&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
   
   *(Use Cmd/Ctrl + Click for best experience)*
   <details>
   <summary><b>Prompt for AI Agent 🤖 </b></summary>
   
   ```mdx
   This is a comment left during a code review.
   
   **Path:** tests/unit_tests/mcp_service/dashboard/test_dashboard_schemas.py
   **Line:** 700:700
   **Comment:**
        *Custom Rule: Add a concise docstring describing the validation 
scenario covered by this new test.
   
   Validate the correctness of the flagged issue. If correct, How can I resolve 
this? If you propose a fix, implement it and please make it concise.
   Once fix is implemented, also check other comments on the same PR, and ask 
user if the user wants to fix the rest of the comments as well. if said yes, 
then fetch all the comments validate the correctness and implement a minimal fix
   ```
   </details>
   <a 
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40399&comment_hash=bc542ee10d79566893ca745133faec2e4eb0f7627e83c7c6f80a2965118af2f5&reaction=like'>👍</a>
 | <a 
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40399&comment_hash=bc542ee10d79566893ca745133faec2e4eb0f7627e83c7c6f80a2965118af2f5&reaction=dislike'>👎</a>



##########
tests/unit_tests/mcp_service/dashboard/test_dashboard_schemas.py:
##########
@@ -87,8 +89,8 @@ def _mock_dashboard(
 class TestSerializeDashboardObject:
     """Tests for serialize_dashboard_object slug handling."""
 
-    @patch("superset.mcp_service.utils.url_utils.get_superset_base_url")
-    def test_slug_none_returns_empty_string(self, mock_base_url):
+    @patch("superset.mcp_service.dashboard.schemas.get_superset_base_url")
+    def test_slug_none_returns_empty_string(self, mock_base_url) -> None:

Review Comment:
   **Suggestion:** Add an explicit type annotation for the patched mock 
argument so the new test method is fully typed. [custom_rule]
   
   **Severity Level:** Minor ⚠️
   <details>
   <summary><b>Why it matters? 🤔 </b></summary>
   
   This is a newly added Python test method, and its patched argument 
`mock_base_url` is not type annotated. The rule requires new or modified Python 
functions/methods to be fully typed, so this is a real violation.
   </details>
   
   [![Fix in 
Cursor](https://new-codeant-butcket.s3.us-west-1.amazonaws.com/badges/fix-in-cursor-flat.svg)](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=1c0396879518401f9545c578edee991f&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
 [![Fix in VSCode 
Claude](https://new-codeant-butcket.s3.us-west-1.amazonaws.com/badges/fix-in-vscode-claude-flat.svg)](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=1c0396879518401f9545c578edee991f&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
   
   *(Use Cmd/Ctrl + Click for best experience)*
   <details>
   <summary><b>Prompt for AI Agent 🤖 </b></summary>
   
   ```mdx
   This is a comment left during a code review.
   
   **Path:** tests/unit_tests/mcp_service/dashboard/test_dashboard_schemas.py
   **Line:** 93:93
   **Comment:**
        *Custom Rule: Add an explicit type annotation for the patched mock 
argument so the new test method is fully typed.
   
   Validate the correctness of the flagged issue. If correct, How can I resolve 
this? If you propose a fix, implement it and please make it concise.
   Once fix is implemented, also check other comments on the same PR, and ask 
user if the user wants to fix the rest of the comments as well. if said yes, 
then fetch all the comments validate the correctness and implement a minimal fix
   ```
   </details>
   <a 
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40399&comment_hash=2d4588e9f5e2bf68dd4cbe57d583f8763b3d251f5315ab028b608d0c471f7d31&reaction=like'>👍</a>
 | <a 
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40399&comment_hash=2d4588e9f5e2bf68dd4cbe57d583f8763b3d251f5315ab028b608d0c471f7d31&reaction=dislike'>👎</a>



##########
tests/unit_tests/mcp_service/dashboard/test_dashboard_schemas.py:
##########
@@ -625,3 +627,212 @@ def 
test_client_warnings_discarded_even_when_server_also_warns(self) -> None:
         assert len(req.sanitization_warnings) == 1
         assert "dashboard_title" in req.sanitization_warnings[0]
         assert "injected" not in req.sanitization_warnings[0]
+
+
+class TestGenerateDashboardRequestLayoutTheme:
+    """generate_dashboard accepts optional position_json, theme overrides, 
CSS."""
+
+    def test_layout_theme_css_fields_default_to_none(self) -> None:
+        req = GenerateDashboardRequest(chart_ids=[1])
+        assert req.position_json is None
+        assert req.json_metadata_overrides is None
+        assert req.css is None
+        assert req.slug is None
+
+    def test_position_json_accepted(self) -> None:
+        position = {
+            "ROOT_ID": {"children": ["GRID_ID"], "type": "ROOT"},
+            "GRID_ID": {"children": ["ROW-1"], "type": "GRID"},
+        }
+        req = GenerateDashboardRequest(chart_ids=[1], position_json=position)
+        assert req.position_json == position
+
+    def test_json_metadata_overrides_accepted(self) -> None:
+        overrides = {
+            "label_colors": {"Electronics": "#4C78A8"},
+            "cross_filters_enabled": False,
+        }
+        req = GenerateDashboardRequest(chart_ids=[1], 
json_metadata_overrides=overrides)
+        assert req.json_metadata_overrides == overrides
+
+    def test_css_accepted(self) -> None:
+        req = GenerateDashboardRequest(
+            chart_ids=[1], css=".header-controls{display:none}"
+        )
+        assert req.css == ".header-controls{display:none}"
+
+    def test_slug_accepted(self) -> None:
+        req = GenerateDashboardRequest(chart_ids=[1], slug="my-dashboard")
+        assert req.slug == "my-dashboard"
+
+    def test_css_max_length_enforced(self) -> None:
+        with pytest.raises(ValidationError, match="at most 50000"):
+            GenerateDashboardRequest(chart_ids=[1], css="x" * 50001)
+
+    def test_title_alias_accepted(self) -> None:
+        """``title`` is one of the AliasChoices for ``dashboard_title``
+        — JSON callers using either name resolve to the same field."""
+        req = GenerateDashboardRequest(chart_ids=[1], title="Q4 Review")
+        assert req.dashboard_title == "Q4 Review"
+
+    def test_name_alias_accepted(self) -> None:
+        """``name`` is the third AliasChoice for ``dashboard_title``
+        and must resolve identically to ``title`` and ``dashboard_title``."""
+        req = GenerateDashboardRequest(chart_ids=[1], name="Q4 Review")
+        assert req.dashboard_title == "Q4 Review"
+
+    def test_published_defaults_to_false(self) -> None:
+        """``published`` defaults to False — newly generated dashboards
+        are drafts by default to avoid accidentally publishing partial
+        work-in-progress to all users."""
+        req = GenerateDashboardRequest(chart_ids=[1])
+        assert req.published is False
+
+    def test_published_true_accepted(self) -> None:
+        """An explicit ``published=True`` is preserved."""
+        req = GenerateDashboardRequest(chart_ids=[1], published=True)
+        assert req.published is True
+
+
+class TestUpdateDashboardRequest:
+    """Schema validation for update_dashboard's request."""
+
+    def test_identifier_required(self) -> None:
+        with pytest.raises(ValidationError, match="Field required"):
+            UpdateDashboardRequest()
+
+    def test_int_identifier_accepted(self) -> None:
+        req = UpdateDashboardRequest(identifier=42)
+        assert req.identifier == 42
+
+    def test_string_identifier_accepted(self) -> None:
+        req = UpdateDashboardRequest(identifier="my-slug")
+        assert req.identifier == "my-slug"
+
+    def test_all_optional_fields_default_to_none(self) -> None:
+        req = UpdateDashboardRequest(identifier=1)
+        assert req.dashboard_title is None
+        assert req.description is None
+        assert req.slug is None
+        assert req.published is None
+        assert req.position_json is None
+        assert req.json_metadata_overrides is None
+        assert req.css is None
+
+    def test_position_json_and_overrides_and_css(self) -> None:
+        req = UpdateDashboardRequest(
+            identifier=42,
+            position_json={"ROOT_ID": {"type": "ROOT"}},
+            json_metadata_overrides={"cross_filters_enabled": True},
+            css=".x{}",
+        )
+        assert req.position_json == {"ROOT_ID": {"type": "ROOT"}}
+        assert req.json_metadata_overrides == {"cross_filters_enabled": True}
+        assert req.css == ".x{}"
+
+    def test_title_alias_accepted(self) -> None:
+        """`title` is accepted as an alias for `dashboard_title`."""
+        req = UpdateDashboardRequest(identifier=1, title="New Title")
+        assert req.dashboard_title == "New Title"
+
+    def test_name_alias_accepted(self) -> None:
+        """`name` is accepted as an alias for `dashboard_title` — mirrors
+        ``GenerateDashboardRequest``'s third AliasChoice so callers can
+        use the same key name on both create and update paths."""
+        req = UpdateDashboardRequest(identifier=1, name="New Title")
+        assert req.dashboard_title == "New Title"
+
+    def test_css_max_length_enforced(self) -> None:
+        with pytest.raises(ValidationError, match="at most 50000"):
+            UpdateDashboardRequest(identifier=1, css="x" * 50001)
+
+    def test_title_partial_strip_emits_warning(self) -> None:
+        """Mirror of ``test_title_partial_strip_emits_warning`` on the
+        create path — sanitization removes the HTML, the title survives,
+        and a warning records that the input was altered."""
+        req = UpdateDashboardRequest(identifier=1, dashboard_title="Q1 
<b>Review</b>")
+        assert req.dashboard_title == "Q1 Review"
+        assert len(req.sanitization_warnings) == 1
+        assert "dashboard_title" in req.sanitization_warnings[0]
+
+    def test_client_supplied_warnings_are_discarded(self) -> None:
+        """``sanitization_warnings`` is server-only on the update path
+        too — caller-supplied entries are dropped so an attacker cannot
+        smuggle warning text through the response."""
+        req = UpdateDashboardRequest(
+            identifier=1,
+            dashboard_title="Clean Title",
+            sanitization_warnings=["<script>injected</script>"],
+        )
+        assert req.sanitization_warnings == []
+
+    def test_title_xss_only_rejected_at_schema_level(self) -> None:
+        """An XSS-only title is rejected by the Pydantic validator
+        before the tool ever runs — matches the create path's guard."""
+        with pytest.raises(ValidationError, match="removed entirely"):
+            UpdateDashboardRequest(
+                identifier=1,
+                dashboard_title="<script>alert(1)</script>",
+            )
+
+    def test_title_alias_xss_rejected(self) -> None:
+        """The ``title`` alias resolves to the same sanitized field, so
+        XSS-only input supplied via the alias must be rejected with the
+        same guard. Otherwise an attacker could bypass sanitization just
+        by choosing a different request key."""
+        with pytest.raises(ValidationError, match="removed entirely"):
+            UpdateDashboardRequest(
+                identifier=1,
+                title="<script>alert(1)</script>",
+            )
+
+    def test_name_alias_xss_rejected(self) -> None:
+        """Same as ``test_title_alias_xss_rejected`` for the ``name``
+        AliasChoice — every alias funnels through the same validator,
+        not just the canonical field name."""
+        with pytest.raises(ValidationError, match="removed entirely"):
+            UpdateDashboardRequest(
+                identifier=1,
+                name="<script>alert(1)</script>",
+            )
+
+
+class TestSafeUserLabel:
+    """``_safe_user_label`` defensively coerces ``*_by_name`` attributes
+    so dashboard serialization never leaks a ``repr(user)`` or trips
+    Pydantic with a non-string value."""
+
+    def test_plain_string_passes_through(self) -> None:

Review Comment:
   **Suggestion:** Add a docstring explaining the expectation verified by this 
newly added test method. [custom_rule]
   
   **Severity Level:** Minor ⚠️
   <details>
   <summary><b>Why it matters? 🤔 </b></summary>
   
   This newly added test method has no docstring, so it violates the custom 
rule that new Python functions and classes must be documented inline.
   </details>
   
   [![Fix in 
Cursor](https://new-codeant-butcket.s3.us-west-1.amazonaws.com/badges/fix-in-cursor-flat.svg)](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=9b77332aab484653ae80a8ec89cdbe01&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
 [![Fix in VSCode 
Claude](https://new-codeant-butcket.s3.us-west-1.amazonaws.com/badges/fix-in-vscode-claude-flat.svg)](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=9b77332aab484653ae80a8ec89cdbe01&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
   
   *(Use Cmd/Ctrl + Click for best experience)*
   <details>
   <summary><b>Prompt for AI Agent 🤖 </b></summary>
   
   ```mdx
   This is a comment left during a code review.
   
   **Path:** tests/unit_tests/mcp_service/dashboard/test_dashboard_schemas.py
   **Line:** 805:805
   **Comment:**
        *Custom Rule: Add a docstring explaining the expectation verified by 
this newly added test method.
   
   Validate the correctness of the flagged issue. If correct, How can I resolve 
this? If you propose a fix, implement it and please make it concise.
   Once fix is implemented, also check other comments on the same PR, and ask 
user if the user wants to fix the rest of the comments as well. if said yes, 
then fetch all the comments validate the correctness and implement a minimal fix
   ```
   </details>
   <a 
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40399&comment_hash=79f85bac624dd323c33ca4c8acf5aea7d0d062f5ff80ac1203ab9a93f452e272&reaction=like'>👍</a>
 | <a 
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40399&comment_hash=79f85bac624dd323c33ca4c8acf5aea7d0d062f5ff80ac1203ab9a93f452e272&reaction=dislike'>👎</a>



##########
tests/unit_tests/mcp_service/dashboard/tool/test_update_dashboard.py:
##########
@@ -0,0 +1,311 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+"""Tests for the update_dashboard MCP tool."""
+
+from datetime import datetime
+from unittest.mock import Mock, patch
+
+import pytest
+from fastmcp import Client
+
+from superset.mcp_service.app import mcp
+from superset.utils import json
+
+
[email protected]
+def mcp_server():
+    return mcp
+
+
[email protected](autouse=True)
+def mock_auth():
+    """Mock authentication for all tests in this module."""
+    with patch("superset.mcp_service.auth.get_user_from_request") as 
mock_get_user:
+        with patch("superset.security_manager.raise_for_ownership"):
+            mock_user = Mock()
+            mock_user.id = 1
+            mock_user.username = "admin"
+            mock_get_user.return_value = mock_user
+            yield mock_get_user
+
+
+def _mock_dashboard(
+    id: int = 42,
+    title: str = "Test Dashboard",
+    slug: str | None = "test-slug",
+    published: bool = True,
+    css: str | None = None,
+    json_metadata: str | None = None,
+    position_json: str | None = None,
+):
+    """Build a Mock with EVERY field the DashboardInfo serializer touches
+    explicitly set. Without this, Mock returns auto-Mock objects for
+    unset attributes, which Pydantic rejects as wrong-type."""
+    dashboard = Mock()
+    dashboard.id = id
+    dashboard.dashboard_title = title
+    dashboard.slug = slug
+    dashboard.description = "desc"
+    dashboard.published = published
+    dashboard.css = css
+    dashboard.json_metadata = json_metadata or json.dumps({"label_colors": {}})
+    dashboard.position_json = position_json
+    dashboard.certified_by = None
+    dashboard.certification_details = None
+    dashboard.is_managed_externally = False
+    dashboard.external_url = None
+    dashboard.created_on = datetime(2024, 1, 1)
+    dashboard.changed_on = datetime(2024, 1, 1)
+    dashboard.created_by = Mock(username="admin")
+    dashboard.changed_by = Mock(username="admin")
+    dashboard.created_by_name = "admin"
+    dashboard.changed_by_name = "admin"
+    dashboard.created_on_humanized = "a day ago"
+    dashboard.changed_on_humanized = "a day ago"
+    dashboard.uuid = f"dashboard-uuid-{id}"
+    dashboard.slices = []
+    dashboard.owners = []
+    dashboard.tags = []
+    return dashboard
+
+
+class TestUpdateDashboard:
+    """update_dashboard patches existing dashboard layout/theme/CSS."""
+
+    @patch("superset.daos.dashboard.DashboardDAO.get_by_id_or_slug")
+    @patch("superset.extensions.db.session")
+    @pytest.mark.asyncio
+    async def test_update_layout_theme_and_css(
+        self, mock_session, mock_get, mcp_server
+    ) -> None:

Review Comment:
   **Suggestion:** Add type annotations for all parameters in this new test 
method signature (including mocked dependencies and fixtures) to satisfy the 
full typing rule. [custom_rule]
   
   **Severity Level:** Minor ⚠️
   <details>
   <summary><b>Why it matters? 🤔 </b></summary>
   
   This new async test method includes untyped parameters `mock_session`, 
`mock_get`, and `mcp_server`. The rule requires new Python functions and 
methods to be fully typed, so this is a real violation.
   </details>
   
   [![Fix in 
Cursor](https://new-codeant-butcket.s3.us-west-1.amazonaws.com/badges/fix-in-cursor-flat.svg)](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=51a49165c4b3401ab9b9a6dd16e129b8&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
 [![Fix in VSCode 
Claude](https://new-codeant-butcket.s3.us-west-1.amazonaws.com/badges/fix-in-vscode-claude-flat.svg)](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=51a49165c4b3401ab9b9a6dd16e129b8&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
   
   *(Use Cmd/Ctrl + Click for best experience)*
   <details>
   <summary><b>Prompt for AI Agent 🤖 </b></summary>
   
   ```mdx
   This is a comment left during a code review.
   
   **Path:** 
tests/unit_tests/mcp_service/dashboard/tool/test_update_dashboard.py
   **Line:** 93:95
   **Comment:**
        *Custom Rule: Add type annotations for all parameters in this new test 
method signature (including mocked dependencies and fixtures) to satisfy the 
full typing rule.
   
   Validate the correctness of the flagged issue. If correct, How can I resolve 
this? If you propose a fix, implement it and please make it concise.
   Once fix is implemented, also check other comments on the same PR, and ask 
user if the user wants to fix the rest of the comments as well. if said yes, 
then fetch all the comments validate the correctness and implement a minimal fix
   ```
   </details>
   <a 
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40399&comment_hash=9576dada0936c94009d25be9b1d9652a322b37d7aee9773c715edb4e648c853b&reaction=like'>👍</a>
 | <a 
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40399&comment_hash=9576dada0936c94009d25be9b1d9652a322b37d7aee9773c715edb4e648c853b&reaction=dislike'>👎</a>



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