From a1065031eec0da26ad1fcc5a268cc4dc2b059847 Mon Sep 17 00:00:00 2001 From: Lebaud Antoine Date: Mon, 11 Mar 2024 11:09:30 +0100 Subject: [PATCH] =?UTF-8?q?=E2=9C=A8(frontend)=20sort=20team's=20members?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We are displaying all team's members in a datagrid. This propose to enable sorting on columns, to easily find members. Please note that few issues were faced when activating the sorting on the Cunningham components. First, custom columns can not be sorted (yet), a PR has been merged on Cunningham's side. We're waiting for the next release. Second, when sorting data rows, if any of the column has some null values, the datagrid sorting state becomes inconsistent. Thx @AntoLC for spotting the issue. It's work in progress on Cunningham's side to fix the issue. Finally, Cunningham export only the SortModel type, which is an array, and doesn't export its items' type. I might have miss something but it feels weird to redefine its items type. Columns wiggle on sorting, because they data is set to undefined while fetching the next batch. it's visually weird, but not a major pain. Next release of Cunningham will allow us to set the column to a fixed size. --- .../members/__tests__/MemberGrid.test.tsx | 130 +++++++++++++++++- .../features/members/api/useTeamsAccesses.tsx | 10 +- .../members/components/MemberGrid.tsx | 75 ++++++++-- 3 files changed, 200 insertions(+), 15 deletions(-) diff --git a/src/frontend/apps/desk/src/features/members/__tests__/MemberGrid.test.tsx b/src/frontend/apps/desk/src/features/members/__tests__/MemberGrid.test.tsx index e9187e6..9103b71 100644 --- a/src/frontend/apps/desk/src/features/members/__tests__/MemberGrid.test.tsx +++ b/src/frontend/apps/desk/src/features/members/__tests__/MemberGrid.test.tsx @@ -1,5 +1,5 @@ import '@testing-library/jest-dom'; -import { render, screen } from '@testing-library/react'; +import { render, screen, waitFor } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; import fetchMock from 'fetch-mock'; @@ -215,4 +215,132 @@ describe('MemberGrid', () => { screen.queryByLabelText('Add members to the team'), ).not.toBeInTheDocument(); }); + + it.each([ + ['name', 'Names'], + ['email', 'Emails'], + ['role', 'Roles'], + ])('checks the sorting', async (ordering, header_name) => { + const mockedData = [ + { + id: '123', + role: Role.ADMIN, + user: { + id: '123', + name: 'albert', + email: 'albert@test.com', + }, + abilities: {} as any, + }, + { + id: '789', + role: Role.OWNER, + user: { + id: '456', + name: 'philipp', + email: 'philipp@test.com', + }, + abilities: {} as any, + }, + { + id: '456', + role: Role.MEMBER, + user: { + id: '789', + name: 'fany', + email: 'fany@test.com', + }, + abilities: {} as any, + }, + ]; + + const sortedMockedData = [...mockedData].sort((a, b) => + a.id > b.id ? 1 : -1, + ); + const reversedMockedData = [...sortedMockedData].reverse(); + + fetchMock.get(`/api/teams/123456/accesses/?page=1`, { + count: 3, + results: mockedData, + }); + + fetchMock.get(`/api/teams/123456/accesses/?page=1&ordering=${ordering}`, { + count: 3, + results: sortedMockedData, + }); + + fetchMock.get(`/api/teams/123456/accesses/?page=1&ordering=-${ordering}`, { + count: 3, + results: reversedMockedData, + }); + + render(, { + wrapper: AppWrapper, + }); + + expect(screen.getByRole('status')).toBeInTheDocument(); + + expect(fetchMock.lastUrl()).toBe(`/api/teams/123456/accesses/?page=1`); + + await waitFor(() => { + expect(screen.queryByRole('status')).not.toBeInTheDocument(); + }); + + let rows = screen.getAllByRole('row'); + expect(rows[1]).toHaveTextContent('albert'); + expect(rows[2]).toHaveTextContent('philipp'); + expect(rows[3]).toHaveTextContent('fany'); + + expect(screen.queryByLabelText('arrow_drop_down')).not.toBeInTheDocument(); + expect(screen.queryByLabelText('arrow_drop_up')).not.toBeInTheDocument(); + + await userEvent.click(screen.getByText(header_name)); + + expect(fetchMock.lastUrl()).toBe( + `/api/teams/123456/accesses/?page=1&ordering=${ordering}`, + ); + + await waitFor(() => { + expect(screen.queryByRole('status')).not.toBeInTheDocument(); + }); + + rows = screen.getAllByRole('row'); + expect(rows[1]).toHaveTextContent('albert'); + expect(rows[2]).toHaveTextContent('fany'); + expect(rows[3]).toHaveTextContent('philipp'); + + expect(await screen.findByText('arrow_drop_up')).toBeInTheDocument(); + + await userEvent.click(screen.getByText(header_name)); + + expect(fetchMock.lastUrl()).toBe( + `/api/teams/123456/accesses/?page=1&ordering=-${ordering}`, + ); + await waitFor(() => { + expect(screen.queryByRole('status')).not.toBeInTheDocument(); + }); + + rows = screen.getAllByRole('row'); + expect(rows[1]).toHaveTextContent('philipp'); + expect(rows[2]).toHaveTextContent('fany'); + expect(rows[3]).toHaveTextContent('albert'); + + expect(await screen.findByText('arrow_drop_down')).toBeInTheDocument(); + + await userEvent.click(screen.getByText(header_name)); + + expect(fetchMock.lastUrl()).toBe('/api/teams/123456/accesses/?page=1'); + + await waitFor(() => { + expect(screen.queryByRole('status')).not.toBeInTheDocument(); + }); + + rows = screen.getAllByRole('row'); + expect(rows[1]).toHaveTextContent('albert'); + expect(rows[2]).toHaveTextContent('philipp'); + expect(rows[3]).toHaveTextContent('fany'); + + expect(screen.queryByLabelText('arrow_drop_down')).not.toBeInTheDocument(); + expect(screen.queryByLabelText('arrow_drop_up')).not.toBeInTheDocument(); + }); }); diff --git a/src/frontend/apps/desk/src/features/members/api/useTeamsAccesses.tsx b/src/frontend/apps/desk/src/features/members/api/useTeamsAccesses.tsx index cfadc9f..ee4f4ea 100644 --- a/src/frontend/apps/desk/src/features/members/api/useTeamsAccesses.tsx +++ b/src/frontend/apps/desk/src/features/members/api/useTeamsAccesses.tsx @@ -7,6 +7,7 @@ import { Access } from '../types'; export type TeamAccessesAPIParams = { page: number; teamId: string; + ordering?: string; }; type AccessesResponse = APIList; @@ -14,8 +15,15 @@ type AccessesResponse = APIList; export const getTeamAccesses = async ({ page, teamId, + ordering, }: TeamAccessesAPIParams): Promise => { - const response = await fetchAPI(`teams/${teamId}/accesses/?page=${page}`); + let url = `teams/${teamId}/accesses/?page=${page}`; + + if (ordering) { + url += '&ordering=' + ordering; + } + + const response = await fetchAPI(url); if (!response.ok) { throw new APIError( diff --git a/src/frontend/apps/desk/src/features/members/components/MemberGrid.tsx b/src/frontend/apps/desk/src/features/members/components/MemberGrid.tsx index 7d6771f..bbf0f8f 100644 --- a/src/frontend/apps/desk/src/features/members/components/MemberGrid.tsx +++ b/src/frontend/apps/desk/src/features/members/components/MemberGrid.tsx @@ -1,4 +1,9 @@ -import { Button, DataGrid, usePagination } from '@openfun/cunningham-react'; +import { + Button, + DataGrid, + SortModel, + usePagination, +} from '@openfun/cunningham-react'; import React, { useEffect, useState } from 'react'; import { useTranslation } from 'react-i18next'; @@ -18,6 +23,33 @@ interface MemberGridProps { currentRole: Role; } +// FIXME : ask Cunningham to export this type +type SortModelItem = { + field: string; + sort: 'asc' | 'desc' | null; +}; + +const defaultOrderingMapping: Record = { + 'user.name': 'name', + 'user.email': 'email', + localizedRole: 'role', +}; + +/** + * Formats the sorting model based on a given mapping. + * @param {SortModelItem} sortModel The sorting model item containing field and sort direction. + * @param {Record} mapping The mapping object to map field names. + * @returns {string} The formatted sorting string. + */ +function formatSortModel( + sortModel: SortModelItem, + mapping = defaultOrderingMapping, +) { + const { field, sort } = sortModel; + const orderingField = mapping[field] || field; + return sort === 'desc' ? `-${orderingField}` : orderingField; +} + export const MemberGrid = ({ team, currentRole }: MemberGridProps) => { const [isModalMemberOpen, setIsModalMemberOpen] = useState(false); const { t } = useTranslation(); @@ -25,24 +57,42 @@ export const MemberGrid = ({ team, currentRole }: MemberGridProps) => { const pagination = usePagination({ pageSize: PAGE_SIZE, }); + const [sortModel, setSortModel] = useState([]); const { page, pageSize, setPagesCount } = pagination; + + const ordering = sortModel.length ? formatSortModel(sortModel[0]) : undefined; + const { data, isLoading, error } = useTeamAccesses({ teamId: team.id, page, + ordering, }); - const accesses = data?.results; - - useEffect(() => { - setPagesCount(data?.count ? Math.ceil(data.count / pageSize) : 0); - }, [data?.count, pageSize, setPagesCount]); - - const dictRole = { + const localizedRoles = { [Role.ADMIN]: t('Admin'), [Role.MEMBER]: t('Member'), [Role.OWNER]: t('Owner'), }; + /* + * Bug occurs from the Cunningham Datagrid component, when applying sorting + * on null values. Sanitize empty values to ensure consistent sorting functionality. + */ + const accesses = + data?.results?.map((access) => ({ + ...access, + localizedRole: localizedRoles[access.role], + user: { + ...access.user, + name: access.user.name || '', + email: access.user.email || '', + }, + })) || []; + + useEffect(() => { + setPagesCount(data?.count ? Math.ceil(data.count / pageSize) : 0); + }, [data?.count, pageSize, setPagesCount]); + return ( <> {currentRole !== Role.MEMBER && ( @@ -104,11 +154,8 @@ export const MemberGrid = ({ team, currentRole }: MemberGridProps) => { headerName: t('Emails'), }, { - id: 'role', + field: 'localizedRole', headerName: t('Roles'), - renderCell({ row }) { - return dictRole[row.role]; - }, }, { id: 'column-actions', @@ -123,9 +170,11 @@ export const MemberGrid = ({ team, currentRole }: MemberGridProps) => { }, }, ]} - rows={accesses || []} + rows={accesses} isLoading={isLoading} pagination={pagination} + onSortModelChange={setSortModel} + sortModel={sortModel} /> {isModalMemberOpen && (