Copilot commented on code in PR #43499:
URL: https://github.com/apache/superset/pull/43499#discussion_r3849615252
##########
superset/models/sql_lab.py:
##########
@@ -551,6 +551,15 @@ 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.
+ """
+ return self.label
Review Comment:
This only fixes the first attribute lookup. For a non-admin request,
`CreateChartCommand`/`UpdateChartCommand` immediately call
`raise_for_access(datasource=saved_query)`, whose datasource branch evaluates
`datasource.perm` (and may later use `datasource.data`); `SavedQuery` defines
neither, so the real API still fails with an `AttributeError` after this
property is added. Please route saved queries through the query-specific access
check or implement the complete access contract, and add a regression test
without mocking `raise_for_access`.
##########
superset/models/sql_lab.py:
##########
@@ -551,6 +551,15 @@ 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.
+ """
+ return self.label
Review Comment:
Chart persistence has another unconditional `SavedQuery` interface
requirement that this property does not address: `Slice`'s `before_insert` and
`before_update` listener resolves the configured datasource and reads
`ds.perm`, `ds.catalog_perm`, and `ds.schema_perm`
(`superset/models/slice.py:407-429`). `SavedQuery` has none of these
attributes, so after validation is fixed, creating or updating a saved-query
chart will still fail during flush with `AttributeError`. Handle `saved_query`
in that listener (or give it the required permission properties) and add an
actual create/update persistence regression test.
--
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]