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></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> [](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