Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 20 additions & 12 deletions superset/commands/report/execute.py
Original file line number Diff line number Diff line change
Expand Up @@ -289,19 +289,27 @@ def get_dashboard_urls(
except json.JSONDecodeError:
logger.debug("Anchor value is not a list, Fall back to single tab")

# Merge native_filters into existing urlParams instead of
# overwriting — dashboard_state may already have urlParams
# (e.g. standalone=true) that must be preserved.
state: DashboardPermalinkState = {**dashboard_state}
state["urlParams"] = self._merge_native_filters_into_url_params(
state.get("urlParams"), native_filter_params
)
return [
self._get_tab_url(
state,
user_friendly=user_friendly,
# Skip the permalink when there is nothing meaningful to encode —
# an empty dashboard_state falls through to the plain URL below.
# A non-empty anchor means a single tab was selected (it failed
# JSON list parsing above) and still needs a permalink. Non-filter
# state such as urlParams (e.g. standalone=true) must also be
# preserved via a permalink.
if (
anchor
or dashboard_state.get("urlParams")
or (native_filter_params and native_filter_params != "()")
):
state: DashboardPermalinkState = {**dashboard_state}
state["urlParams"] = self._merge_native_filters_into_url_params(
state.get("urlParams"), native_filter_params
)
]
return [
self._get_tab_url(
state,
user_friendly=user_friendly,
)
]

native_filter_params, filter_warnings = (
self._report_schedule.get_native_filters_params()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ def test_report_for_dashboard_with_tabs(
with create_dashboard_report(
dashboard=tabbed_dashboard,
extra={
"anchor": "TAB-L1B",
"activeTabs": ["TAB-L1B", "TAB-L2BB"],
"urlParams": [["native_filters", "()"]],
},
Expand Down Expand Up @@ -95,7 +96,8 @@ def test_report_with_header_data(
with create_dashboard_report(
dashboard=tabbed_dashboard,
extra={
"active_tabs": ["TAB-L1B", "TAB-L2BB"],
"anchor": "TAB-L1B",
"activeTabs": ["TAB-L1B", "TAB-L2BB"],
"urlParams": [["native_filters", "()"]],
},
name="test report tabbed dashboard",
Expand Down
92 changes: 82 additions & 10 deletions tests/unit_tests/commands/report/execute_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -331,19 +331,15 @@ def test_get_dashboard_urls_with_multiple_tabs(
assert result == expected_uris


@patch(
"superset.commands.dashboard.permalink.create.CreateDashboardPermalinkCommand.run"
)
@with_feature_flags(ALERT_REPORT_TABS=True)
def test_get_dashboard_urls_with_exporting_dashboard_only(
mock_run,
mocker: MockerFixture,
app,
) -> None:
mock_report_schedule: ReportSchedule = mocker.Mock(spec=ReportSchedule)
mock_report_schedule.chart = False
mock_report_schedule.chart_id = None
mock_report_schedule.dashboard_id = 123
mock_report_schedule.force_screenshot = False
mock_report_schedule.type = "report_type"
mock_report_schedule.report_format = "report_format"
mock_report_schedule.owners = [1, 2]
Expand All @@ -360,7 +356,9 @@ def test_get_dashboard_urls_with_exporting_dashboard_only(
"()",
[],
)
mock_run.return_value = "url1"
mock_dashboard = mocker.MagicMock()
mock_dashboard.uuid = UUID("12345678-1234-1234-1234-123456789abc")
mock_report_schedule.dashboard = mock_dashboard

class_instance: BaseReportState = BaseReportState(
mock_report_schedule, "January 1, 2021", "execution_id_example"
Expand All @@ -369,11 +367,85 @@ def test_get_dashboard_urls_with_exporting_dashboard_only(

result: list[str] = class_instance.get_dashboard_urls()

import urllib.parse
assert len(result) == 1
assert "/dashboard/p/" not in result[0]
assert "12345678-1234-1234-1234-123456789abc" in result[0]

base_url = app.config.get("WEBDRIVER_BASEURL", "http://0.0.0.0:8080/")
expected_url = urllib.parse.urljoin(base_url, "superset/dashboard/p/url1/")
assert expected_url == result[0]

@patch("superset.commands.report.execute.CreateDashboardPermalinkCommand")
@with_feature_flags(ALERT_REPORT_TABS=True)
def test_get_dashboard_urls_empty_dashboard_state_skips_permalink(
mock_permalink_cls,
mocker: MockerFixture,
) -> None:
"""When both ALERT_REPORT_TABS and ALERT_REPORTS_FILTER are enabled but the
report has no tab or filter configured, get_dashboard_urls() must return
a plain dashboard URL and must not create a permalink. A permalink with
nothing to encode causes a server-side redirect that fails the Playwright
screenshot (domcontentloaded timeout)."""
mock_report_schedule: ReportSchedule = mocker.Mock(spec=ReportSchedule)
mock_report_schedule.chart = False
mock_report_schedule.force_screenshot = False
mock_report_schedule.extra = {"dashboard": {}}
mock_report_schedule.get_native_filters_params.return_value = ("()", []) # type: ignore

mock_dashboard = mocker.MagicMock()
mock_dashboard.uuid = UUID("12345678-1234-1234-1234-123456789abc")
mock_report_schedule.dashboard = mock_dashboard

class_instance: BaseReportState = BaseReportState(
mock_report_schedule, "January 1, 2021", "execution_id_example"
)
class_instance._report_schedule = mock_report_schedule

result: list[str] = class_instance.get_dashboard_urls()

mock_permalink_cls.assert_not_called()
assert len(result) == 1
assert "/dashboard/p/" not in result[0]
assert "12345678-1234-1234-1234-123456789abc" in result[0]


@patch("superset.commands.report.execute.CreateDashboardPermalinkCommand")
@with_feature_flags(ALERT_REPORT_TABS=True)
def test_get_dashboard_urls_url_params_only_creates_permalink(
mock_permalink_cls,
mocker: MockerFixture,
) -> None:
"""When the dashboard state carries no anchor and no native filters but
does carry meaningful urlParams (e.g. standalone=true), get_dashboard_urls()
must still build a permalink so that state survives in the screenshot,
rather than falling through to the plain dashboard URL."""
mock_permalink_cls.return_value.run.return_value = "key1"
mock_report_schedule: ReportSchedule = mocker.Mock(spec=ReportSchedule)
mock_report_schedule.chart = False
mock_report_schedule.force_screenshot = False
mock_report_schedule.extra = {
"dashboard": {
"anchor": "",
"dataMask": None,
"activeTabs": None,
"urlParams": [["standalone", "true"]],
}
}
mock_report_schedule.get_native_filters_params.return_value = ("()", []) # type: ignore

mock_dashboard = mocker.MagicMock()
mock_dashboard.uuid = UUID("12345678-1234-1234-1234-123456789abc")
mock_report_schedule.dashboard = mock_dashboard

class_instance: BaseReportState = BaseReportState(
mock_report_schedule, "January 1, 2021", "execution_id_example"
)
class_instance._report_schedule = mock_report_schedule

result: list[str] = class_instance.get_dashboard_urls()

mock_permalink_cls.assert_called_once()
state = mock_permalink_cls.call_args.kwargs["state"]
assert ["standalone", "true"] in state["urlParams"]
assert len(result) == 1
assert "/dashboard/p/" in result[0]


@patch("superset.commands.report.execute.CreateDashboardPermalinkCommand")
Expand Down
Loading