diff --git a/api/environments/views.py b/api/environments/views.py index 8fda3026fe6d..5533bc2d6bb2 100644 --- a/api/environments/views.py +++ b/api/environments/views.py @@ -19,6 +19,7 @@ from rest_framework.serializers import BaseSerializer from rest_framework.viewsets import GenericViewSet +from app.pagination import CustomPagination from core.models import AbstractBaseExportableModel from environments.permissions.permissions import ( EnvironmentAdminPermission, @@ -86,6 +87,7 @@ ) class EnvironmentViewSet(viewsets.ModelViewSet): # type: ignore[type-arg] lookup_field = "api_key" + pagination_class = CustomPagination permission_classes = [EnvironmentPermissions] def get_serializer_class(self): # type: ignore[no-untyped-def] diff --git a/api/tests/unit/environments/test_unit_environments_views.py b/api/tests/unit/environments/test_unit_environments_views.py index 30b0a9a71cd4..706e0e52449c 100644 --- a/api/tests/unit/environments/test_unit_environments_views.py +++ b/api/tests/unit/environments/test_unit_environments_views.py @@ -16,6 +16,7 @@ from rest_framework.test import APIClient from api_keys.models import MasterAPIKey +from app.pagination import CustomPagination from audit.models import AuditLog, RelatedObjectType # type: ignore[attr-defined] from core.constants import STRING from environments.identities.models import Identity @@ -1395,3 +1396,114 @@ def test_create_environment__required_metadata_on_other_project__returns_201( # Then assert response.status_code == status.HTTP_201_CREATED + + +def test_list_environments__returns_paginated_response( + admin_client_new: APIClient, + project: Project, +) -> None: + # Given + environments = [ + Environment.objects.create(name=f"Env {i}", project=project) for i in range(11) + ] + another_project = Project.objects.create( + name="Other", organisation=project.organisation + ) + Environment.objects.create(name="Other Project Env", project=another_project) + url = reverse("api-v1:environments:environment-list") + + # When + response = admin_client_new.get( + url, data={"project": project.id}, content_type="application/json" + ) + + # Then + assert response.status_code == status.HTTP_200_OK + data = response.json() + assert data["count"] == 11 + assert len(data["results"]) == 11 + assert data["next"] is None + assert {environment["id"] for environment in data["results"]} == { + environment.id for environment in environments + } + + +def test_list_environments__page_size_param_is_respected( + admin_client_new: APIClient, + project: Project, + mocker: MockerFixture, +) -> None: + # Given + environments = [ + Environment.objects.create(name=f"Env {i}", project=project) for i in range(5) + ] + mocker.patch.object(CustomPagination, "max_page_size", 3) + another_project = Project.objects.create( + name="Other", organisation=project.organisation + ) + Environment.objects.create(name="Other Project Env", project=another_project) + url = reverse("api-v1:environments:environment-list") + + # When + response = admin_client_new.get( + url, + data={"project": project.id, "page_size": 2, "page": 1}, + content_type="application/json", + ) + page_2_response = admin_client_new.get( + url, + data={"project": project.id, "page_size": 2, "page": 2}, + content_type="application/json", + ) + page_3_response = admin_client_new.get( + url, + data={"project": project.id, "page_size": 2, "page": 3}, + content_type="application/json", + ) + page_size_3_response = admin_client_new.get( + url, + data={"project": project.id, "page_size": 3}, + content_type="application/json", + ) + max_page_size_response = admin_client_new.get( + url, + data={"project": project.id, "page_size": 10}, + content_type="application/json", + ) + + # Then + assert response.status_code == status.HTTP_200_OK + assert page_2_response.status_code == status.HTTP_200_OK + assert page_3_response.status_code == status.HTTP_200_OK + assert page_size_3_response.status_code == status.HTTP_200_OK + assert max_page_size_response.status_code == status.HTTP_200_OK + data = response.json() + page_2_data = page_2_response.json() + page_3_data = page_3_response.json() + page_size_3_data = page_size_3_response.json() + max_page_size_data = max_page_size_response.json() + assert len(data["results"]) == 2 + assert data["count"] == 5 + assert data["next"] is not None + page_1_ids = {environment["id"] for environment in data["results"]} + page_2_ids = {environment["id"] for environment in page_2_data["results"]} + page_3_ids = {environment["id"] for environment in page_3_data["results"]} + assert len(page_2_data["results"]) == 2 + assert page_2_data["next"] is not None + assert len(page_3_data["results"]) == 1 + assert page_3_data["next"] is None + page_size_3_ids = {environment["id"] for environment in page_size_3_data["results"]} + assert len(page_size_3_ids) == 3 + assert page_size_3_ids.issubset({environment.id for environment in environments}) + assert len(max_page_size_data["results"]) == 3 + assert max_page_size_data["next"] is not None + assert page_1_ids.isdisjoint(page_2_ids) + assert page_1_ids.isdisjoint(page_3_ids) + assert page_2_ids.isdisjoint(page_3_ids) + assert page_1_ids | page_2_ids | page_3_ids == { + environment.id for environment in environments + } + assert page_size_3_data["count"] == 5 + assert {environment["id"] for environment in page_size_3_data["results"]}.issubset( + {environment.id for environment in environments} + ) diff --git a/frontend/common/services/useEnvironment.ts b/frontend/common/services/useEnvironment.ts index 0799aea8c666..40d71733220a 100644 --- a/frontend/common/services/useEnvironment.ts +++ b/frontend/common/services/useEnvironment.ts @@ -1,8 +1,6 @@ -import { FetchBaseQueryError } from '@reduxjs/toolkit/query' -import { Environment, Res } from 'common/types/responses' +import { Res } from 'common/types/responses' import { Req } from 'common/types/requests' import { service } from 'common/service' -import { fetchAllPages } from 'common/utils/fetchAllPages' export const environmentService = service .enhanceEndpoints({ addTagTypes: ['Environment'] }) @@ -19,19 +17,6 @@ export const environmentService = service url: `environments/`, }), }), - getAllEnvironments: builder.query< - Res['allEnvironments'], - Req['getAllEnvironments'] - >({ - providesTags: [{ id: 'LIST', type: 'Environment' }], - queryFn: ({ projectId }, _, _2, baseQuery) => - fetchAllPages(async (page) => { - const { data, error } = await baseQuery({ - url: `environments/?project=${projectId}&page=${page}`, - }) - return { data: data as Res['environments'] | undefined, error } - }), - }), getEnvironment: builder.query({ providesTags: (res) => [{ id: res?.id, type: 'Environment' }], query: (query: Req['getEnvironment']) => ({ @@ -111,7 +96,6 @@ export async function updateEnvironment( export const { useCreateEnvironmentMutation, - useGetAllEnvironmentsQuery, useGetEnvironmentMetricsQuery, useGetEnvironmentQuery, useGetEnvironmentsQuery, diff --git a/frontend/common/types/requests.ts b/frontend/common/types/requests.ts index a99864c3f2e8..c640d555ef54 100644 --- a/frontend/common/types/requests.ts +++ b/frontend/common/types/requests.ts @@ -273,9 +273,6 @@ export type Req = { getEnvironments: { projectId: number } - getAllEnvironments: { - projectId: number - } getOrganisationUsage: { organisationId: number projectId?: number diff --git a/frontend/common/types/responses.ts b/frontend/common/types/responses.ts index f8dee3a87451..6f769cac03fe 100644 --- a/frontend/common/types/responses.ts +++ b/frontend/common/types/responses.ts @@ -1550,7 +1550,6 @@ export type Res = { projects: ProjectSummary[] project: Project environments: PagedResponse - allEnvironments: Environment[] webhook: Webhook webhooks: Webhook[] organisationUsage: { diff --git a/frontend/web/components/pages/usage/components/UsageBreakdown/useGroupedBreakdown.ts b/frontend/web/components/pages/usage/components/UsageBreakdown/useGroupedBreakdown.ts index f88434f293ed..baff79d7470e 100644 --- a/frontend/web/components/pages/usage/components/UsageBreakdown/useGroupedBreakdown.ts +++ b/frontend/web/components/pages/usage/components/UsageBreakdown/useGroupedBreakdown.ts @@ -3,7 +3,7 @@ import { skipToken } from '@reduxjs/toolkit/query' import { BillingPeriod } from 'common/types/requests' import { useGetOrganisationUsageQuery } from 'common/services/useOrganisationUsage' import { useGetProjectsQuery } from 'common/services/useProject' -import { useGetAllEnvironmentsQuery } from 'common/services/useEnvironment' +import { useGetEnvironmentsQuery } from 'common/services/useEnvironment' import { breakdownStatusOf, retryFailed } from './breakdownView' import { byScope, @@ -50,7 +50,7 @@ export const useGroupedBreakdown = ({ const projects = useGetProjectsQuery( groupBy === 'project' && organisationId ? { organisationId } : skipToken, ) - const environments = useGetAllEnvironmentsQuery( + const environments = useGetEnvironmentsQuery( groupBy === 'environment' && projectId ? { projectId } : skipToken, ) @@ -67,7 +67,7 @@ export const useGroupedBreakdown = ({ return byScope( grouped.currentData, 'environment_id', - new Map(environments.currentData?.map(({ id, name }) => [id, name])), + new Map(environments.currentData?.results?.map(({ id, name }) => [id, name])), 'Deleted environments', ) }