codeant-ai-for-open-source[bot] commented on code in PR #41472:
URL: https://github.com/apache/superset/pull/41472#discussion_r3485210923
##########
superset/mcp_service/dashboard/schemas.py:
##########
@@ -1528,3 +1528,30 @@ def dashboard_layout_serializer(dashboard: "Dashboard")
-> DashboardLayout:
has_layout=bool(position_json_str),
)
)
+
+
+class DeleteDashboardRequest(BaseModel):
+ """Request schema for delete_dashboard."""
+
+ identifier: int | str = Field(
+ ...,
+ description="Dashboard identifier - numeric ID, UUID string, or slug.",
+ )
+
+
+class DeleteDashboardResponse(BaseModel):
+ """Result of a delete_dashboard operation."""
+
+ success: bool = Field(description="Whether the dashboard was deleted")
+ deleted_id: int | None = Field(None, description="ID of the deleted
dashboard")
Review Comment:
**Suggestion:** Replace the integer deletion identifier in the response with
a UUID-based public identifier field to prevent exposing internal IDs.
[custom_rule]
**Severity Level:** Minor ⚠️
<details>
<summary><b>Why it matters? 🤔 </b></summary>
This is a real exposure of an internal integer ID in a public API response.
The rule explicitly flags public API identifiers that expose internal integer
IDs when UUID-based identifiers are being used, so this violation is present.
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=367654c72fec44e9a18c969a00481c3f&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
[](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=367654c72fec44e9a18c969a00481c3f&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:** superset/mcp_service/dashboard/schemas.py
**Line:** 1546:1546
**Comment:**
*Custom Rule: Replace the integer deletion identifier in the response
with a UUID-based public identifier field to prevent exposing internal IDs.
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%2F41472&comment_hash=2fe682a917c2c3f1e36cc2875e4ba91651e76fb61d5cf61884412877b2639ee8&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41472&comment_hash=2fe682a917c2c3f1e36cc2875e4ba91651e76fb61d5cf61884412877b2639ee8&reaction=dislike'>👎</a>
##########
tests/unit_tests/mcp_service/chart/tool/test_delete_chart.py:
##########
@@ -0,0 +1,131 @@
+# 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.
+
+"""Unit tests for the delete_chart MCP tool.
+
+Run through the async MCP Client (not direct calls); auth is mocked via the
+autouse mock_auth fixture, matching the other chart tool test files.
+"""
+
+from unittest.mock import Mock, patch
+
+import pytest
+from fastmcp import Client
+
+from superset.mcp_service.app import mcp
+
+
[email protected]
+def mcp_server() -> object:
+ return mcp
+
+
[email protected](autouse=True)
+def mock_auth():
+ with patch("superset.mcp_service.auth.get_user_from_request") as
mock_get_user:
+ mock_user = Mock()
+ mock_user.id = 1
+ mock_user.username = "admin"
+ mock_get_user.return_value = mock_user
+ yield mock_get_user
+
+
+def _mock_chart(chart_id: int = 10, slice_name: str = "Test Chart") -> Mock:
+ chart = Mock()
+ chart.id = chart_id
+ chart.slice_name = slice_name
+ return chart
+
+
+@patch("superset.mcp_service.chart.tool.delete_chart.find_chart_by_identifier")
[email protected]
+async def test_delete_chart_not_found(mock_find: Mock, mcp_server: object) ->
None:
+ mock_find.return_value = None
+
+ async with Client(mcp_server) as client:
+ result = await client.call_tool(
+ "delete_chart", {"request": {"identifier": 999}}
+ )
+
+ content = result.structured_content
+ assert content["success"] is False
+ assert content["error_type"] == "NotFound"
+ assert "999" in (content["error"] or "")
Review Comment:
**Suggestion:** Add a docstring to this newly added async test function.
[custom_rule]
**Severity Level:** Minor ⚠️
<details>
<summary><b>Why it matters? 🤔 </b></summary>
The test function is newly added and has no docstring. The custom rule
applies to new Python functions broadly, including test functions.
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=c2023ee0d5444bcbbf52b28f64ead90d&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
[](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=c2023ee0d5444bcbbf52b28f64ead90d&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/chart/tool/test_delete_chart.py
**Line:** 56:67
**Comment:**
*Custom Rule: Add a docstring to this newly added async test function.
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%2F41472&comment_hash=082ae1d19d140943c7abb89e703b8bf38fcc04853b2816408a3d3c8b9e006829&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41472&comment_hash=082ae1d19d140943c7abb89e703b8bf38fcc04853b2816408a3d3c8b9e006829&reaction=dislike'>👎</a>
##########
tests/unit_tests/mcp_service/chart/tool/test_delete_chart.py:
##########
@@ -0,0 +1,131 @@
+# 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.
+
+"""Unit tests for the delete_chart MCP tool.
+
+Run through the async MCP Client (not direct calls); auth is mocked via the
+autouse mock_auth fixture, matching the other chart tool test files.
+"""
+
+from unittest.mock import Mock, patch
+
+import pytest
+from fastmcp import Client
+
+from superset.mcp_service.app import mcp
+
+
[email protected]
+def mcp_server() -> object:
+ return mcp
+
+
[email protected](autouse=True)
+def mock_auth():
+ with patch("superset.mcp_service.auth.get_user_from_request") as
mock_get_user:
+ mock_user = Mock()
+ mock_user.id = 1
+ mock_user.username = "admin"
+ mock_get_user.return_value = mock_user
+ yield mock_get_user
Review Comment:
**Suggestion:** Add an explicit return type annotation to this fixture so
the new function is fully typed. [custom_rule]
**Severity Level:** Minor ⚠️
<details>
<summary><b>Why it matters? 🤔 </b></summary>
The rule requires newly added Python functions to be fully typed. This
fixture has parameterless definition `def mock_auth():` with no return type
annotation, so it violates the typing requirement.
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=74df72b276664f5aaf86958a1c163640&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
[](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=74df72b276664f5aaf86958a1c163640&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/chart/tool/test_delete_chart.py
**Line:** 37:44
**Comment:**
*Custom Rule: Add an explicit return type annotation to this fixture so
the new function 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%2F41472&comment_hash=d98b5194cbbbacea2943a8e918ab39d820b1aa3a3e4f10b964a544dfcbc8857a&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41472&comment_hash=d98b5194cbbbacea2943a8e918ab39d820b1aa3a3e4f10b964a544dfcbc8857a&reaction=dislike'>👎</a>
##########
tests/unit_tests/mcp_service/chart/tool/test_delete_chart.py:
##########
@@ -0,0 +1,131 @@
+# 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.
+
+"""Unit tests for the delete_chart MCP tool.
+
+Run through the async MCP Client (not direct calls); auth is mocked via the
+autouse mock_auth fixture, matching the other chart tool test files.
+"""
+
+from unittest.mock import Mock, patch
+
+import pytest
+from fastmcp import Client
+
+from superset.mcp_service.app import mcp
+
+
[email protected]
+def mcp_server() -> object:
+ return mcp
+
+
[email protected](autouse=True)
+def mock_auth():
+ with patch("superset.mcp_service.auth.get_user_from_request") as
mock_get_user:
+ mock_user = Mock()
+ mock_user.id = 1
+ mock_user.username = "admin"
+ mock_get_user.return_value = mock_user
+ yield mock_get_user
+
+
+def _mock_chart(chart_id: int = 10, slice_name: str = "Test Chart") -> Mock:
+ chart = Mock()
+ chart.id = chart_id
+ chart.slice_name = slice_name
+ return chart
Review Comment:
**Suggestion:** Add a docstring to this newly added helper function.
[custom_rule]
**Severity Level:** Minor ⚠️
<details>
<summary><b>Why it matters? 🤔 </b></summary>
This newly added helper function has no docstring. Since the rule requires
new Python functions to be documented inline, this is a valid violation.
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=5ea5f5c1361c4a4d91c72f80a19f47b2&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
[](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=5ea5f5c1361c4a4d91c72f80a19f47b2&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/chart/tool/test_delete_chart.py
**Line:** 47:51
**Comment:**
*Custom Rule: Add a docstring to this newly added helper function.
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%2F41472&comment_hash=e91868fa72f6fea9f0376ecf00335ec1f59e470b0b73ecdc366c84e836684e45&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41472&comment_hash=e91868fa72f6fea9f0376ecf00335ec1f59e470b0b73ecdc366c84e836684e45&reaction=dislike'>👎</a>
##########
tests/unit_tests/mcp_service/chart/tool/test_delete_chart.py:
##########
@@ -0,0 +1,131 @@
+# 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.
+
+"""Unit tests for the delete_chart MCP tool.
+
+Run through the async MCP Client (not direct calls); auth is mocked via the
+autouse mock_auth fixture, matching the other chart tool test files.
+"""
+
+from unittest.mock import Mock, patch
+
+import pytest
+from fastmcp import Client
+
+from superset.mcp_service.app import mcp
+
+
[email protected]
+def mcp_server() -> object:
+ return mcp
Review Comment:
**Suggestion:** Add a docstring to this newly added fixture function.
[custom_rule]
**Severity Level:** Minor ⚠️
<details>
<summary><b>Why it matters? 🤔 </b></summary>
The rule requires newly added Python functions to include docstrings. This
fixture has no docstring above its definition, so the suggestion identifies a
real violation.
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=79017b1d423b4a40a7d90bc9dffda7cc&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
[](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=79017b1d423b4a40a7d90bc9dffda7cc&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/chart/tool/test_delete_chart.py
**Line:** 32:34
**Comment:**
*Custom Rule: Add a docstring to this newly added fixture function.
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%2F41472&comment_hash=b8aa760cab3a25c6e339ed859e2a0c565f8eb294ef35bcffb466bdc9cb9814d3&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41472&comment_hash=b8aa760cab3a25c6e339ed859e2a0c565f8eb294ef35bcffb466bdc9cb9814d3&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]
