aminghadersohi commented on code in PR #43900:
URL: https://github.com/apache/superset/pull/43900#discussion_r4163173276
##########
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:
Still says the flag gates enforcement, contradicting the sentence above and
mcp.md: `raise_for_access` restricts on any nonempty `viewers` with
`ENABLE_VIEWERS` off. This is the LLM-facing tool description, and the flag-off
warning at L224-228 makes the same "no effect" claim.
```suggestion
This applies regardless of the ``ENABLE_VIEWERS`` feature flag; the
response's ``viewers_enabled`` field reports the flag's state, and
``warnings`` notes when it is disabled.
```
--
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]
