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

🔥(teams) remove pagination of teams listing #496

Merged
merged 2 commits into from
Oct 31, 2024
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
9 changes: 5 additions & 4 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,15 @@ and this project adheres to

## [Unreleased]

### Added

- ✨(teams) register contacts on admin views

### Changed

- 🔥(teams) remove pagination of teams listing
- 🔥(teams) remove search users by trigram

### Added

- ✨(teams) register contacts on admin views

### Fixed

- 💚(ci) improve E2E tests #492
Expand Down
1 change: 1 addition & 0 deletions src/backend/core/api/viewsets.py
Original file line number Diff line number Diff line change
Expand Up @@ -246,6 +246,7 @@ class TeamViewSet(
ordering_fields = ["created_at"]
ordering = ["-created_at"]
queryset = models.Team.objects.all()
pagination_class = None

def get_queryset(self):
"""Custom queryset to get user related teams."""
Expand Down
59 changes: 6 additions & 53 deletions src/backend/core/tests/teams/test_core_api_teams_list.py
Original file line number Diff line number Diff line change
Expand Up @@ -47,60 +47,13 @@ def test_api_teams_list_authenticated():
)

assert response.status_code == HTTP_200_OK
results = response.json()["results"]
results = response.json()

assert len(results) == 5
results_id = {result["id"] for result in results}
assert expected_ids == results_id


@mock.patch.object(PageNumberPagination, "get_page_size", return_value=2)
def test_api_teams_list_pagination(
_mock_page_size,
):
"""Pagination should work as expected."""
user = factories.UserFactory()

client = APIClient()
client.force_login(user)

team_ids = [
str(access.team.id)
for access in factories.TeamAccessFactory.create_batch(3, user=user)
]

# Get page 1
response = client.get(
"/api/v1.0/teams/",
)

assert response.status_code == HTTP_200_OK
content = response.json()

assert content["count"] == 3
assert content["next"] == "http://testserver/api/v1.0/teams/?page=2"
assert content["previous"] is None

assert len(content["results"]) == 2
for item in content["results"]:
team_ids.remove(item["id"])

# Get page 2
response = client.get(
"/api/v1.0/teams/?page=2",
)

assert response.status_code == HTTP_200_OK
content = response.json()

assert content["count"] == 3
assert content["next"] is None
assert content["previous"] == "http://testserver/api/v1.0/teams/"

assert len(content["results"]) == 1
team_ids.remove(content["results"][0]["id"])
assert team_ids == []


def test_api_teams_list_authenticated_distinct():
"""A team with several related users should only be listed once."""
user = factories.UserFactory()
Expand All @@ -118,8 +71,8 @@ def test_api_teams_list_authenticated_distinct():

assert response.status_code == HTTP_200_OK
content = response.json()
assert len(content["results"]) == 1
assert content["results"][0]["id"] == str(team.id)
assert len(content) == 1
assert content[0]["id"] == str(team.id)


def test_api_teams_order():
Expand All @@ -142,7 +95,7 @@ def test_api_teams_order():
assert response.status_code == 200

response_data = response.json()
response_team_ids = [team["id"] for team in response_data["results"]]
response_team_ids = [team["id"] for team in response_data]

team_ids.reverse()
assert (
Expand Down Expand Up @@ -171,7 +124,7 @@ def test_api_teams_order_param():

response_data = response.json()

response_team_ids = [team["id"] for team in response_data["results"]]
response_team_ids = [team["id"] for team in response_data]

assert (
response_team_ids == team_ids
Expand Down
2 changes: 1 addition & 1 deletion src/frontend/apps/desk/next-env.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,4 @@
/// <reference types="next/image-types/global" />

// NOTE: This file should not be edited
// see https://nextjs.org/docs/basic-features/typescript for more information.
// see https://nextjs.org/docs/pages/building-your-application/configuring/typescript for more information.
Original file line number Diff line number Diff line change
@@ -1,11 +1,6 @@
import {
DefinedInitialDataInfiniteOptions,
InfiniteData,
QueryKey,
useInfiniteQuery,
} from '@tanstack/react-query';
import { useQuery } from '@tanstack/react-query';

import { APIError, APIList, errorCauses, fetchAPI } from '@/api';
import { APIError, errorCauses, fetchAPI } from '@/api';

import { Team } from '../types';

Expand All @@ -17,18 +12,14 @@ export enum TeamsOrdering {
export type TeamsParams = {
ordering: TeamsOrdering;
};
type TeamsAPIParams = TeamsParams & {
page: number;
};

type TeamsResponse = APIList<Team>;
type TeamsResponse = Team[];

export const getTeams = async ({
ordering,
page,
}: TeamsAPIParams): Promise<TeamsResponse> => {
const orderingQuery = ordering ? `&ordering=${ordering}` : '';
const response = await fetchAPI(`teams/?page=${page}${orderingQuery}`);
}: TeamsParams): Promise<TeamsResponse> => {
const orderingQuery = ordering ? `?ordering=${ordering}` : '';
const response = await fetchAPI(`teams/${orderingQuery}`);

if (!response.ok) {
throw new APIError('Failed to get the teams', await errorCauses(response));
Expand All @@ -39,33 +30,9 @@ export const getTeams = async ({

export const KEY_LIST_TEAM = 'teams';

export function useTeams(
param: TeamsParams,
queryConfig?: DefinedInitialDataInfiniteOptions<
TeamsResponse,
APIError,
InfiniteData<TeamsResponse>,
QueryKey,
number
>,
) {
return useInfiniteQuery<
TeamsResponse,
APIError,
InfiniteData<TeamsResponse>,
QueryKey,
number
>({
initialPageParam: 1,
queryKey: [KEY_LIST_TEAM, param],
queryFn: ({ pageParam }) =>
getTeams({
...param,
page: pageParam,
}),
getNextPageParam(lastPage, allPages) {
return lastPage.next ? allPages.length + 1 : undefined;
},
...queryConfig,
export function useTeams(params: TeamsParams) {
return useQuery({
queryKey: [KEY_LIST_TEAM, params],
queryFn: () => getTeams(params),
});
}
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,7 @@ describe('PanelTeams', () => {
});

it('renders with no team to display', async () => {
fetchMock.mock(`end:/teams/?page=1&ordering=-created_at`, {
count: 0,
results: [],
});
fetchMock.mock(`end:/teams/?ordering=-created_at`, []);

render(<TeamList />, { wrapper: AppWrapper });

Expand All @@ -40,16 +37,13 @@ describe('PanelTeams', () => {
});

it('renders an empty team', async () => {
fetchMock.mock(`end:/teams/?page=1&ordering=-created_at`, {
count: 1,
results: [
{
id: '1',
name: 'Team 1',
accesses: [],
},
],
});
fetchMock.mock(`end:/teams/?ordering=-created_at`, [
{
id: '1',
name: 'Team 1',
accesses: [],
},
]);

render(<TeamList />, { wrapper: AppWrapper });

Expand All @@ -59,21 +53,18 @@ describe('PanelTeams', () => {
});

it('renders a team with only 1 member', async () => {
fetchMock.mock(`end:/teams/?page=1&ordering=-created_at`, {
count: 1,
results: [
{
id: '1',
name: 'Team 1',
accesses: [
{
id: '1',
role: 'owner',
},
],
},
],
});
fetchMock.mock(`end:/teams/?ordering=-created_at`, [
{
id: '1',
name: 'Team 1',
accesses: [
{
id: '1',
role: 'owner',
},
],
},
]);

render(<TeamList />, { wrapper: AppWrapper });

Expand All @@ -83,33 +74,30 @@ describe('PanelTeams', () => {
});

it('renders a non-empty team', () => {
fetchMock.mock(`end:/teams/?page=1&ordering=-created_at`, {
count: 1,
results: [
{
id: '1',
name: 'Team 1',
accesses: [
{
id: '1',
role: 'admin',
},
{
id: '2',
role: 'member',
},
],
},
],
});
fetchMock.mock(`end:/teams/?ordering=-created_at`, [
{
id: '1',
name: 'Team 1',
accesses: [
{
id: '1',
role: 'admin',
},
{
id: '2',
role: 'member',
},
],
},
]);

render(<TeamList />, { wrapper: AppWrapper });

expect(screen.getByRole('status')).toBeInTheDocument();
});

it('renders the error', async () => {
fetchMock.mock(`end:/teams/?page=1&ordering=-created_at`, {
fetchMock.mock(`end:/teams/?ordering=-created_at`, {
status: 500,
});

Expand All @@ -125,10 +113,7 @@ describe('PanelTeams', () => {
});

it('renders with team panel open', async () => {
fetchMock.mock(`end:/teams/?page=1&ordering=-created_at`, {
count: 1,
results: [],
});
fetchMock.mock(`end:/teams/?ordering=-created_at`, []);

render(<Panel />, { wrapper: AppWrapper });

Expand All @@ -140,10 +125,7 @@ describe('PanelTeams', () => {
});

it('closes and opens the team panel', async () => {
fetchMock.get(`end:/teams/?page=1&ordering=-created_at`, {
count: 1,
results: [],
});
fetchMock.get(`end:/teams/?ordering=-created_at`, []);

render(<Panel />, { wrapper: AppWrapper });

Expand Down
Original file line number Diff line number Diff line change
@@ -1,9 +1,8 @@
import { Loader } from '@openfun/cunningham-react';
import React, { useMemo, useRef } from 'react';
import React, { useRef } from 'react';
import { useTranslation } from 'react-i18next';

import { Box, Text } from '@/components';
import { InfiniteScroll } from '@/components/InfiniteScroll';
import { Team, useTeams } from '@/features/teams/team-management';

import { useTeamStore } from '../store';
Expand Down Expand Up @@ -57,39 +56,14 @@ const TeamListState = ({ isLoading, isError, teams }: PanelTeamsStateProps) => {

export const TeamList = () => {
const ordering = useTeamStore((state) => state.ordering);
const {
data,
isError,
isLoading,
fetchNextPage,
hasNextPage,
isFetchingNextPage,
} = useTeams({
const { data, isError, isLoading } = useTeams({
ordering,
});
const containerRef = useRef<HTMLDivElement>(null);
const teams = useMemo(() => {
return data?.pages.reduce((acc, page) => {
return acc.concat(page.results);
}, [] as Team[]);
}, [data?.pages]);

return (
<Box $css="overflow-y: auto; overflow-x: hidden;" ref={containerRef}>
<InfiniteScroll
hasMore={hasNextPage}
isLoading={isFetchingNextPage}
next={() => {
void fetchNextPage();
}}
scrollContainer={containerRef.current}
as="ul"
$margin={{ top: 'none' }}
$padding="none"
role="listbox"
>
<TeamListState isLoading={isLoading} isError={isError} teams={teams} />
</InfiniteScroll>
<TeamListState isLoading={isLoading} isError={isError} teams={data} />
</Box>
);
};
Loading
Loading