QT/superset2
1
0
Fork 0
mirror of https://github.com/apache/superset.git synced 2026-05-07 08:54:23 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix(reports): preserve urlParams in multi-tab report fan-out (#39884)

Browse Source
This commit is contained in:
Enzo Martellucci
2026-05-06 16:29:45 +02:00
committed by GitHub
parent adfbbf1433
commit 9aaa12c7d4
3 changed files with 161 additions and 15 deletions
Download Patch File Download Diff File Expand all files Collapse all files

59
superset/commands/report/execute.py
View File

@@ -15,6 +15,7 @@
# specific language governing permissions and limitations
# under the License.
import logging
from collections.abc import Sequence
from datetime import datetime, timedelta
from typing import Any, Optional, Union
from uuid import UUID
@@ -196,7 +197,7 @@ class BaseReportState:
db.session.commit() # pylint: disable=consider-using-transaction
except StaleDataError as ex:
# Report schedule was modified or deleted by another process
db.session.rollback()
db.session.rollback() # pylint: disable=consider-using-transaction
logger.warning(
"Report schedule (execution %s) was modified or deleted "
"during execution. This can occur when a report is deleted "
@@ -280,6 +281,7 @@ class BaseReportState:
)
urls = self._get_tabs_urls(
anchor_list,
dashboard_state=dashboard_state,
native_filter_params=native_filter_params,
user_friendly=user_friendly,
)
@@ -291,12 +293,9 @@ class BaseReportState:
# overwriting — dashboard_state may already have urlParams
# (e.g. standalone=true) that must be preserved.
state: DashboardPermalinkState = {**dashboard_state}
existing_params: list[tuple[str, str]] = state.get("urlParams") or []
merged_params: list[list[str]] = [
list(p) for p in existing_params if p[0] != "native_filters"
]
merged_params.append(["native_filters", native_filter_params or ""])
state["urlParams"] = merged_params # type: ignore[typeddict-item]
state["urlParams"] = self._merge_native_filters_into_url_params(
state.get("urlParams"), native_filter_params
)
return [
self._get_tab_url(
state,
@@ -310,12 +309,17 @@ class BaseReportState:
if filter_warnings:
self._filter_warnings.extend(filter_warnings)
if native_filter_params and native_filter_params != "()":
# Preserve any urlParams from extra.dashboard (e.g. standalone=true)
# set via API even when ALERT_REPORT_TABS is off — same merge
# semantics as the protected branch above.
fallback_state = self._report_schedule.extra.get("dashboard") or {}
return [
self._get_tab_url(
{
"urlParams": [
["native_filters", native_filter_params] # type: ignore
],
"urlParams": self._merge_native_filters_into_url_params(
fallback_state.get("urlParams"),
native_filter_params,
)
},
user_friendly=user_friendly,
)
@@ -353,24 +357,51 @@ class BaseReportState:
user_friendly=user_friendly,
)
@staticmethod
def _merge_native_filters_into_url_params(
existing: Optional[Sequence[Sequence[str]]],
native_filter_params: Optional[str],
) -> list[Sequence[str]]:
"""
Merge the report's ``native_filters`` into a permalink's existing
``urlParams``, deduping any prior ``native_filters`` entry so the
report's value wins. All other params (e.g. ``standalone=true``)
survive in their original order.
"""
merged: list[Sequence[str]] = [
list(p) for p in (existing or []) if p[0] != "native_filters"
]
merged.append(["native_filters", native_filter_params or ""])
return merged
def _get_tabs_urls(
self,
tab_anchors: list[str],
dashboard_state: Optional[DashboardPermalinkState] = None,
native_filter_params: Optional[str] = None,
user_friendly: bool = False,
) -> list[str]:
"""
Get multple tabs urls
Get multiple tabs urls.
Each per-tab permalink merges the report's ``native_filters`` into
the original ``dashboard_state.urlParams`` (deduping any prior
``native_filters`` entry), so params like ``standalone=true`` are
preserved — matching the precedence rules of the single-tab branch
in :meth:`get_dashboard_urls`.
"""
base_state: DashboardPermalinkState = dashboard_state or {}
merged_params = self._merge_native_filters_into_url_params(
base_state.get("urlParams"), native_filter_params
)
return [
self._get_tab_url(
{
"anchor": tab_anchor,
"dataMask": None,
"activeTabs": None,
"urlParams": [
["native_filters", native_filter_params] # type: ignore
],
"urlParams": merged_params,
},
user_friendly=user_friendly,
)

6
superset/dashboards/permalink/types.py
View File

@@ -14,6 +14,7 @@
# KIND, either express or implied. See the License for the
# specific language governing permissions and limitations
# under the License.
from collections.abc import Sequence
from typing import Any, Optional, TypedDict
@@ -21,7 +22,10 @@ class DashboardPermalinkState(TypedDict, total=False):
dataMask: Optional[dict[str, Any]]
activeTabs: Optional[list[str]]
anchor: Optional[str]
urlParams: Optional[list[tuple[str, str]]]
# urlParams items are stored/transmitted as JSON arrays, so they
# arrive at runtime as ``list[str]``; ``Sequence[str]`` keeps the
# annotation permissive of both list and tuple shapes.
urlParams: Optional[list[Sequence[str]]]
chartStates: Optional[dict[str, Any]]

111
tests/unit_tests/commands/report/execute_test.py
View File

@@ -624,6 +624,65 @@ def test_get_tab_urls(
]
@patch("superset.commands.report.execute.CreateDashboardPermalinkCommand")
@with_feature_flags(ALERT_REPORT_TABS=True)
def test_get_dashboard_urls_multitab_preserves_url_params(
mock_permalink_cls,
mocker: MockerFixture,
app,
) -> None:
"""Multi-tab fan-out must preserve dashboard_state.urlParams (e.g. standalone)
and replace any pre-existing native_filters entry with the report's value —
matching the single-tab branch's merge semantics."""
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.type = "report_type"
mock_report_schedule.report_format = "report_format"
mock_report_schedule.owners = [1, 2]
mock_report_schedule.recipients = []
native_filter_rison = "(NATIVE_FILTER-1:(filterType:filter_select))"
# Use list-of-lists (not tuples) — extra_json deserializes urlParams from
# JSON arrays. Includes a stale native_filters entry to exercise the
# dedup-then-append step in the merge.
mock_report_schedule.extra = {
"dashboard": {
"anchor": json.dumps(["TAB-1", "TAB-2"]),
"urlParams": [
["standalone", "true"],
["native_filters", "(STALE_FILTER:(filterType:filter_select))"],
["show_filters", "0"],
],
}
}
mock_report_schedule.get_native_filters_params.return_value = ( # type: ignore[attr-defined]
native_filter_rison,
[],
)
mock_permalink_cls.return_value.run.side_effect = ["key1", "key2"]
class_instance: BaseReportState = BaseReportState(
mock_report_schedule, "January 1, 2021", "execution_id_example"
)
class_instance._report_schedule = mock_report_schedule
class_instance.get_dashboard_urls()
assert mock_permalink_cls.call_count == 2
for idx, expected_anchor in enumerate(["TAB-1", "TAB-2"]):
state = mock_permalink_cls.call_args_list[idx].kwargs["state"]
# Stale native_filters is replaced (not duplicated); other params
# survive in their original order; report's native_filters appended.
assert state["urlParams"] == [
["standalone", "true"],
["show_filters", "0"],
["native_filters", native_filter_rison],
]
# Each per-tab permalink targets exactly that tab.
assert state["anchor"] == expected_anchor
@patch(
"superset.commands.dashboard.permalink.create.CreateDashboardPermalinkCommand.run"
)
@@ -702,6 +761,58 @@ def test_get_dashboard_urls_native_filters_without_tabs(
assert "permalink_key" in result[0]
@patch("superset.commands.report.execute.CreateDashboardPermalinkCommand")
@with_feature_flags(ALERT_REPORT_TABS=False)
def test_get_dashboard_urls_flag_off_preserves_url_params(
mock_permalink_cls,
mocker: MockerFixture,
app,
) -> None:
"""The post-``if``-block fall-through in ``get_dashboard_urls`` must
honor any urlParams set in ``extra.dashboard`` (e.g. via API) — same
merge semantics as the protected branch.
Reachability: only when ``dashboard_state`` is falsy OR
``ALERT_REPORT_TABS=False``. The flag-on / no-anchor case lands in
the single-tab merge at L290-306, not here.
"""
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
native_filter_rison = "(NATIVE_FILTER-abc:!(val1))"
mock_report_schedule.extra = {
"dashboard": {
"urlParams": [
["standalone", "true"],
["native_filters", "(STALE_FILTER:!(stale))"],
["show_filters", "0"],
],
}
}
mock_report_schedule.get_native_filters_params.return_value = ( # type: ignore[attr-defined]
native_filter_rison,
[],
)
class_instance: BaseReportState = BaseReportState(
mock_report_schedule, "January 1, 2021", "execution_id_example"
)
class_instance._report_schedule = mock_report_schedule
mock_permalink_cls.return_value.run.return_value = "permalink_key"
class_instance.get_dashboard_urls()
state = mock_permalink_cls.call_args_list[0].kwargs["state"]
# Stale native_filters replaced; existing params survive in order;
# report's native_filters appended.
assert state["urlParams"] == [
["standalone", "true"],
["show_filters", "0"],
["native_filters", native_filter_rison],
]
def create_report_schedule(
mocker: MockerFixture,
custom_width: int | None = None,