refactor: nest role fields in UserProjectMembership API response

ami/main/models.py

@@ -274,6 +274,23 @@ def ensure_owner_membership(self):
         if self.owner and not self.members.filter(id=self.owner.pk).exists():
             self.members.add(self.owner)
 
+    def members_with_roles(self):
+        """
+        Get users who are members of this project with a role assigned.
+
+        This filters out memberships where the user has no role groups assigned,
+        which indicates a data inconsistency.
+
+        Returns:
+            User queryset filtered to users with roles
+
+        Example:
+            project.members_with_roles()
+            project.members_with_roles().values_list('email', flat=True)
+        """
+        memberships_with_roles = self.project_memberships.with_role()
+        return User.objects.filter(project_memberships__in=memberships_with_roles).distinct()
+
     def deployments_count(self) -> int:
         return self.deployments.count()
 
@@ -489,6 +506,78 @@ class Meta:
         ]
 
 
+class UserProjectMembershipQuerySet(BaseQuerySet):
+    """Custom queryset for UserProjectMembership with role filtering."""
+
+    def with_role(self, log_invalid=False):
+        """
+        Filter memberships to only include users who have a role assigned.
+
+        Memberships without roles indicate a data inconsistency between the
+        UserProjectMembership table and permission groups (roles).
+
+        Args:
+            log_invalid: If True, log warnings for memberships without roles
+
+        Returns:
+            Queryset filtered to members with at least one role for their project
+
+        Example:
+            # In a ViewSet:
+            UserProjectMembership.objects.filter(project=project).with_role(log_invalid=True)
+
+            # In a management command:
+            UserProjectMembership.objects.filter(project=project).with_role()
+        """
+        from django.contrib.auth.models import Group
+
+        # TODO: Once we migrate to FK-based role-project associations (custom Group model
+        # with project_id FK), this method should be updated to use a direct FK join:
+        #
+        # has_role=Exists(
+        #     Group.objects.filter(
+        #         user=OuterRef('user'),
+        #         project_id=OuterRef('project_id')  # Direct FK instead of string parsing
+        #     )
+        # )
+        #
+        # This will be more efficient and eliminate the need for name prefix matching.
+        # Related refactor: Remove string-based group naming pattern in ami/users/roles.py:24
+        # Subquery: Check if user has ANY group for their project
+        # Groups follow naming pattern: {project_id}_{project_name}_{RoleClassName}
+        # See: ami/users/roles.py:24 (Role.get_group_name)
+        queryset = self.annotate(
+            has_role=Exists(
+                Group.objects.filter(
+                    user=OuterRef("user"),
+                    name__startswith=models.functions.Concat(OuterRef("project_id"), models.Value("_")),
+                )
+            )
+        )
+
+        # Log invalid memberships if requested (before filtering them out)
+        if log_invalid:
+            invalid = queryset.filter(has_role=False).select_related("user", "project")
+            if invalid.exists():
+                for membership in invalid:
+                    logger.warning(
+                        f"Data inconsistency detected: UserProjectMembership {membership.pk} "
+                        f"for user '{membership.user.email}' in project '{membership.project.name}' "
+                        f"(ID: {membership.project.pk}) has no role assigned. This indicates "
+                        f"the permission groups are out of sync. "
+                        f"Fix by running: python manage.py update_roles --project-id={membership.project.pk}"
+                    )
+
+        # Return only members with valid roles
+        return queryset.filter(has_role=True)
+
+
+class UserProjectMembershipManager(models.Manager.from_queryset(UserProjectMembershipQuerySet)):
+    """Custom manager for UserProjectMembership."""
+
+    pass
+
+
 class UserProjectMembership(BaseModel):
     """
     Through model connecting User <-> Project.
@@ -508,6 +597,9 @@ class UserProjectMembership(BaseModel):
         related_name="project_memberships",
     )
 
+    # Add custom manager
+    objects = UserProjectMembershipManager()
+
     def check_permission(self, user: AbstractUser | AnonymousUser, action: str) -> bool:
         project = self.project
         # Allow viewing membership details if the user has view permission on the project

ami/users/api/serializers.py

@@ -109,8 +109,6 @@ class UserProjectMembershipSerializer(DefaultSerializer):
 
     user = MemberUserSerializer(read_only=True)
     role = serializers.SerializerMethodField(read_only=True)
-    role_display_name = serializers.SerializerMethodField(read_only=True)
-    role_description = serializers.SerializerMethodField(read_only=True)
 
     class Meta:
         model = UserProjectMembership
@@ -121,8 +119,6 @@ class Meta:
             "user",
             "project",
             "role",
-            "role_display_name",
-            "role_description",
             "created_at",
             "updated_at",
         ]
@@ -132,8 +128,6 @@ class Meta:
             "created_at",
             "updated_at",
             "role",
-            "role_display_name",
-            "role_description",
         ]
 
     def validate_email(self, value):
@@ -158,22 +152,23 @@ def validate_role_id(self, value):
         return value
 
     def get_role(self, obj):
-        from ami.users.roles import Role
-
-        role_cls = Role.get_primary_role(obj.project, obj.user)
-        return role_cls.__name__ if role_cls else None
-
-    def get_role_display_name(self, obj):
-        from ami.users.roles import Role
-
-        role_cls = Role.get_primary_role(obj.project, obj.user)
-        return role_cls.display_name if role_cls else None
+        """
+        Get the primary role for this membership.
 
-    def get_role_description(self, obj):
+        Note: Due to queryset filtering in UserProjectMembershipViewSet.get_queryset(),
+        this method should never return None in API responses. However, we maintain
+        the None check for safety (e.g., when called outside API context).
+        """
         from ami.users.roles import Role
 
         role_cls = Role.get_primary_role(obj.project, obj.user)
-        return role_cls.description if role_cls else None
+        if role_cls is None:
+            return None
+        return {
+            "id": role_cls.__name__,
+            "name": role_cls.display_name,
+            "description": role_cls.description,
+        }
 
     def validate(self, attrs):
         project = self.context["project"]
@@ -207,17 +202,13 @@ def validate(self, attrs):
 class UserProjectMembershipListSerializer(UserProjectMembershipSerializer):
     user = MemberUserSerializer(read_only=True)
     role = serializers.SerializerMethodField()
-    role_display_name = serializers.SerializerMethodField()
-    role_description = serializers.SerializerMethodField()
 
     class Meta:
         model = UserProjectMembership
         fields = [
             "id",
             "user",
             "role",
-            "role_display_name",
-            "role_description",
             "created_at",
             "updated_at",
         ]

ami/users/api/views.py

@@ -36,7 +36,7 @@ class UserProjectMembershipViewSet(DefaultViewSet, ProjectMixin):
 
     def get_queryset(self):
         project = self.get_active_project()
-        return UserProjectMembership.objects.filter(project=project).select_related("user")
+        return UserProjectMembership.objects.filter(project=project).select_related("user").with_role(log_invalid=True)
 
     def get_serializer_class(self):
         if self.action == "list":

ami/users/tests/test_membership_management_api.py

@@ -22,18 +22,28 @@ def setUp(self):
         self.roles_url = "/api/v2/users/roles/"
         self.members_url = f"/api/v2/projects/{self.project.pk}/members/"
 
-    def create_membership(self, user=None):
+    def create_membership(self, user=None, role_cls=None):
         """
-        Create a membership for a user in this project.
-        Used in tests to guarantee isolation.
+        Create a membership for a user in this project with a role assigned.
+
+        Args:
+            user: User to add as member (defaults to self.user1)
+            role_cls: Role class to assign (defaults to BasicMember)
+
+        Returns:
+            UserProjectMembership instance with role assigned
         """
         if user is None:
             user = self.user1
+        if role_cls is None:
+            role_cls = BasicMember  # Default role for test memberships
 
         membership = UserProjectMembership.objects.create(
             project=self.project,
             user=user,
         )
+        # Assign role to ensure membership is valid
+        role_cls.assign_user(user, self.project)
         return membership
 
     def auth_super(self):
@@ -112,7 +122,9 @@ def test_update_membership_functionality(self):
         self.assertEqual(resp.status_code, 200)
 
         updated = resp.json()
-        self.assertEqual(updated["role"], ProjectManager.__name__)
+        self.assertEqual(updated["role"]["id"], ProjectManager.__name__)
+        self.assertEqual(updated["role"]["name"], ProjectManager.display_name)
+        self.assertEqual(updated["role"]["description"], ProjectManager.description)
 
         membership = UserProjectMembership.objects.get(
             project=self.project,

ui/src/data-services/hooks/team/useMembers.ts

@@ -14,11 +14,7 @@ const convertServerRecord = (record: ServerMember): Member => ({
   id: `${record.id}`,
   image: record.user.image,
   name: record.user.name,
-  role: {
-    description: record.role_description,
-    id: record.role,
-    name: record.role_display_name,
-  },
+  role: record.role,
   updatedAt: record.updated_at ? new Date(record.updated_at) : undefined,
   userId: `${record.user.id}`,
 })

ui/src/data-services/models/member.ts

@@ -1,6 +1,19 @@
 import { Role } from './role'
+import { UserPermission } from 'utils/user/types'
 
-export type ServerMember = any // TODO: Update this type
+export type ServerMember = {
+  created_at: string
+  id: string
+  role: Role
+  updated_at: string
+  user: {
+    id: string
+    name: string
+    email: string
+    image?: string
+  }
+  user_permissions: UserPermission[]
+}
 
 export type Member = {
   addedAt: Date

ui/src/data-services/models/role.ts

@@ -1,5 +1,5 @@
 export type Role = {
   name: string
   id: string
-  description?: string
+  description: string
 }