fix(charts): make SavedQuery a working chart datasource, not just past the first AttributeError

CreateChartCommand/UpdateChartCommand.validate() only reads
datasource.name; SavedQuery still crashed one step later for
non-admin callers (SecurityManager.raise_for_access needs .perm and
.data) and for every caller at flush time (Slice's
before_insert/before_update listener copies .perm/.catalog_perm/
.schema_perm onto the chart unconditionally). Add those properties
plus a None-safe fallback for name/perm when label is unset, and
test the real (unmocked) raise_for_access and set_related_perm paths
so a saved_query chart can actually be created end to end.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

superset/models/sql_lab.py

@@ -551,6 +551,66 @@ class SavedQuery(
             "id": self.id,
         }
 
+    @property
+    def name(self) -> str:
+        """
+        Expose ``label`` as ``name`` so callers that treat a ``SavedQuery`` as
+        a generic datasource (e.g. chart create/update commands) can rely on
+        a uniform ``name`` attribute across all datasource types.
+
+        ``label`` is a nullable column, so fall back to an id-based name
+        rather than returning ``None`` from a property typed as ``str``.
+        """
+        return self.label or f"Saved query {self.id}"
+
+    @property
+    def schema_perm(self) -> Optional[str]:
+        """
+        Schema-level permission string, mirroring ``Query.schema_perm``.
+
+        Required so that ``SecurityManager.raise_for_access(datasource=...)``
+        and the ``Slice`` ``before_insert``/``before_update`` listener (which
+        copies ``perm``/``catalog_perm``/``schema_perm`` onto the chart) can
+        treat a ``SavedQuery`` like any other datasource instead of raising
+        ``AttributeError``.
+        """
+        return f"{self.database.database_name}.{self.schema}"
+
+    @property
+    def catalog_perm(self) -> Optional[str]:
+        """Catalog-level permission string; see ``schema_perm`` above."""
+        return security_manager.get_catalog_perm(
+            self.database.database_name, self.catalog
+        )
+
+    @property
+    def perm(self) -> str:
+        """Object-level permission string; see ``schema_perm`` above."""
+        return f"[{self.database.database_name}].[{self.name}](id:{self.id})"
+
+    @property
+    def data(self) -> ExplorableData:
+        """
+        Minimal explorable payload.
+
+        ``SecurityManager`` builds access-denied error messages (and a few
+        dashboard-RBAC lookups) off ``datasource.data["id"]``/``["name"]``
+        for *every* datasource type it can be asked to authorize, not just
+        ones that are actually explorable; without this a denied saved-query
+        chart request fails with ``AttributeError`` instead of the intended
+        403.
+        """
+        result: ExplorableData = {
+            "id": self.id,
+            "name": self.name,
+            "type": "saved_query",
+            "schema": self.schema,
+            "catalog": self.catalog,
+        }
+        if self.database:
+            result["database"] = {"id": self.db_id, "backend": self.database.backend}
+        return result
+
     @property
     def pop_tab_link(self) -> Markup:
         return Markup(

tests/unit_tests/commands/chart/test_access_checks.py

@@ -20,9 +20,12 @@ from unittest.mock import MagicMock, patch
 
 import pytest
 
+from superset import security_manager
 from superset.commands.chart.exceptions import ChartForbiddenError
 from superset.errors import ErrorLevel, SupersetError, SupersetErrorType
 from superset.exceptions import SupersetSecurityException
+from superset.models.core import Database
+from superset.models.sql_lab import SavedQuery
 
 
 def _security_exception() -> SupersetSecurityException:
@@ -100,6 +103,129 @@ def test_create_chart_command_allowed_when_access_passes() -> None:
                     command.validate()  # should not raise
 
 
+def test_create_chart_command_supports_saved_query_datasource() -> None:
+    """CreateChartCommand.validate() must populate ``datasource_name`` for a
+    ``saved_query`` datasource, which exposes ``label`` rather than ``name``.
+
+    Regression test for https://github.com/apache/superset/issues/29697.
+    """
+    from superset.commands.chart.create import CreateChartCommand
+
+    saved_query = SavedQuery(label="My saved query")
+
+    with patch(
+        "superset.commands.chart.create.get_datasource_by_id",
+        return_value=saved_query,
+    ):
+        with patch("superset.commands.chart.create.security_manager.raise_for_access"):
+            with patch(
+                "superset.commands.chart.create.populate_subjects",
+                return_value=[],
+            ):
+                with patch(
+                    "superset.commands.chart.create.DashboardDAO.find_by_ids",
+                    return_value=[],
+                ):
+                    command = CreateChartCommand(
+                        {
+                            "slice_name": "test",
+                            "viz_type": "bar",
+                            "datasource_id": 1,
+                            "datasource_type": "saved_query",
+                        }
+                    )
+                    command.validate()  # should not raise AttributeError
+
+    assert command._properties["datasource_name"] == "My saved query"
+
+
+def test_saved_query_exposes_perm_properties() -> None:
+    """``SavedQuery`` must expose ``perm``/``schema_perm``/``catalog_perm`` so
+    it can stand in for a generic datasource: ``Slice``'s ``before_insert``/
+    ``before_update`` listener (``superset/models/slice.py::set_related_perm``)
+    unconditionally reads all three off the resolved datasource when
+    persisting a chart, regardless of ``datasource_type``.
+    """
+    database = Database(database_name="my_db", sqlalchemy_uri="sqlite://")
+    saved_query = SavedQuery(
+        id=1, label="My saved query", schema="main", database=database
+    )
+
+    assert saved_query.perm == "[my_db].[My saved query](id:1)"
+    assert saved_query.schema_perm == "my_db.main"
+    assert saved_query.catalog_perm is None
+
+
+def test_saved_query_name_falls_back_when_label_is_none() -> None:
+    """``label`` is a nullable column; ``name`` (typed ``str``) must not
+    return ``None`` for a saved query that was persisted without one.
+    """
+    saved_query = SavedQuery(id=7, label=None)
+
+    assert saved_query.name == "Saved query 7"
+
+
+def test_raise_for_access_does_not_crash_on_saved_query_datasource(
+    app_context: None,
+) -> None:
+    """``SecurityManager.raise_for_access(datasource=...)`` must not raise
+    ``AttributeError`` for a ``saved_query`` datasource.
+
+    Regression test for the ``datasource.perm`` lookup performed in the
+    non-admin branch of ``raise_for_access`` (previously unreachable for
+    ``SavedQuery`` since it had no ``perm`` attribute at all). Called for
+    real, without mocking ``raise_for_access`` itself, per the follow-up
+    review on https://github.com/apache/superset/issues/29697.
+    """
+    from flask import g
+
+    database = Database(database_name="my_db", sqlalchemy_uri="sqlite://")
+    saved_query = SavedQuery(
+        id=1, label="My saved query", schema="main", database=database
+    )
+
+    non_admin_user = MagicMock(is_anonymous=False, id=99, roles=[])
+    g.user = non_admin_user
+    try:
+        with patch.object(
+            security_manager, "can_access_all_datasources", return_value=False
+        ):
+            with patch.object(security_manager, "can_access", return_value=False):
+                # A denied, non-admin caller must be rejected with the expected
+                # security exception, not an AttributeError from a missing
+                # `.perm`/`.schema_perm` attribute on SavedQuery.
+                with pytest.raises(SupersetSecurityException):
+                    security_manager.raise_for_access(datasource=saved_query)
+    finally:
+        del g.user
+
+
+def test_slice_set_related_perm_does_not_crash_on_saved_query_datasource() -> None:
+    """``Slice``'s ``before_insert``/``before_update`` listener
+    (``set_related_perm``) unconditionally reads ``perm``/``catalog_perm``/
+    ``schema_perm`` off the resolved datasource for *every* chart flush,
+    independent of ``datasource_type`` — it must not crash for a
+    ``saved_query``-backed chart either.
+    """
+    from superset.models.slice import set_related_perm, Slice
+
+    database = Database(database_name="my_db", sqlalchemy_uri="sqlite://")
+    saved_query = SavedQuery(
+        id=1, label="My saved query", schema="main", database=database
+    )
+    slice_ = Slice(datasource_type="saved_query", datasource_id=1)
+
+    with patch("superset.models.slice.db") as mock_db:
+        mock_db.session.query.return_value.filter_by.return_value.first.return_value = (
+            saved_query  # noqa: E501
+        )
+        set_related_perm(MagicMock(), MagicMock(), slice_)
+
+    assert slice_.perm == "[my_db].[My saved query](id:1)"
+    assert slice_.schema_perm == "my_db.main"
+    assert slice_.catalog_perm is None
+
+
 def test_create_chart_command_delegates_editors_to_subjects() -> None:
     """CreateChartCommand.validate() must resolve editor subject IDs."""
     from superset.commands.chart.create import CreateChartCommand