From d6d9a9aedcd74821573673a6d8c89a2827e3959f Mon Sep 17 00:00:00 2001
From: David Nguyen <davidngu28@gmail.com>
Date: Fri, 19 Jun 2026 16:52:00 +1000
Subject: [PATCH] fix: improve team member removal ux

---
 .../dialogs/team-member-delete-dialog.tsx     |  31 +++++-
 .../components/tables/team-members-table.tsx  |  56 ++++++----
 .../app-tests/e2e/teams/team-members.spec.ts  | 105 ++++++++++++++++++
 3 files changed, 167 insertions(+), 25 deletions(-)
 create mode 100644 packages/app-tests/e2e/teams/team-members.spec.ts

diff --git a/apps/remix/app/components/dialogs/team-member-delete-dialog.tsx b/apps/remix/app/components/dialogs/team-member-delete-dialog.tsx
index 841268dfc..1dda81788 100644
--- a/apps/remix/app/components/dialogs/team-member-delete-dialog.tsx
+++ b/apps/remix/app/components/dialogs/team-member-delete-dialog.tsx
@@ -16,6 +16,17 @@ import { msg } from '@lingui/core/macro';
 import { useLingui } from '@lingui/react';
 import { Trans } from '@lingui/react/macro';
 import { useState } from 'react';
+import { match } from 'ts-pattern';
+
+/**
+ * The reason a team member cannot be removed from the team. When set, the delete
+ * dialog explains the reason instead of offering a confirm button.
+ */
+export type TeamMemberDeleteDisableReason =
+  | 'TEAM_OWNER'
+  | 'HIGHER_ROLE'
+  | 'INHERIT_MEMBER_ENABLED'
+  | 'INHERITED_MEMBER';
 
 export type TeamMemberDeleteDialogProps = {
   teamId: number;
@@ -23,7 +34,7 @@ export type TeamMemberDeleteDialogProps = {
   memberId: string;
   memberName: string;
   memberEmail: string;
-  isInheritMemberEnabled: boolean | null;
+  disableReason?: TeamMemberDeleteDisableReason | null;
   trigger?: React.ReactNode;
 };
 
@@ -34,7 +45,7 @@ export const TeamMemberDeleteDialog = ({
   memberId,
   memberName,
   memberEmail,
-  isInheritMemberEnabled,
+  disableReason,
 }: TeamMemberDeleteDialogProps) => {
   const [open, setOpen] = useState(false);
 
@@ -86,10 +97,19 @@ export const TeamMemberDeleteDialog = ({
           </DialogDescription>
         </DialogHeader>
 
-        {isInheritMemberEnabled ? (
+        {disableReason ? (
           <Alert variant="neutral">
             <AlertDescription>
-              <Trans>You cannot remove members from this team if the inherit member feature is enabled.</Trans>
+              {match(disableReason)
+                .with('TEAM_OWNER', () => <Trans>You cannot remove the organisation owner from the team.</Trans>)
+                .with('HIGHER_ROLE', () => <Trans>You cannot remove a member with a role higher than your own.</Trans>)
+                .with('INHERIT_MEMBER_ENABLED', () => (
+                  <Trans>You cannot remove members from this team while the inherit member feature is enabled.</Trans>
+                ))
+                .with('INHERITED_MEMBER', () => (
+                  <Trans>This member is inherited from a group and cannot be removed from the team directly.</Trans>
+                ))
+                .exhaustive()}
             </AlertDescription>
           </Alert>
         ) : (
@@ -109,11 +129,10 @@ export const TeamMemberDeleteDialog = ({
               <Trans>Close</Trans>
             </Button>
 
-            {!isInheritMemberEnabled && (
+            {!disableReason && (
               <Button
                 type="submit"
                 variant="destructive"
-                disabled={Boolean(isInheritMemberEnabled)}
                 loading={isDeletingTeamMember}
                 onClick={async () => deleteTeamMember({ teamId, memberId })}
               >
diff --git a/apps/remix/app/components/tables/team-members-table.tsx b/apps/remix/app/components/tables/team-members-table.tsx
index c8a9f0faa..612920efd 100644
--- a/apps/remix/app/components/tables/team-members-table.tsx
+++ b/apps/remix/app/components/tables/team-members-table.tsx
@@ -29,7 +29,7 @@ import { useSearchParams } from 'react-router';
 
 import { useCurrentTeam } from '~/providers/team';
 
-import { TeamMemberDeleteDialog } from '../dialogs/team-member-delete-dialog';
+import { TeamMemberDeleteDialog, type TeamMemberDeleteDisableReason } from '../dialogs/team-member-delete-dialog';
 import { TeamMemberUpdateDialog } from '../dialogs/team-member-update-dialog';
 import { TeamInheritMemberAlert } from '../general/teams/team-inherit-member-alert';
 
@@ -86,6 +86,39 @@ export const TeamMembersTable = () => {
   );
 
   const columns = useMemo(() => {
+    // A member is a direct team member when they belong to one of the team's
+    // INTERNAL_TEAM groups. Otherwise they are inherited from an organisation or
+    // custom group and cannot be managed directly from this team.
+    const isMemberPartOfInternalTeamGroup = (memberId: string) =>
+      groups.some(
+        (group) =>
+          group.organisationGroupType === OrganisationGroupType.INTERNAL_TEAM &&
+          group.members.some((member) => member.id === memberId),
+      );
+
+    // Determine why a member can't be removed from the team (if at all). The delete
+    // dialog uses this to explain the reason instead of attempting a removal that
+    // would fail.
+    const getDeleteDisableReason = (member: (typeof results)['data'][number]): TeamMemberDeleteDisableReason | null => {
+      if (organisation.ownerUserId === member.userId) {
+        return 'TEAM_OWNER';
+      }
+
+      if (!isTeamRoleWithinUserHierarchy(team.currentTeamRole, member.teamRole)) {
+        return 'HIGHER_ROLE';
+      }
+
+      if (memberAccessTeamGroup !== undefined) {
+        return 'INHERIT_MEMBER_ENABLED';
+      }
+
+      if (!isMemberPartOfInternalTeamGroup(member.id)) {
+        return 'INHERITED_MEMBER';
+      }
+
+      return null;
+    };
+
     return [
       {
         header: _(msg`Team Member`),
@@ -111,15 +144,7 @@ export const TeamMembersTable = () => {
       },
       {
         header: _(msg`Source`),
-        cell: ({ row }) => {
-          const internalTeamGroupFound = groups.find(
-            (group) =>
-              group.organisationGroupType === OrganisationGroupType.INTERNAL_TEAM &&
-              group.members.some((member) => member.id === row.original.id),
-          );
-
-          return internalTeamGroupFound ? _(msg`Member`) : _(msg`Group`);
-        },
+        cell: ({ row }) => (isMemberPartOfInternalTeamGroup(row.original.id) ? _(msg`Member`) : _(msg`Group`)),
       },
       {
         header: _(msg`Actions`),
@@ -161,16 +186,9 @@ export const TeamMembersTable = () => {
                 memberId={row.original.id}
                 memberName={row.original.name ?? ''}
                 memberEmail={row.original.email}
-                isInheritMemberEnabled={memberAccessTeamGroup !== undefined}
+                disableReason={getDeleteDisableReason(row.original)}
                 trigger={
-                  <DropdownMenuItem
-                    onSelect={(e) => e.preventDefault()}
-                    disabled={
-                      organisation.ownerUserId === row.original.userId ||
-                      !isTeamRoleWithinUserHierarchy(team.currentTeamRole, row.original.teamRole)
-                    }
-                    title={_(msg`Remove team member`)}
-                  >
+                  <DropdownMenuItem onSelect={(e) => e.preventDefault()} title={_(msg`Remove team member`)}>
                     <Trash2Icon className="mr-2 h-4 w-4" />
                     <Trans>Remove</Trans>
                   </DropdownMenuItem>
diff --git a/packages/app-tests/e2e/teams/team-members.spec.ts b/packages/app-tests/e2e/teams/team-members.spec.ts
new file mode 100644
index 000000000..0064b35dc
--- /dev/null
+++ b/packages/app-tests/e2e/teams/team-members.spec.ts
@@ -0,0 +1,105 @@
+import { seedOrganisationMembers } from '@documenso/prisma/seed/organisations';
+import { seedTeamMember } from '@documenso/prisma/seed/teams';
+import { seedUser } from '@documenso/prisma/seed/users';
+import { expect, test } from '@playwright/test';
+import { OrganisationMemberRole, TeamMemberRole } from '@prisma/client';
+
+import { apiSignin } from '../fixtures/authentication';
+import { openDropdownMenu } from '../fixtures/generic';
+
+/**
+ * Reproduces the "Team has no internal team groups" bug.
+ *
+ * When a team has member inheritance turned OFF, organisation admins/managers are
+ * still inherited into the team as team admins (shown with the "Group" source).
+ * These members are not part of the team's INTERNAL_TEAM group, so they cannot be
+ * removed via the team members page - attempting to do so threw a 500 ("Team has no
+ * internal team groups").
+ *
+ * Instead of crashing, the delete dialog must explain why the inherited member can't
+ * be removed and not offer a confirm button.
+ */
+test('[TEAMS]: explains why an inherited organisation member cannot be removed', async ({ page }) => {
+  // Team created with member inheritance OFF.
+  const { user: owner, organisation, team } = await seedUser({ inheritMembers: false });
+
+  const inheritedAdminEmail = `inherited-admin-${team.url}@test.documenso.com`;
+
+  // A second organisation admin is inherited into the team as a team admin (source "Group").
+  await seedOrganisationMembers({
+    organisationId: organisation.id,
+    members: [
+      {
+        name: 'Inherited Admin',
+        email: inheritedAdminEmail,
+        organisationRole: OrganisationMemberRole.ADMIN,
+      },
+    ],
+  });
+
+  await apiSignin({
+    page,
+    email: owner.email,
+    redirectPath: `/t/${team.url}/settings/members`,
+  });
+
+  const inheritedMemberRow = page.getByRole('row').filter({ hasText: inheritedAdminEmail });
+
+  // Sanity check: the member is inherited from a group, not a direct team member.
+  await expect(inheritedMemberRow).toBeVisible();
+  await expect(inheritedMemberRow).toContainText('Group');
+
+  await openDropdownMenu(page, inheritedMemberRow.getByRole('button').last());
+
+  // The action stays enabled - opening it shows a dialog explaining why the inherited
+  // member can't be removed, rather than triggering the 500.
+  const removeMenuItem = page.getByRole('menuitem', { name: 'Remove' });
+  await expect(removeMenuItem).toBeEnabled();
+  await removeMenuItem.click();
+
+  await expect(page.getByText('inherited from a group').first()).toBeVisible();
+
+  // No confirm button is offered, so the broken removal can never be triggered.
+  await expect(page.getByRole('button', { name: 'Remove' })).toHaveCount(0);
+});
+
+/**
+ * Guards against over-disabling the remove action: a direct team member (one that
+ * belongs to the team's INTERNAL_TEAM group) must still be removable.
+ */
+test('[TEAMS]: can remove a direct team member', async ({ page }) => {
+  const { user: owner, team } = await seedUser({ inheritMembers: false });
+
+  const directMember = await seedTeamMember({
+    teamId: team.id,
+    name: 'Direct Member',
+    role: TeamMemberRole.MEMBER,
+  });
+
+  await apiSignin({
+    page,
+    email: owner.email,
+    redirectPath: `/t/${team.url}/settings/members`,
+  });
+
+  const directMemberRow = page.getByRole('row').filter({ hasText: directMember.email });
+
+  await expect(directMemberRow).toBeVisible();
+
+  await openDropdownMenu(page, directMemberRow.getByRole('button').last());
+
+  const removeMenuItem = page.getByRole('menuitem', { name: 'Remove' });
+
+  // The "Remove" action is enabled for direct members and removing them succeeds.
+  await expect(removeMenuItem).toBeEnabled();
+  await removeMenuItem.click();
+
+  await page.getByRole('button', { name: 'Remove' }).click();
+
+  await expect(page.getByText('You have successfully removed this user from the team.').first()).toBeVisible();
+
+  // The member is actually gone after reloading the members list.
+  await page.reload();
+  await expect(page.getByRole('row').filter({ hasText: owner.email })).toBeVisible();
+  await expect(page.getByRole('row').filter({ hasText: directMember.email })).toHaveCount(0);
+});