bito-code-review[bot] commented on PR #44339:
URL: https://github.com/apache/superset/pull/44339#issuecomment-5694040261

   <!-- Bito Reply -->
   The current implementation of `_role_names` is defined within 
`superset/mcp_service/system/schemas.py` and is indeed scoped only to the 
`serialize_user_object` function in that same file. As you noted, other 
services like `superset.mcp_service.user.schemas.serialize_user_object` 
continue to use their own logic, which only reads `user.roles` and does not 
account for group-based roles.
   
   To ensure consistent role serialization across the application, you should 
move `_role_names` to a shared utility module (e.g., 
`superset/mcp_service/utils.py` or a common schema utility file) and update the 
other serialization functions to import and use this shared helper. 
Additionally, you should add unit tests for those other serialization functions 
that specifically cover users with group-assigned roles to verify the fix.
   
   **superset/mcp_service/system/schemas.py**
   ```
   def _role_names(user: Any) -> list[str]:
       """Return the names of every role the user holds, direct or through a 
group."""
       names: list[str] = []
       def add(roles: Any) -> None:
           for role in roles or []:
               name = getattr(role, "name", None)
               if name is not None and name not in names:
                   names.append(name)
       # ... (logic to add direct and group roles)
   ```


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