betodealmeida commented on code in PR #34319:
URL: https://github.com/apache/superset/pull/34319#discussion_r2252409285


##########
superset/security/manager.py:
##########
@@ -2401,6 +2428,32 @@ def raise_for_access(  # noqa: C901
                             and slc in dashboard_.slices
                             and slc.datasource == datasource
                         )
+                        or (

Review Comment:
   Oh my, what is this monster expression stating on line 2379?!? 😱
   
   We should break this down into methods that are self-explanatotry, like:
   
   ```python
   return (
       self.is_d2d_allowed(form_data) or
       self.is_dashbaord_rbac(form_data) or
       ...
   )
   ```
   
   (Not saying _you_ should do it, but you could start by moving the code 
you're adding into a method and call that instead here.)



##########
superset-frontend/src/components/Chart/ChartContextMenu/ChartContextMenu.tsx:
##########
@@ -151,6 +166,75 @@ const ChartContextMenu = (
     canDrillBy &&
     isDisplayed(ContextMenuItem.DrillBy);
 
+  useEffect(() => {
+    async function fetchDataset() {
+      if (!visible || dataset || (!showDrillBy && !showDrillToDetail)) return;
+
+      const datasetId = Number(formData.datasource.split('__')[0]);
+      try {
+        setIsLoadingDataset(true);
+        let response;
+
+        if (loadDrillByOptionsExtension) {
+          response = await loadDrillByOptionsExtension(datasetId, formData);
+        } else {
+          const endpoint = 
`/api/v1/dataset/${datasetId}/drill_info/?q=(dashboard_id:${dashboardId})`;
+          response = await cachedSupersetGet({ endpoint });
+        }
+
+        const { json } = response;
+        const { result } = json;
+
+        setDataset(result);
+      } catch (error) {
+        logging.error('Failed to load dataset:', error);
+        supersetGetCache.delete(`/api/v1/dataset/${datasetId}/drill_info/`);
+      } finally {
+        setIsLoadingDataset(false);
+      }
+    }
+
+    fetchDataset();
+  }, [
+    visible,

Review Comment:
   Don't we need to add `dataset` here as well?



-- 
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-unsubscr...@superset.apache.org

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


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

Reply via email to