Ryan/documenso
1
0
Fork 1
mirror of https://github.com/documenso/documenso.git synced 2026-06-22 04:12:06 +10:00
Code Issues Packages Projects Releases Wiki Activity

fix: improve team member removal ux

Browse Source
This commit is contained in:
David Nguyen
2026-06-19 16:52:00 +10:00
parent 40d20ad068
commit d6d9a9aedc
3 changed files with 167 additions and 25 deletions
Download Patch File Download Diff File Expand all files Collapse all files
apps/remix/app/components/dialogs/team-member-delete-dialog.tsx
+25 -6
View File
@@ -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 })}
>
apps/remix/app/components/tables/team-members-table.tsx
+37 -19
View File
@@ -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>
packages/app-tests/e2e/teams/team-members.spec.ts
+105
View File
@@ -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);
});