rusackas commented on code in PR #43900:
URL: https://github.com/apache/superset/pull/43900#discussion_r4163858276
##########
superset/mcp_service/dashboard/tool/manage_dashboard_roles.py:
##########
@@ -171,12 +171,13 @@ def manage_dashboard_roles(
Add or remove dashboard access roles with explicit operations.
Dashboard access roles restrict who can view a dashboard to members of
- the listed roles, on top of normal Superset permissions. An empty roles
- list means "no role restriction" — the dashboard is visible per standard
- permissions instead. This only takes effect when the ``ENABLE_VIEWERS``
- feature flag is enabled; the response's ``viewers_enabled`` field
- reports whether it is, and ``warnings`` notes when a change was applied
- but has no live effect.
+ the listed roles, on top of normal Superset permissions. Removing every
+ role only restores standard permissions if no USER- or GROUP-type
+ viewers remain on the dashboard — ``raise_for_access`` restricts access
+ whenever the ``viewers`` list is nonempty, regardless of subject type.
+ This only takes effect when the ``ENABLE_VIEWERS`` feature flag is
+ enabled; the response's ``viewers_enabled`` field reports whether it is,
+ and ``warnings`` notes when a change was applied but has no live effect.
Review Comment:
@aminghadersohi good catch. Fixed the docstring and the flag-off warning to
say role changes apply regardless of `ENABLE_VIEWERS`, and dropped the
"disguised directory lookup" line in both the roles and owners tool docstrings.
--
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]
