feat(roles): update team and project serializers with all org roles (#43997)

src/sentry/api/serializers/models/project.py

@@ -94,24 +94,20 @@ def get_access_by_project(
         project_team_map[pt.project_id].append(pt.team)
 
     team_memberships = set(_get_team_memberships([pt.team for pt in project_teams], user))
-    org_roles = get_org_roles({i.organization_id for i in projects}, user)
+    all_org_roles = get_org_roles({i.organization_id for i in projects}, user)
     prefetch_related_objects(projects, "organization")
 
     is_superuser = request and is_active_superuser(request) and request.user == user
     result = {}
     for project in projects:
         is_member = any(t.id in team_memberships for t in project_team_map.get(project.id, []))
-        org_role = org_roles.get(project.organization_id)
-        if is_member:
-            has_access = True
-        elif is_superuser:
-            has_access = True
-        elif project.organization.flags.allow_joinleave:
-            has_access = True
-        elif org_role and roles.get(org_role).is_global:
-            has_access = True
-        else:
-            has_access = False
+        org_roles = all_org_roles.get(project.organization_id) or []
+        has_access = bool(
+            is_member
+            or is_superuser
+            or project.organization.flags.allow_joinleave
+            or any(roles.get(org_role).is_global for org_role in org_roles)
+        )
         result[project] = {"is_member": is_member, "has_access": has_access}
     return result
 

src/sentry/api/serializers/models/team.py

@@ -39,6 +39,7 @@ from sentry.models import (
     TeamAvatar,
     User,
 )
+from sentry.roles import team_roles
 from sentry.scim.endpoints.constants import SCIM_SCHEMA_GROUP
 from sentry.utils.query import RangeQuerySetWrapper
 
@@ -94,27 +95,36 @@ def get_member_totals(team_list: Sequence[Team], user: User) -> Mapping[str, int
 
 def get_org_roles(
     org_ids: Set[int], user: User, optimization: SingularApiAccessOrgOptimization | None = None
-) -> Mapping[int, str]:
-    """Get the role the user has in each org"""
+) -> Mapping[int, Sequence[str]]:
+    """
+    Get the roles the user has in each org
+    Roles are ordered from highest to lower priority (descending priority)
+    """
     if not user.is_authenticated:
         return {}
 
     if optimization:
-        if optimization.access.role is not None:
+        if optimization.access.roles is not None:
             return {
-                optimization.access.api_user_organization_context.organization.id: optimization.access.role
+                optimization.access.api_user_organization_context.organization.id: list(
+                    optimization.access.roles
+                )
             }
         return {}
 
     # map of org id to role
+    # can have multiple org roles through team membership
+    # return them sorted to figure out highest team role
     return {
-        om["organization_id"]: om["role"]
-        for om in OrganizationMember.objects.filter(
-            user_id=user.id, organization__in=set(org_ids)
-        ).values("role", "organization_id")
+        om.organization.id: _get_all_org_roles(om)
+        for om in OrganizationMember.objects.filter(user_id=user.id, organization__in=set(org_ids))
     }
 
 
+def _get_all_org_roles(member: OrganizationMember) -> Sequence[str]:
+    return [role.id for role in member.get_all_org_roles_sorted()]
+
+
 def get_access_requests(item_list: Sequence[Team], user: User) -> AbstractSet[Team]:
     if user.is_authenticated:
         return frozenset(
@@ -143,6 +153,7 @@ class TeamSerializerResponse(_TeamSerializerResponseOptional):
     isPending: bool
     memberCount: int
     avatar: SerializedAvatarFields
+    orgRole: str
 
 
 @register(Team)
@@ -181,7 +192,7 @@ class TeamSerializer(Serializer):  # type: ignore
             maybe_singular_api_access_org_context(self.access, org_ids) if self.access else None
         )
 
-        org_roles = get_org_roles(org_ids, user, optimization=optimization)
+        all_org_roles = get_org_roles(org_ids, user, optimization=optimization)
 
         member_totals = get_member_totals(item_list, user)
         team_memberships = _get_team_memberships(item_list, user, optimization=optimization)
@@ -193,27 +204,30 @@ class TeamSerializer(Serializer):  # type: ignore
         result: MutableMapping[Team, MutableMapping[str, Any]] = {}
 
         for team in item_list:
-            org_role = org_roles.get(team.organization_id)
+            org_roles = all_org_roles.get(team.organization_id) or []
+            is_member = team.id in team_memberships
+            team_role = None
 
-            if team.id in team_memberships:
-                is_member = True
+            if is_member:
                 team_role = team_memberships[team.id]
-                if team_role is None:
-                    team_role = roles.get_minimum_team_role(org_role).id
-            else:
-                is_member = False
-                team_role = None
+                top_org_role = org_roles[0] if org_roles else None
+
+                if top_org_role is not None:
+                    minimum_team_role = roles.get_minimum_team_role(top_org_role)
+                    if (
+                        not team_role
+                        or minimum_team_role.priority
+                        > team_roles.get(team_role.lower().replace("team ", "")).priority
+                    ):
+                        team_role = minimum_team_role.id
+
+            has_access = bool(
+                is_member
+                or is_superuser
+                or team.organization.flags.allow_joinleave
+                or any(roles.get(org_role).is_global for org_role in org_roles)
+            )
 
-            if is_member:
-                has_access = True
-            elif is_superuser:
-                has_access = True
-            elif team.organization.flags.allow_joinleave:
-                has_access = True
-            elif org_role and roles.get(org_role).is_global:
-                has_access = True
-            else:
-                has_access = False
             result[team] = {
                 "pending_request": team.id in access_requests,
                 "is_member": is_member,
@@ -275,6 +289,7 @@ class TeamSerializer(Serializer):  # type: ignore
             "isPending": attrs["pending_request"],
             "memberCount": attrs["member_count"],
             "avatar": avatar,
+            "orgRole": obj.org_role,
         }
 
         # Expandable attributes.

src/sentry/models/organizationmember.py

@@ -415,6 +415,13 @@ class OrganizationMember(Model):
         all_org_roles.append(self.role)
         return all_org_roles
 
+    def get_all_org_roles_sorted(self):
+        return sorted(
+            [organization_roles.get(role) for role in self.get_all_org_roles()],
+            key=lambda r: r.priority,
+            reverse=True,
+        )
+
     def validate_invitation(self, user_to_approve, allowed_roles):
         """
         Validates whether an org has the options to invite members, handle join requests,

tests/sentry/api/endpoints/test_organization_details.py

@@ -139,7 +139,7 @@ class OrganizationDetailsTest(OrganizationDetailsTestBase):
             options.delete("store.symbolicate-event-lpq-never")
 
         # TODO(dcramer): We need to pare this down. Lots of duplicate queries for membership data.
-        expected_queries = 45 if SiloMode.get_current_mode() == SiloMode.MONOLITH else 47
+        expected_queries = 49 if SiloMode.get_current_mode() == SiloMode.MONOLITH else 51
 
         with self.assertNumQueries(expected_queries, using="default"):
             response = self.get_success_response(self.organization.slug)

tests/sentry/api/serializers/test_project.py

@@ -130,6 +130,35 @@ class ProjectSerializerTest(TestCase):
         assert result["hasAccess"] is True
         assert result["isMember"] is True
 
+    def test_member_on_owner_team_access(self):
+        organization = self.create_organization()
+        manager_team = self.create_team(organization=organization, org_role="manager")
+        owner_team = self.create_team(organization=organization, org_role="owner")
+        self.create_member(
+            user=self.user,
+            organization=self.organization,
+            role="member",
+            teams=[manager_team, owner_team],
+        )
+
+        result = serialize(self.project, self.user)
+
+        assert result["hasAccess"] is True
+        assert result["isMember"] is False
+
+        self.organization.flags.allow_joinleave = False
+        self.organization.save()
+        result = serialize(self.project, self.user)
+        # after changing to allow_joinleave=False
+        assert result["hasAccess"] is True
+        assert result["isMember"] is False
+
+        self.create_team_membership(user=self.user, team=self.team)
+        result = serialize(self.project, self.user)
+        # after giving them access to team
+        assert result["hasAccess"] is True
+        assert result["isMember"] is True
+
     @mock.patch("sentry.features.batch_has")
     def test_project_batch_has(self, mock_batch):
         mock_batch.return_value = {

tests/sentry/api/serializers/test_team.py

@@ -1,6 +1,7 @@
 from sentry.api.serializers import serialize
 from sentry.api.serializers.models.team import TeamSCIMSerializer, TeamWithProjectsSerializer
 from sentry.models import InviteStatus
+from sentry.models.organizationmemberteam import OrganizationMemberTeam
 from sentry.testutils import TestCase
 from sentry.testutils.silo import region_silo_test
 
@@ -25,6 +26,7 @@ class TeamSerializerTest(TestCase):
             "flags": {"idp:provisioned": False},
             "id": str(team.id),
             "avatar": {"avatarType": "letter_avatar", "avatarUuid": None},
+            "orgRole": None,
             "memberCount": 0,
         }
 
@@ -175,6 +177,79 @@ class TeamSerializerTest(TestCase):
         assert result["isMember"] is True
         assert result["teamRole"] == "admin"
 
+    def test_member_on_owner_team_access(self):
+        user = self.create_user(username="foo")
+        organization = self.create_organization()
+        manager_team = self.create_team(organization=organization, org_role="manager")
+        owner_team = self.create_team(organization=organization, org_role="owner")
+        self.create_member(
+            user=user, organization=organization, role="member", teams=[manager_team, owner_team]
+        )
+        team = self.create_team(organization=organization)
+
+        result = serialize(team, user)
+        result.pop("dateCreated")
+
+        assert result["hasAccess"] is True
+        assert result["isMember"] is False
+        assert result["teamRole"] is None
+
+        organization.flags.allow_joinleave = False
+        organization.save()
+        result = serialize(team, user)
+        # after changing to allow_joinleave=False
+        assert result["hasAccess"] is True
+        assert result["isMember"] is False
+        assert result["teamRole"] is None
+
+        self.create_team_membership(user=user, team=team)
+        result = serialize(team, user)
+        # after giving them access to team
+        assert result["hasAccess"] is True
+        assert result["isMember"] is True
+        assert result["teamRole"] == "admin"
+
+    def test_member_with_team_role_on_owner_team_access(self):
+        user = self.create_user(username="foo")
+        organization = self.create_organization()
+        manager_team = self.create_team(organization=organization, org_role="manager")
+        member = self.create_member(
+            user=user, organization=organization, role="member", teams=[manager_team]
+        )
+        team = self.create_team(organization=organization)
+        OrganizationMemberTeam(organizationmember=member, team=team, role="admin")
+
+        result = serialize(team, user)
+        result.pop("dateCreated")
+
+        assert result["hasAccess"] is True
+        assert result["isMember"] is False
+        assert result["teamRole"] is None
+
+        organization.flags.allow_joinleave = False
+        organization.save()
+        result = serialize(team, user)
+        # after changing to allow_joinleave=False
+        assert result["hasAccess"] is True
+        assert result["isMember"] is False
+        assert result["teamRole"] is None
+
+        self.create_team_membership(user=user, team=team)
+        result = serialize(team, user)
+        # after giving them access to team
+        assert result["hasAccess"] is True
+        assert result["isMember"] is True
+        assert result["teamRole"] == "admin"
+
+    def test_org_role(self):
+        user = self.create_user(username="foo")
+        organization = self.create_organization()
+        self.create_member(user=user, organization=organization, role="owner")
+        team = self.create_team(organization=organization, org_role="manager")
+        result = serialize(team, user)
+
+        assert result["orgRole"] == "manager"
+
 
 @region_silo_test
 class TeamWithProjectsSerializerTest(TestCase):
@@ -199,6 +274,7 @@ class TeamWithProjectsSerializerTest(TestCase):
             "id": str(team.id),
             "projects": serialized_projects,
             "avatar": {"avatarType": "letter_avatar", "avatarUuid": None},
+            "orgRole": None,
             "memberCount": 0,
             "dateCreated": team.date_added,
             "externalTeams": [],