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]

Reply via email to