rusackas commented on code in PR #44256:
URL: https://github.com/apache/superset/pull/44256#discussion_r4129630194
##########
superset/dashboards/filters.py:
##########
@@ -220,12 +220,14 @@ def _apply_viewers(self, query: Query) -> Query:
and_(
Dashboard.published.is_(True),
~dashboard_has_viewers,
- get_dataset_access_filters(
- Slice,
- security_manager.can_access_all_datasources(),
- layer_grant_clause,
+ or_(
+ ~Dashboard.slices.any(),
+ get_dataset_access_filters(
+ Slice,
+ security_manager.can_access_all_datasources(),
+ layer_grant_clause,
),
- )
+ ),
)
)
Review Comment:
This got rebased pretty heavily since (the join logic moved into shared
helpers for sc-119501/sc-120032), so the exact suggestion doesn't apply
verbatim anymore, but the missing paren is gone as of 58d2c0c.
##########
superset/dashboards/filters.py:
##########
@@ -220,13 +220,15 @@ def _apply_viewers(self, query: Query) -> Query:
and_(
Dashboard.published.is_(True),
~dashboard_has_viewers,
+ or_(
+ ~Dashboard.slices.any(),
Review Comment:
Fixed via the outer join's `Slice.id.is_(None)` arm instead of the EXISTS,
so it agrees with the join on which slices actually exist. Added a regression
test for it too
(`test_list_filter_hides_dashboard_with_only_soft_deleted_charts`) in 58d2c0c.
--
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]
