From 808e45dc42cc65c8aec99f034131721d34d32656 Mon Sep 17 00:00:00 2001 From: Haris Chaniotakis Date: Thu, 20 Jul 2023 15:41:01 +0300 Subject: [PATCH] fix(types,clerk-js): Allow organization admin to leave, if there are more admins This commit fixes a bug in our component where an admin couldn't leave the organization even though there were more org admins present. --- .changeset/weak-ducks-double.md | 5 ++ .../OrganizationSettings.tsx | 16 +++-- .../__tests__/OrganizationMembers.test.tsx | 40 ++--------- .../__tests__/OrganizationSettings.test.tsx | 66 +++++++++++++++---- .../OrganizationProfile/__tests__/utils.ts | 35 ++++++++++ .../src/ui/utils/test/createFixtures.tsx | 37 ++++++----- packages/types/src/organization.ts | 4 +- 7 files changed, 131 insertions(+), 72 deletions(-) create mode 100644 .changeset/weak-ducks-double.md create mode 100644 packages/clerk-js/src/ui/components/OrganizationProfile/__tests__/utils.ts diff --git a/.changeset/weak-ducks-double.md b/.changeset/weak-ducks-double.md new file mode 100644 index 0000000000..37599371c1 --- /dev/null +++ b/.changeset/weak-ducks-double.md @@ -0,0 +1,5 @@ +--- +'@clerk/clerk-js': patch +--- + +In `` component, allow an admin to leave the current organization if there are more admins present. diff --git a/packages/clerk-js/src/ui/components/OrganizationProfile/OrganizationSettings.tsx b/packages/clerk-js/src/ui/components/OrganizationProfile/OrganizationSettings.tsx index 2939944eee..c6a4e03107 100644 --- a/packages/clerk-js/src/ui/components/OrganizationProfile/OrganizationSettings.tsx +++ b/packages/clerk-js/src/ui/components/OrganizationProfile/OrganizationSettings.tsx @@ -58,14 +58,22 @@ const OrganizationProfileSection = () => { }; const OrganizationDangerSection = () => { - const { organization, membership } = useCoreOrganization(); + const { + organization, + membership, + membershipList: adminMembers, + } = useCoreOrganization({ + membershipList: { role: ['admin'] }, + }); const { navigate } = useRouter(); - if (!organization || !membership) { + if (!organization || !membership || !adminMembers) { return null; } const adminDeleteEnabled = organization.adminDeleteEnabled; + const hasMoreThanOneAdmin = adminMembers.length > 1; + const isAdmin = membership.role === 'admin'; return ( { colorScheme='danger' textVariant='buttonExtraSmallBold' onClick={() => navigate('leave')} - isDisabled={membership.role === 'admin'} + isDisabled={isAdmin && !hasMoreThanOneAdmin} localizationKey={localizationKeys('organizationProfile.profilePage.dangerSection.leaveOrganization.title')} /> - {membership.role === 'admin' && adminDeleteEnabled && ( + {isAdmin && adminDeleteEnabled && ( { - return { - destroy: jest.fn() as any, - update: jest.fn() as any, - organization: { id: params.orgId } as any as OrganizationResource, - id: params.id, - role: params?.role || 'admin', - createdAt: params?.createdAt || new Date(), - updatedAt: new Date(), - publicMetadata: {}, - publicUserData: { - userId: params.id, - identifier: params?.identifier || 'test_user', - firstName: params?.firstName || 'test_firstName', - lastName: params?.lastName || 'test_lastName', - profileImageUrl: params?.profileImageUrl || '', - imageUrl: params?.imageUrl || '', - }, - } as any; -}; - describe('OrganizationMembers', () => { it('renders the Organization Members page', async () => { const { wrapper } = await createFixtures(f => { @@ -144,7 +112,7 @@ describe('OrganizationMembers', () => { it.todo('changes role on a member from organization when clicking the respective button on a user row'); it.todo('changes tab and renders the pending invites list'); - it.skip('shows the "You" badge when the member id from the list matches the current session user id', async () => { + it('shows the "You" badge when the member id from the list matches the current session user id', async () => { const membersList: OrganizationMembershipResource[] = [ createFakeMember({ id: '1', orgId: '1', role: 'admin', identifier: 'test_user1' }), createFakeMember({ id: '2', orgId: '1', role: 'basic_member', identifier: 'test_user2' }), diff --git a/packages/clerk-js/src/ui/components/OrganizationProfile/__tests__/OrganizationSettings.test.tsx b/packages/clerk-js/src/ui/components/OrganizationProfile/__tests__/OrganizationSettings.test.tsx index 396e195b04..cba8930640 100644 --- a/packages/clerk-js/src/ui/components/OrganizationProfile/__tests__/OrganizationSettings.test.tsx +++ b/packages/clerk-js/src/ui/components/OrganizationProfile/__tests__/OrganizationSettings.test.tsx @@ -1,27 +1,57 @@ +import { render, userEvent, waitFor } from '@clerk/shared/testUtils'; +import type { OrganizationMembershipResource } from '@clerk/types'; import { describe, it } from '@jest/globals'; -import React from 'react'; -import { render } from '../../../../testUtils'; import { bindCreateFixtures } from '../../../utils/test/createFixtures'; import { OrganizationSettings } from '../OrganizationSettings'; +import { createFakeMember } from './utils'; const { createFixtures } = bindCreateFixtures('OrganizationProfile'); describe('OrganizationSettings', () => { - it('enables organization profile button and disables leave when user is admin', async () => { - const { wrapper } = await createFixtures(f => { + it('enables organization profile button and disables leave when user is the only admin', async () => { + const adminsList: OrganizationMembershipResource[] = [createFakeMember({ id: '1', orgId: '1', role: 'admin' })]; + + const { wrapper, fixtures } = await createFixtures(f => { + f.withOrganizations(); + f.withUser({ email_addresses: ['test@clerk.dev'], organization_memberships: [{ name: 'Org1', role: 'admin' }] }); + }); + + fixtures.clerk.organization?.getMemberships.mockReturnValue(Promise.resolve(adminsList)); + const { getByText } = render(, { wrapper }); + await waitFor(() => { + expect(fixtures.clerk.organization?.getMemberships).toHaveBeenCalled(); + expect(getByText('Settings')).toBeDefined(); + expect(getByText('Org1', { exact: false }).closest('button')).not.toBeNull(); + expect(getByText(/leave organization/i, { exact: false }).closest('button')).toHaveAttribute('disabled'); + }); + }); + + it('enables organization profile button and enables leave when user is admin and there is more', async () => { + const adminsList: OrganizationMembershipResource[] = [ + createFakeMember({ id: '1', orgId: '1', role: 'admin' }), + createFakeMember({ id: '2', orgId: '1', role: 'admin' }), + ]; + + const { wrapper, fixtures } = await createFixtures(f => { f.withOrganizations(); f.withUser({ email_addresses: ['test@clerk.dev'], organization_memberships: [{ name: 'Org1', role: 'admin' }] }); }); + fixtures.clerk.organization?.getMemberships.mockReturnValue(Promise.resolve(adminsList)); const { getByText } = render(, { wrapper }); - expect(getByText('Settings')).toBeDefined(); - expect(getByText('Org1', { exact: false }).closest('button')).not.toBeNull(); - expect(getByText(/leave organization/i, { exact: false }).closest('button')).toHaveAttribute('disabled'); + await waitFor(() => { + expect(fixtures.clerk.organization?.getMemberships).toHaveBeenCalled(); + expect(getByText('Settings')).toBeDefined(); + expect(getByText('Org1', { exact: false }).closest('button')).not.toBeNull(); + expect(getByText(/leave organization/i, { exact: false }).closest('button')).not.toHaveAttribute('disabled'); + }); }); it('disables organization profile button and enables leave when user is not admin', async () => { - const { wrapper } = await createFixtures(f => { + const adminsList: OrganizationMembershipResource[] = [createFakeMember({ id: '1', orgId: '1', role: 'admin' })]; + + const { wrapper, fixtures } = await createFixtures(f => { f.withOrganizations(); f.withUser({ email_addresses: ['test@clerk.dev'], @@ -29,10 +59,14 @@ describe('OrganizationSettings', () => { }); }); + fixtures.clerk.organization?.getMemberships.mockReturnValue(Promise.resolve(adminsList)); const { getByText } = render(, { wrapper }); - expect(getByText('Settings')).toBeDefined(); - expect(getByText('Org1', { exact: false }).closest('button')).toBeNull(); - expect(getByText(/leave organization/i, { exact: false }).closest('button')).not.toHaveAttribute('disabled'); + await waitFor(() => { + expect(fixtures.clerk.organization?.getMemberships).toHaveBeenCalled(); + expect(getByText('Settings')).toBeDefined(); + expect(getByText('Org1', { exact: false }).closest('button')).toBeNull(); + expect(getByText(/leave organization/i, { exact: false }).closest('button')).not.toHaveAttribute('disabled'); + }); }); describe('Navigation', () => { @@ -45,12 +79,14 @@ describe('OrganizationSettings', () => { }); }); - const { getByText, userEvent } = render(, { wrapper }); + const { getByText } = render(, { wrapper }); await userEvent.click(getByText('Org1', { exact: false })); expect(fixtures.router.navigate).toHaveBeenCalledWith('profile'); }); it('navigates to Leave Organization page when clicking on the respective button and user is not admin', async () => { + const adminsList: OrganizationMembershipResource[] = [createFakeMember({ id: '1', orgId: '1', role: 'admin' })]; + const { wrapper, fixtures } = await createFixtures(f => { f.withOrganizations(); f.withUser({ @@ -59,8 +95,10 @@ describe('OrganizationSettings', () => { }); }); - const { getByText, userEvent } = render(, { wrapper }); - await userEvent.click(getByText(/leave organization/i, { exact: false })); + fixtures.clerk.organization?.getMemberships.mockReturnValue(Promise.resolve(adminsList)); + const { findByText } = render(, { wrapper }); + await waitFor(() => expect(fixtures.clerk.organization?.getMemberships).toHaveBeenCalled()); + await userEvent.click(await findByText(/leave organization/i, { exact: false })); expect(fixtures.router.navigate).toHaveBeenCalledWith('leave'); }); }); diff --git a/packages/clerk-js/src/ui/components/OrganizationProfile/__tests__/utils.ts b/packages/clerk-js/src/ui/components/OrganizationProfile/__tests__/utils.ts new file mode 100644 index 0000000000..255d55ce69 --- /dev/null +++ b/packages/clerk-js/src/ui/components/OrganizationProfile/__tests__/utils.ts @@ -0,0 +1,35 @@ +import type { MembershipRole, OrganizationMembershipResource, OrganizationResource } from '@clerk/types'; +import { jest } from '@jest/globals'; + +type FakeMemberParams = { + id: string; + orgId: string; + role?: MembershipRole; + identifier?: string; + firstName?: string; + lastName?: string; + profileImageUrl?: string; + imageUrl?: string; + createdAt?: Date; +}; + +export const createFakeMember = (params: FakeMemberParams): OrganizationMembershipResource => { + return { + destroy: jest.fn() as any, + update: jest.fn() as any, + organization: { id: params.orgId } as any as OrganizationResource, + id: params.id, + role: params?.role || 'admin', + createdAt: params?.createdAt || new Date(), + updatedAt: new Date(), + publicMetadata: {}, + publicUserData: { + userId: params.id, + identifier: params?.identifier || 'test_user', + firstName: params?.firstName || 'test_firstName', + lastName: params?.lastName || 'test_lastName', + profileImageUrl: params?.profileImageUrl || '', + imageUrl: params?.imageUrl || '', + }, + } as any; +}; diff --git a/packages/clerk-js/src/ui/utils/test/createFixtures.tsx b/packages/clerk-js/src/ui/utils/test/createFixtures.tsx index 264913eb9f..7284b2fabc 100644 --- a/packages/clerk-js/src/ui/utils/test/createFixtures.tsx +++ b/packages/clerk-js/src/ui/utils/test/createFixtures.tsx @@ -1,6 +1,7 @@ import type { ClerkOptions, ClientJSON, EnvironmentJSON, LoadedClerk } from '@clerk/types'; import { jest } from '@jest/globals'; import React from 'react'; +import { SWRConfig } from 'swr'; import { default as ClerkCtor } from '../../../core/clerk'; import { Client, Environment } from '../../../core/resources'; @@ -84,23 +85,25 @@ const unboundCreateFixtures = [ const MockClerkProvider = (props: any) => { const { children } = props; return ( - - - - - - - - - {children} - - - - - - - - + new Map(), dedupingInterval: 0 }}> + + + + + + + + + {children} + + + + + + + + + ); }; diff --git a/packages/types/src/organization.ts b/packages/types/src/organization.ts index c87a193364..2ee32d9e4c 100644 --- a/packages/types/src/organization.ts +++ b/packages/types/src/organization.ts @@ -51,7 +51,9 @@ export interface OrganizationResource extends ClerkResource { setLogo: (params: SetOrganizationLogoParams) => Promise; } -export type GetMembershipsParams = ClerkPaginationParams; +export type GetMembershipsParams = { + role?: MembershipRole[]; +} & ClerkPaginationParams; export type GetPendingInvitationsParams = ClerkPaginationParams;