From 767c5352fa2cc36f0fdb1cd39e7cf7268cc18b89 Mon Sep 17 00:00:00 2001 From: Joey Orlando Date: Fri, 14 Jul 2023 17:19:40 +0200 Subject: [PATCH] augment API response pagination attributes (#2471) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit # What this PR does This PR: - adds a few attributes to paginated API responses - removes channel filter "send demo alert" internal API endpoint + tests (this endpoint was marked as deprecated + not consumed by the web UI) With the new paginated API response schema, the web UI will no longer need to: - hardcode `ITEMS_PER_PAGE` for each table - manually calculate total number of pages (these two things ☝️ will be done in https://github.com/grafana/oncall/issues/2476) For `GET /api/internal/v1/alertgroups` the response will now look like this: ```diff { "next": | None, "previous": | None, "results": [], ++ "page_size": } ``` For all other paginated API responses, the response will now look like: ```diff { "count": , "next": | None, "previous": | None, "results": [], ++ "page_size": , ++ "current_page_number": , ++ "total_pages": } ``` ## TODO - [x] update public API docs to include these new attributes ## Checklist - [x] Unit, integration, and e2e (if applicable) tests updated - [x] 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 | 4 + .../oncall-api-reference/alertgroups.md | 5 +- docs/sources/oncall-api-reference/alerts.md | 5 +- .../oncall-api-reference/escalation_chains.md | 5 +- .../escalation_policies.md | 5 +- .../oncall-api-reference/integrations.md | 7 +- .../oncall-api-reference/on_call_shifts.md | 5 +- .../oncall-api-reference/outgoing_webhooks.md | 5 +- .../personal_notification_rules.md | 5 +- .../oncall-api-reference/resolution_notes.md | 13 +-- docs/sources/oncall-api-reference/routes.md | 5 +- .../sources/oncall-api-reference/schedules.md | 10 ++- .../oncall-api-reference/slack_channels.md | 5 +- .../oncall-api-reference/user_groups.md | 5 +- docs/sources/oncall-api-reference/users.md | 5 +- .../alerts/models/alert_receive_channel.py | 9 +-- engine/apps/alerts/models/channel_filter.py | 5 -- .../tests/test_alert_receiver_channel.py | 2 - .../apps/alerts/tests/test_channel_filter.py | 49 +----------- engine/apps/api/tests/test_channel_filter.py | 35 -------- engine/apps/api/tests/test_oncall_shift.py | 9 +++ engine/apps/api/tests/test_schedules.py | 12 +++ engine/apps/api/tests/test_user.py | 3 + engine/apps/api/views/channel_filter.py | 13 --- .../oss_installation/views/cloud_users.py | 37 ++++----- engine/apps/public_api/tests/test_alerts.py | 3 + .../public_api/tests/test_custom_actions.py | 16 +++- .../public_api/tests/test_escalation_chain.py | 3 + .../tests/test_escalation_policies.py | 3 + .../apps/public_api/tests/test_incidents.py | 13 ++- .../public_api/tests/test_integrations.py | 3 + .../tests/test_personal_notification_rules.py | 9 +++ .../public_api/tests/test_resolution_notes.py | 3 + engine/apps/public_api/tests/test_routes.py | 6 ++ .../apps/public_api/tests/test_schedules.py | 15 +++- .../public_api/tests/test_slack_channels.py | 3 + engine/apps/public_api/tests/test_teams.py | 3 + .../apps/public_api/tests/test_user_groups.py | 16 +++- engine/apps/public_api/tests/test_users.py | 9 +++ engine/apps/public_api/views/schedules.py | 3 + engine/common/api_helpers/paginators.py | 79 ++++++++++++++----- 41 files changed, 272 insertions(+), 178 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4b70a384..fc678497 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## Unreleased +### Added + +- Add `page_size`, `current_page_number`, and `total_pages` attributes to paginated API responses by @joeyorlando ([#2471](https://github.com/grafana/oncall/pull/2471)) + ## v1.3.11 (2023-07-13) ### Added diff --git a/docs/sources/oncall-api-reference/alertgroups.md b/docs/sources/oncall-api-reference/alertgroups.md index 9dfe58e4..dd5efbbd 100644 --- a/docs/sources/oncall-api-reference/alertgroups.md +++ b/docs/sources/oncall-api-reference/alertgroups.md @@ -36,7 +36,10 @@ The above command returns JSON structured in the following way: "telegram": "https://t.me/c/5354/1234?thread=1234" } } - ] + ], + "current_page_number": 1, + "page_size": 50, + "total_pages": 1 } ``` diff --git a/docs/sources/oncall-api-reference/alerts.md b/docs/sources/oncall-api-reference/alerts.md index 8cbc46a0..7d8935b7 100644 --- a/docs/sources/oncall-api-reference/alerts.md +++ b/docs/sources/oncall-api-reference/alerts.md @@ -96,7 +96,10 @@ The above command returns JSON structured in the following way: ] } } - ] + ], + "current_page_number": 1, + "page_size": 50, + "total_pages": 1 } ``` diff --git a/docs/sources/oncall-api-reference/escalation_chains.md b/docs/sources/oncall-api-reference/escalation_chains.md index 301deb2a..4957402d 100644 --- a/docs/sources/oncall-api-reference/escalation_chains.md +++ b/docs/sources/oncall-api-reference/escalation_chains.md @@ -80,7 +80,10 @@ The above command returns JSON structured in the following way: "name": "default", "team_id": null } - ] + ], + "current_page_number": 1, + "page_size": 50, + "total_pages": 1 } ``` diff --git a/docs/sources/oncall-api-reference/escalation_policies.md b/docs/sources/oncall-api-reference/escalation_policies.md index 02c740a8..c13e1fee 100644 --- a/docs/sources/oncall-api-reference/escalation_policies.md +++ b/docs/sources/oncall-api-reference/escalation_policies.md @@ -105,7 +105,10 @@ The above command returns JSON structured in the following way: "type": "notify_person_next_each_time", "persons_to_notify_next_each_time": ["U4DNY931HHJS5"] } - ] + ], + "current_page_number": 1, + "page_size": 50, + "total_pages": 1 } ``` diff --git a/docs/sources/oncall-api-reference/integrations.md b/docs/sources/oncall-api-reference/integrations.md index e9365c85..b09cbfdd 100644 --- a/docs/sources/oncall-api-reference/integrations.md +++ b/docs/sources/oncall-api-reference/integrations.md @@ -33,7 +33,7 @@ The above command returns JSON structured in the following way: "channel_id": "CH23212D" } }, - "templates": { + "templates": { "grouping_key": null, "resolve_signal": null, "acknowledge_signal": null, @@ -219,7 +219,10 @@ The above command returns JSON structured in the following way: } } } - ] + ], + "current_page_number": 1, + "page_size": 50, + "total_pages": 1 } ``` diff --git a/docs/sources/oncall-api-reference/on_call_shifts.md b/docs/sources/oncall-api-reference/on_call_shifts.md index 98eeab49..979755f3 100644 --- a/docs/sources/oncall-api-reference/on_call_shifts.md +++ b/docs/sources/oncall-api-reference/on_call_shifts.md @@ -141,7 +141,10 @@ The above command returns JSON structured in the following way: "by_monthday": null, "users": ["U4DNY931HHJS5"] } - ] + ], + "current_page_number": 1, + "page_size": 50, + "total_pages": 1 } ``` diff --git a/docs/sources/oncall-api-reference/outgoing_webhooks.md b/docs/sources/oncall-api-reference/outgoing_webhooks.md index 20bd59f0..547bdb77 100644 --- a/docs/sources/oncall-api-reference/outgoing_webhooks.md +++ b/docs/sources/oncall-api-reference/outgoing_webhooks.md @@ -29,7 +29,10 @@ The above command returns JSON structured in the following way: "id": "KGEFG74LU1D8L", "name": "Publish alert group notification to JIRA" } - ] + ], + "current_page_number": 1, + "page_size": 50, + "total_pages": 1 } ``` diff --git a/docs/sources/oncall-api-reference/personal_notification_rules.md b/docs/sources/oncall-api-reference/personal_notification_rules.md index 853cc47c..22c3d5d5 100644 --- a/docs/sources/oncall-api-reference/personal_notification_rules.md +++ b/docs/sources/oncall-api-reference/personal_notification_rules.md @@ -113,7 +113,10 @@ The above command returns JSON structured in the following ways: "important": true, "type": "notify_by_phone_call" } - ] + ], + "current_page_number": 1, + "page_size": 50, + "total_pages": 1 } ``` diff --git a/docs/sources/oncall-api-reference/resolution_notes.md b/docs/sources/oncall-api-reference/resolution_notes.md index b5d8cc77..24d4eecd 100644 --- a/docs/sources/oncall-api-reference/resolution_notes.md +++ b/docs/sources/oncall-api-reference/resolution_notes.md @@ -30,10 +30,10 @@ The above command returns JSON structured in the following way: } ``` -| Parameter | Required | Description | -| --------------- | :------: | :--------------------- | -| `alert_group_id`| Yes | Alert group ID | | -| `text` | Yes | Resolution note text | +| Parameter | Required | Description | +| ---------------- | :------: | :------------------- | --- | +| `alert_group_id` | Yes | Alert group ID | | +| `text` | Yes | Resolution note text | **HTTP request** @@ -90,7 +90,10 @@ The above command returns JSON structured in the following way: "created_at": "2020-06-19T12:40:01.429805Z", "text": "Demo resolution note" } - ] + ], + "current_page_number": 1, + "page_size": 50, + "total_pages": 1 } ``` diff --git a/docs/sources/oncall-api-reference/routes.md b/docs/sources/oncall-api-reference/routes.md index 45c39ae6..81940477 100644 --- a/docs/sources/oncall-api-reference/routes.md +++ b/docs/sources/oncall-api-reference/routes.md @@ -125,7 +125,10 @@ The above command returns JSON structured in the following way: "channel_id": "CH23212D" } } - ] + ], + "current_page_number": 1, + "page_size": 25, + "total_pages": 1 } ``` diff --git a/docs/sources/oncall-api-reference/schedules.md b/docs/sources/oncall-api-reference/schedules.md index 9076985c..32a8d309 100644 --- a/docs/sources/oncall-api-reference/schedules.md +++ b/docs/sources/oncall-api-reference/schedules.md @@ -129,7 +129,10 @@ The above command returns JSON structured in the following way: "user_group_id": "MEOW_SLACK_ID" } } - ] + ], + "current_page_number": 1, + "page_size": 50, + "total_pages": 1 } ``` @@ -294,7 +297,10 @@ The above command returns JSON structured in the following way: "shift_start": "2023-01-27T09:00:00Z", "shift_end": "2023-01-27T17:00:00Z" } - ] + ], + "current_page_number": 1, + "page_size": 50, + "total_pages": 1 } ``` diff --git a/docs/sources/oncall-api-reference/slack_channels.md b/docs/sources/oncall-api-reference/slack_channels.md index 82e78152..e2c39b44 100644 --- a/docs/sources/oncall-api-reference/slack_channels.md +++ b/docs/sources/oncall-api-reference/slack_channels.md @@ -25,7 +25,10 @@ The above command returns JSON structured in the following way: "name": "meow_channel", "slack_id": "MEOW_SLACK_ID" } - ] + ], + "current_page_number": 1, + "page_size": 50, + "total_pages": 1 } ``` diff --git a/docs/sources/oncall-api-reference/user_groups.md b/docs/sources/oncall-api-reference/user_groups.md index e8f6a806..49d58942 100644 --- a/docs/sources/oncall-api-reference/user_groups.md +++ b/docs/sources/oncall-api-reference/user_groups.md @@ -32,7 +32,10 @@ The above command returns JSON structured in the following way: "handle": "meow_group" } } - ] + ], + "current_page_number": 1, + "page_size": 50, + "total_pages": 1 } ``` diff --git a/docs/sources/oncall-api-reference/users.md b/docs/sources/oncall-api-reference/users.md index 9d205fb0..f703e870 100644 --- a/docs/sources/oncall-api-reference/users.md +++ b/docs/sources/oncall-api-reference/users.md @@ -76,7 +76,10 @@ The above command returns JSON structured in the following way: "username": "alex", "role": "admin" } - ] + ], + "current_page_number": 1, + "page_size": 100, + "total_pages": 1 } ``` diff --git a/engine/apps/alerts/models/alert_receive_channel.py b/engine/apps/alerts/models/alert_receive_channel.py index 8579ddb3..083e56d0 100644 --- a/engine/apps/alerts/models/alert_receive_channel.py +++ b/engine/apps/alerts/models/alert_receive_channel.py @@ -527,8 +527,8 @@ class AlertReceiveChannel(IntegrationOptionsMixin, MaintainableObject): return getattr(heartbeat, self.INTEGRATIONS_TO_REVERSE_URL_MAP[self.integration], None) # Demo alerts - def send_demo_alert(self, force_route_id=None, payload=None): - logger.info(f"send_demo_alert integration={self.pk} force_route_id={force_route_id}") + def send_demo_alert(self, payload=None): + logger.info(f"send_demo_alert integration={self.pk}") if not self.is_demo_alert_enabled: raise UnableToSendDemoAlert("Unable to send demo alert for this integration.") @@ -543,9 +543,7 @@ class AlertReceiveChannel(IntegrationOptionsMixin, MaintainableObject): "Unable to send demo alert as payload has no 'alerts' key, it is not array, or it is empty." ) for alert in alerts: - create_alertmanager_alerts.delay( - alert_receive_channel_pk=self.pk, alert=alert, is_demo=True, force_route_id=force_route_id - ) + create_alertmanager_alerts.delay(alert_receive_channel_pk=self.pk, alert=alert, is_demo=True) else: create_alert.delay( title="Demo alert", @@ -556,7 +554,6 @@ class AlertReceiveChannel(IntegrationOptionsMixin, MaintainableObject): integration_unique_data=None, raw_request_data=payload, is_demo=True, - force_route_id=force_route_id, ) @property diff --git a/engine/apps/alerts/models/channel_filter.py b/engine/apps/alerts/models/channel_filter.py index 677f3a19..915aeaff 100644 --- a/engine/apps/alerts/models/channel_filter.py +++ b/engine/apps/alerts/models/channel_filter.py @@ -165,11 +165,6 @@ class ChannelFilter(OrderedModel): return str(self.filtering_term).replace("`", "") raise Exception("Unknown filtering term") - def send_demo_alert(self): - """Deprecated. May be used in the older versions of the plugin""" - integration = self.alert_receive_channel - integration.send_demo_alert(force_route_id=self.pk) - # Insight logs @property def insight_logs_type_verbal(self): diff --git a/engine/apps/alerts/tests/test_alert_receiver_channel.py b/engine/apps/alerts/tests/test_alert_receiver_channel.py index 566b5be6..c829913b 100644 --- a/engine/apps/alerts/tests/test_alert_receiver_channel.py +++ b/engine/apps/alerts/tests/test_alert_receiver_channel.py @@ -110,7 +110,6 @@ def test_send_demo_alert(mocked_create_alert, make_organization, make_alert_rece mocked_create_alert.call_args.args[1]["raw_request_data"] == payload or alert_receive_channel.config.example_payload ) - assert mocked_create_alert.call_args.args[1]["force_route_id"] is None @mock.patch("apps.integrations.tasks.create_alertmanager_alerts.apply_async", return_value=None) @@ -143,7 +142,6 @@ def test_send_demo_alert_alertmanager_payload_shape( if payload else alert_receive_channel.config.example_payload["alerts"][0] ) - assert mocked_create_alert.call_args.args[1]["force_route_id"] is None @mock.patch("apps.integrations.tasks.create_alert.apply_async", return_value=None) diff --git a/engine/apps/alerts/tests/test_channel_filter.py b/engine/apps/alerts/tests/test_channel_filter.py index 3c9de7e8..cf66c632 100644 --- a/engine/apps/alerts/tests/test_channel_filter.py +++ b/engine/apps/alerts/tests/test_channel_filter.py @@ -1,8 +1,6 @@ -from unittest import mock - import pytest -from apps.alerts.models import AlertReceiveChannel, ChannelFilter +from apps.alerts.models import ChannelFilter @pytest.mark.django_db @@ -93,48 +91,3 @@ def test_channel_filter_select_filter_jinja2(make_organization, make_alert_recei alert_receive_channel, raw_request_data, force_route_id=channel_filter.pk ) assert satisfied_filter == channel_filter - - -@mock.patch("apps.integrations.tasks.create_alert.apply_async", return_value=None) -@pytest.mark.django_db -def test_send_demo_alert( - mocked_create_alert, - make_organization, - make_alert_receive_channel, - make_channel_filter, -): - organization = make_organization() - alert_receive_channel = make_alert_receive_channel( - organization, integration=AlertReceiveChannel.INTEGRATION_WEBHOOK - ) - filtering_term = "test alert" - channel_filter = make_channel_filter(alert_receive_channel, filtering_term=filtering_term, is_default=False) - - channel_filter.send_demo_alert() - assert mocked_create_alert.called - assert mocked_create_alert.call_args.args[1]["is_demo"] - assert mocked_create_alert.call_args.args[1]["force_route_id"] == channel_filter.id - - -@mock.patch("apps.integrations.tasks.create_alertmanager_alerts.apply_async", return_value=None) -@pytest.mark.django_db -@pytest.mark.parametrize( - "integration", - [ - AlertReceiveChannel.INTEGRATION_ALERTMANAGER, - AlertReceiveChannel.INTEGRATION_GRAFANA, - AlertReceiveChannel.INTEGRATION_GRAFANA_ALERTING, - ], -) -def test_send_demo_alert_alertmanager_payload_shape( - mocked_create_alert, make_organization, make_alert_receive_channel, make_channel_filter, integration -): - organization = make_organization() - alert_receive_channel = make_alert_receive_channel(organization) - filtering_term = "test alert" - channel_filter = make_channel_filter(alert_receive_channel, filtering_term=filtering_term, is_default=False) - - channel_filter.send_demo_alert() - assert mocked_create_alert.called - assert mocked_create_alert.call_args.args[1]["is_demo"] - assert mocked_create_alert.call_args.args[1]["force_route_id"] == channel_filter.pk diff --git a/engine/apps/api/tests/test_channel_filter.py b/engine/apps/api/tests/test_channel_filter.py index baf845e3..484090c8 100644 --- a/engine/apps/api/tests/test_channel_filter.py +++ b/engine/apps/api/tests/test_channel_filter.py @@ -219,41 +219,6 @@ def test_channel_filter_move_to_position_permissions( assert response.status_code == expected_status -@pytest.mark.django_db -@pytest.mark.parametrize( - "role,expected_status", - [ - (LegacyAccessControlRole.ADMIN, status.HTTP_200_OK), - (LegacyAccessControlRole.EDITOR, status.HTTP_200_OK), - (LegacyAccessControlRole.VIEWER, status.HTTP_403_FORBIDDEN), - ], -) -def test_alert_receive_channel_send_demo_alert_permissions( - make_organization_and_user_with_plugin_token, - make_user_auth_headers, - make_alert_receive_channel, - make_channel_filter, - role, - expected_status, -): - organization, user, token = make_organization_and_user_with_plugin_token(role) - alert_receive_channel = make_alert_receive_channel(organization) - channel_filter = make_channel_filter(alert_receive_channel, is_default=True) - client = APIClient() - - url = reverse("api-internal:channel_filter-send-demo-alert", kwargs={"pk": channel_filter.public_primary_key}) - - with patch( - "apps.api.views.channel_filter.ChannelFilterView.send_demo_alert", - return_value=Response( - status=status.HTTP_200_OK, - ), - ): - response = client.post(url, format="json", **make_user_auth_headers(user, token)) - - assert response.status_code == expected_status - - @pytest.mark.django_db def test_channel_filter_create_with_order( make_organization_and_user_with_plugin_token, diff --git a/engine/apps/api/tests/test_oncall_shift.py b/engine/apps/api/tests/test_oncall_shift.py index 7e708daf..e8b435ed 100644 --- a/engine/apps/api/tests/test_oncall_shift.py +++ b/engine/apps/api/tests/test_oncall_shift.py @@ -222,6 +222,9 @@ def test_list_on_call_shift( "updated_shift": None, } ], + "current_page_number": 1, + "page_size": 50, + "total_pages": 1, } assert response.status_code == status.HTTP_200_OK @@ -280,6 +283,9 @@ def test_list_on_call_shift_filter_schedule_id( "updated_shift": None, } ], + "current_page_number": 1, + "page_size": 50, + "total_pages": 1, } assert response.status_code == status.HTTP_200_OK @@ -290,6 +296,9 @@ def test_list_on_call_shift_filter_schedule_id( "next": None, "previous": None, "results": [], + "current_page_number": 1, + "page_size": 50, + "total_pages": 1, } response = client.get( diff --git a/engine/apps/api/tests/test_schedules.py b/engine/apps/api/tests/test_schedules.py index 123851cb..4cf8eb9c 100644 --- a/engine/apps/api/tests/test_schedules.py +++ b/engine/apps/api/tests/test_schedules.py @@ -137,6 +137,9 @@ def test_get_list_schedules( "enable_web_overrides": True, }, ], + "current_page_number": 1, + "page_size": 15, + "total_pages": 1, } response = client.get(url, format="json", **make_user_auth_headers(user, token)) assert response.status_code == status.HTTP_200_OK @@ -247,6 +250,9 @@ def test_get_list_schedules_pagination( "next": next_url, "previous": previous_url, "results": [schedule], + "current_page_number": p, + "page_size": 1, + "total_pages": 3, } assert response.json() == expected_payload @@ -334,6 +340,9 @@ def test_get_list_schedules_by_type( "next": None, "previous": None, "results": [available_schedules[schedule_type]], + "current_page_number": 1, + "page_size": 15, + "total_pages": 1, } assert response.json() == expected_payload @@ -346,6 +355,9 @@ def test_get_list_schedules_by_type( "next": None, "previous": None, "results": [available_schedules[0], available_schedules[1]], + "current_page_number": 1, + "page_size": 15, + "total_pages": 1, } assert response.json() == expected_payload diff --git a/engine/apps/api/tests/test_user.py b/engine/apps/api/tests/test_user.py index ec30db63..8c9cd843 100644 --- a/engine/apps/api/tests/test_user.py +++ b/engine/apps/api/tests/test_user.py @@ -173,6 +173,9 @@ def test_list_users( "cloud_connection_status": None, }, ], + "current_page_number": 1, + "page_size": 100, + "total_pages": 1, } response = client.get(url, format="json", **make_user_auth_headers(admin, token)) diff --git a/engine/apps/api/views/channel_filter.py b/engine/apps/api/views/channel_filter.py index f00e195e..18d72066 100644 --- a/engine/apps/api/views/channel_filter.py +++ b/engine/apps/api/views/channel_filter.py @@ -12,7 +12,6 @@ from apps.api.serializers.channel_filter import ( ChannelFilterSerializer, ChannelFilterUpdateSerializer, ) -from apps.api.throttlers import DemoAlertThrottler from apps.auth_token.auth import PluginAuthentication from apps.slack.models import SlackChannel from common.api_helpers.exceptions import BadRequest @@ -23,7 +22,6 @@ from common.api_helpers.mixins import ( UpdateSerializerMixin, ) from common.api_helpers.serializers import get_move_to_position_param -from common.exceptions import UnableToSendDemoAlert from common.insight_log import EntityEvent, write_resource_insight_log @@ -41,7 +39,6 @@ class ChannelFilterView( "partial_update": [RBACPermission.Permissions.INTEGRATIONS_WRITE], "destroy": [RBACPermission.Permissions.INTEGRATIONS_WRITE], "move_to_position": [RBACPermission.Permissions.INTEGRATIONS_WRITE], - "send_demo_alert": [RBACPermission.Permissions.INTEGRATIONS_TEST], "convert_from_regex_to_jinja2": [RBACPermission.Permissions.INTEGRATIONS_WRITE], } @@ -131,16 +128,6 @@ class ChannelFilterView( return Response(status=status.HTTP_200_OK) - @action(detail=True, methods=["post"], throttle_classes=[DemoAlertThrottler]) - def send_demo_alert(self, request, pk): - """Deprecated action. May be used in the older version of the plugin.""" - instance = self.get_object() - try: - instance.send_demo_alert() - except UnableToSendDemoAlert as e: - raise BadRequest(detail=str(e)) - return Response(status=status.HTTP_200_OK) - @action(detail=True, methods=["post"]) def convert_from_regex_to_jinja2(self, request, pk): instance = self.get_object() diff --git a/engine/apps/oss_installation/views/cloud_users.py b/engine/apps/oss_installation/views/cloud_users.py index 75d3886a..d62fd612 100644 --- a/engine/apps/oss_installation/views/cloud_users.py +++ b/engine/apps/oss_installation/views/cloud_users.py @@ -1,5 +1,3 @@ -from collections import OrderedDict - from rest_framework import mixins, status, viewsets from rest_framework.decorators import action from rest_framework.permissions import IsAuthenticated @@ -13,12 +11,24 @@ from apps.oss_installation.serializers import CloudUserSerializer from apps.oss_installation.utils import cloud_user_identity_status from apps.user_management.models import User from common.api_helpers.mixins import PublicPrimaryKeyMixin -from common.api_helpers.paginators import HundredPageSizePaginator +from common.api_helpers.paginators import HundredPageSizePaginator, PaginatedData PERMISSIONS = [RBACPermission.Permissions.OTHER_SETTINGS_WRITE] -class CloudUsersView(HundredPageSizePaginator, APIView): +class CloudUsersPagination(HundredPageSizePaginator): + # the override ignore here is expected. The parent classes' get_paginated_response method does not + # take a matched_users_count argument. This is fine in this case + def get_paginated_response(self, data: PaginatedData, matched_users_count: int) -> Response: # type: ignore[override] + return Response( + { + **self._get_paginated_response_data(data), + "matched_users_count": matched_users_count, + } + ) + + +class CloudUsersView(CloudUsersPagination, APIView): authentication_classes = (PluginAuthentication,) permission_classes = (IsAuthenticated, RBACPermission) @@ -44,14 +54,14 @@ class CloudUsersView(HundredPageSizePaginator, APIView): cloud_identities = list(CloudUserIdentity.objects.filter(email__in=emails)) cloud_identities = {cloud_identity.email: cloud_identity for cloud_identity in cloud_identities} - response = [] + data = [] connector = CloudConnector.objects.first() for user in results: cloud_identity = cloud_identities.get(user.email, None) status, link = cloud_user_identity_status(connector, cloud_identity) - response.append( + data.append( { "id": user.public_primary_key, "email": user.email, @@ -60,20 +70,7 @@ class CloudUsersView(HundredPageSizePaginator, APIView): } ) - return self.get_paginated_response_with_matched_users_count(response, len(cloud_identities)) - - def get_paginated_response_with_matched_users_count(self, data, matched_users_count): - return Response( - OrderedDict( - [ - ("count", self.page.paginator.count), - ("matched_users_count", matched_users_count), - ("next", self.get_next_link()), - ("previous", self.get_previous_link()), - ("results", data), - ] - ) - ) + return self.get_paginated_response(data, len(cloud_identities)) def post(self, request): connector = CloudConnector.objects.first() diff --git a/engine/apps/public_api/tests/test_alerts.py b/engine/apps/public_api/tests/test_alerts.py index 525ea842..8cb0de2c 100644 --- a/engine/apps/public_api/tests/test_alerts.py +++ b/engine/apps/public_api/tests/test_alerts.py @@ -90,6 +90,9 @@ def test_get_list_alerts( }, }, ], + "current_page_number": 1, + "page_size": 50, + "total_pages": 1, } assert response.status_code == status.HTTP_200_OK assert response.json() == expected_response diff --git a/engine/apps/public_api/tests/test_custom_actions.py b/engine/apps/public_api/tests/test_custom_actions.py index ae15aafb..36904940 100644 --- a/engine/apps/public_api/tests/test_custom_actions.py +++ b/engine/apps/public_api/tests/test_custom_actions.py @@ -37,6 +37,9 @@ def test_get_custom_actions( "forward_whole_payload": custom_action.forward_whole_payload, } ], + "current_page_number": 1, + "page_size": 50, + "total_pages": 1, } assert response.status_code == status.HTTP_200_OK @@ -74,6 +77,9 @@ def test_get_custom_actions_filter_by_name( "forward_whole_payload": custom_action.forward_whole_payload, } ], + "current_page_number": 1, + "page_size": 50, + "total_pages": 1, } assert response.status_code == status.HTTP_200_OK @@ -94,7 +100,15 @@ def test_get_custom_actions_filter_by_name_empty_result( response = client.get(f"{url}?name=NonExistentName", format="json", HTTP_AUTHORIZATION=f"{token}") - expected_payload = {"count": 0, "next": None, "previous": None, "results": []} + expected_payload = { + "count": 0, + "next": None, + "previous": None, + "results": [], + "current_page_number": 1, + "page_size": 50, + "total_pages": 1, + } assert response.status_code == status.HTTP_200_OK assert response.data == expected_payload diff --git a/engine/apps/public_api/tests/test_escalation_chain.py b/engine/apps/public_api/tests/test_escalation_chain.py index 99f6778e..f8bab811 100644 --- a/engine/apps/public_api/tests/test_escalation_chain.py +++ b/engine/apps/public_api/tests/test_escalation_chain.py @@ -25,6 +25,9 @@ def test_get_escalation_chains(make_organization_and_user_with_token): "name": "test", } ], + "current_page_number": 1, + "page_size": 50, + "total_pages": 1, } assert response.status_code == status.HTTP_200_OK diff --git a/engine/apps/public_api/tests/test_escalation_policies.py b/engine/apps/public_api/tests/test_escalation_policies.py index b1b705e1..728fc383 100644 --- a/engine/apps/public_api/tests/test_escalation_policies.py +++ b/engine/apps/public_api/tests/test_escalation_policies.py @@ -62,6 +62,9 @@ def escalation_policies_setup(): escalation_policy_wait_payload, escalation_policy_notify_persons_empty_payload, ], + "current_page_number": 1, + "page_size": 50, + "total_pages": 1, } return ( escalation_chain, diff --git a/engine/apps/public_api/tests/test_incidents.py b/engine/apps/public_api/tests/test_incidents.py index 77678762..f98b2479 100644 --- a/engine/apps/public_api/tests/test_incidents.py +++ b/engine/apps/public_api/tests/test_incidents.py @@ -46,8 +46,15 @@ def construct_expected_response_from_incidents(incidents): }, } ) - expected_response = {"count": incidents.count(), "next": None, "previous": None, "results": results} - return expected_response + return { + "count": incidents.count(), + "next": None, + "previous": None, + "results": results, + "current_page_number": 1, + "page_size": 50, + "total_pages": 1, + } @pytest.fixture() @@ -275,7 +282,7 @@ def test_pagination(settings, incident_public_api_setup): url = reverse("api-public:alert_groups-list") - with patch("common.api_helpers.paginators.PathPrefixedPagination.get_page_size", return_value=1): + with patch("common.api_helpers.paginators.PathPrefixedPagePagination.get_page_size", return_value=1): response = client.get(url, HTTP_AUTHORIZATION=f"{token}") assert response.status_code == status.HTTP_200_OK diff --git a/engine/apps/public_api/tests/test_integrations.py b/engine/apps/public_api/tests/test_integrations.py index 6b75c897..c154c00f 100644 --- a/engine/apps/public_api/tests/test_integrations.py +++ b/engine/apps/public_api/tests/test_integrations.py @@ -74,6 +74,9 @@ def test_get_list_integrations( "maintenance_end_at": None, } ], + "current_page_number": 1, + "page_size": 50, + "total_pages": 1, } url = reverse("api-public:integrations-list") response = client.get(url, format="json", HTTP_AUTHORIZATION=f"{token}") diff --git a/engine/apps/public_api/tests/test_personal_notification_rules.py b/engine/apps/public_api/tests/test_personal_notification_rules.py index 7409a3f2..b6414e0d 100644 --- a/engine/apps/public_api/tests/test_personal_notification_rules.py +++ b/engine/apps/public_api/tests/test_personal_notification_rules.py @@ -98,6 +98,9 @@ def test_get_personal_notification_rules_list(personal_notification_rule_public_ "important": True, }, ], + "current_page_number": 1, + "page_size": 50, + "total_pages": 1, } assert response.status_code == status.HTTP_200_OK @@ -126,6 +129,9 @@ def test_get_personal_notification_rules_list_important(personal_notification_ru "important": True, } ], + "current_page_number": 1, + "page_size": 50, + "total_pages": 1, } assert response.status_code == status.HTTP_200_OK @@ -169,6 +175,9 @@ def test_get_personal_notification_rules_list_non_important(personal_notificatio "important": False, }, ], + "current_page_number": 1, + "page_size": 50, + "total_pages": 1, } assert response.status_code == status.HTTP_200_OK diff --git a/engine/apps/public_api/tests/test_resolution_notes.py b/engine/apps/public_api/tests/test_resolution_notes.py index f976ef05..7c5e3b94 100644 --- a/engine/apps/public_api/tests/test_resolution_notes.py +++ b/engine/apps/public_api/tests/test_resolution_notes.py @@ -57,6 +57,9 @@ def test_get_resolution_notes( "text": resolution_note_1.text, }, ], + "current_page_number": 1, + "page_size": 50, + "total_pages": 1, } assert response.status_code == status.HTTP_200_OK diff --git a/engine/apps/public_api/tests/test_routes.py b/engine/apps/public_api/tests/test_routes.py index 017883fb..c77ee531 100644 --- a/engine/apps/public_api/tests/test_routes.py +++ b/engine/apps/public_api/tests/test_routes.py @@ -86,6 +86,9 @@ def test_get_routes_list( TEST_MESSAGING_BACKEND_FIELD: {"id": None, "enabled": False}, } ], + "current_page_number": 1, + "page_size": 25, + "total_pages": 1, } assert response.status_code == status.HTTP_200_OK @@ -123,6 +126,9 @@ def test_get_routes_filter_by_integration_id( TEST_MESSAGING_BACKEND_FIELD: {"id": None, "enabled": False}, } ], + "current_page_number": 1, + "page_size": 25, + "total_pages": 1, } assert response.status_code == status.HTTP_200_OK diff --git a/engine/apps/public_api/tests/test_schedules.py b/engine/apps/public_api/tests/test_schedules.py index 6dba8332..a1b402ed 100644 --- a/engine/apps/public_api/tests/test_schedules.py +++ b/engine/apps/public_api/tests/test_schedules.py @@ -702,6 +702,9 @@ def test_get_schedule_list( "slack": {"channel_id": slack_channel_id, "user_group_id": user_group_id}, }, ], + "current_page_number": 1, + "page_size": 50, + "total_pages": 1, } assert response.status_code == status.HTTP_200_OK @@ -912,6 +915,12 @@ def test_oncall_shifts_export( assert total_time_on_call[user2_public_primary_key] == expected_time_on_call # pagination parameters are mocked out for now - assert response_json["next"] is None - assert response_json["previous"] is None - assert response_json["count"] == len(shifts) + del response_json["results"] + assert response_json == { + "next": None, + "previous": None, + "count": len(shifts), + "current_page_number": 1, + "page_size": 50, + "total_pages": 1, + } diff --git a/engine/apps/public_api/tests/test_slack_channels.py b/engine/apps/public_api/tests/test_slack_channels.py index 5884dced..2e0efb8d 100644 --- a/engine/apps/public_api/tests/test_slack_channels.py +++ b/engine/apps/public_api/tests/test_slack_channels.py @@ -32,6 +32,9 @@ def test_get_slack_channels_list( "next": None, "previous": None, "results": [{"name": slack_channel.name, "slack_id": slack_channel.slack_id}], + "current_page_number": 1, + "page_size": 50, + "total_pages": 1, } assert response.status_code == status.HTTP_200_OK diff --git a/engine/apps/public_api/tests/test_teams.py b/engine/apps/public_api/tests/test_teams.py index 946585df..ce31993e 100644 --- a/engine/apps/public_api/tests/test_teams.py +++ b/engine/apps/public_api/tests/test_teams.py @@ -38,6 +38,9 @@ def test_get_teams_list(team_public_api_setup): "avatar_url": team.avatar_url, } ], + "current_page_number": 1, + "page_size": 50, + "total_pages": 1, } assert response.status_code == status.HTTP_200_OK diff --git a/engine/apps/public_api/tests/test_user_groups.py b/engine/apps/public_api/tests/test_user_groups.py index 055d93c6..48aa3967 100644 --- a/engine/apps/public_api/tests/test_user_groups.py +++ b/engine/apps/public_api/tests/test_user_groups.py @@ -42,6 +42,9 @@ def test_get_user_groups( }, } ], + "current_page_number": 1, + "page_size": 50, + "total_pages": 1, } assert response.status_code == status.HTTP_200_OK @@ -80,6 +83,9 @@ def test_get_user_groups_filter_by_handle( }, } ], + "current_page_number": 1, + "page_size": 50, + "total_pages": 1, } assert response.status_code == status.HTTP_200_OK @@ -98,7 +104,15 @@ def test_get_user_groups_filter_by_handle_empty_result( response = client.get(f"{url}?slack_handle=NonExistentSlackHandle", format="json", HTTP_AUTHORIZATION=f"{token}") - expected_payload = {"count": 0, "next": None, "previous": None, "results": []} + expected_payload = { + "count": 0, + "next": None, + "previous": None, + "results": [], + "current_page_number": 1, + "page_size": 50, + "total_pages": 1, + } assert response.status_code == status.HTTP_200_OK assert response.data == expected_payload diff --git a/engine/apps/public_api/tests/test_users.py b/engine/apps/public_api/tests/test_users.py index 0a0cf612..fc77fcd4 100644 --- a/engine/apps/public_api/tests/test_users.py +++ b/engine/apps/public_api/tests/test_users.py @@ -82,6 +82,9 @@ def test_get_users_list( "is_phone_number_verified": False, }, ], + "current_page_number": 1, + "page_size": 100, + "total_pages": 1, } assert response.status_code == status.HTTP_200_OK @@ -121,6 +124,9 @@ def test_get_users_list_short( "is_phone_number_verified": False, }, ], + "current_page_number": 1, + "page_size": 100, + "total_pages": 1, } assert response.status_code == status.HTTP_200_OK @@ -170,6 +176,9 @@ def test_get_users_list_all_role_users(user_public_api_setup, make_user_for_orga } for user, role in expected_users ], + "current_page_number": 1, + "page_size": 100, + "total_pages": 1, } assert response.status_code == status.HTTP_200_OK diff --git a/engine/apps/public_api/views/schedules.py b/engine/apps/public_api/views/schedules.py index 6ba33f55..83aa3207 100644 --- a/engine/apps/public_api/views/schedules.py +++ b/engine/apps/public_api/views/schedules.py @@ -175,5 +175,8 @@ class OnCallScheduleChannelView(RateLimitHeadersMixin, UpdateSerializerMixin, Mo "next": None, "previous": None, "results": data, + "current_page_number": 1, + "page_size": 50, + "total_pages": 1, } ) diff --git a/engine/common/api_helpers/paginators.py b/engine/common/api_helpers/paginators.py index 997d48b4..34a20c27 100644 --- a/engine/common/api_helpers/paginators.py +++ b/engine/common/api_helpers/paginators.py @@ -1,45 +1,82 @@ -from rest_framework.pagination import CursorPagination, PageNumberPagination +import typing + +from rest_framework.pagination import BasePagination, CursorPagination, PageNumberPagination +from rest_framework.response import Response from common.api_helpers.utils import create_engine_url -MAX_PAGE_SIZE = 100 -PAGE_QUERY_PARAM = "page" -PAGE_SIZE_QUERY_PARAM = "perpage" +PaginatedData = typing.List[typing.Any] -class PathPrefixedPagination(PageNumberPagination): - max_page_size = MAX_PAGE_SIZE - page_query_param = PAGE_QUERY_PARAM - page_size_query_param = PAGE_SIZE_QUERY_PARAM +class BasePaginatedResponseData(typing.TypedDict): + next: str | None + previous: str | None + results: PaginatedData + page_size: int + + +class PageBasedPaginationResponseData(BasePaginatedResponseData): + count: int + current_page_number: int + total_pages: int + + +class BasePathPrefixedPagination(BasePagination): + max_page_size = 100 + page_query_param = "page" + page_size_query_param = "perpage" def paginate_queryset(self, queryset, request, view=None): request.build_absolute_uri = lambda: create_engine_url(request.get_full_path()) + + # we're setting the request object explicitly here because the way the paginate_quersey works + # between PageNumberPagination and CursorPagination is slightly different. In the latter class, + # it does not set self.request in the paginate_queryset method, whereas in the former it does. + # this leads to an issue in _get_base_paginated_response_data where the self.request would not be set + self.request = request + 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) + def _get_base_paginated_response_data(self, data: PaginatedData) -> BasePaginatedResponseData: + return { + "next": self.get_next_link(), + "previous": self.get_previous_link(), + "results": data, + "page_size": self.get_page_size(self.request), + } -class HundredPageSizePaginator(PathPrefixedPagination): +class PathPrefixedPagePagination(BasePathPrefixedPagination, PageNumberPagination): + def _get_paginated_response_data(self, data: PaginatedData) -> PageBasedPaginationResponseData: + return { + **self._get_base_paginated_response_data(data), + "count": self.page.paginator.count, + "current_page_number": self.page.number, + "total_pages": self.page.paginator.num_pages, + } + + def get_paginated_response(self, data: PaginatedData) -> Response: + return Response(self._get_paginated_response_data(data)) + + +class PathPrefixedCursorPagination(BasePathPrefixedPagination, CursorPagination): + def get_paginated_response(self, data: PaginatedData) -> Response: + return Response(self._get_base_paginated_response_data(data)) + + +class HundredPageSizePaginator(PathPrefixedPagePagination): page_size = 100 -class FiftyPageSizePaginator(PathPrefixedPagination): +class FiftyPageSizePaginator(PathPrefixedPagePagination): page_size = 50 -class TwentyFivePageSizePaginator(PathPrefixedPagination): +class TwentyFivePageSizePaginator(PathPrefixedPagePagination): page_size = 25 -class FifteenPageSizePaginator(PathPrefixedPagination): +class FifteenPageSizePaginator(PathPrefixedPagePagination): page_size = 15