Skip to content

Commit

Permalink
fix(clerk-js): Incorrect result counters in data tables (#2056)
Browse files Browse the repository at this point in the history
* fix(clerk-js): Incorrect result counters in data tables

* test(clerk-js): Unit test for pagination counts
  • Loading branch information
panteliselef committed Nov 7, 2023
1 parent f827e55 commit 64d3763
Show file tree
Hide file tree
Showing 7 changed files with 114 additions and 35 deletions.
6 changes: 6 additions & 0 deletions .changeset/chatty-boats-tease.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
'@clerk/clerk-js': patch
'@clerk/shared': patch
---

Fix incorrect pagination counters in data tables inside `<OrganizationProfile/>`.
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,16 @@ import { useFetchRoles, useLocalizeCustomRoles } from '../../hooks/useFetchRoles
import { handleError } from '../../utils';
import { DataTable, RoleSelect, RowContainer } from './MemberListTable';

const membershipsParams = {
memberships: {
pageSize: 10,
keepPreviousData: true,
},
};

export const ActiveMembersList = () => {
const card = useCardState();
const { organization, memberships } = useCoreOrganization({
memberships: true,
});
const { organization, memberships } = useCoreOrganization(membershipsParams);

const { options, isLoading: loadingRoles } = useFetchRoles();

Expand Down Expand Up @@ -44,6 +49,7 @@ export const ActiveMembersList = () => {
onPageChange={n => memberships?.fetchPage?.(n)}
itemCount={memberships?.count || 0}
pageCount={memberships?.pageCount || 0}
itemsPerPage={membershipsParams.memberships.pageSize}
isLoading={memberships?.isLoading || loadingRoles}
emptyStateLocalizationKey={localizationKeys('organizationProfile.membersPage.detailsTitle__emptyRow')}
headers={[
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,16 @@ import { useLocalizeCustomRoles } from '../../hooks/useFetchRoles';
import { handleError } from '../../utils';
import { DataTable, RowContainer } from './MemberListTable';

const invitationsParams = {
invitations: {
pageSize: 10,
keepPreviousData: true,
},
};

export const InvitedMembersList = () => {
const card = useCardState();
const { organization, invitations } = useCoreOrganization({
invitations: true,
});
const { organization, invitations } = useCoreOrganization(invitationsParams);

if (!organization) {
return null;
Expand All @@ -32,6 +37,7 @@ export const InvitedMembersList = () => {
onPageChange={invitations?.fetchPage || (() => null)}
itemCount={invitations?.count || 0}
pageCount={invitations?.pageCount || 0}
itemsPerPage={invitationsParams.invitations.pageSize}
isLoading={invitations?.isLoading}
emptyStateLocalizationKey={localizationKeys('organizationProfile.membersPage.invitationsTab.table__emptyRow')}
headers={[
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,16 +15,9 @@ type MembersListTableProps = {
onPageChange: (page: number) => void;
itemCount: number;
emptyStateLocalizationKey: LocalizationKey;
} & (
| {
itemsPerPage?: never;
pageCount: number;
}
| {
itemsPerPage: number;
pageCount?: never;
}
);
pageCount: number;
itemsPerPage: number;
};

export const DataTable = (props: MembersListTableProps) => {
const {
Expand All @@ -35,13 +28,12 @@ export const DataTable = (props: MembersListTableProps) => {
isLoading,
itemCount,
itemsPerPage,
pageCount: pageCountProp,
pageCount,
emptyStateLocalizationKey,
} = props;

const pageCount = rows.length !== 0 ? pageCountProp ?? Math.ceil(itemCount / itemsPerPage) : 1;
const startRowIndex = (page - 1) * rows.length;
const endRowIndex = Math.min(page * rows.length);
const startingRow = itemCount > 0 ? Math.max(0, (page - 1) * itemsPerPage) + 1 : 0;
const endingRow = Math.min(page * itemsPerPage, itemCount);

return (
<Col
Expand Down Expand Up @@ -90,8 +82,8 @@ export const DataTable = (props: MembersListTableProps) => {
siblingCount={1}
rowInfo={{
allRowsCount: itemCount,
startingRow: rows.length ? startRowIndex + 1 : startRowIndex,
endingRow: endRowIndex,
startingRow,
endingRow,
}}
/>
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,16 +6,16 @@ import { useCardState, UserPreview, withCardStateProvider } from '../../elements
import { handleError } from '../../utils';
import { DataTable, RowContainer } from './MemberListTable';

const ITEMS_PER_PAGE = 10;

const membershipRequestsParams = {
pageSize: ITEMS_PER_PAGE,
membershipRequests: {
pageSize: 10,
keepPreviousData: true,
},
};

export const RequestToJoinList = () => {
const card = useCardState();
const { organization, membershipRequests } = useCoreOrganization({
membershipRequests: membershipRequestsParams,
});
const { organization, membershipRequests } = useCoreOrganization(membershipRequestsParams);

if (!organization) {
return null;
Expand All @@ -25,8 +25,9 @@ export const RequestToJoinList = () => {
<DataTable
page={membershipRequests?.page || 1}
onPageChange={membershipRequests?.fetchPage ?? (() => null)}
itemCount={membershipRequests?.count ?? 0}
itemsPerPage={ITEMS_PER_PAGE}
itemCount={membershipRequests?.count || 0}
pageCount={membershipRequests?.pageCount || 0}
itemsPerPage={membershipRequestsParams.membershipRequests.pageSize}
isLoading={membershipRequests?.isLoading}
emptyStateLocalizationKey={localizationKeys('organizationProfile.membersPage.requestsTab.table__emptyRow')}
headers={[
Expand All @@ -49,9 +50,7 @@ const RequestRow = withCardStateProvider(
(props: { request: OrganizationMembershipRequestResource; onError: ReturnType<typeof useCardState>['setError'] }) => {
const { request, onError } = props;
const card = useCardState();
const { membership, membershipRequests } = useCoreOrganization({
membershipRequests: membershipRequestsParams,
});
const { membership, membershipRequests } = useCoreOrganization(membershipRequestsParams);

const onAccept = () => {
if (!membership || !membershipRequests) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,76 @@ describe('OrganizationMembers', () => {
});
});

it('display pagination counts for 2 pages', async () => {
const { wrapper, fixtures } = await createFixtures(f => {
f.withOrganizations();
f.withUser({
email_addresses: ['test@clerk.com'],
organization_memberships: [{ name: 'Org1', id: '1' }],
});
});

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

fixtures.clerk.organization?.getRoles.mockRejectedValue(null);

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

await waitFor(async () => {
expect(fixtures.clerk.organization?.getMemberships).toHaveBeenCalled();
expect(fixtures.clerk.organization?.getInvitations).not.toHaveBeenCalled();
expect(fixtures.clerk.organization?.getMembershipRequests).not.toHaveBeenCalled();

expect(queryByText(/displaying/i)).toBeInTheDocument();

expect(queryByText(/1 – 10/i)).toBeInTheDocument();
expect(queryByText(/of/i)).toBeInTheDocument();
expect(queryByText(/^14/i)).toBeInTheDocument();
});

await act(async () => await userEvent.click(getByText(/next/i)));

await waitFor(async () => {
expect(queryByText(/11 – 14/i)).toBeInTheDocument();
expect(queryByText(/of/i)).toBeInTheDocument();
expect(queryByText(/^14/i)).toBeInTheDocument();
});
});

it('display pagination counts for 1 page', async () => {
const { wrapper, fixtures } = await createFixtures(f => {
f.withOrganizations();
f.withUser({
email_addresses: ['test@clerk.com'],
organization_memberships: [{ name: 'Org1', id: '1' }],
});
});

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

fixtures.clerk.organization?.getRoles.mockRejectedValue(null);

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

await waitFor(async () => {
expect(queryByText(/displaying/i)).toBeInTheDocument();
expect(queryByText(/1 – 5/i)).toBeInTheDocument();
expect(queryByText(/of/i)).toBeInTheDocument();
expect(queryByText(/^5/i)).toBeInTheDocument();
expect(getByText(/next/i)).toBeDisabled();
});
});

// TODO: Bring this test back once we can determine the last admin by permissions.
it.skip('Last admin cannot change to member', async () => {
const membersList: OrganizationMembershipResource[] = [
Expand Down
4 changes: 2 additions & 2 deletions packages/shared/src/react/hooks/useOrganization.tsx
Original file line number Diff line number Diff line change
@@ -1,18 +1,18 @@
import type {
ClerkPaginatedResponse,
ClerkPaginationParams,
GetDomainsParams,
GetInvitationsParams,
GetMembershipRequestParams,
GetMembershipsParams,
GetMembersParams,
GetPendingInvitationsParams,
OrganizationDomainResource,
OrganizationInvitationResource,
OrganizationMembershipRequestResource,
OrganizationMembershipResource,
OrganizationResource,
} from '@clerk/types';
import type { ClerkPaginatedResponse } from '@clerk/types';
import type { GetMembersParams } from '@clerk/types';

import { deprecated } from '../../deprecated';
import { useSWR } from '../clerk-swr';
Expand Down

0 comments on commit 64d3763

Please sign in to comment.