Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore(clerk-js): Fix broken Organization methods #1871

Merged
merged 1 commit into from
Oct 12, 2023
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
7 changes: 7 additions & 0 deletions .changeset/unlucky-carrots-exist.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
'@clerk/clerk-js': patch
'@clerk/clerk-react': patch
'@clerk/types': patch
---

Fix methods in clerk-js that consumede paginated endpoints in order to retrieve single resources.
Copy link
Contributor

Choose a reason for hiding this comment

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

🙃 Should we replace consumede with consume ?

26 changes: 25 additions & 1 deletion packages/clerk-js/src/core/clerk.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,15 @@ import { mockNativeRuntime } from '../testUtils';
import Clerk from './clerk';
import { eventBus, events } from './events';
import type { AuthConfig, DisplayConfig, Organization } from './resources/internal';
import { Client, EmailLinkErrorCode, Environment, MagicLinkErrorCode, SignIn, SignUp } from './resources/internal';
import {
BaseResource,
Client,
EmailLinkErrorCode,
Environment,
MagicLinkErrorCode,
SignIn,
SignUp,
} from './resources/internal';
import { SessionCookieService } from './services';
import { mockJwt } from './test/fixtures';

Expand Down Expand Up @@ -2012,4 +2020,20 @@ describe('Clerk singleton', () => {
expect(url).toBe('https://rested-anemone-14.accounts.dev/?__dev_session=deadbeef');
});
});

describe('Organizations', () => {
it('getOrganization', async () => {
// @ts-ignore
BaseResource._fetch = jest.fn().mockResolvedValue({});
const sut = new Clerk(devFrontendApi);

await sut.getOrganization('some-org-id');

// @ts-ignore
expect(BaseResource._fetch).toHaveBeenCalledWith({
method: 'GET',
path: '/organizations/some-org-id',
});
});
});
});
6 changes: 2 additions & 4 deletions packages/clerk-js/src/core/clerk.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1152,10 +1152,8 @@ export default class Clerk implements ClerkInterface {
return await OrganizationMembership.retrieve();
};

public getOrganization = async (organizationId: string): Promise<Organization | undefined> => {
return (await OrganizationMembership.retrieve()).find(orgMem => orgMem.organization.id === organizationId)
?.organization;
};
public getOrganization = async (organizationId: string): Promise<OrganizationResource> =>
Organization.get(organizationId);

public updateEnvironment(environment: EnvironmentResource) {
this.#environment = environment;
Expand Down
25 changes: 13 additions & 12 deletions packages/clerk-js/src/core/resources/Organization.ts
Original file line number Diff line number Diff line change
Expand Up @@ -342,18 +342,19 @@ export class Organization extends BaseResource implements OrganizationResource {

public async reload(params?: ClerkResourceReloadParams): Promise<this> {
const { rotatingTokenNonce } = params || {};
const json = await BaseResource._fetch(
{
method: 'GET',
path: `/me/organization_memberships`,
rotatingTokenNonce,
},
{ forceUpdateClient: true },
Copy link
Member

Choose a reason for hiding this comment

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

Don't we need to include this parameter as well?

Copy link
Member Author

Choose a reason for hiding this comment

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

brought it back

);
const currentOrganization = (json?.response as unknown as OrganizationMembershipJSON[]).find(
orgMem => orgMem.organization.id === this.id,
);
return this.fromJSON(currentOrganization?.organization as OrganizationJSON);

const json = (
await BaseResource._fetch<OrganizationJSON>(
{
path: `/organizations/${this.id}`,
method: 'GET',
rotatingTokenNonce,
},
{ forceUpdateClient: true },
)
)?.response as unknown as OrganizationJSON;

return this.fromJSON(json);
}
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
import type { OrganizationResource, UserOrganizationInvitationResource } from '@clerk/types';
import { useState } from 'react';

import { Organization } from '../../../core/resources/Organization';
import { useCoreOrganizationList } from '../../contexts';
import { useCoreClerk, useCoreOrganizationList } from '../../contexts';
import { localizationKeys } from '../../customizables';
import { useCardState, withCardStateProvider } from '../../elements';
import { handleError } from '../../utils';
Expand All @@ -25,6 +24,7 @@ export const AcceptRejectInvitationButtons = (props: { onAccept: () => void }) =

export const InvitationPreview = withCardStateProvider((props: UserOrganizationInvitationResource) => {
const card = useCardState();
const { getOrganization } = useCoreClerk();
const [acceptedOrganization, setAcceptedOrganization] = useState<OrganizationResource | null>(null);
const { userInvitations } = useCoreOrganizationList({
userInvitations: organizationListParams.userInvitations,
Expand All @@ -34,7 +34,7 @@ export const InvitationPreview = withCardStateProvider((props: UserOrganizationI
return card
.runAsync(async () => {
const updatedItem = await props.accept();
const organization = await Organization.get(props.publicOrganizationData.id);
const organization = await getOrganization(props.publicOrganizationData.id);
return [updatedItem, organization] as const;
})
.then(([updatedItem, organization]) => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,11 @@ import { describe } from '@jest/globals';

import { render, waitFor } from '../../../../testUtils';
import { bindCreateFixtures } from '../../../utils/test/createFixtures';
import { createFakeUserOrganizationMembership } from '../../OrganizationSwitcher/__tests__/utlis';
import { createFakeOrganization } from '../../CreateOrganization/__tests__/CreateOrganization.test';
import {
createFakeUserOrganizationInvitation,
createFakeUserOrganizationMembership,
} from '../../OrganizationSwitcher/__tests__/utlis';
import { OrganizationList } from '../OrganizationList';

const { createFixtures } = bindCreateFixtures('OrganizationList');
Expand Down Expand Up @@ -109,6 +113,85 @@ describe('OrganizationList', () => {
expect(queryByRole('button', { name: 'Create organization' })).toBeInTheDocument();
});
});

it('lists invitations', async () => {
const { wrapper, props, fixtures } = await createFixtures(f => {
f.withOrganizations();
f.withUser({
email_addresses: ['[email protected]'],
});
});

fixtures.clerk.user?.getOrganizationMemberships.mockReturnValueOnce(
Promise.resolve({
data: [
createFakeUserOrganizationMembership({
id: '1',
organization: {
id: '1',
name: 'Org1',
slug: 'org1',
membersCount: 1,
adminDeleteEnabled: false,
maxAllowedMemberships: 1,
pendingInvitationsCount: 1,
},
}),
],
total_count: 1,
}),
);

const invitation = createFakeUserOrganizationInvitation({
id: '1',
emailAddress: '[email protected]',
publicOrganizationData: {
name: 'OrgOne',
},
});

invitation.accept = jest.fn().mockResolvedValue(
createFakeUserOrganizationInvitation({
id: '1',
emailAddress: '[email protected]',
publicOrganizationData: {
name: 'OrgOne',
},
status: 'accepted',
}),
);

fixtures.clerk.user?.getOrganizationInvitations.mockReturnValueOnce(
Promise.resolve({
data: [invitation],
total_count: 1,
}),
);

fixtures.clerk.getOrganization.mockResolvedValue(
createFakeOrganization({
adminDeleteEnabled: false,
id: '2',
maxAllowedMemberships: 0,
membersCount: 1,
name: 'OrgOne',
pendingInvitationsCount: 0,
slug: '',
}),
);

props.setProps({ hidePersonal: true });
const { queryByText, userEvent, getByRole, queryByRole } = render(<OrganizationList />, { wrapper });

await waitFor(async () => {
// Display membership
expect(queryByText('Org1')).toBeInTheDocument();
// Display invitation
expect(queryByText('OrgOne')).toBeInTheDocument();
await userEvent.click(getByRole('button', { name: 'Join' }));
expect(queryByRole('button', { name: 'Join' })).not.toBeInTheDocument();
});
});
});

describe('CreateOrganization', () => {
Expand Down
4 changes: 2 additions & 2 deletions packages/react/src/isomorphicClerk.ts
Original file line number Diff line number Diff line change
Expand Up @@ -719,10 +719,10 @@ export default class IsomorphicClerk {
}
};

getOrganization = async (organizationId: string): Promise<OrganizationResource | undefined | void> => {
getOrganization = async (organizationId: string): Promise<OrganizationResource | void> => {
const callback = () => this.clerkjs?.getOrganization(organizationId);
if (this.clerkjs && this.#loaded) {
return callback() as Promise<OrganizationResource | undefined>;
return callback() as Promise<OrganizationResource>;
} else {
this.premountMethodCalls.set('getOrganization', callback);
}
Expand Down
2 changes: 1 addition & 1 deletion packages/types/src/clerk.ts
Original file line number Diff line number Diff line change
Expand Up @@ -442,7 +442,7 @@ export interface Clerk {
/**
* Retrieves a single organization by id.
*/
getOrganization: (organizationId: string) => Promise<OrganizationResource | undefined>;
getOrganization: (organizationId: string) => Promise<OrganizationResource>;

/**
* Handles a 401 response from Frontend API by refreshing the client and session object accordingly
Expand Down