From 69767bdd29214288542d2999747a98791933977d Mon Sep 17 00:00:00 2001 From: Sabrina Demagny Date: Tue, 29 Oct 2024 10:51:13 +0100 Subject: [PATCH 1/2] =?UTF-8?q?=F0=9F=94=A5(teams)=20remove=20pagination?= =?UTF-8?q?=20of=20teams=20listing?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit For frontend pagination is useless for teams, so we remove it. --- CHANGELOG.md | 9 +-- src/backend/core/api/viewsets.py | 1 + .../tests/teams/test_core_api_teams_list.py | 59 ++----------------- 3 files changed, 12 insertions(+), 57 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b8ba748ac..b39df489e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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 diff --git a/src/backend/core/api/viewsets.py b/src/backend/core/api/viewsets.py index 7ed8d7d1f..6c8973a48 100644 --- a/src/backend/core/api/viewsets.py +++ b/src/backend/core/api/viewsets.py @@ -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.""" diff --git a/src/backend/core/tests/teams/test_core_api_teams_list.py b/src/backend/core/tests/teams/test_core_api_teams_list.py index c4aed5828..420dd59e7 100644 --- a/src/backend/core/tests/teams/test_core_api_teams_list.py +++ b/src/backend/core/tests/teams/test_core_api_teams_list.py @@ -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() @@ -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(): @@ -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 ( @@ -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 From b46b0aa94cd33b5d360147550ef3eabea138d464 Mon Sep 17 00:00:00 2001 From: Nathan Panchout Date: Wed, 30 Oct 2024 20:35:10 +0100 Subject: [PATCH 2/2] =?UTF-8?q?=E2=99=BB=EF=B8=8F(frontend)=20delete=20inf?= =?UTF-8?q?inite=20scroll?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In order to be in agreement with the back, we must modify the front so as to no longer have infinite scroll in the team part. It is also necessary to modify the tests accordingly --- src/frontend/apps/desk/next-env.d.ts | 2 +- .../teams/team-management/api/useTeams.tsx | 53 ++-------- .../teams-panel/__tests__/PanelTeams.test.tsx | 96 ++++++++----------- .../teams/teams-panel/components/TeamList.tsx | 32 +------ .../app-desk/keyboard-navigation.spec.ts | 4 +- .../__tests__/app-desk/teams-panel.spec.ts | 24 +---- src/frontend/yarn.lock | 8 +- 7 files changed, 61 insertions(+), 158 deletions(-) diff --git a/src/frontend/apps/desk/next-env.d.ts b/src/frontend/apps/desk/next-env.d.ts index 62b8a52d3..95ddcaed0 100644 --- a/src/frontend/apps/desk/next-env.d.ts +++ b/src/frontend/apps/desk/next-env.d.ts @@ -2,4 +2,4 @@ /// // 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. diff --git a/src/frontend/apps/desk/src/features/teams/team-management/api/useTeams.tsx b/src/frontend/apps/desk/src/features/teams/team-management/api/useTeams.tsx index f259f2634..ac7784096 100644 --- a/src/frontend/apps/desk/src/features/teams/team-management/api/useTeams.tsx +++ b/src/frontend/apps/desk/src/features/teams/team-management/api/useTeams.tsx @@ -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'; @@ -17,18 +12,14 @@ export enum TeamsOrdering { export type TeamsParams = { ordering: TeamsOrdering; }; -type TeamsAPIParams = TeamsParams & { - page: number; -}; -type TeamsResponse = APIList; +type TeamsResponse = Team[]; export const getTeams = async ({ ordering, - page, -}: TeamsAPIParams): Promise => { - const orderingQuery = ordering ? `&ordering=${ordering}` : ''; - const response = await fetchAPI(`teams/?page=${page}${orderingQuery}`); +}: TeamsParams): Promise => { + 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)); @@ -39,33 +30,9 @@ export const getTeams = async ({ export const KEY_LIST_TEAM = 'teams'; -export function useTeams( - param: TeamsParams, - queryConfig?: DefinedInitialDataInfiniteOptions< - TeamsResponse, - APIError, - InfiniteData, - QueryKey, - number - >, -) { - return useInfiniteQuery< - TeamsResponse, - APIError, - InfiniteData, - 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), }); } diff --git a/src/frontend/apps/desk/src/features/teams/teams-panel/__tests__/PanelTeams.test.tsx b/src/frontend/apps/desk/src/features/teams/teams-panel/__tests__/PanelTeams.test.tsx index b8ec69ab4..68d355c37 100644 --- a/src/frontend/apps/desk/src/features/teams/teams-panel/__tests__/PanelTeams.test.tsx +++ b/src/frontend/apps/desk/src/features/teams/teams-panel/__tests__/PanelTeams.test.tsx @@ -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(, { wrapper: AppWrapper }); @@ -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(, { wrapper: AppWrapper }); @@ -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(, { wrapper: AppWrapper }); @@ -83,25 +74,22 @@ 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(, { wrapper: AppWrapper }); @@ -109,7 +97,7 @@ describe('PanelTeams', () => { }); it('renders the error', async () => { - fetchMock.mock(`end:/teams/?page=1&ordering=-created_at`, { + fetchMock.mock(`end:/teams/?ordering=-created_at`, { status: 500, }); @@ -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(, { wrapper: AppWrapper }); @@ -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(, { wrapper: AppWrapper }); diff --git a/src/frontend/apps/desk/src/features/teams/teams-panel/components/TeamList.tsx b/src/frontend/apps/desk/src/features/teams/teams-panel/components/TeamList.tsx index ac1002b61..6331cc96a 100644 --- a/src/frontend/apps/desk/src/features/teams/teams-panel/components/TeamList.tsx +++ b/src/frontend/apps/desk/src/features/teams/teams-panel/components/TeamList.tsx @@ -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'; @@ -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(null); - const teams = useMemo(() => { - return data?.pages.reduce((acc, page) => { - return acc.concat(page.results); - }, [] as Team[]); - }, [data?.pages]); return ( - { - void fetchNextPage(); - }} - scrollContainer={containerRef.current} - as="ul" - $margin={{ top: 'none' }} - $padding="none" - role="listbox" - > - - + ); }; diff --git a/src/frontend/apps/e2e/__tests__/app-desk/keyboard-navigation.spec.ts b/src/frontend/apps/e2e/__tests__/app-desk/keyboard-navigation.spec.ts index 48eae6cee..594bf1002 100644 --- a/src/frontend/apps/e2e/__tests__/app-desk/keyboard-navigation.spec.ts +++ b/src/frontend/apps/e2e/__tests__/app-desk/keyboard-navigation.spec.ts @@ -26,9 +26,9 @@ const payloadGetTeams = { }; const mockApiRequests = (page: Page) => { - void page.route('**/teams/?page=1&ordering=-created_at', (route) => { + void page.route('**/teams/?ordering=-created_at', (route) => { void route.fulfill({ - json: payloadGetTeams, + json: payloadGetTeams.results, }); }); }; diff --git a/src/frontend/apps/e2e/__tests__/app-desk/teams-panel.spec.ts b/src/frontend/apps/e2e/__tests__/app-desk/teams-panel.spec.ts index d06b12953..02749701d 100644 --- a/src/frontend/apps/e2e/__tests__/app-desk/teams-panel.spec.ts +++ b/src/frontend/apps/e2e/__tests__/app-desk/teams-panel.spec.ts @@ -1,7 +1,5 @@ import { expect, test } from '@playwright/test'; -import { waitForElementCount } from '../helpers'; - import { createTeam, keyCloakSignIn } from './common'; test.beforeEach(async ({ page, browserName }) => { @@ -31,13 +29,13 @@ test.describe('Teams Panel', () => { test('checks the sort button', async ({ page }) => { const responsePromiseSortDesc = page.waitForResponse( (response) => - response.url().includes('/teams/?page=1&ordering=-created_at') && + response.url().includes('/teams/?ordering=-created_at') && response.status() === 200, ); const responsePromiseSortAsc = page.waitForResponse( (response) => - response.url().includes('/teams/?page=1&ordering=created_at') && + response.url().includes('/teams/?ordering=created_at') && response.status() === 200, ); @@ -62,24 +60,6 @@ test.describe('Teams Panel', () => { expect(responseSortDesc.ok()).toBeTruthy(); }); - test('checks the infinite scroll', async ({ page, browserName }) => { - test.setTimeout(90000); - const panel = page.getByLabel('Teams panel').first(); - - const randomTeams = await createTeam( - page, - 'team-infinite', - browserName, - 40, - ); - - await expect(panel.locator('li')).toHaveCount(20); - await panel.getByText(randomTeams[24]).click(); - - await waitForElementCount(panel.locator('li'), 21, 10000); - expect(await panel.locator('li').count()).toBeGreaterThan(20); - }); - test('checks the hover and selected state', async ({ page, browserName }) => { const panel = page.getByLabel('Teams panel').first(); await createTeam(page, 'team-hover', browserName, 2); diff --git a/src/frontend/yarn.lock b/src/frontend/yarn.lock index e11c321c6..1199f0035 100644 --- a/src/frontend/yarn.lock +++ b/src/frontend/yarn.lock @@ -3492,7 +3492,7 @@ resolved "https://registry.yarnpkg.com/@types/minimatch/-/minimatch-3.0.5.tgz#1001cc5e6a3704b83c236027e77f2f58ea010f40" integrity sha512-Klz949h02Gz2uZCMGwDUSDS1YBlTdDDgbWHi+81l29tQALUtvz4rAYi5uoVhE5Lagoq6DeqAUlbrHvW/mXDgdQ== -"@types/node@*": +"@types/node@*", "@types/node@20.16.10": version "20.16.10" resolved "https://registry.yarnpkg.com/@types/node/-/node-20.16.10.tgz#0cc3fdd3daf114a4776f54ba19726a01c907ef71" integrity sha512-vQUKgWTjEIRFCvK6CyriPH3MZYiYlNy0fKiEYHWbcoWLEgs4opurGGKlebrTLqdSMIbXImH6XExNiIyNUv3WpA== @@ -3509,7 +3509,7 @@ resolved "https://registry.yarnpkg.com/@types/prop-types/-/prop-types-15.7.12.tgz#12bb1e2be27293c1406acb6af1c3f3a1481d98c6" integrity sha512-5zvhXYtRNRluoE/jAp4GVsSduVUzNWKkOZrCDBWYtE7biZywwdC2AcEzg+cSMLFRfVgeAFqpfNabiPjxFddV1Q== -"@types/react-dom@*": +"@types/react-dom@*", "@types/react-dom@18.3.0": version "18.3.0" resolved "https://registry.yarnpkg.com/@types/react-dom/-/react-dom-18.3.0.tgz#0cbc818755d87066ab6ca74fbedb2547d74a82b0" integrity sha512-EhwApuTmMBmXuFOikhQLIBUn6uFg81SwLMOAUgodJF14SOBOCMdU04gDoYi0WOJJHD144TL32z4yDqCW3dnkQg== @@ -8485,7 +8485,7 @@ safe-regex-test@^1.0.3: resolved "https://registry.yarnpkg.com/safer-buffer/-/safer-buffer-2.1.2.tgz#44fa161b0187b9549dd84bb91802f9bd8385cd6a" integrity sha512-YZo3K82SD7Riyi0E1EQPojLz7kpepnSQI9IyPbHHg1XXXevb5dJI7tpyN2ADxGcQbHG7vcyRHk0cbwqcQriUtg== -sass@^1.80.3: +sass@1.80.3: version "1.80.3" resolved "https://registry.yarnpkg.com/sass/-/sass-1.80.3.tgz#3f63dd527647d2b3de35f36acb971bda80517423" integrity sha512-ptDWyVmDMVielpz/oWy3YP3nfs7LpJTHIJZboMVs8GEC9eUmtZTZhMHlTW98wY4aEorDfjN38+Wr/XjskFWcfA== @@ -9259,7 +9259,7 @@ typed-array-length@^1.0.6: is-typed-array "^1.1.13" possible-typed-array-names "^1.0.0" -typescript@*, typescript@^5.0.4: +typescript@*, typescript@5.6.2, typescript@^5.0.4: version "5.6.2" resolved "https://registry.yarnpkg.com/typescript/-/typescript-5.6.2.tgz#d1de67b6bef77c41823f822df8f0b3bcff60a5a0" integrity sha512-NW8ByodCSNCwZeghjN3o+JX5OFH0Ojg6sadjEKY4huZ52TqbJTJnDo5+Tw98lSy63NZvi4n+ez5m2u5d4PkZyw==