From e2ed9896399664d12452e08558d65f2ad30669b3 Mon Sep 17 00:00:00 2001 From: Mehmet Salih Yavuz Date: Tue, 2 Jun 2026 11:37:30 +0300 Subject: [PATCH] fix(reports): skip permalink when dashboard state has no anchor or filters (#40530) --- superset/commands/report/execute.py | 32 ++++--- .../execute_dashboard_report_tests.py | 4 +- .../commands/report/execute_test.py | 92 +++++++++++++++++-- 3 files changed, 105 insertions(+), 23 deletions(-) diff --git a/superset/commands/report/execute.py b/superset/commands/report/execute.py index 0cca27ef880f..4f20783c4d45 100644 --- a/superset/commands/report/execute.py +++ b/superset/commands/report/execute.py @@ -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() diff --git a/tests/integration_tests/reports/commands/execute_dashboard_report_tests.py b/tests/integration_tests/reports/commands/execute_dashboard_report_tests.py index 9492a45ce291..a3304e86aa76 100644 --- a/tests/integration_tests/reports/commands/execute_dashboard_report_tests.py +++ b/tests/integration_tests/reports/commands/execute_dashboard_report_tests.py @@ -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", "()"]], }, @@ -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", diff --git a/tests/unit_tests/commands/report/execute_test.py b/tests/unit_tests/commands/report/execute_test.py index 9aef3245ec93..bf8c38f1e314 100644 --- a/tests/unit_tests/commands/report/execute_test.py +++ b/tests/unit_tests/commands/report/execute_test.py @@ -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] @@ -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" @@ -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")