korbit-ai[bot] commented on code in PR #32289:
URL: https://github.com/apache/superset/pull/32289#discussion_r1959817729


##########
superset/security/manager.py:
##########
@@ -2289,66 +2289,46 @@ def raise_for_access(  # noqa: C901
                 form_data = viz.form_data
 
             assert datasource
-
-            if not (
-                self.can_access_schema(datasource)
-                or self.can_access("datasource_access", datasource.perm or "")
-                or self.is_owner(datasource)
-                or (
-                    # Grant access to the datasource only if dashboard RBAC is 
enabled
-                    # or the user is an embedded guest user with access to the 
dashboard
-                    # and said datasource is associated with the dashboard 
chart in
-                    # question.
-                    form_data
-                    and (dashboard_id := form_data.get("dashboardId"))
-                    and (
-                        dashboard_ := self.get_session.query(Dashboard)
-                        .filter(Dashboard.id == dashboard_id)
-                        .one_or_none()
-                    )
-                    and (
-                        (is_feature_enabled("DASHBOARD_RBAC") and 
dashboard_.roles)
-                        or (
-                            is_feature_enabled("EMBEDDED_SUPERSET")
-                            and self.is_guest_user()
-                        )
-                    )
-                    and (
-                        (
-                            # Native filter.
-                            form_data.get("type") == "NATIVE_FILTER"
-                            and (native_filter_id := 
form_data.get("native_filter_id"))
-                            and dashboard_.json_metadata
-                            and (json_metadata := 
json.loads(dashboard_.json_metadata))
-                            and any(
-                                target.get("datasetId") == datasource.id
-                                for fltr in json_metadata.get(
-                                    "native_filter_configuration",
-                                    [],
-                                )
-                                for target in fltr.get("targets", [])
-                                if native_filter_id == fltr.get("id")
-                            )
-                        )
-                        or (
-                            # Chart.
-                            form_data.get("type") != "NATIVE_FILTER"
-                            and (slice_id := form_data.get("slice_id"))
-                            and (
-                                slc := self.get_session.query(Slice)
-                                .filter(Slice.id == slice_id)
-                                .one_or_none()
-                            )
-                            and slc in dashboard_.slices
-                            and slc.datasource == datasource
+                
+            can_access_schema = self.can_access_schema(datasource)
+            can_access_datasource_permission = 
self.can_access("datasource_access", datasource.perm or "")
+            is_owner = self.is_owner(datasource)
+            # Grant access to the datasource only if dashboard RBAC is enabled
+            # or the user is an embedded guest user with access to the 
dashboard
+            # and said datasource is associated with the dashboard chart in
+            # question.
+            dashboard_accessible = False
+            dashboard_ = None
+
+            if form_data and (dashboard_id := form_data.get("dashboardId")):
+                dashboard_ = 
self.get_session.query(Dashboard).filter(Dashboard.id == 
dashboard_id).one_or_none()

Review Comment:
   ### Unclear variable assignment pattern <sub>![category 
Readability](https://img.shields.io/badge/Readability-0284c7)</sub>
   
   <details>
     <summary>Tell me more</summary>
   
   ###### What is the issue?
   The use of walrus operator (:=) combined with underscore suffix naming makes 
the code less immediately readable.
   
   ###### Why this matters
   While the walrus operator can be useful, combining it with underscore suffix 
naming reduces code clarity at first glance.
   
   ###### Suggested change ∙ *Feature Preview*
   ```python
   dashboard_id = form_data.get("dashboardId") if form_data else None
   if dashboard_id:
       dashboard = self.get_session.query(Dashboard).filter(Dashboard.id == 
dashboard_id).one_or_none()
   ```
   
   
   </details>
   
   <sub>
   
   [![Report a problem with this 
comment](https://img.shields.io/badge/Report%20a%20problem%20with%20this%20comment-gray.svg?logo=data:image/svg+xml;base64,PHN2ZyB4bWxucz0iaHR0cDovL3d3dy53My5vcmcvMjAwMC9zdmciIHdpZHRoPSIyNCIgaGVpZ2h0PSIyNCIgdmlld0JveD0iMCAwIDI0IDI0IiBmaWxsPSJub25lIiBzdHJva2U9IiNmNWVjMDAiIHN0cm9rZS13aWR0aD0iMiIgc3Ryb2tlLWxpbmVjYXA9InJvdW5kIiBzdHJva2UtbGluZWpvaW49InJvdW5kIiBjbGFzcz0ibHVjaWRlIGx1Y2lkZS10cmlhbmdsZS1hbGVydCI+PHBhdGggZD0ibTIxLjczIDE4LTgtMTRhMiAyIDAgMCAwLTMuNDggMGwtOCAxNEEyIDIgMCAwIDAgNCAyMWgxNmEyIDIgMCAwIDAgMS43My0zIi8+PHBhdGggZD0iTTEyIDl2NCIvPjxwYXRoIGQ9Ik0xMiAxN2guMDEiLz48L3N2Zz4=)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/f4489a93-a7ba-48c9-aa78-2789f9e0055e?suggestedFixEnabled=true)
   
   💬 Chat with Korbit by mentioning @korbit-ai.
   </sub>
   
   <!--- korbi internal id:eaf7e006-4457-4b18-a6b6-da33e77ff414 -->
   



-- 
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