fix(types,clerk-js): Allow organization admin to leave, if there are …

…more admins This commit fixes a bug in our <OrganizationProfile /> component where an admin couldn't leave the organization even though there were more org admins present.
clerk · Jul 20, 2023 · 808e45d · 808e45d
1 parent 86f2fbc
commit 808e45d
Show file tree

Hide file tree

Showing 7 changed files with 131 additions and 72 deletions.
diff --git a/.changeset/weak-ducks-double.md b/.changeset/weak-ducks-double.md
@@ -0,0 +1,5 @@
+---
+'@clerk/clerk-js': patch
+---
+
+In `<OrganizationProfile />` 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
@@ -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 (
     <ProfileSection
@@ -87,10 +95,10 @@ const OrganizationDangerSection = () => {
           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 && (
           <IconButton
             aria-label='Delete organization'
             icon={

diff --git a/...ges/clerk-js/src/ui/components/OrganizationProfile/__tests__/OrganizationMembers.test.tsx b/...ges/clerk-js/src/ui/components/OrganizationProfile/__tests__/OrganizationMembers.test.tsx
@@ -1,45 +1,13 @@
 import { render, userEvent, waitFor } from '@clerk/shared/testUtils';
-import type { MembershipRole, OrganizationMembershipResource, OrganizationResource } from '@clerk/types';
-import { describe, it, jest } from '@jest/globals';
+import type { OrganizationMembershipResource } from '@clerk/types';
+import { describe, it } from '@jest/globals';
 
 import { bindCreateFixtures } from '../../../utils/test/createFixtures';
 import { OrganizationMembers } from '../OrganizationMembers';
+import { createFakeMember } from './utils';
 
 const { createFixtures } = bindCreateFixtures('OrganizationProfile');
 
-type FakeMemberParams = {
-  id: string;
-  orgId: string;
-  role?: MembershipRole;
-  identifier?: string;
-  firstName?: string;
-  lastName?: string;
-  profileImageUrl?: string;
-  imageUrl?: string;
-  createdAt?: Date;
-};
-
-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;
-};
-
 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/...es/clerk-js/src/ui/components/OrganizationProfile/__tests__/OrganizationSettings.test.tsx b/...es/clerk-js/src/ui/components/OrganizationProfile/__tests__/OrganizationSettings.test.tsx
@@ -1,38 +1,72 @@
+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(<OrganizationSettings />, { 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(<OrganizationSettings />, { 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'],
         organization_memberships: [{ name: 'Org1', role: 'basic_member' }],
       });
     });
 
+    fixtures.clerk.organization?.getMemberships.mockReturnValue(Promise.resolve(adminsList));
     const { getByText } = render(<OrganizationSettings />, { 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(<OrganizationSettings />, { wrapper });
+      const { getByText } = render(<OrganizationSettings />, { 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(<OrganizationSettings />, { wrapper });
-      await userEvent.click(getByText(/leave organization/i, { exact: false }));
+      fixtures.clerk.organization?.getMemberships.mockReturnValue(Promise.resolve(adminsList));
+      const { findByText } = render(<OrganizationSettings />, { 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
@@ -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
@@ -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 = <N extends UnpackContext<typeof ComponentContext>[
     const MockClerkProvider = (props: any) => {
       const { children } = props;
       return (
-        <CoreClerkContextWrapper clerk={clerkMock}>
-          <EnvironmentProvider value={environmentMock}>
-            <OptionsProvider value={optionsMock}>
-              <RouteContext.Provider value={routerMock}>
-                <AppearanceProvider appearanceKey={'signIn'}>
-                  <FlowMetadataProvider flow={componentName as any}>
-                    <InternalThemeProvider>
-                      <ComponentContext.Provider value={{ ...componentContextProps, componentName }}>
-                        {children}
-                      </ComponentContext.Provider>
-                    </InternalThemeProvider>
-                  </FlowMetadataProvider>
-                </AppearanceProvider>
-              </RouteContext.Provider>
-            </OptionsProvider>
-          </EnvironmentProvider>
-        </CoreClerkContextWrapper>
+        <SWRConfig value={{ provider: () => new Map(), dedupingInterval: 0 }}>
+          <CoreClerkContextWrapper clerk={clerkMock}>
+            <EnvironmentProvider value={environmentMock}>
+              <OptionsProvider value={optionsMock}>
+                <RouteContext.Provider value={routerMock}>
+                  <AppearanceProvider appearanceKey={'signIn'}>
+                    <FlowMetadataProvider flow={componentName as any}>
+                      <InternalThemeProvider>
+                        <ComponentContext.Provider value={{ ...componentContextProps, componentName }}>
+                          {children}
+                        </ComponentContext.Provider>
+                      </InternalThemeProvider>
+                    </FlowMetadataProvider>
+                  </AppearanceProvider>
+                </RouteContext.Provider>
+              </OptionsProvider>
+            </EnvironmentProvider>
+          </CoreClerkContextWrapper>
+        </SWRConfig>
       );
     };
 

diff --git 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<OrganizationResource>;
 }
 
-export type GetMembershipsParams = ClerkPaginationParams;
+export type GetMembershipsParams = {
+  role?: MembershipRole[];
+} & ClerkPaginationParams;
 
 export type GetPendingInvitationsParams = ClerkPaginationParams;