Browse Source

feat(team-roles): Disable deprecated roles in OrgRoleSelect (#35358)

Danny Lee 2 years ago
parent
commit
6b70fefd9e

+ 6 - 1
src/sentry/api/serializers/models/organization_member/expand/roles.py

@@ -2,7 +2,6 @@ from __future__ import annotations
 
 from typing import Any, Iterable, Mapping, Sequence, cast
 
-from sentry import features
 from sentry.api.serializers import serialize
 from sentry.api.serializers.models.user import DetailedUserSerializer
 from sentry.models import OrganizationMember, User
@@ -15,11 +14,17 @@ from ..response import OrganizationMemberWithRolesResponse
 
 
 def _is_retired_role_hidden(role: OrganizationRole, member: OrganizationMember) -> bool:
+    """
+    During EA, we will show the role but make it greyed out and prevent the user
+    from assigning more people to the role.
+
     return (
         role.is_retired
         and role.id != member.role
         and features.has("organizations:team-roles", member.organization)
     )
+    """
+    return False
 
 
 class OrganizationMemberWithRolesSerializer(OrganizationMemberWithTeamsSerializer):

+ 4 - 0
static/app/types/organization.tsx

@@ -78,6 +78,10 @@ export type MemberRole = {
   id: string;
   name: string;
   allowed?: boolean;
+
+  isGlobal?: boolean;
+  isRetired?: boolean;
+  minimumTeamRole?: string;
 };
 
 /**

+ 22 - 11
static/app/views/settings/organizationMembers/inviteMember/roleSelect.tsx → static/app/views/settings/organizationMembers/inviteMember/orgRoleSelect.tsx

@@ -17,33 +17,44 @@ const Label = styled('label')`
 type Props = {
   disabled: boolean;
   enforceAllowed: boolean;
+  enforceRetired: boolean;
   roleList: MemberRole[];
-  selectedRole: string;
-  setRole: (id: string) => void;
+  roleSelected: string;
+  setSelected: (id: string) => void;
 };
 
-class RoleSelect extends Component<Props> {
+class OrganizationRoleSelect extends Component<Props> {
   render() {
-    const {disabled, enforceAllowed, roleList, selectedRole} = this.props;
+    const {
+      disabled,
+      enforceRetired,
+      enforceAllowed,
+      roleList,
+      roleSelected,
+      setSelected,
+    } = this.props;
 
     return (
       <Panel>
-        <PanelHeader>{t('Role')}</PanelHeader>
+        <PanelHeader>{t('Organization Role')}</PanelHeader>
 
         <PanelBody>
           {roleList.map(role => {
-            const {desc, name, id, allowed} = role;
-            const isDisabled = disabled || (enforceAllowed && !allowed);
+            const {desc, name, id, allowed, isRetired: roleRetired} = role;
+
+            const isRetired = enforceRetired && roleRetired;
+            const isDisabled = disabled || isRetired || (enforceAllowed && !allowed);
+
             return (
               <PanelItem
                 key={id}
-                onClick={() => !isDisabled && this.props.setRole(id)}
+                onClick={() => !isDisabled && setSelected(id)}
                 css={!isDisabled ? {} : {color: 'grey', cursor: 'default'}}
               >
                 <Label>
-                  <Radio id={id} value={name} checked={id === selectedRole} readOnly />
+                  <Radio id={id} value={name} checked={id === roleSelected} readOnly />
                   <div style={{flex: 1, padding: '0 16px'}}>
-                    {name}
+                    {name} {isRetired && t('(Deprecated)')}
                     <TextBlock noMargin>
                       <div className="help-block">{desc}</div>
                     </TextBlock>
@@ -58,4 +69,4 @@ class RoleSelect extends Component<Props> {
   }
 }
 
-export default RoleSelect;
+export default OrganizationRoleSelect;

+ 7 - 9
static/app/views/settings/organizationMembers/organizationMemberDetail.tsx

@@ -32,7 +32,7 @@ import AsyncView from 'sentry/views/asyncView';
 import SettingsPageHeader from 'sentry/views/settings/components/settingsPageHeader';
 import TeamSelect from 'sentry/views/settings/components/teamSelect';
 
-import RoleSelect from './inviteMember/roleSelect';
+import OrganizationRoleSelect from './inviteMember/orgRoleSelect';
 
 const MULTIPLE_ORGS = t('Cannot be reset since user is in more than one organization');
 const NOT_ENROLLED = t('Not enrolled in two-factor authentication');
@@ -52,8 +52,6 @@ type Props = {
 
 type State = {
   member: Member | null;
-  roleList: Member['roles'];
-  selectedRole: Member['role'];
 } & AsyncView['state'];
 
 const DisabledMemberTooltip = HookOrDefault({
@@ -65,8 +63,6 @@ class OrganizationMemberDetail extends AsyncView<Props, State> {
   getDefaultState(): State {
     return {
       ...super.getDefaultState(),
-      roleList: [],
-      selectedRole: '',
       member: null,
     };
   }
@@ -240,9 +236,10 @@ class OrganizationMemberDetail extends AsyncView<Props, State> {
       return <NotFound />;
     }
 
-    const {access} = organization;
+    const {access, features} = organization;
     const inviteLink = member.invite_link;
     const canEdit = access.includes('org:write') && !this.memberDeactivated;
+    const hasTeamRoles = features.includes('team-roles');
 
     const {email, expired, pending} = member;
     const canResend = !expired;
@@ -353,12 +350,13 @@ class OrganizationMemberDetail extends AsyncView<Props, State> {
           </Panel>
         )}
 
-        <RoleSelect
+        <OrganizationRoleSelect
           enforceAllowed={false}
+          enforceRetired={hasTeamRoles}
           disabled={!canEdit}
           roleList={member.roles}
-          selectedRole={member.role}
-          setRole={slug => this.setState({member: {...member, role: slug}})}
+          roleSelected={member.role}
+          setSelected={slug => this.setState({member: {...member, role: slug}})}
         />
 
         <Teams slugs={member.teams}>

+ 6 - 4
tests/js/spec/views/settings/organizationMembers/organizationMemberDetail.spec.jsx

@@ -81,11 +81,13 @@ describe('OrganizationMemberDetail', function () {
       );
 
       // Should have 4 roles
-      expect(wrapper.find('RoleSelect Radio')).toHaveLength(4);
+      expect(wrapper.find('OrganizationRoleSelect Radio')).toHaveLength(4);
 
-      wrapper.find('RoleSelect Radio').last().simulate('click');
+      wrapper.find('OrganizationRoleSelect Radio').last().simulate('click');
 
-      expect(wrapper.find('RoleSelect Radio').last().prop('checked')).toBe(true);
+      expect(wrapper.find('OrganizationRoleSelect Radio').last().prop('checked')).toBe(
+        true
+      );
 
       // Save Member
       wrapper.find('Button[priority="primary"]').simulate('click');
@@ -172,7 +174,7 @@ describe('OrganizationMemberDetail', function () {
       wrapper.update();
 
       // Should have 4 roles
-      expect(wrapper.find('RoleSelect').prop('disabled')).toBe(true);
+      expect(wrapper.find('OrganizationRoleSelect').prop('disabled')).toBe(true);
       expect(wrapper.find('TeamSelect').prop('disabled')).toBe(true);
       expect(wrapper.find('TeamRow Button').first().prop('disabled')).toBe(true);
 

+ 4 - 1
tests/sentry/api/endpoints/test_organization_member_details.py

@@ -112,10 +112,13 @@ class GetOrganizationMemberTest(OrganizationMemberTestBase):
 
     @with_feature("organizations:team-roles")
     def test_hides_retired_organization_roles(self):
+        """
+        Note: Admin will be hidden after team-roles EA.
+        """
         response = self.get_success_response(self.organization.slug, "me")
 
         role_ids = [role["id"] for role in response.data["roles"]]
-        assert role_ids == ["member", "manager", "owner"]
+        assert role_ids == ["member", "admin", "manager", "owner"]
 
     def test_lists_team_roles(self):
         response = self.get_success_response(self.organization.slug, "me")