Skip to content

Commit

Permalink
fix(clerk-js): Disable role toggle in <OrganizationMembers />, if las…
Browse files Browse the repository at this point in the history
…t admin (#1721)
  • Loading branch information
panteliselef authored Sep 15, 2023
1 parent 75be1d6 commit 23c0739
Show file tree
Hide file tree
Showing 3 changed files with 103 additions and 19 deletions.
5 changes: 5 additions & 0 deletions .changeset/lucky-rockets-yawn.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@clerk/clerk-js': patch
---

Disable role selection for the last admin in OrganizationProfile
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,13 @@ export const ActiveMembersList = () => {
organization,
membershipList,
membership: currentUserMembership,
memberships: adminMembers,
...rest
} = useCoreOrganization({
membershipList: { offset: (page - 1) * ITEMS_PER_PAGE, limit: ITEMS_PER_PAGE },
memberships: {
role: ['admin'],
},
});
const isAdmin = currentUserMembership?.role === 'admin';

Expand All @@ -32,22 +36,28 @@ export const ActiveMembersList = () => {
return null;
}

//TODO: calculate if user is the only admin
const canChangeOwnAdminRole = isAdmin && organization?.membersCount > 1;

const handleRoleChange = (membership: OrganizationMembershipResource) => (newRole: MembershipRole) => {
if (!isAdmin) {
return;
}
return card.runAsync(membership.update({ role: newRole })).catch(err => handleError(err, [], card.setError));
return card
.runAsync(async () => {
await membership.update({ role: newRole });
await (adminMembers as any).unstable__mutate?.();
})
.catch(err => handleError(err, [], card.setError));
};

const handleRemove = (membership: OrganizationMembershipResource) => () => {
if (!isAdmin) {
return;
}
return card
.runAsync(membership.destroy())
.runAsync(async () => {
const destroyedMembership = membership.destroy();
await (adminMembers as any).unstable__mutate?.();
return destroyedMembership;
})
.then(mutateSwrState)
.catch(err => handleError(err, [], card.setError));
};
Expand All @@ -70,8 +80,9 @@ export const ActiveMembersList = () => {
<MemberRow
key={m.id}
membership={m}
onRoleChange={canChangeOwnAdminRole ? handleRoleChange(m) : undefined}
onRoleChange={handleRoleChange(m)}
onRemove={handleRemove(m)}
adminCount={adminMembers?.count || 0}
/>
))}
/>
Expand All @@ -81,15 +92,17 @@ export const ActiveMembersList = () => {
const MemberRow = (props: {
membership: OrganizationMembershipResource;
onRemove: () => unknown;
adminCount: number;
onRoleChange?: (role: MembershipRole) => unknown;
}) => {
const { membership, onRemove, onRoleChange } = props;
const { membership, onRemove, onRoleChange, adminCount } = props;
const card = useCardState();
const { membership: currentUserMembership } = useCoreOrganization();
const user = useCoreUser();

const isAdmin = currentUserMembership?.role === 'admin';
const isCurrentUser = user.id === membership.publicUserData.userId;
const isLastAdmin = adminCount <= 1 && membership.role === 'admin';

return (
<RowContainer>
Expand All @@ -112,7 +125,7 @@ const MemberRow = (props: {
<Td>
{isAdmin ? (
<RoleSelect
isDisabled={card.isLoading || !onRoleChange}
isDisabled={card.isLoading || !onRoleChange || isLastAdmin}
value={membership.role}
onChange={onRoleChange}
/>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,15 @@ describe('OrganizationMembers', () => {
lastName: 'Last2',
createdAt: new Date('2022-01-01'),
}),
createFakeMember({
id: '3',
orgId: '1',
role: 'admin',
identifier: 'test_user3',
firstName: 'First3',
lastName: 'Last3',
createdAt: new Date('2022-01-01'),
}),
];
const { wrapper, fixtures } = await createFixtures(f => {
f.withOrganizations();
Expand All @@ -120,17 +129,74 @@ describe('OrganizationMembers', () => {
});
});

fixtures.clerk.organization?.getMemberships.mockReturnValue(Promise.resolve(membersList));
const { queryByText } = render(<OrganizationMembers />, { wrapper });
expect(fixtures.clerk.organization?.getMemberships).toHaveBeenCalled();
expect(fixtures.clerk.organization?.getPendingInvitations).not.toHaveBeenCalled();
expect(fixtures.clerk.organization?.getMembershipRequests).not.toHaveBeenCalled();
expect(queryByText('test_user1')).toBeDefined();
expect(queryByText('First1 Last1')).toBeDefined();
expect(queryByText('Admin')).toBeDefined();
expect(queryByText('test_user2')).toBeDefined();
expect(queryByText('First2 Last2')).toBeDefined();
expect(queryByText('Member')).toBeDefined();
fixtures.clerk.organization?.getMemberships.mockReturnValueOnce(
Promise.resolve({
data: [],
total_count: 2,
}),
);

fixtures.clerk.organization?.getMemberships.mockReturnValueOnce(Promise.resolve(membersList));

const { queryByText, queryAllByRole } = render(<OrganizationMembers />, { wrapper });

await waitFor(() => {
expect(fixtures.clerk.organization?.getMemberships).toHaveBeenCalled();
expect(fixtures.clerk.organization?.getPendingInvitations).not.toHaveBeenCalled();
expect(fixtures.clerk.organization?.getMembershipRequests).not.toHaveBeenCalled();
expect(queryByText('test_user1')).toBeInTheDocument();
expect(queryByText('First1 Last1')).toBeInTheDocument();
const buttons = queryAllByRole('button', { name: 'Admin' });
buttons.forEach(button => expect(button).not.toBeDisabled());
expect(queryByText('test_user2')).toBeInTheDocument();
expect(queryByText('First2 Last2')).toBeInTheDocument();
expect(queryByText('Member')).toBeInTheDocument();
});
});

it('Last admin cannot change to member', async () => {
const membersList: OrganizationMembershipResource[] = [
createFakeMember({
id: '1',
orgId: '1',
role: 'admin',
identifier: 'test_user1',
firstName: 'First1',
lastName: 'Last1',
createdAt: new Date('2022-01-01'),
}),
createFakeMember({
id: '2',
orgId: '1',
role: 'basic_member',
identifier: 'test_user2',
firstName: 'First2',
lastName: 'Last2',
createdAt: new Date('2022-01-01'),
}),
];
const { wrapper, fixtures } = await createFixtures(f => {
f.withOrganizations();
f.withUser({
email_addresses: ['[email protected]'],
organization_memberships: [{ name: 'Org1', id: '1', role: 'admin' }],
});
});

fixtures.clerk.organization?.getMemberships.mockReturnValueOnce(
Promise.resolve({
data: [],
total_count: 0,
}),
);

fixtures.clerk.organization?.getMemberships.mockReturnValueOnce(Promise.resolve(membersList));

const { queryByRole } = render(<OrganizationMembers />, { wrapper });

await waitFor(() => {
expect(queryByRole('button', { name: 'Admin' })).toBeDisabled();
});
});

it('displays counter in requests tab', async () => {
Expand Down

0 comments on commit 23c0739

Please sign in to comment.