Просмотр исходного кода

chore(roles): remove effectiveOrgRole (#64735)

Cathy Teng 1 год назад
Родитель
Сommit
1ce7041288

+ 5 - 15
static/app/components/teamRoleSelect.tsx

@@ -4,7 +4,6 @@ import type {ControlProps} from 'sentry/components/forms/controls/selectControl'
 import RoleSelectControl from 'sentry/components/roleSelectControl';
 import {space} from 'sentry/styles/space';
 import type {Organization, Team, TeamMember, TeamRole} from 'sentry/types';
-import {getEffectiveOrgRole} from 'sentry/utils/orgRole';
 import {
   hasOrgRoleOverwrite,
   RoleOverwriteIcon,
@@ -30,30 +29,21 @@ function TeamRoleSelect({
   const {orgRoleList, teamRoleList, features} = organization;
   const hasTeamRoles = features.includes('team-roles');
 
-  // Determine the org-role, including if the current team has an org role
-  // and adding the user to the current team changes their minimum team-role
-  const possibleOrgRoles = [member.orgRole];
-  if (member.groupOrgRoles && member.groupOrgRoles.length > 0) {
-    possibleOrgRoles.push(member.groupOrgRoles[0].role.id);
-  }
-  if (team.orgRole) {
-    possibleOrgRoles.push(team.orgRole);
-  }
-  const effectiveOrgRole = getEffectiveOrgRole(possibleOrgRoles, orgRoleList);
+  const memberOrgRole = orgRoleList.find(r => r.id === member.orgRole);
 
   // If the member's org-role has elevated permission, their team-role will
   // inherit scopes from it
-  if (hasOrgRoleOverwrite({orgRole: effectiveOrgRole?.id, orgRoleList, teamRoleList})) {
+  if (hasOrgRoleOverwrite({orgRole: memberOrgRole?.id, orgRoleList, teamRoleList})) {
     const effectiveTeamRole = teamRoleList.find(
-      r => r.id === effectiveOrgRole?.minimumTeamRole
+      r => r.id === memberOrgRole?.minimumTeamRole
     );
 
     return (
       <RoleName>
-        {effectiveTeamRole?.name || effectiveOrgRole?.minimumTeamRole}
+        {effectiveTeamRole?.name || memberOrgRole?.minimumTeamRole}
         <IconWrapper>
           <RoleOverwriteIcon
-            orgRole={effectiveOrgRole?.id}
+            orgRole={memberOrgRole?.id}
             orgRoleList={orgRoleList}
             teamRoleList={teamRoleList}
           />

+ 0 - 16
static/app/utils/orgRole.tsx

@@ -1,16 +0,0 @@
-import type {OrgRole} from 'sentry/types';
-
-export function getEffectiveOrgRole(
-  memberOrgRoles: string[],
-  orgRoleList: OrgRole[]
-): 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;
-}

+ 4 - 24
static/app/views/settings/components/teamSelect/teamSelectForMember.tsx

@@ -1,6 +1,5 @@
 import {Fragment} from 'react';
 import styled from '@emotion/styled';
-import startCase from 'lodash/startCase';
 
 import {Button} from 'sentry/components/button';
 import EmptyMessage from 'sentry/components/emptyMessage';
@@ -17,7 +16,6 @@ import {IconSubtract} from 'sentry/icons';
 import {t} from 'sentry/locale';
 import {space} from 'sentry/styles/space';
 import type {Member, Organization, Team} from 'sentry/types';
-import {getEffectiveOrgRole} from 'sentry/utils/orgRole';
 import {useTeams} from 'sentry/utils/useTeams';
 import {RoleOverwritePanelAlert} from 'sentry/views/settings/organizationTeams/roleOverwriteWarning';
 import {getButtonHelpText} from 'sentry/views/settings/organizationTeams/utils';
@@ -62,19 +60,6 @@ function TeamSelect({
   const selectedTeamSlugs = new Set(selectedTeamRoles.map(tm => tm.teamSlug));
   const selectedTeams = teams.filter(tm => selectedTeamSlugs.has(tm.slug));
 
-  // Determine if adding a team changes the minimum team-role
-  // Get org-roles from team membership, if any
-  const groupOrgRoles = selectedTeams
-    .filter(team => team.orgRole)
-    .map(team => team.orgRole as string);
-  if (selectedOrgRole) {
-    groupOrgRoles.push(selectedOrgRole);
-  }
-
-  // Sort them and to get the highest priority role
-  // Highest priority role may change minimum team role
-  const effectiveOrgRole = getEffectiveOrgRole(groupOrgRoles, orgRoleList);
-
   const renderBody = () => {
     if (selectedTeams.length === 0) {
       return <EmptyMessage>{t('No Teams assigned')}</EmptyMessage>;
@@ -82,9 +67,9 @@ function TeamSelect({
 
     return (
       <Fragment>
-        {effectiveOrgRole && (
+        {selectedOrgRole && (
           <RoleOverwritePanelAlert
-            orgRole={effectiveOrgRole?.id}
+            orgRole={selectedOrgRole}
             orgRoleList={orgRoleList}
             teamRoleList={teamRoleList}
           />
@@ -98,7 +83,6 @@ function TeamSelect({
             team={team}
             member={{
               ...member,
-              groupOrgRoles: [{role: effectiveOrgRole, teamSlug: ''}],
               orgRole: selectedOrgRole,
               teamRoles: selectedTeamRoles,
             }}
@@ -156,11 +140,9 @@ function TeamRow({
 }) {
   const hasOrgAdmin = organization.access.includes('org:admin');
   const isIdpProvisioned = team.flags['idp:provisioned'];
-  const isPermissionGroup = !!team.orgRole && !hasOrgAdmin;
-  const isRemoveDisabled = disabled || isIdpProvisioned || isPermissionGroup;
+  const isRemoveDisabled = disabled || isIdpProvisioned || !hasOrgAdmin;
 
-  const buttonHelpText = getButtonHelpText(isIdpProvisioned, isPermissionGroup);
-  const orgRoleFromTeam = team.orgRole ? `${startCase(team.orgRole)} Team` : null;
+  const buttonHelpText = getButtonHelpText(isIdpProvisioned, !hasOrgAdmin);
 
   return (
     <TeamPanelItem data-test-id="team-row-for-member">
@@ -170,8 +152,6 @@ function TeamRow({
         </Link>
       </div>
 
-      <div>{orgRoleFromTeam}</div>
-
       <div>
         <TeamRoleSelect
           disabled={disabled}

+ 0 - 115
static/app/views/settings/organizationMembers/organizationMemberDetail.spec.tsx

@@ -35,14 +35,6 @@ describe('OrganizationMemberDetail', function () {
       'idp:provisioned': true,
     },
   });
-  const managerTeam = TeamFixture({id: '5', orgRole: 'manager', slug: 'manager-team'});
-  const otherManagerTeam = TeamFixture({
-    id: '4',
-    slug: 'org-role-team',
-    name: 'Org Role Team',
-    isMember: true,
-    orgRole: 'manager',
-  });
   const teams = [
     team,
     TeamFixture({
@@ -52,8 +44,6 @@ describe('OrganizationMemberDetail', function () {
       isMember: false,
     }),
     idpTeam,
-    managerTeam,
-    otherManagerTeam,
   ];
 
   const teamAssignment = {
@@ -100,18 +90,6 @@ describe('OrganizationMemberDetail', function () {
       },
     ],
   });
-  const managerTeamMember = MemberFixture({
-    id: '5',
-    roles: OrgRoleListFixture(),
-    dateCreated: new Date().toISOString(),
-    teams: [otherManagerTeam.slug],
-    teamRoles: [
-      {
-        teamSlug: otherManagerTeam.slug,
-        role: null,
-      },
-    ],
-  });
   const managerMember = MemberFixture({
     id: '6',
     roles: OrgRoleListFixture(),
@@ -149,10 +127,6 @@ describe('OrganizationMemberDetail', function () {
         url: `/organizations/${organization.slug}/members/${idpTeamMember.id}/`,
         body: idpTeamMember,
       });
-      MockApiClient.addMockResponse({
-        url: `/organizations/${organization.slug}/members/${managerTeamMember.id}/`,
-        body: managerTeamMember,
-      });
       MockApiClient.addMockResponse({
         url: `/organizations/${organization.slug}/members/${managerMember.id}/`,
         body: managerMember,
@@ -239,57 +213,6 @@ describe('OrganizationMemberDetail', function () {
       expect(screen.getByRole('button', {name: 'Remove'})).toBeDisabled();
     });
 
-    it('cannot leave org role team if missing org:admin', function () {
-      const regularOrg = OrganizationFixture({
-        teams,
-        features: ['team-roles'],
-        access: [],
-      });
-
-      const {routerContext, routerProps} = initializeOrg({organization: regularOrg});
-
-      render(
-        <OrganizationMemberDetail
-          {...routerProps}
-          params={{memberId: managerTeamMember.id}}
-        />,
-        {
-          context: routerContext,
-          organization: regularOrg,
-        }
-      );
-      expect(screen.getByText('Manager Team')).toBeInTheDocument();
-      expect(screen.getByRole('button', {name: 'Remove'})).toBeDisabled();
-    });
-
-    it('cannot join org role team if missing org:admin', async function () {
-      const regularOrg = OrganizationFixture({
-        teams,
-        features: ['team-roles'],
-        access: ['org:write'],
-      });
-
-      const {routerContext, routerProps} = initializeOrg({organization: regularOrg});
-      render(
-        <OrganizationMemberDetail
-          {...routerProps}
-          params={{memberId: managerMember.id}}
-        />,
-        {
-          context: routerContext,
-          organization: regularOrg,
-        }
-      );
-
-      await userEvent.click(screen.getByText('Add Team'));
-      await userEvent.hover(screen.getByText('#org-role-team'));
-      expect(
-        await screen.findByText(
-          'Membership to a team with an organization role is managed by org owners.'
-        )
-      ).toBeInTheDocument();
-    });
-
     it('joins a team and assign a team-role', async function () {
       const {routerContext, routerProps} = initializeOrg({organization});
 
@@ -875,43 +798,5 @@ describe('OrganizationMemberDetail', function () {
       selectEvent.openMenu(teamRoleSelect);
       expect(screen.queryAllByText('...').length).toBe(0);
     });
-
-    it('overwrites when member joins a manager team', async () => {
-      const {routerContext, routerProps} = initializeOrg({});
-      render(
-        <OrganizationMemberDetail {...routerProps} params={{memberId: member.id}} />,
-        {
-          context: routerContext,
-          organization,
-        }
-      );
-
-      // 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
-      await userEvent.click(screen.getByText('Add Team'));
-      // Click the first item
-      await 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);
-    });
   });
 });

+ 1 - 68
static/app/views/settings/organizationTeams/teamMembers.spec.tsx

@@ -23,7 +23,7 @@ describe('TeamMembers', function () {
 
   const organization = OrganizationFixture();
   const team = TeamFixture();
-  const managerTeam = TeamFixture({orgRole: 'manager'});
+  const managerTeam = TeamFixture();
   const members = MembersFixture();
   const member = MemberFixture({
     id: '9',
@@ -381,31 +381,6 @@ describe('TeamMembers', function () {
     expect(contributors).toHaveLength(2);
   });
 
-  it('adding member to manager team makes them team admin', async function () {
-    MockApiClient.addMockResponse({
-      url: `/teams/${organization.slug}/${managerTeam.slug}/members/`,
-      method: 'GET',
-      body: [],
-    });
-    const orgWithTeamRoles = OrganizationFixture({features: ['team-roles']});
-    render(
-      <TeamMembers
-        {...routerProps}
-        params={{teamId: managerTeam.slug}}
-        organization={orgWithTeamRoles}
-        team={managerTeam}
-      />
-    );
-
-    await userEvent.click(
-      (await screen.findAllByRole('button', {name: 'Add Member'}))[0]
-    );
-    await 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 = TeamFixture({
       flags: {
@@ -458,46 +433,4 @@ describe('TeamMembers', function () {
       expect(screen.findByRole('button', {name: 'Remove'})).toBeDisabled();
     });
   });
-
-  it('cannot add or remove members or leave if team has org role and no access', function () {
-    const team2 = TeamFixture({orgRole: 'manager'});
-
-    const me = MemberFixture({
-      id: '123',
-      email: 'foo@example.com',
-      role: 'member',
-    });
-
-    MockApiClient.clearMockResponses();
-    MockApiClient.addMockResponse({
-      url: `/organizations/${organization.slug}/members/`,
-      method: 'GET',
-      body: [...members, me],
-    });
-    MockApiClient.addMockResponse({
-      url: `/teams/${organization.slug}/${team2.slug}/members/`,
-      method: 'GET',
-      body: members,
-    });
-    MockApiClient.addMockResponse({
-      url: `/teams/${organization.slug}/${team2.slug}/`,
-      method: 'GET',
-      body: team2,
-    });
-
-    render(
-      <TeamMembers
-        {...routerProps}
-        params={{teamId: team2.slug}}
-        organization={organization}
-        team={team2}
-      />
-    );
-
-    waitFor(() => {
-      expect(screen.findByRole('button', {name: 'Add Member'})).toBeDisabled();
-      expect(screen.findByRole('button', {name: 'Remove'})).toBeDisabled();
-      expect(screen.findByRole('button', {name: 'Leave'})).toBeDisabled();
-    });
-  });
 });