rusackas commented on PR #41404:
URL: https://github.com/apache/superset/pull/41404#issuecomment-4827283778

   Piggybacking on @sha174n's review here with a different angle... I think 
this is aimed at the wrong copy. The `serialize_role_object` in 
`dashboard/schemas.py` doesn't actually have any callers, it's a leftover 
duplicate from the big MCP consolidation in #35877. The one MCP really uses 
lives in `role/schemas.py` (via `get_role_info` and `list_roles`).
   
   And since the dashboard path strips roles entirely (your own testing 
confirms it), the serialization failure this fixes can't really happen there to 
begin with.
   
   If there's a real `PermissionViewMenu` problem, it'd be in the 
`role/schemas.py` version, which filters with `hasattr(p, "name")` and would 
quietly drop FAB perms that only expose `.permission`/`.view_menu`. Might be 
worth redirecting the fix there, or folding the two copies into one so we're 
not maintaining both. Curious too whether roles are even meant to be exposed on 
the dashboard path... if not, the dead one could probably just go. Thoughts?


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

Reply via email to