Browse Source

ref(invites): Disable team selector when non-team role is selected (#67706)

Fixes https://github.com/getsentry/sentry/issues/67427 with better user
clarity

Requires https://github.com/getsentry/sentry/pull/67708

Disable the teams field when selecting a role that cannot be applied to
teams (e.g. billing).



https://github.com/getsentry/sentry/assets/35509934/d7d39d57-65cf-48e0-a890-2e846a58bfe1
Leander Rodrigues 11 months ago
parent
commit
d7f7c55c5a

+ 4 - 4
fixtures/js-stubs/roleList.tsx

@@ -9,45 +9,45 @@ export function OrgRoleListFixture(
       id: 'member',
       name: 'Member',
       desc: 'Members can view and act on events, as well as view most other data within the organization.',
-      allowed: true,
       isAllowed: true,
       is_global: false,
       isGlobal: false,
       isRetired: false,
       minimumTeamRole: 'contributor',
+      isTeamRolesAllowed: true,
     },
     {
       id: 'admin',
       name: 'Admin',
       desc: "Admin privileges on any teams of which they're a member. They can create new teams and projects, as well as remove teams and projects on which they already hold membership (or all teams, if open membership is enabled). Additionally, they can manage memberships of teams that they are members of. They cannot invite members to the organization.",
-      allowed: fullAccess,
       isAllowed: fullAccess,
       is_global: false,
       isGlobal: false,
       isRetired: false,
       minimumTeamRole: 'admin',
+      isTeamRolesAllowed: true,
     },
     {
       id: 'manager',
       name: 'Manager',
       desc: 'Gains admin access on all teams as well as the ability to add and remove members.',
-      allowed: fullAccess,
       isAllowed: fullAccess,
       is_global: true,
       isGlobal: true,
       isRetired: false,
       minimumTeamRole: 'admin',
+      isTeamRolesAllowed: true,
     },
     {
       id: 'owner',
       name: 'Owner',
       desc: 'Gains full permission across the organization. Can manage members as well as perform catastrophic operations such as removing the organization.',
-      allowed: fullAccess,
       isAllowed: fullAccess,
       is_global: true,
       isGlobal: true,
       isRetired: false,
       minimumTeamRole: 'admin',
+      isTeamRolesAllowed: true,
     },
     ...params,
   ];

+ 4 - 0
static/app/components/acl/role.spec.tsx

@@ -16,24 +16,28 @@ describe('Role', function () {
         name: 'Member',
         desc: '...',
         minimumTeamRole: 'contributor',
+        isTeamRolesAllowed: true,
       },
       {
         id: 'admin',
         name: 'Admin',
         desc: '...',
         minimumTeamRole: 'admin',
+        isTeamRolesAllowed: true,
       },
       {
         id: 'manager',
         name: 'Manager',
         desc: '...',
         minimumTeamRole: 'admin',
+        isTeamRolesAllowed: true,
       },
       {
         id: 'owner',
         name: 'Owner',
         desc: '...',
         minimumTeamRole: 'admin',
+        isTeamRolesAllowed: true,
       },
     ],
   });

+ 5 - 3
static/app/components/modals/inviteMembersModal/index.spec.tsx

@@ -28,7 +28,7 @@ describe('InviteMembersModal', function () {
     return client.addMockResponse({
       url: `/organizations/${orgSlug}/members/me/`,
       method: 'GET',
-      body: {roles},
+      body: {orgRoleList: roles},
     });
   };
 
@@ -55,13 +55,15 @@ describe('InviteMembersModal', function () {
         id: 'admin',
         name: 'Admin',
         desc: 'This is the admin role',
-        allowed: true,
+        isAllowed: true,
+        isTeamRolesAllowed: true,
       },
       {
         id: 'member',
         name: 'Member',
         desc: 'This is the member role',
-        allowed: true,
+        isAllowed: true,
+        isTeamRolesAllowed: true,
       },
     ],
     modalProps = defaultMockModalProps,

+ 1 - 1
static/app/components/modals/inviteMembersModal/inviteMembersModalview.tsx

@@ -107,7 +107,7 @@ export default function InviteMembersModalView({
             emails={[...emails]}
             role={role}
             teams={[...teams]}
-            roleOptions={member ? member.roles : ORG_ROLES}
+            roleOptions={member?.orgRoleList ?? ORG_ROLES}
             roleDisabledUnallowed={willInvite}
             inviteStatus={inviteStatus}
             onRemove={() => removeInviteRow(i)}

+ 22 - 5
static/app/components/modals/inviteMembersModal/inviteRowControl.tsx

@@ -1,4 +1,4 @@
-import {useState} from 'react';
+import {useCallback, useState} from 'react';
 import type {MultiValueProps} from 'react-select';
 import type {Theme} from '@emotion/react';
 import {useTheme} from '@emotion/react';
@@ -63,6 +63,18 @@ function InviteRowControl({
 
   const theme = useTheme();
 
+  const isTeamRolesAllowedForRole = useCallback<(roleId: string) => boolean>(
+    roleId => {
+      const roleOptionsMap = roleOptions.reduce(
+        (rolesMap, roleOption) => ({...rolesMap, [roleOption.id]: roleOption}),
+        {}
+      );
+      return roleOptionsMap[roleId]?.isTeamRolesAllowed ?? true;
+    },
+    [roleOptions]
+  );
+  const isTeamRolesAllowed = isTeamRolesAllowedForRole(role);
+
   const handleKeyDown = (event: React.KeyboardEvent<HTMLElement>) => {
     switch (event.key) {
       case 'Enter':
@@ -114,14 +126,19 @@ function InviteRowControl({
         value={role}
         roles={roleOptions}
         disableUnallowed={roleDisabledUnallowed}
-        onChange={onChangeRole}
+        onChange={roleOption => {
+          onChangeRole(roleOption);
+          if (!isTeamRolesAllowedForRole(roleOption.value)) {
+            onChangeTeams([]);
+          }
+        }}
       />
       <TeamSelector
         aria-label={t('Add to Team')}
         data-test-id="select-teams"
-        disabled={disabled}
-        placeholder={t('None')}
-        value={teams}
+        disabled={isTeamRolesAllowed ? disabled : true}
+        placeholder={isTeamRolesAllowed ? t('None') : t('Role cannot join teams')}
+        value={isTeamRolesAllowed ? teams : []}
         onChange={onChangeTeams}
         useTeamDefaultIfOnlyOne
         multiple

+ 2 - 2
static/app/components/modals/inviteMissingMembersModal/index.spec.tsx

@@ -17,13 +17,13 @@ const roles = [
     id: 'admin',
     name: 'Admin',
     desc: 'This is the admin role',
-    allowed: true,
+    isAllowed: true,
   },
   {
     id: 'member',
     name: 'Member',
     desc: 'This is the member role',
-    allowed: true,
+    isAllowed: true,
   },
 ] as OrgRole[];
 

+ 50 - 41
static/app/components/modals/inviteMissingMembersModal/index.tsx

@@ -1,4 +1,4 @@
-import {Fragment, useState} from 'react';
+import {Fragment, useCallback, useMemo, useState} from 'react';
 import {css} from '@emotion/react';
 import styled from '@emotion/styled';
 
@@ -58,42 +58,53 @@ export function InviteMissingMembersModal({
 
   const api = useApi();
 
-  if (memberInvites.length === 0 || !organization.access.includes('org:write')) {
-    return null;
-  }
+  const allowedRolesMap = useMemo<Record<string, OrgRole>>(
+    () => allowedRoles.reduce((rolesMap, role) => ({...rolesMap, [role.id]: role}), {}),
+    [allowedRoles]
+  );
 
-  const setRole = (role: string, index: number) => {
-    setMemberInvites(currentMemberInvites =>
-      currentMemberInvites.map((member, i) => {
-        if (i === index) {
-          member.role = role;
+  const setRole = useCallback(
+    (role: string, index: number) => {
+      setMemberInvites(prevInvites => {
+        const invites = prevInvites.map(i => ({...i}));
+        invites[index].role = role;
+        if (!allowedRolesMap[role].isTeamRolesAllowed) {
+          invites[index].teamSlugs = new Set([]);
         }
-        return member;
-      })
-    );
-  };
+        return invites;
+      });
+    },
+    [allowedRolesMap]
+  );
 
-  const setTeams = (teamSlugs: string[], index: number) => {
-    setMemberInvites(currentMemberInvites =>
-      currentMemberInvites.map((member, i) => {
-        if (i === index) {
-          member.teamSlugs = new Set(teamSlugs);
-        }
-        return member;
-      })
-    );
-  };
+  const setTeams = useCallback((teamSlugs: string[], index: number) => {
+    setMemberInvites(prevInvites => {
+      const invites = prevInvites.map(i => ({...i}));
+      invites[index].teamSlugs = new Set(teamSlugs);
+      return invites;
+    });
+  }, []);
+
+  const selectAll = useCallback(
+    (checked: boolean) => {
+      const selectedMembers = memberInvites.map(m => ({...m, selected: checked}));
+      setMemberInvites(selectedMembers);
+    },
+    [memberInvites]
+  );
 
-  const selectAll = (checked: boolean) => {
-    const selectedMembers = memberInvites.map(m => ({...m, selected: checked}));
-    setMemberInvites(selectedMembers);
-  };
+  const toggleCheckbox = useCallback(
+    (checked: boolean, index: number) => {
+      const selectedMembers = [...memberInvites];
+      selectedMembers[index].selected = checked;
+      setMemberInvites(selectedMembers);
+    },
+    [memberInvites]
+  );
 
-  const toggleCheckbox = (checked: boolean, index: number) => {
-    const selectedMembers = [...memberInvites];
-    selectedMembers[index].selected = checked;
-    setMemberInvites(selectedMembers);
-  };
+  if (memberInvites.length === 0 || !organization.access.includes('org:write')) {
+    return null;
+  }
 
   const renderStatusMessage = () => {
     if (sendingInvites) {
@@ -189,13 +200,9 @@ export function InviteMissingMembersModal({
   const selectedAll = memberInvites.length === selectedCount;
 
   const inviteButtonLabel = () => {
-    return tct('Invite [memberCount] missing member[isPlural]', {
-      memberCount:
-        memberInvites.length === selectedCount
-          ? `all ${selectedCount}`
-          : selectedCount === 0
-            ? ''
-            : selectedCount,
+    return tct('Invite [prefix][memberCount] missing member[isPlural]', {
+      prefix: memberInvites.length === selectedCount ? 'all ' : '',
+      memberCount: selectedCount === 0 ? '' : selectedCount,
       isPlural: selectedCount !== 1 ? 's' : '',
     });
   };
@@ -227,6 +234,8 @@ export function InviteMissingMembersModal({
         {memberInvites?.map((member, i) => {
           const checked = memberInvites[i].selected;
           const username = member.externalId.split(':').pop();
+          const isTeamRolesAllowed =
+            allowedRolesMap[member.role]?.isTeamRolesAllowed ?? true;
           return (
             <Fragment key={i}>
               <div>
@@ -264,8 +273,8 @@ export function InviteMissingMembersModal({
                 organization={organization}
                 aria-label={t('Add to Team')}
                 data-test-id="select-teams"
-                disabled={false}
-                placeholder={t('None')}
+                disabled={!isTeamRolesAllowed}
+                placeholder={isTeamRolesAllowed ? t('None') : t('Role cannot join teams')}
                 onChange={opts => setTeams(opts ? opts.map(v => v.value) : [], i)}
                 multiple
                 clearable

+ 4 - 4
static/app/components/roleSelectControl.tsx

@@ -2,7 +2,7 @@ import styled from '@emotion/styled';
 
 import type {ControlProps} from 'sentry/components/forms/controls/selectControl';
 import SelectControl from 'sentry/components/forms/controls/selectControl';
-import type {MemberRole} from 'sentry/types';
+import type {BaseRole} from 'sentry/types';
 
 type OptionType = {
   details: React.ReactNode;
@@ -13,7 +13,7 @@ type OptionType = {
 
 type Props = Omit<ControlProps<OptionType>, 'onChange' | 'value'> & {
   disableUnallowed: boolean;
-  roles: MemberRole[];
+  roles: BaseRole[];
   /**
    * Narrower type than SelectControl because there is no empty value
    */
@@ -27,11 +27,11 @@ function RoleSelectControl({roles, disableUnallowed, ...props}: Props) {
       options={roles
         ?.filter(r => !r.isRetired)
         .map(
-          (r: MemberRole) =>
+          (r: BaseRole) =>
             ({
               value: r.id,
               label: r.name,
-              disabled: disableUnallowed && !r.allowed,
+              disabled: disableUnallowed && !r.isAllowed,
               details: <Details>{r.desc}</Details>,
             }) as OptionType
         )}

+ 8 - 4
static/app/constants/index.tsx

@@ -80,30 +80,34 @@ export const ORG_ROLES: OrgRole[] = [
   {
     id: 'member',
     name: 'Member',
-    allowed: true,
+    isAllowed: true,
     desc: 'Members can view and act on events, as well as view most other data within the organization.',
     minimumTeamRole: 'contributor',
+    isTeamRolesAllowed: true,
   },
   {
     id: 'admin',
     name: 'Admin',
-    allowed: true,
+    isAllowed: true,
     desc: "Admin privileges on any teams of which they're a member. They can create new teams and projects, as well as remove teams and projects on which they already hold membership (or all teams, if open membership is enabled). Additionally, they can manage memberships of teams that they are members of. They cannot invite members to the organization.",
     minimumTeamRole: 'admin',
+    isTeamRolesAllowed: true,
   },
   {
     id: 'manager',
     name: 'Manager',
-    allowed: true,
+    isAllowed: true,
     desc: 'Gains admin access on all teams as well as the ability to add and remove members.',
     minimumTeamRole: 'admin',
+    isTeamRolesAllowed: true,
   },
   {
     id: 'owner',
     name: 'Organization Owner',
-    allowed: true,
+    isAllowed: true,
     desc: 'Unrestricted access to the organization, its data, and its settings. Can add, modify, and delete projects and members, as well as make billing and plan changes.',
     minimumTeamRole: 'admin',
+    isTeamRolesAllowed: true,
   },
 ];
 

+ 4 - 4
static/app/data/forms/organizationGeneralSettings.tsx

@@ -1,7 +1,7 @@
 import type {JsonFormObject} from 'sentry/components/forms/types';
 import ExternalLink from 'sentry/components/links/externalLink';
 import {t, tct} from 'sentry/locale';
-import type {MemberRole} from 'sentry/types';
+import type {BaseRole} from 'sentry/types';
 import slugify from 'sentry/utils/slugify';
 
 // Export route to make these forms searchable by label/help
@@ -68,7 +68,7 @@ const formGroups: JsonFormObject[] = [
         label: t('Default Role'),
         // seems weird to have choices in initial form data
         choices: ({initialData} = {}) =>
-          initialData?.orgRoleList?.map((r: MemberRole) => [r.id, r.name]) ?? [],
+          initialData?.orgRoleList?.map((r: BaseRole) => [r.id, r.name]) ?? [],
         help: t('The default role new members will receive'),
         disabled: ({access}) => !access.has('org:admin'),
       },
@@ -99,7 +99,7 @@ const formGroups: JsonFormObject[] = [
         name: 'attachmentsRole',
         type: 'select',
         choices: ({initialData = {}}) =>
-          initialData?.orgRoleList?.map((r: MemberRole) => [r.id, r.name]) ?? [],
+          initialData?.orgRoleList?.map((r: BaseRole) => [r.id, r.name]) ?? [],
         label: t('Attachments Access'),
         help: t(
           'Role required to download event attachments, such as native crash reports or log files.'
@@ -110,7 +110,7 @@ const formGroups: JsonFormObject[] = [
         name: 'debugFilesRole',
         type: 'select',
         choices: ({initialData = {}}) =>
-          initialData?.orgRoleList?.map((r: MemberRole) => [r.id, r.name]) ?? [],
+          initialData?.orgRoleList?.map((r: BaseRole) => [r.id, r.name]) ?? [],
         label: t('Debug Files Access'),
         help: t(
           'Role required to download debug information files, proguard mappings and source maps.'

Some files were not shown because too many files changed in this diff