rusackas commented on code in PR #43499:
URL: https://github.com/apache/superset/pull/43499#discussion_r4289253553
##########
superset/models/sql_lab.py:
##########
@@ -602,6 +602,68 @@ def to_dict(self) -> dict[str, Any]:
"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 security_manager.get_schema_perm(
+ self.database.database_name, self.catalog, self.schema
+ )
Review Comment:
Good catch, fixed. `schema_perm` now falls back to the database's default
catalog the same way `Query.schema_perm` does, so it builds the same
`[db].[catalog].[schema]` string a schema grant actually matches.
##########
superset/models/sql_lab.py:
##########
@@ -602,6 +602,68 @@ def to_dict(self) -> dict[str, Any]:
"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 security_manager.get_schema_perm(
+ self.database.database_name, self.catalog, 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
Review Comment:
Good catch. Added a `before_update` listener on `SavedQuery` that re-syncs
`perm`/`catalog_perm`/`schema_perm` onto its dependent charts when the query
moves, same as `semantic_view_before_update` does for views.
##########
tests/unit_tests/commands/chart/update_test.py:
##########
@@ -409,19 +409,18 @@ def
test_update_chart_query_context_without_datasource_is_allowed(
).validate()
[email protected]("datasource_type", ["saved_query", "query", "bogus"])
[email protected]("datasource_type", ["query", "bogus"])
Review Comment:
Added a couple of `update_test.py` cases with a real `SavedQuery`, covering
the effective-id lookup, `datasource_name`, and the forbidden path.
--
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]
