codeant-ai-for-open-source[bot] commented on code in PR #41404:
URL: https://github.com/apache/superset/pull/41404#discussion_r3472748335
##########
tests/unit_tests/mcp_service/dashboard/tool/test_dashboard_tools.py:
##########
@@ -134,6 +137,109 @@ async def test_list_dashboards_basic(mock_list,
mcp_server):
assert "slug" in data["columns_loaded"]
+def test_dashboard_role_serializer_serializes_permission_view_names() -> None:
Review Comment:
**Suggestion:** Add a short docstring to this new test function describing
the serializer behavior it validates. [custom_rule]
**Severity Level:** Minor ⚠️
<details>
<summary><b>Why it matters? 🤔 </b></summary>
This is a newly added Python test function and it has no docstring. The
custom rule requires new functions to be documented inline, so the suggestion
identifies a real violation.
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=398d717c6eee4fca8f57042980d4d2c9&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=398d717c6eee4fca8f57042980d4d2c9&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_dashboard_tools.py
**Line:** 140:140
**Comment:**
*Custom Rule: Add a short docstring to this new test function
describing the serializer behavior it validates.
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%2F41404&comment_hash=2290885bc0b93cf09c78767c11e2a167bf691c283188e130baf672bd21e6e36f&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41404&comment_hash=2290885bc0b93cf09c78767c11e2a167bf691c283188e130baf672bd21e6e36f&reaction=dislike'>👎</a>
##########
tests/unit_tests/mcp_service/dashboard/tool/test_dashboard_tools.py:
##########
@@ -134,6 +137,109 @@ async def test_list_dashboards_basic(mock_list,
mcp_server):
assert "slug" in data["columns_loaded"]
+def test_dashboard_role_serializer_serializes_permission_view_names() -> None:
+ permission_view = SimpleNamespace(
+ permission=SimpleNamespace(name="can_read"),
+ view_menu=SimpleNamespace(name="Dashboard"),
+ )
+ role = SimpleNamespace(id=1, name="Gamma", permissions=[permission_view])
+
+ role_info = serialize_role_object(role)
+
+ assert role_info is not None
+ assert role_info.model_dump() == {
+ "id": 1,
+ "name": "Gamma",
+ "permissions": ["can_read on Dashboard"],
+ }
+
+
+def test_dashboard_role_serializer_skips_bad_permission_items() -> None:
Review Comment:
**Suggestion:** Add a docstring to this new test function so its edge-case
intent is explicitly documented. [custom_rule]
**Severity Level:** Minor ⚠️
<details>
<summary><b>Why it matters? 🤔 </b></summary>
This is a newly added Python test function and there is no docstring
immediately under it. That violates the rule requiring new functions to include
docstrings.
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=8a154bda8b22470e83c67a0a30457a00&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=8a154bda8b22470e83c67a0a30457a00&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_dashboard_tools.py
**Line:** 157:157
**Comment:**
*Custom Rule: Add a docstring to this new test function so its
edge-case intent is explicitly documented.
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%2F41404&comment_hash=4dc845107053f3b04b5969a8248ccd20ee6e508e2565b1ed59c4a0d67cb4716d&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41404&comment_hash=4dc845107053f3b04b5969a8248ccd20ee6e508e2565b1ed59c4a0d67cb4716d&reaction=dislike'>👎</a>
##########
tests/unit_tests/mcp_service/dashboard/tool/test_dashboard_tools.py:
##########
@@ -134,6 +137,109 @@ async def test_list_dashboards_basic(mock_list,
mcp_server):
assert "slug" in data["columns_loaded"]
+def test_dashboard_role_serializer_serializes_permission_view_names() -> None:
+ permission_view = SimpleNamespace(
+ permission=SimpleNamespace(name="can_read"),
+ view_menu=SimpleNamespace(name="Dashboard"),
+ )
+ role = SimpleNamespace(id=1, name="Gamma", permissions=[permission_view])
+
+ role_info = serialize_role_object(role)
+
+ assert role_info is not None
+ assert role_info.model_dump() == {
+ "id": 1,
+ "name": "Gamma",
+ "permissions": ["can_read on Dashboard"],
+ }
+
+
+def test_dashboard_role_serializer_skips_bad_permission_items() -> None:
+ class DetachedPermission:
+ @property
+ def name(self) -> str:
+ raise DetachedInstanceError("detached permission")
+
+ permission_view = SimpleNamespace(
+ permission=SimpleNamespace(name="can_read"),
+ view_menu=SimpleNamespace(name="Dashboard"),
+ )
+ role = SimpleNamespace(
+ id=1,
+ name="Gamma",
+ permissions=[
+ permission_view,
+ DetachedPermission(),
+ SimpleNamespace(name="can_write"),
+ ],
+ )
+
+ role_info = serialize_role_object(role)
+
+ assert role_info is not None
+ assert role_info.permissions == ["can_read on Dashboard", "can_write"]
+
+
+def test_dashboard_role_serializer_handles_detached_permissions() -> None:
+ class DetachedRole:
Review Comment:
**Suggestion:** Add a class docstring to describe why this detached-role
test double exists. [custom_rule]
**Severity Level:** Minor ⚠️
<details>
<summary><b>Why it matters? 🤔 </b></summary>
This is a newly added helper class inside a test function and it lacks a
docstring. That matches the custom rule for new classes needing inline
documentation.
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=292c9534cfa949c4a9b5c19dddfe7aee&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=292c9534cfa949c4a9b5c19dddfe7aee&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_dashboard_tools.py
**Line:** 184:184
**Comment:**
*Custom Rule: Add a class docstring to describe why this detached-role
test double exists.
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%2F41404&comment_hash=a4a5b1298a86b12fa90f9d3ecd1ba47e63cf2b7c9b40a55fb5be6b2970882079&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41404&comment_hash=a4a5b1298a86b12fa90f9d3ecd1ba47e63cf2b7c9b40a55fb5be6b2970882079&reaction=dislike'>👎</a>
##########
tests/unit_tests/mcp_service/dashboard/tool/test_dashboard_tools.py:
##########
@@ -134,6 +137,109 @@ async def test_list_dashboards_basic(mock_list,
mcp_server):
assert "slug" in data["columns_loaded"]
+def test_dashboard_role_serializer_serializes_permission_view_names() -> None:
+ permission_view = SimpleNamespace(
+ permission=SimpleNamespace(name="can_read"),
+ view_menu=SimpleNamespace(name="Dashboard"),
+ )
+ role = SimpleNamespace(id=1, name="Gamma", permissions=[permission_view])
+
+ role_info = serialize_role_object(role)
+
+ assert role_info is not None
+ assert role_info.model_dump() == {
+ "id": 1,
+ "name": "Gamma",
+ "permissions": ["can_read on Dashboard"],
+ }
+
+
+def test_dashboard_role_serializer_skips_bad_permission_items() -> None:
+ class DetachedPermission:
Review Comment:
**Suggestion:** Add a brief class docstring explaining the purpose of this
helper class used in the test. [custom_rule]
**Severity Level:** Minor ⚠️
<details>
<summary><b>Why it matters? 🤔 </b></summary>
This newly introduced helper class does not have a docstring. The custom
rule explicitly requires new classes to be documented inline.
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=8b691e47d04649c79fb1797c8b536b18&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=8b691e47d04649c79fb1797c8b536b18&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_dashboard_tools.py
**Line:** 158:158
**Comment:**
*Custom Rule: Add a brief class docstring explaining the purpose of
this helper class used in the 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%2F41404&comment_hash=0b24613ac122970a9b066749b3f7e613063c2566f51bd6f8a3da1a015db0b74d&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41404&comment_hash=0b24613ac122970a9b066749b3f7e613063c2566f51bd6f8a3da1a015db0b74d&reaction=dislike'>👎</a>
##########
tests/unit_tests/mcp_service/dashboard/tool/test_dashboard_tools.py:
##########
@@ -134,6 +137,109 @@ async def test_list_dashboards_basic(mock_list,
mcp_server):
assert "slug" in data["columns_loaded"]
+def test_dashboard_role_serializer_serializes_permission_view_names() -> None:
+ permission_view = SimpleNamespace(
+ permission=SimpleNamespace(name="can_read"),
+ view_menu=SimpleNamespace(name="Dashboard"),
+ )
+ role = SimpleNamespace(id=1, name="Gamma", permissions=[permission_view])
+
+ role_info = serialize_role_object(role)
+
+ assert role_info is not None
+ assert role_info.model_dump() == {
+ "id": 1,
+ "name": "Gamma",
+ "permissions": ["can_read on Dashboard"],
+ }
+
+
+def test_dashboard_role_serializer_skips_bad_permission_items() -> None:
+ class DetachedPermission:
+ @property
+ def name(self) -> str:
+ raise DetachedInstanceError("detached permission")
+
+ permission_view = SimpleNamespace(
+ permission=SimpleNamespace(name="can_read"),
+ view_menu=SimpleNamespace(name="Dashboard"),
+ )
+ role = SimpleNamespace(
+ id=1,
+ name="Gamma",
+ permissions=[
+ permission_view,
+ DetachedPermission(),
+ SimpleNamespace(name="can_write"),
+ ],
+ )
+
+ role_info = serialize_role_object(role)
+
+ assert role_info is not None
+ assert role_info.permissions == ["can_read on Dashboard", "can_write"]
+
+
+def test_dashboard_role_serializer_handles_detached_permissions() -> None:
+ class DetachedRole:
+ id = 1
+ name = "Gamma"
+
+ @property
+ def permissions(self) -> list[object]:
+ raise DetachedInstanceError("detached permissions")
+
+ role_info = serialize_role_object(DetachedRole())
+
+ assert role_info is not None
+ assert role_info.permissions is None
+
+
+def test_dashboard_role_serializer_handles_non_iterable_permissions() -> None:
+ role = SimpleNamespace(id=1, name="Gamma", permissions=object())
+
+ role_info = serialize_role_object(role)
+
+ assert role_info is not None
+ assert role_info.permissions == []
+
+
+def test_dashboard_role_serializer_stops_on_detached_permission_iterator() ->
None:
+ class DetachedPermissionIterator:
Review Comment:
**Suggestion:** Add a short docstring to this iterator helper class to
clarify the failure scenario it simulates. [custom_rule]
**Severity Level:** Minor ⚠️
<details>
<summary><b>Why it matters? 🤔 </b></summary>
This new helper class also lacks a docstring. Since the rule requires newly
added classes to be documented, the suggestion is valid.
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=eb1d379b8397453f82e7f5542e603eb9&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=eb1d379b8397453f82e7f5542e603eb9&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_dashboard_tools.py
**Line:** 208:208
**Comment:**
*Custom Rule: Add a short docstring to this iterator helper class to
clarify the failure scenario it simulates.
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%2F41404&comment_hash=d088b7e3005d98c907f06efcc6715150036b29123aaf1f8d3c2866bcb25264d7&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41404&comment_hash=d088b7e3005d98c907f06efcc6715150036b29123aaf1f8d3c2866bcb25264d7&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]
