mikebridge commented on code in PR #45538:
URL: https://github.com/apache/superset/pull/45538#discussion_r4383646728
##########
tests/unit_tests/dao/dashboard_test.py:
##########
@@ -229,6 +232,108 @@ def
test_install_membership_baseline_keeps_unflushed_change(
assert inspect(dashboard).attrs.slices.history.added == [added]
+@with_feature_flags(SOFT_DELETE=True)
+def test_set_dash_metadata_keeps_only_stored_archived_members(
+ session: Session,
+) -> None:
+ """Archived charts kept outside the layout come from the stored rows, so
+ an unflushed archived addition is not linked past the access check."""
+ Dashboard.metadata.create_all(session.get_bind())
+ archived_at: datetime = datetime(2026, 1, 1, tzinfo=timezone.utc)
+ live: Slice = Slice(slice_name="stored_live", datasource_type="table")
+ stored: Slice = Slice(
+ slice_name="stored_archived", datasource_type="table",
deleted_at=archived_at
+ )
+ pending: Slice = Slice(
+ slice_name="pending_archived", datasource_type="table",
deleted_at=archived_at
+ )
+ dashboard: Dashboard = Dashboard(
+ dashboard_title="stored_archived_dash", slices=[live, stored]
+ )
+ db.session.add_all([live, stored, pending, dashboard])
+ db.session.commit()
+ positions: dict[str, dict[str, Any]] = {
+ "CHART-live": {"type": "CHART", "meta": {"chartId": live.id}}
+ }
+
+ with db.session.no_autoflush:
+ dashboard.slices.append(pending)
+ 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 == {live.id, stored.id}
+
+
+@with_feature_flags(SOFT_DELETE=True)
+def test_set_dash_metadata_checks_new_charts_beside_archived_member(
+ session: Session,
+) -> None:
+ """With an archived member outside the layout, a new unreadable chart is
+ refused against the stored rows, and a new readable chart is linked
+ alongside the kept archived member, which is never access-checked."""
+ Dashboard.metadata.create_all(session.get_bind())
+ live: Slice = Slice(slice_name="mixed_live", datasource_type="table")
+ archived: Slice = Slice(
+ slice_name="mixed_archived",
+ datasource_type="table",
+ deleted_at=datetime(2026, 1, 1, tzinfo=timezone.utc),
+ )
+ readable: Slice = Slice(slice_name="mixed_readable",
datasource_type="table")
+ unreadable: Slice = Slice(slice_name="mixed_unreadable",
datasource_type="table")
Review Comment:
Good catch, that case wasn't covered. I added
`test_set_dash_metadata_checks_archived_chart_that_is_not_a_member`, which puts
an archived chart that isn't already a member, and that the caller can't read,
into the layout.
- The save raises `DashboardLayoutInvalidError` naming only that chart, and
the stored membership is unchanged.
- The code already handled it: layout charts are resolved with the
soft-delete filter bypassed, and the only exemption from the check is existing
membership, not `deleted_at`. So this is test-only.
- To confirm the test bites, I temporarily made the check skip charts with
`deleted_at` set: the new test fails, and it's the only test in the file that
does.
--
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]
