Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/weak-ducks-double.md
Original file line number Diff line number Diff line change
@@ -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.
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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={
Expand Down
Original file line number Diff line number Diff line change
@@ -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 => {
Expand Down Expand Up @@ -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' }),
Expand Down
Original file line number Diff line number Diff line change
@@ -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', () => {
Expand All @@ -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({
Expand All @@ -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');
});
});
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
import type { MembershipRole, OrganizationMembershipResource, OrganizationResource } from '@clerk/types';
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure if this is the right place to place a common test helper for this component

Copy link
Copy Markdown
Member

@anagstef anagstef Jul 19, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can see that this helper is specific to this component, so it's a good place I guess.

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;
};
37 changes: 20 additions & 17 deletions packages/clerk-js/src/ui/utils/test/createFixtures.tsx
Original file line number Diff line number Diff line change
@@ -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';
Expand Down Expand Up @@ -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 }}>
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We were forced to wrap our test fixture with a SWR config, as swr uses a global cache for tests and there is no easy way to clear it before every test. This lead to use the same initial response when mocking a method organizations.getMemberships() in our case and every test expect the first one to fail
Related issue

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💯 All tests seem to pass correctly, so it is good news this does not mess with anything else.

<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>
);
};

Expand Down
4 changes: 3 additions & 1 deletion packages/types/src/organization.ts
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,9 @@ export interface OrganizationResource extends ClerkResource {
setLogo: (params: SetOrganizationLogoParams) => Promise<OrganizationResource>;
}

export type GetMembershipsParams = ClerkPaginationParams;
export type GetMembershipsParams = {
role?: MembershipRole[];
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this optional?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You don't have to specify the role, If it is left empty or undefined it will return memberships of any role

} & ClerkPaginationParams;

export type GetPendingInvitationsParams = ClerkPaginationParams;

Expand Down