This is an automated email from the ASF dual-hosted git repository.
rebenitez1802 pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/superset.git
The following commit(s) were added to refs/heads/master by this push:
new 99b472bef74 fix(dashboard): saving a dashboard that contains an
archived chart no longer fails (#45543)
99b472bef74 is described below
commit 99b472bef749767692c547e496d342ee2157242c
Author: Mike Bridge <[email protected]>
AuthorDate: Fri Oct 23 18:09:16 2026 -0600
fix(dashboard): saving a dashboard that contains an archived chart no
longer fails (#45543)
Co-authored-by: Claude Opus 5.5 <[email protected]>
---
superset/daos/dashboard.py | 64 +++++++++++---
tests/integration_tests/dashboards/api_tests.py | 100 +++++++++++++++++++++-
tests/unit_tests/dao/dashboard_test.py | 108 ++++++++++++++++++++++--
3 files changed, 254 insertions(+), 18 deletions(-)
diff --git a/superset/daos/dashboard.py b/superset/daos/dashboard.py
index a46a7157237..a4cbc4e6af7 100644
--- a/superset/daos/dashboard.py
+++ b/superset/daos/dashboard.py
@@ -25,9 +25,11 @@ from typing import Any, Callable, Dict, List, TYPE_CHECKING
from flask import g
from flask_appbuilder.models.sqla.interface import SQLAInterface
from marshmallow import ValidationError
-from sqlalchemy import or_, select
+from sqlalchemy import inspect as sa_inspect, or_, select
from sqlalchemy.engine import Row
from sqlalchemy.orm import lazyload, Query, selectinload
+from sqlalchemy.orm.attributes import set_committed_value
+from sqlalchemy.orm.state import InstanceState
from superset import security_manager
from superset.commands.dashboard.exceptions import (
@@ -328,6 +330,36 @@ def reconcile_position_json(positions: object,
dashboard_id: int | None = None)
)
+def _install_membership_baseline(dashboard: Dashboard) -> None:
+ """Install *dashboard*'s stored chart membership, archived charts
+ included, as the committed state of ``dashboard.slices``.
+
+ Call inside ``skip_visibility_filter``. Only a persistent dashboard has
+ stored membership. Pending changes to the collection must be flushed
+ first: the baseline query autoflushes them, and a caller running under
+ ``no_autoflush`` flushes explicitly (as ``UpdateDashboardCommand`` does).
+ A change still pending is kept, by leaving the collection alone, because
+ installing the baseline would drop it.
+ """
+ state: InstanceState[Dashboard] = sa_inspect(dashboard)
+ if not state.persistent:
+ return
+ baseline: list[Slice] = (
+ db.session.query(Slice)
+ .join(dashboard_slices, dashboard_slices.c.slice_id == Slice.id)
+ .filter(dashboard_slices.c.dashboard_id == dashboard.id)
+ .all()
+ )
+ if state.attrs.slices.history.has_changes():
+ logger.warning(
+ "dashboard id=%s has unflushed chart-list changes; saving it with "
+ "an archived chart may fail",
+ dashboard.id,
+ )
+ return
+ set_committed_value(dashboard, "slices", baseline)
+
+
class DashboardDAO(BaseDAO[Dashboard]):
base_filter = DashboardAccessFilter
# Column used by MCP tools for title-based identifier fallback, so a
@@ -665,21 +697,31 @@ class DashboardDAO(BaseDAO[Dashboard]):
# documented restore-reattach contract) and writing
# ``uuid: None`` into its position slot via ``uuid_map`` below.
#
- # The bypass must be session-scoped and cover the ASSIGNMENT,
- # not just the resolution query: assigning to
+ # The bypass must also cover the ASSIGNMENT: assigning to
# ``dashboard.slices`` makes the unit of work diff the new
- # collection against the existing one, which it lazy-loads at
- # that moment. A filtered baseline load would exclude the
- # trashed member, so the diff would treat it as net-new and
- # INSERT a ``dashboard_slices`` row that still exists (soft
- # delete never removes junction rows) — an IntegrityError on
- # the composite primary key on every save of a dashboard
- # containing a trashed chart.
+ # collection against the existing one. A baseline missing the
+ # trashed member makes the diff treat it as net-new and INSERT a
+ # ``dashboard_slices`` row that still exists (soft delete never
+ # removes junction rows), an IntegrityError on the composite
+ # primary key on every save of a dashboard containing a trashed
+ # chart. Lazy-loading the baseline inside the bypass is not
+ # enough: a dashboard loaded through a query carries the
+ # visibility filter into its own lazy loads. Read the baseline
+ # from the association table instead and install it as the
+ # committed state.
with skip_visibility_filter(db.session, Slice):
current_slices = (
db.session.query(Slice).filter(Slice.id.in_(slice_ids)).all()
)
- dashboard.slices = current_slices
+ _install_membership_baseline(dashboard)
+ # Preserve archived membership even when no layout slot
represents it.
+ layout_ids: set[int] = {chart.id for chart in current_slices}
+ kept_archived: list[Slice] = [
+ chart
+ for chart in dashboard.slices
+ if chart.deleted_at is not None and chart.id not in
layout_ids
+ ]
+ dashboard.slices = current_slices + kept_archived
# add UUID to positions
uuid_map = {slice.id: str(slice.uuid) for slice in current_slices}
diff --git a/tests/integration_tests/dashboards/api_tests.py
b/tests/integration_tests/dashboards/api_tests.py
index c14b49c018f..c396af1d663 100644
--- a/tests/integration_tests/dashboards/api_tests.py
+++ b/tests/integration_tests/dashboards/api_tests.py
@@ -60,7 +60,7 @@ from superset.exceptions import (
ReleaseDistributedLockFailedException,
)
from superset.security.guest_token import GuestTokenResourceType, GuestUser
-from superset.models.dashboard import Dashboard
+from superset.models.dashboard import Dashboard, dashboard_slices
from superset.models.core import FavStar, FavStarClassName
from superset.reports.models import ReportSchedule, ReportScheduleType
from superset.models.slice import Slice
@@ -2991,6 +2991,104 @@ class TestDashboardApi(ApiEditorsTestCaseMixin,
InsertChartMixin, SupersetTestCa
db.session.delete(model)
db.session.commit()
+ @staticmethod
+ def _expunge_dashboards_and_charts() -> None:
+ """Drop dashboards and charts from the session so the next request
+ loads them through a query, as a fresh request would."""
+ for obj in list(db.session):
+ if isinstance(obj, (Dashboard, Slice)):
+ db.session.expunge(obj)
+
+ def test_update_dashboard_keeps_archived_chart_slot(self) -> None:
+ """PUT: saving back the layout of a dashboard that holds an archived
+ chart succeeds, and the chart keeps its slot and its membership, so
+ recovering it puts it back in place."""
+ from superset.connectors.sqla.models import SqlaTable
+ from superset.models.helpers import skip_visibility_filter
+
+ admin: User = self.get_user("admin")
+ dataset_id: int = db.session.query(SqlaTable.id).first()[0]
+ live: Slice = self.insert_chart("archived-slot-live", [admin.id],
dataset_id)
+ archived: Slice = self.insert_chart(
+ "archived-slot-archived", [admin.id], dataset_id
+ )
+ live_id: int = live.id
+ archived_id: int = archived.id
+ archived_uuid: str = str(archived.uuid)
+ positions: dict[str, Any] = {
+ "DASHBOARD_VERSION_KEY": "v2",
+ "ROOT_ID": {"id": "ROOT_ID", "type": "ROOT", "children":
["GRID_ID"]},
+ "GRID_ID": {
+ "id": "GRID_ID",
+ "type": "GRID",
+ "children": ["CHART-live", "CHART-archived"],
+ "parents": ["ROOT_ID"],
+ },
+ }
+ for slot, chart_id in (
+ ("CHART-live", live_id),
+ ("CHART-archived", archived_id),
+ ):
+ positions[slot] = {
+ "id": slot,
+ "type": "CHART",
+ "children": [],
+ "parents": ["ROOT_ID", "GRID_ID"],
+ "meta": {"chartId": chart_id, "width": 4, "height": 50},
+ }
+ dashboard: Dashboard = self.insert_dashboard(
+ "archived-slot",
+ None,
+ [admin.id],
+ slices=[live, archived],
+ position_json=json.dumps(positions),
+ )
+ dashboard_id: int = dashboard.id
+ self.login(ADMIN_USERNAME)
+ rv: TestResponse = self.client.delete(f"api/v1/chart/{archived_id}")
+ assert rv.status_code == 200, rv.data
+ try:
+ # Load the dashboard and its chart list fresh, as a real request
+ # does: through a query that hides archived charts, not from the
+ # identity map this test populated.
+ self._expunge_dashboards_and_charts()
+ rv = self.client.get(f"api/v1/dashboard/{dashboard_id}")
+ assert rv.status_code == 200, rv.data
+ self._expunge_dashboards_and_charts()
+ rv = self.client.put(
+ f"api/v1/dashboard/{dashboard_id}",
+ json={"json_metadata": json.dumps({"positions": positions})},
+ )
+ assert rv.status_code == 200, rv.data
+
+ self._expunge_dashboards_and_charts()
+ model: Dashboard = db.session.get(Dashboard, dashboard_id)
+ stored: dict[str, Any] = json.loads(model.position_json)
+ assert stored["CHART-archived"]["type"] == "CHART"
+ assert stored["CHART-archived"]["meta"]["chartId"] == archived_id
+ member_ids: set[int] = {
+ row[0]
+ for row in
db.session.query(dashboard_slices.c.slice_id).filter(
+ dashboard_slices.c.dashboard_id == dashboard_id
+ )
+ }
+ assert member_ids == {live_id, archived_id}
+
+ rv = self.client.post(f"api/v1/chart/{archived_uuid}/restore")
+ assert rv.status_code == 200, rv.data
+ self._expunge_dashboards_and_charts()
+ model = db.session.get(Dashboard, dashboard_id)
+ assert {s.id for s in model.slices} == {live_id, archived_id}
+ finally:
+ db.session.rollback()
+ self.client.post(f"api/v1/chart/{archived_uuid}/restore")
+ self._expunge_dashboards_and_charts()
+ with skip_visibility_filter(db.session, Slice):
+ db.session.delete(db.session.get(Dashboard, dashboard_id))
+ for chart_id in (live_id, archived_id):
+ db.session.delete(db.session.get(Slice, chart_id))
+ db.session.commit()
+
def test_update_dashboard_reconciles_dangling_position_json(self) -> None:
"""PUT: a raw ``position_json`` layout node referencing a chart absent
from every Slice row is persisted as a markdown placeholder
(sc-115325).
diff --git a/tests/unit_tests/dao/dashboard_test.py
b/tests/unit_tests/dao/dashboard_test.py
index f3291b633aa..2f9d415d7b0 100644
--- a/tests/unit_tests/dao/dashboard_test.py
+++ b/tests/unit_tests/dao/dashboard_test.py
@@ -19,7 +19,7 @@ from typing import Any
from unittest.mock import patch
import pytest
-from sqlalchemy import event, inspect
+from sqlalchemy import event, inspect, select
from sqlalchemy.orm.session import Session
from superset import db, security_manager
@@ -27,12 +27,13 @@ from superset.commands.dashboard.exceptions import
DashboardInvalidError
from superset.commands.dashboard.update import UpdateDashboardCommand
from superset.connectors.sqla.models import Database, SqlaTable
from superset.daos.dashboard import (
+ _install_membership_baseline,
_layout_chart_id,
DashboardDAO,
reconcile_position_json,
)
from superset.dashboards.filter_scope import derive_metadata_scopes
-from superset.models.dashboard import Dashboard
+from superset.models.dashboard import Dashboard, dashboard_slices
from superset.models.helpers import skip_visibility_filter
from superset.models.slice import Slice
from superset.subjects.models import Subject
@@ -97,10 +98,13 @@ def test_set_dash_metadata_preserves_soft_deleted_members(
db.session.add_all([live_chart, trashed_chart, dashboard])
db.session.flush()
- # Production shape: the PUT flow loads a fresh Dashboard whose
- # ``slices`` collection is unloaded; expiring forces the baseline
- # reload through the visibility listener during the assignment.
- db.session.expire(dashboard, ["slices"])
+ # Production shape: the PUT flow loads the Dashboard through a query,
+ # which carries the visibility filter into the lazy load of its
+ # ``slices`` baseline. Expiring the collection alone is not enough: an
+ # object built in Python carries no filter into its lazy loads.
+ dashboard_id: int = dashboard.id
+ db.session.expunge(dashboard)
+ dashboard = db.session.query(Dashboard).filter_by(id=dashboard_id).one()
positions: dict[str, dict[str, Any]] = {
"CHART-live": {
@@ -129,10 +133,102 @@ def
test_set_dash_metadata_preserves_soft_deleted_members(
"set_dash_metadata must bypass the visibility filter when "
"resolving incoming chart ids"
)
+ # An archived chart that has a layout slot is listed once, not twice.
+ assert [chart.id for chart in dashboard.slices] == [
+ live_chart.id,
+ trashed_chart.id,
+ ]
# And the position slot kept its UUID rather than being nulled.
assert positions["CHART-trashed"]["meta"]["uuid"] ==
str(trashed_chart.uuid)
[email protected]("previously_in_layout", [False, True])
+@with_feature_flags(SOFT_DELETE=True)
+def test_set_dash_metadata_preserves_archived_member_missing_from_layout(
+ session: Session, previously_in_layout: bool
+) -> None:
+ """A layout save keeps omitted archived members but removes omitted live
ones."""
+ Dashboard.metadata.create_all(session.get_bind())
+ live: Slice = Slice(slice_name="layout_member", datasource_type="table")
+ archived: Slice = Slice(
+ slice_name="archived_member",
+ datasource_type="table",
+ deleted_at=datetime(2026, 1, 1, tzinfo=timezone.utc),
+ )
+ removed: Slice = Slice(slice_name="removed_member",
datasource_type="table")
+ positions: dict[str, dict[str, Any]] = {
+ "CHART-live": {"type": "CHART", "meta": {"chartId": None}}
+ }
+ dashboard: Dashboard = Dashboard(
+ dashboard_title="archived_outside_layout", slices=[live, archived,
removed]
+ )
+ db.session.add(dashboard)
+ db.session.flush()
+ positions["CHART-live"]["meta"]["chartId"] = live.id
+ stored_positions: dict[str, dict[str, Any]] = dict(positions)
+ if previously_in_layout:
+ stored_positions["CHART-archived"] = {
+ "type": "CHART",
+ "meta": {"chartId": archived.id},
+ }
+ dashboard.position_json = json.dumps(stored_positions)
+ # The layout save runs in a separate transaction from the initial
membership.
+ db.session.commit()
+ dashboard_id: int = dashboard.id
+ expected_ids: set[int] = {live.id, archived.id}
+ db.session.expunge(dashboard)
+ dashboard = db.session.query(Dashboard).filter_by(id=dashboard_id).one()
+
+ DashboardDAO.set_dash_metadata(dashboard, {"positions": positions})
+ db.session.flush()
+
+ member_ids: set[int] = set(
+ db.session.scalars(
+ select(dashboard_slices.c.slice_id).where(
+ dashboard_slices.c.dashboard_id == dashboard_id
+ )
+ )
+ )
+ assert member_ids == expected_ids
+ assert json.loads(dashboard.position_json) == positions
+
+
+def test_install_membership_baseline_skips_transient_dashboard(
+ session: Session,
+) -> None:
+ """A dashboard with no stored rows keeps its in-memory chart list."""
+ Dashboard.metadata.create_all(session.get_bind())
+ chart: Slice = Slice(slice_name="transient_chart", datasource_type="table")
+ db.session.add(chart)
+ db.session.flush()
+ dashboard: Dashboard = Dashboard(dashboard_title="transient_dash",
slices=[chart])
+
+ _install_membership_baseline(dashboard)
+
+ assert dashboard.slices == [chart]
+ assert inspect(dashboard).attrs.slices.history.added == [chart]
+
+
+def test_install_membership_baseline_keeps_unflushed_change(
+ session: Session,
+) -> None:
+ """A pending change to the chart list is kept, not replaced by the
+ stored baseline, when autoflush cannot flush it first."""
+ Dashboard.metadata.create_all(session.get_bind())
+ kept: Slice = Slice(slice_name="kept_chart", datasource_type="table")
+ added: Slice = Slice(slice_name="added_chart", datasource_type="table")
+ dashboard: Dashboard = Dashboard(dashboard_title="pending_dash",
slices=[kept])
+ db.session.add_all([kept, added, dashboard])
+ db.session.flush()
+
+ with db.session.no_autoflush:
+ dashboard.slices.append(added)
+ _install_membership_baseline(dashboard)
+
+ assert dashboard.slices == [kept, added]
+ assert inspect(dashboard).attrs.slices.history.added == [added]
+
+
def test_set_dash_metadata_preserves_refresh_frequency(session: Session) ->
None:
"""set_dash_metadata must not reset refresh_frequency when absent from
data.
