feat(roles): determine team roles from all org roles (#45277)

static/app/types/organization.tsx

@@ -127,6 +127,7 @@ export interface Member {
   name: string;
   orgRole: OrgRole['id'];
   orgRoleList: OrgRole[]; // TODO: Move to global store
+  orgRolesFromTeams: {role: OrgRole; teamSlug: string}[];
   pending: boolean | undefined;
   projects: string[];
 

static/app/utils/orgRole.tsx

@@ -0,0 +1,13 @@
+import {OrgRole} from 'sentry/types';
+
+export function getEffectiveOrgRole(memberOrgRoles: string[], orgRoleList: OrgRole[]) {
+  const orgRoleMap = orgRoleList.reduce((acc, role, index) => {
+    acc[role.id] = {index, role};
+    return acc;
+  }, {});
+
+  // sort by ascending index (high to low priority)
+  memberOrgRoles.sort((a, b) => orgRoleMap[b].index - orgRoleMap[a].index);
+
+  return orgRoleMap[memberOrgRoles[0]]?.role;
+}

static/app/views/settings/components/teamSelect.tsx

@@ -19,6 +19,7 @@ import {IconSubtract} from 'sentry/icons';
 import {t, tct} from 'sentry/locale';
 import {space} from 'sentry/styles/space';
 import {Member, Organization, Team} from 'sentry/types';
+import {getEffectiveOrgRole} from 'sentry/utils/orgRole';
 import useTeams from 'sentry/utils/useTeams';
 import {
   hasOrgRoleOverwrite,
@@ -92,6 +93,23 @@ function TeamSelect({
   const {teams, onSearch, fetching} = useTeams();
   const {orgRoleList, teamRoleList} = organization;
 
+  const slugsToFilter: string[] =
+    selectedTeams?.map(tm => tm.slug) || selectedTeamRoles?.map(tm => tm.teamSlug) || [];
+
+  // Determine if adding a team changes the minimum team-role
+  // Get org roles from team membership, if any
+  const orgRolesFromTeams = teams
+    .filter(team => slugsToFilter.includes(team.slug) && team.orgRole)
+    .map(team => team.orgRole as string);
+
+  if (selectedOrgRole) {
+    orgRolesFromTeams.push(selectedOrgRole);
+  }
+
+  // Sort them and to get the highest priority role
+  // Highest prio role may change minimum team role
+  const effectiveOrgRole = getEffectiveOrgRole(orgRolesFromTeams, orgRoleList)?.id;
+
   const renderBody = () => {
     const numTeams = selectedTeams?.length || selectedTeamRoles?.length;
     if (numTeams === 0) {
@@ -105,9 +123,9 @@ function TeamSelect({
 
     return (
       <React.Fragment>
-        {organization.features.includes('team-roles') && selectedOrgRole && (
+        {organization.features.includes('team-roles') && effectiveOrgRole && (
           <RoleOverwritePanelAlert
-            orgRole={selectedOrgRole}
+            orgRole={effectiveOrgRole}
             orgRoleList={orgRoleList}
             teamRoleList={teamRoleList}
           />
@@ -125,7 +143,7 @@ function TeamSelect({
             />
           ))}
 
-        {selectedOrgRole &&
+        {effectiveOrgRole &&
           selectedTeamRoles &&
           /**
            * "Map + Find" operation is O(n * n), leaving it as it us because it is unlikely to cause performance issues because a Member is unlikely to be in 1000+ teams
@@ -148,7 +166,7 @@ function TeamSelect({
                 confirmMessage={confirmMessage}
                 organization={organization}
                 team={team}
-                selectedOrgRole={selectedOrgRole}
+                selectedOrgRole={effectiveOrgRole}
                 selectedTeamRole={r.role}
                 onChangeTeamRole={onChangeTeamRole}
                 onRemoveTeam={slug => onRemoveTeam(slug)}
@@ -159,9 +177,6 @@ function TeamSelect({
     );
   };
 
-  const slugsToFilter =
-    selectedTeams?.map(tm => tm.slug) || selectedTeamRoles?.map(tm => tm.teamSlug) || [];
-
   // Only show options that aren't selected in the dropdown
   const options = teams
     .filter(team => !slugsToFilter.some(slug => slug === team.slug))

static/app/views/settings/organizationMembers/organizationMemberDetail.spec.jsx

@@ -31,6 +31,7 @@ describe('OrganizationMemberDetail', function () {
       'idp:provisioned': true,
     },
   });
+  const managerTeam = TestStubs.Team({id: '5', orgRole: 'manager', slug: 'manager-team'});
   const teams = [
     team,
     TestStubs.Team({
@@ -49,6 +50,7 @@ describe('OrganizationMemberDetail', function () {
       },
     }),
     idpTeam,
+    managerTeam,
   ];
 
   const teamAssignment = {
@@ -683,4 +685,37 @@ describe('OrganizationMemberDetail', function () {
       expect(screen.queryAllByText('...').length).toBe(0);
     });
   });
+
+  it('overwrites when member joins a manager team', () => {
+    render(<OrganizationMemberDetail params={{memberId: member.id}} />, {
+      context: routerContext,
+    });
+
+    // Role info box is hidden
+    expect(screen.queryByTestId('alert-role-overwrite')).not.toBeInTheDocument();
+
+    // Dropdown has correct value set
+    const teamRow = screen.getByTestId('team-row-for-member');
+    const teamRoleSelect = within(teamRow).getByText('Contributor');
+
+    // Join manager team
+    userEvent.click(screen.getByText('Add Team'));
+    // Click the first item
+    userEvent.click(screen.getByText('#manager-team'));
+
+    // Role info box is shown
+    expect(screen.queryByTestId('alert-role-overwrite')).toBeInTheDocument();
+
+    // Dropdowns have correct value set
+    const teamRows = screen.getAllByTestId('team-row-for-member');
+    within(teamRows[0]).getByText('Team Admin');
+    within(teamRows[1]).getByText('Team Admin');
+
+    // Dropdown options are not visible
+    expect(screen.queryAllByText('...').length).toBe(0);
+
+    // Dropdown cannot be opened
+    selectEvent.openMenu(teamRoleSelect);
+    expect(screen.queryAllByText('...').length).toBe(0);
+  });
 });

static/app/views/settings/organizationTeams/teamMembers.spec.jsx

@@ -18,6 +18,7 @@ describe('TeamMembers', function () {
 
   const organization = TestStubs.Organization();
   const team = TestStubs.Team();
+  const managerTeam = TestStubs.Team({orgRole: 'manager'});
   const members = TestStubs.Members();
   const member = TestStubs.Member({
     id: '9',
@@ -42,6 +43,11 @@ describe('TeamMembers', function () {
       method: 'GET',
       body: team,
     });
+    Client.addMockResponse({
+      url: `/teams/${organization.slug}/${managerTeam.slug}/`,
+      method: 'GET',
+      body: managerTeam,
+    });
 
     createMock = Client.addMockResponse({
       url: `/organizations/${organization.slug}/members/${member.id}/teams/${team.slug}/`,
@@ -308,6 +314,28 @@ describe('TeamMembers', function () {
     expect(contributors).toHaveLength(2);
   });
 
+  it('adding member to manager team makes them team admin', async function () {
+    Client.addMockResponse({
+      url: `/teams/${organization.slug}/${managerTeam.slug}/members/`,
+      method: 'GET',
+      body: [],
+    });
+    const orgWithTeamRoles = TestStubs.Organization({features: ['team-roles']});
+    render(
+      <TeamMembers
+        params={{orgId: orgWithTeamRoles.slug, teamId: managerTeam.slug}}
+        organization={orgWithTeamRoles}
+        team={managerTeam}
+      />
+    );
+
+    userEvent.click((await screen.findAllByRole('button', {name: 'Add Member'}))[0]);
+    userEvent.click(screen.getAllByTestId('letter_avatar-avatar')[0]);
+
+    const admin = screen.queryByText('Team Admin');
+    expect(admin).toBeInTheDocument();
+  });
+
   it('cannot add or remove members if team is idp:provisioned', function () {
     const team2 = TestStubs.Team({
       flags: {

static/app/views/settings/organizationTeams/teamMembers.tsx

@@ -289,7 +289,7 @@ class TeamMembers extends AsyncView<Props, State> {
       return <LoadingError onRetry={this.fetchData} />;
     }
 
-    const {organization, config} = this.props;
+    const {organization, config, team} = this.props;
     const {teamMembersPageLinks} = this.state;
     const {access} = organization;
     const hasWriteAccess = access.includes('org:write') || access.includes('team:admin');
@@ -311,6 +311,7 @@ class TeamMembers extends AsyncView<Props, State> {
                   hasWriteAccess={hasWriteAccess}
                   member={member}
                   organization={organization}
+                  team={team}
                   removeMember={this.removeTeamMember}
                   updateMemberRole={this.updateTeamMemberRole}
                   user={config.user}

static/app/views/settings/organizationTeams/teamMembersRow.tsx

@@ -7,7 +7,8 @@ import RoleSelectControl from 'sentry/components/roleSelectControl';
 import {IconSubtract} from 'sentry/icons';
 import {t} from 'sentry/locale';
 import {space} from 'sentry/styles/space';
-import {Member, Organization, TeamMember, User} from 'sentry/types';
+import {Member, Organization, Team, TeamMember, User} from 'sentry/types';
+import {getEffectiveOrgRole} from 'sentry/utils/orgRole';
 import {
   hasOrgRoleOverwrite,
   RoleOverwriteIcon,
@@ -18,11 +19,19 @@ const TeamMembersRow = (props: {
   member: TeamMember;
   organization: Organization;
   removeMember: (member: Member) => void;
+  team: Team;
   updateMemberRole: (member: Member, newRole: string) => void;
   user: User;
 }) => {
-  const {organization, member, user, hasWriteAccess, removeMember, updateMemberRole} =
-    props;
+  const {
+    organization,
+    team,
+    member,
+    user,
+    hasWriteAccess,
+    removeMember,
+    updateMemberRole,
+  } = props;
 
   return (
     <TeamRolesPanelItem key={member.id}>
@@ -34,6 +43,7 @@ const TeamMembersRow = (props: {
           hasWriteAccess={hasWriteAccess}
           updateMemberRole={updateMemberRole}
           organization={organization}
+          team={team}
           member={member}
         />
       </div>
@@ -53,30 +63,39 @@ const TeamRoleSelect = (props: {
   hasWriteAccess: boolean;
   member: TeamMember;
   organization: Organization;
+  team: Team;
   updateMemberRole: (member: TeamMember, newRole: string) => void;
 }) => {
-  const {hasWriteAccess, organization, member, updateMemberRole} = props;
+  const {hasWriteAccess, organization, team, member, updateMemberRole} = props;
   const {orgRoleList, teamRoleList, features} = organization;
   if (!features.includes('team-roles')) {
     return null;
   }
 
-  const {orgRole: orgRoleId} = member;
-  const orgRole = orgRoleList.find(r => r.id === orgRoleId);
+  // Determine the team-role, including if the current team has an org role
+  // and if adding the user to the current team changes their minimum team-role
+  const possibleOrgRoles = [member.orgRole];
+  if (member.orgRolesFromTeams) {
+    possibleOrgRoles.push(member.orgRolesFromTeams[0].role.id);
+  }
+  if (team.orgRole) {
+    possibleOrgRoles.push(team.orgRole);
+  }
+  const effectiveOrgRole = getEffectiveOrgRole(possibleOrgRoles, orgRoleList);
 
-  const teamRoleId = member.teamRole || orgRole?.minimumTeamRole;
+  const teamRoleId = member.teamRole || effectiveOrgRole?.minimumTeamRole;
   const teamRole = teamRoleList.find(r => r.id === teamRoleId) || teamRoleList[0];
 
   if (
     !hasWriteAccess ||
-    hasOrgRoleOverwrite({orgRole: orgRoleId, orgRoleList, teamRoleList})
+    hasOrgRoleOverwrite({orgRole: effectiveOrgRole?.id, orgRoleList, teamRoleList})
   ) {
     return (
       <RoleName>
         {teamRole.name}
         <IconWrapper>
           <RoleOverwriteIcon
-            orgRole={orgRoleId}
+            orgRole={effectiveOrgRole?.id}
             orgRoleList={orgRoleList}
             teamRoleList={teamRoleList}
           />