aminghadersohi commented on code in PR #44096:
URL: https://github.com/apache/superset/pull/44096#discussion_r4014585338
##########
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:
False positive. In Flask's `test_request_context`, when `SCRIPT_NAME` is not
explicitly provided, the request builder seeds it from `APPLICATION_ROOT`. I
reproduced this with `APPLICATION_ROOT="/analytics"`: inside
`app.test_request_context("/mcp")`, `url_for` returns
`/analytics/dashboard/p/shared-key/`, matching the existing expectation. The
separate test with explicit `SCRIPT_NAME="/proxy"` covers the proxy override.
The full Superset test could not start locally because `superset_core` is
unavailable, but the isolated Flask reproduction directly exercises the
disputed behavior. No test change is warranted.
--
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]
