aminghadersohi commented on code in PR #40957:
URL: https://github.com/apache/superset/pull/40957#discussion_r3493287157
##########
tests/unit_tests/mcp_service/dashboard/tool/test_update_dashboard.py:
##########
@@ -309,3 +309,152 @@ async def test_xss_only_title_is_rejected(self,
mcp_server) -> None:
}
},
)
+
+ @patch("superset.commands.utils.update_tags")
+ @patch("superset.commands.utils.validate_tags")
+ @patch("superset.daos.dashboard.DashboardDAO.get_by_id_or_slug")
+ @patch("superset.extensions.db.session")
+ @pytest.mark.asyncio
+ async def test_update_tags_replaces(
+ self, mock_session, mock_get, mock_validate_tags, mock_update_tags,
mcp_server
+ ) -> None:
Review Comment:
Declining as a nit: every other `@patch`-injected test method in this file
leaves the injected `Mock` params unannotated (e.g.
`test_update_layout_theme_and_css`, `test_update_with_no_fields_is_noop`).
Adding `MagicMock` types only to these two methods would be asymmetric without
a broader pass. mypy passes on the file as-is, so there is no CI enforcement
gap.
##########
tests/unit_tests/mcp_service/dashboard/tool/test_update_dashboard.py:
##########
@@ -309,3 +309,152 @@ async def test_xss_only_title_is_rejected(self,
mcp_server) -> None:
}
},
)
+
+ @patch("superset.commands.utils.update_tags")
+ @patch("superset.commands.utils.validate_tags")
+ @patch("superset.daos.dashboard.DashboardDAO.get_by_id_or_slug")
+ @patch("superset.extensions.db.session")
+ @pytest.mark.asyncio
+ async def test_update_tags_replaces(
+ self, mock_session, mock_get, mock_validate_tags, mock_update_tags,
mcp_server
+ ) -> None:
+ """``tags`` routes through the same validate/update helpers the REST
+ UpdateDashboardCommand uses, and reports ``tags`` as changed."""
+ from superset.tags.models import ObjectType
+
+ dash = _mock_dashboard(id=42)
+ mock_get.return_value = dash
+
+ async with Client(mcp_server) as client:
+ result = await client.call_tool(
+ "update_dashboard",
+ {"request": {"identifier": 42, "tags": [3, 7]}},
+ )
+
+ mock_validate_tags.assert_called_once()
+ mock_update_tags.assert_called_once()
+ update_args = mock_update_tags.call_args.args
+ assert update_args[0] == ObjectType.dashboard
+ assert update_args[1] == 42
+ assert update_args[3] == [3, 7]
+ payload = json.loads(result.content[0].text)
+ assert "tags" in payload.get("changed_fields", [])
+
+ @patch("superset.commands.utils.update_tags")
+ @patch("superset.commands.utils.validate_tags")
+ @patch("superset.daos.dashboard.DashboardDAO.get_by_id_or_slug")
+ @patch("superset.extensions.db.session")
+ @pytest.mark.asyncio
+ async def test_update_tags_empty_list_clears(
+ self, mock_session, mock_get, mock_validate_tags, mock_update_tags,
mcp_server
+ ) -> None:
Review Comment:
Same as above — deferring to a consistent pass rather than annotating
selectively.
--
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]
