bito-code-review[bot] commented on code in PR #44096:
URL: https://github.com/apache/superset/pull/44096#discussion_r4014586831
##########
tests/unit_tests/mcp_service/dashboard/test_permalink.py:
##########
@@ -372,3 +375,119 @@ def
test_lookup_dashboard_reference_unresolvable_reference(mock_get_permalink) -
assert result.permalink_key == "nonexistent"
assert result.permalink_value is None
assert result.resolved_from_permalink is False
+
+
[email protected]("denied", [False, True])
+def test_stacking_resolves_with_calling_user(app: Flask, denied: bool) -> None:
+ """The real get command checks access using the refreshed calling user."""
+ user = Mock(username="viewer", is_anonymous=False)
+ refreshed_user = Mock(username="viewer")
+ value: DashboardPermalinkValue = {
+ "dashboardId": "42",
+ "state": {
+ "dataMask": {
+ "legacy-filter": {
+ "extraFormData": {"filters": [{"col": "region", "val":
["EMEA"]}]},
+ "ownState": {"arbitrary": "preserved"},
+ }
+ }
+ },
+ }
+
+ def check_access(reference: str) -> Mock:
+ """Observe the request principal at the command's authorization
gate."""
+ assert reference == "42"
+ assert g.user is refreshed_user
+ if denied:
+ raise DashboardAccessDeniedError()
+ return Mock()
+
+ with (
+ app.test_request_context("/mcp"),
+ patch(
+
"superset.mcp_service.dashboard.permalink.load_user_with_relationships",
+ return_value=refreshed_user,
+ ) as load_user,
+ patch(
+ "superset.commands.dashboard.permalink.get."
+ "GetDashboardPermalinkCommand.salt",
+ new_callable=PropertyMock,
+ return_value="test-salt",
+ ),
+ patch(
+ "superset.commands.dashboard.permalink.get.decode_permalink_id",
+ return_value=1,
+ ),
+ patch(
+ "superset.commands.dashboard.permalink.get.KeyValueDAO.get_value",
+ return_value=value,
+ ),
+ patch(
+ "superset.daos.dashboard.DashboardDAO.get_by_id_or_slug",
+ side_effect=check_access,
+ ) as access,
+ ):
+ g.user = user
+ if denied:
+ with pytest.raises(DashboardAccessDeniedError):
+ get_dashboard_permalink_data_mask("base-key", 42)
+ else:
+ mask = get_dashboard_permalink_data_mask("base-key", 42)
+ assert mask == value["state"]["dataMask"]
+ assert mask is not value["state"]["dataMask"]
+ load_user.assert_called_once_with(username="viewer")
+ access.assert_called_once_with("42")
+
+
[email protected]("state", [{}, {"dataMask": None}, {"dataMask": {}}])
+def test_stacking_base_without_selections(state: dict[str, object]) -> None:
+ """A valid permalink without explicit filter selections supplies an empty
mask."""
+ with (
+ patch(
+ "superset.mcp_service.dashboard.permalink."
+ "refresh_request_user_for_permalink_access"
+ ),
+ patch(
+
"superset.mcp_service.dashboard.permalink.GetDashboardPermalinkCommand"
+ ) as command,
+ ):
+ command.return_value.run.return_value = {"dashboardId": "42", "state":
state}
+ assert get_dashboard_permalink_data_mask("base-key", 42) == {}
+
+
[email protected]("application_root", ["/", "/analytics"])
[email protected]("in_request", [False, True])
+def test_build_dashboard_permalink_url(
+ app: Flask, application_root: str, in_request: bool
+) -> None:
+ """Use the configured public origin and the Flask deployment prefix."""
+ with patch.dict(
+ app.config,
+ WEBDRIVER_BASEURL_USER_FRIENDLY="https://dashboards.example.test/",
+ APPLICATION_ROOT=application_root,
+ ):
+ expected = (
+ f"https://dashboards.example.test{application_root.rstrip('/')}"
+ "/dashboard/p/shared-key/"
+ )
+ if in_request:
+ with app.test_request_context("/mcp"):
+ assert build_dashboard_permalink_url("shared-key") == expected
+ else:
+ with app.app_context():
+ assert build_dashboard_permalink_url("shared-key") == expected
Review Comment:
<!-- Bito Reply -->
The suggestion provided by the reviewer is not necessary because Flask's
`test_request_context` correctly handles `SCRIPT_NAME` when seeded from
`APPLICATION_ROOT`. The existing test behavior is consistent with how Flask
resolves URLs in this context, and the proxy override is already covered by a
separate test case. Therefore, no changes to the test code are required.
--
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]