From 1988ac106846b0bec6535b3f824409f6cfd2851d Mon Sep 17 00:00:00 2001 From: Joey Orlando Date: Fri, 7 Jul 2023 18:24:21 +0200 Subject: [PATCH] fix pagination next and previous links for schedules and integrations GET endpoints (#2467) # Which issue(s) this PR fixes closes https://github.com/grafana/oncall/issues/2463 ## Checklist - [ ] Unit, integration, and e2e (if applicable) tests updated - [ ] Documentation added (or `pr:no public docs` PR label added if not required) - [x] `CHANGELOG.md` updated (or `pr:no changelog` PR label added if not required) --- CHANGELOG.md | 6 ++++++ .../api/tests/test_alert_receive_channel.py | 15 +++++++------- engine/apps/api/tests/test_schedules.py | 4 ++-- .../apps/api/views/alert_receive_channel.py | 20 ++----------------- engine/apps/api/views/schedule.py | 11 ++-------- engine/common/api_helpers/paginators.py | 18 +++++++++++++++-- 6 files changed, 36 insertions(+), 38 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8fc1e169..4db19143 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Add `event.users.avatar_full` field to `GET /api/internal/v1/schedules/{schedule_id}/filter_events` payload by @joeyorlando ([#2459](https://github.com/grafana/oncall/pull/2459)) +### Changed + +- Modified DRF pagination class used by `GET /api/internal/v1/alert_receive_channels` and `GET /api/internal/v1/schedules` + endpoints so that the `next` and `previous` pagination links are properly set when OnCall is run behind + a reverse proxy by @joeyorlando ([#TBD](https://github.com/grafana/oncall/pull/TBD)) + ## v1.3.7 (2023-07-06) ### Changed diff --git a/engine/apps/api/tests/test_alert_receive_channel.py b/engine/apps/api/tests/test_alert_receive_channel.py index dd59726f..e2f42d39 100644 --- a/engine/apps/api/tests/test_alert_receive_channel.py +++ b/engine/apps/api/tests/test_alert_receive_channel.py @@ -141,7 +141,7 @@ def test_integration_filter_by_maintenance( ) assert response.status_code == status.HTTP_200_OK - assert len(response.data) == 1 + assert len(response.json()["results"]) == 1 @pytest.mark.django_db @@ -165,7 +165,7 @@ def test_integration_filter_by_debug( ) assert response.status_code == status.HTTP_200_OK - assert len(response.data) == 1 + assert len(response.json()["results"]) == 1 @pytest.mark.django_db @@ -186,19 +186,19 @@ def test_integration_search( ) assert response.status_code == status.HTTP_200_OK - assert len(response.data) == 2 + assert len(response.json()["results"]) == 2 response = client.get( f"{url}?search=zabbix", content_type="application/json", **make_user_auth_headers(user, token) ) assert response.status_code == status.HTTP_200_OK - assert len(response.data) == 0 + assert len(response.json()["results"]) == 0 response = client.get(f"{url}?search=prod", content_type="application/json", **make_user_auth_headers(user, token)) assert response.status_code == status.HTTP_200_OK - assert len(response.data) == 1 + assert len(response.json()["results"]) == 1 @pytest.mark.django_db @@ -688,7 +688,8 @@ def test_get_alert_receive_channels_direct_paging_hidden_from_list( # Check no direct paging integrations in the response assert response.status_code == status.HTTP_200_OK - assert response.json() == [] + assert response.json()["count"] == 0 + assert len(response.json()["results"]) == 0 @pytest.mark.django_db @@ -706,7 +707,7 @@ def test_get_alert_receive_channels_direct_paging_present_for_filters( # Check direct paging integration is in the response assert response.status_code == status.HTTP_200_OK - assert response.json()[0]["value"] == alert_receive_channel.public_primary_key + assert response.json()["results"][0]["value"] == alert_receive_channel.public_primary_key @pytest.mark.django_db diff --git a/engine/apps/api/tests/test_schedules.py b/engine/apps/api/tests/test_schedules.py index 6a239f68..123851cb 100644 --- a/engine/apps/api/tests/test_schedules.py +++ b/engine/apps/api/tests/test_schedules.py @@ -147,7 +147,7 @@ def test_get_list_schedules( def test_get_list_schedules_pagination( schedule_internal_api_setup, make_escalation_chain, make_escalation_policy, make_user_auth_headers ): - user, token, calendar_schedule, ical_schedule, web_schedule, slack_channel = schedule_internal_api_setup + user, token, calendar_schedule, ical_schedule, web_schedule, _ = schedule_internal_api_setup # setup escalation chain linked to web schedule escalation_chain = make_escalation_chain(user.organization) @@ -219,7 +219,7 @@ def test_get_list_schedules_pagination( client = APIClient() schedule_list_url = reverse("api-internal:schedule-list") - absolute_url = create_engine_url(schedule_list_url, override_base="http://testserver") + absolute_url = create_engine_url(schedule_list_url) for p, schedule in enumerate(available_schedules, start=1): # patch oncall_users to check a paginated queryset is used def mock_oncall_now(qs, events_datetime): diff --git a/engine/apps/api/views/alert_receive_channel.py b/engine/apps/api/views/alert_receive_channel.py index a0fff6ad..9bcb3d8e 100644 --- a/engine/apps/api/views/alert_receive_channel.py +++ b/engine/apps/api/views/alert_receive_channel.py @@ -4,7 +4,6 @@ from django_filters.rest_framework import DjangoFilterBackend from rest_framework import status from rest_framework.decorators import action from rest_framework.filters import SearchFilter -from rest_framework.pagination import PageNumberPagination from rest_framework.permissions import IsAuthenticated from rest_framework.response import Response from rest_framework.viewsets import ModelViewSet @@ -29,26 +28,11 @@ from common.api_helpers.mixins import ( TeamFilteringMixin, UpdateSerializerMixin, ) +from common.api_helpers.paginators import FifteenPageSizePaginator from common.exceptions import MaintenanceCouldNotBeStartedError, TeamCanNotBeChangedError, UnableToSendDemoAlert from common.insight_log import EntityEvent, write_resource_insight_log -class AlertReceiveChannelPagination(PageNumberPagination): - page_size = 15 - page_query_param = "page" - page_size_query_param = "perpage" - max_page_size = 50 - - def paginate_queryset(self, queryset, request, view=None): - """Override to apply pagination only if ?page= is present in query params - Required for backwards compatibility with older versions - """ - page_number = request.query_params.get(self.page_query_param, None) - if not page_number: - return None - return super().paginate_queryset(queryset, request, view) - - class AlertReceiveChannelFilter(ByTeamModelFieldFilterMixin, filters.FilterSet): maintenance_mode = filters.MultipleChoiceFilter( choices=AlertReceiveChannel.MAINTENANCE_MODE_CHOICES, method="filter_maintenance_mode" @@ -98,7 +82,7 @@ class AlertReceiveChannelView( search_fields = ("verbal_name",) filterset_class = AlertReceiveChannelFilter - pagination_class = AlertReceiveChannelPagination + pagination_class = FifteenPageSizePaginator rbac_permissions = { "metadata": [RBACPermission.Permissions.INTEGRATIONS_READ], diff --git a/engine/apps/api/views/schedule.py b/engine/apps/api/views/schedule.py index 7e958f9e..d6960c9d 100644 --- a/engine/apps/api/views/schedule.py +++ b/engine/apps/api/views/schedule.py @@ -13,7 +13,6 @@ from rest_framework.decorators import action from rest_framework.exceptions import NotFound from rest_framework.fields import BooleanField from rest_framework.filters import SearchFilter -from rest_framework.pagination import PageNumberPagination from rest_framework.permissions import IsAuthenticated from rest_framework.request import Request from rest_framework.views import Response @@ -44,6 +43,7 @@ from common.api_helpers.mixins import ( TeamFilteringMixin, UpdateSerializerMixin, ) +from common.api_helpers.paginators import FifteenPageSizePaginator from common.api_helpers.utils import create_engine_url, get_date_range_from_request from common.insight_log import EntityEvent, write_resource_insight_log from common.timezones import raise_exception_if_not_valid_timezone @@ -57,13 +57,6 @@ SCHEDULE_TYPE_TO_CLASS = { } -class SchedulePagination(PageNumberPagination): - page_size = 10 - page_query_param = "page" - page_size_query_param = "perpage" - max_page_size = 50 - - class ScheduleFilter(ByTeamModelFieldFilterMixin, ModelFieldFilterMixin, filters.FilterSet): team = TeamModelMultipleChoiceFilter() @@ -113,7 +106,7 @@ class ScheduleView( create_serializer_class = PolymorphicScheduleCreateSerializer update_serializer_class = PolymorphicScheduleUpdateSerializer short_serializer_class = ScheduleFastSerializer - pagination_class = SchedulePagination + pagination_class = FifteenPageSizePaginator @cached_property def can_update_user_groups(self): diff --git a/engine/common/api_helpers/paginators.py b/engine/common/api_helpers/paginators.py index 2a3ad974..997d48b4 100644 --- a/engine/common/api_helpers/paginators.py +++ b/engine/common/api_helpers/paginators.py @@ -2,14 +2,26 @@ from rest_framework.pagination import CursorPagination, PageNumberPagination from common.api_helpers.utils import create_engine_url +MAX_PAGE_SIZE = 100 +PAGE_QUERY_PARAM = "page" +PAGE_SIZE_QUERY_PARAM = "perpage" + class PathPrefixedPagination(PageNumberPagination): + max_page_size = MAX_PAGE_SIZE + page_query_param = PAGE_QUERY_PARAM + page_size_query_param = PAGE_SIZE_QUERY_PARAM + def paginate_queryset(self, queryset, request, view=None): request.build_absolute_uri = lambda: create_engine_url(request.get_full_path()) return super().paginate_queryset(queryset, request, view) class PathPrefixedCursorPagination(CursorPagination): + max_page_size = MAX_PAGE_SIZE + page_query_param = PAGE_QUERY_PARAM + page_size_query_param = PAGE_SIZE_QUERY_PARAM + def paginate_queryset(self, queryset, request, view=None): request.build_absolute_uri = lambda: create_engine_url(request.get_full_path()) return super().paginate_queryset(queryset, request, view) @@ -27,8 +39,10 @@ class TwentyFivePageSizePaginator(PathPrefixedPagination): page_size = 25 +class FifteenPageSizePaginator(PathPrefixedPagination): + page_size = 15 + + class TwentyFiveCursorPaginator(PathPrefixedCursorPagination): page_size = 25 - max_page_size = 100 - page_size_query_param = "perpage" ordering = "-pk"