[GitHub] [superset] hughhhh commented on a diff in pull request #19981: feat!: pass datasource_type and datasource_id to form_data

[GitBox <gi...@apache.org>]

hughhhh commented on code in PR #19981:
URL: https://github.com/apache/superset/pull/19981#discussion_r867278462


##########
superset/explore/form_data/commands/delete.py:
##########
@@ -46,14 +48,15 @@ def run(self) -> bool:
                 key
             )
             if state:
-                dataset_id = state["dataset_id"]
-                chart_id = state["chart_id"]
-                check_access(dataset_id, chart_id, actor)
+                datasource_id: int = state["datasource_id"]
+                chart_id: Optional[int] = state["chart_id"]
+                datasource_type: str = state["datasource_type"]
+                check_chart_access(datasource_id, chart_id, actor, datasource_type)
                 if state["owner"] != actor.get_user_id():
                     raise TemporaryCacheAccessDeniedError()
                 tab_id = self._cmd_params.tab_id
                 contextual_key = cache_key(
-                    session.get("_id"), tab_id, dataset_id, chart_id
+                    session.get("_id"), tab_id, datasource_id, chart_id

Review Comment:
   wouldn't we need to add `datasource_type` or we would get the same cache hit for datasource_id but different datasource_type?
   
   ```suggestion
                       session.get("_id"), tab_id, datasource_id, datasource_id, chart_id
   ```



-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org