Remove hybrid property, add displayed_name property

lib/galaxy/managers/roles.py

@@ -6,6 +6,7 @@
 from typing import List
 
 from sqlalchemy import (
+    and_,
     false,
     select,
 )
@@ -71,10 +72,22 @@ def get(self, trans: ProvidesUserContext, role_id: int) -> model.Role:
 
     def list_displayable_roles(self, trans: ProvidesUserContext) -> List[Role]:
         roles = []
-        stmt = select(Role).where(Role.deleted == false())
-        for role in trans.sa_session.scalars(stmt):
+        stmt = (
+            select(Role, model.User.email)
+            .outerjoin(
+                model.UserRoleAssociation,
+                and_(Role.id == model.UserRoleAssociation.role_id, Role.type == Role.types.PRIVATE),
+            )
+            .outerjoin(model.User)
+            .where(Role.deleted == false())
+        )
+
+        for role, user_email in trans.sa_session.execute(stmt):
             if trans.user_is_admin or trans.app.security_agent.ok_to_display(trans.user, role):
                 roles.append(role)
+                if role.type == Role.types.PRIVATE:
+                    assert user_email, "Did not find user for private role {role}"
+                    role.displayed_name = user_email
         return roles
 
     def create_role(self, trans: ProvidesUserContext, role_definition_model: RoleDefinitionModel) -> model.Role:
@@ -83,11 +96,7 @@ def create_role(self, trans: ProvidesUserContext, role_definition_model: RoleDef
         user_ids = role_definition_model.user_ids or []
         group_ids = role_definition_model.group_ids or []
 
-        stmt = (
-            select(Role)
-            .where(Role.name == name)  # type:ignore[arg-type,comparison-overlap]  # Role.name is a SA hybrid property
-            .limit(1)
-        )
+        stmt = select(Role).where(Role.name == name).limit(1)
         if trans.sa_session.scalars(stmt).first():
             raise Conflict(f"A role with that name already exists [{name}]")
 

lib/galaxy/model/__init__.py

@@ -109,7 +109,6 @@
     association_proxy,
     AssociationProxy,
 )
-from sqlalchemy.ext.hybrid import hybrid_property
 from sqlalchemy.ext.orderinglist import ordering_list
 from sqlalchemy.orm import (
     aliased,
@@ -3798,7 +3797,7 @@ class Role(Base, Dictifiable, RepresentById):
     id: Mapped[int] = mapped_column(primary_key=True)
     create_time: Mapped[datetime] = mapped_column(default=now, nullable=True)
     update_time: Mapped[datetime] = mapped_column(default=now, onupdate=now, nullable=True)
-    _name: Mapped[str] = mapped_column("name", String(255), index=True)
+    name: Mapped[str] = mapped_column(String(255), index=True)
     description: Mapped[Optional[str]] = mapped_column(TEXT)
     type: Mapped[Optional[str]] = mapped_column(String(40), index=True)
     deleted: Mapped[Optional[bool]] = mapped_column(index=True, default=False)
@@ -3810,6 +3809,10 @@ class Role(Base, Dictifiable, RepresentById):
     dict_element_visible_keys = ["id", "name", "description", "type"]
     private_id = None
 
+    # For generic names, we may want to display a more descriptive name, derived from role
+    # associations and generated at runtime. This value is not persisted.
+    _displayed_name = None
+
     class types(str, Enum):
         PRIVATE = "private"
         SYSTEM = "system"
@@ -3821,18 +3824,13 @@ class types(str, Enum):
     def default_name(role_type):
         return f"{role_type.value} role"
 
-    @hybrid_property
-    def name(self):
-        if self.type == Role.types.PRIVATE:
-            user_assocs = self.users
-            assert len(user_assocs) == 1, f"Did not find exactly one user for private role {self}"
-            return user_assocs[0].user.email
-        else:
-            return self._name
+    @property
+    def displayed_name(self):
+        return self._displayed_name or self.name
 
-    @name.setter  # type:ignore[no-redef]  # property setter
-    def name(self, name):
-        self._name = name
+    @displayed_name.setter
+    def displayed_name(self, value):
+        self._displayed_name = value
 
     def __init__(self, name=None, description=None, type=types.SYSTEM, deleted=False):
         self.name = name or Role.default_name(type)