From 98b3b918a50a0b0e75dd15d059daf4b3d9547da5 Mon Sep 17 00:00:00 2001 From: Matias Bordese Date: Fri, 24 Feb 2023 11:59:03 -0300 Subject: [PATCH] Add schedule pagination to plugin API (#1309) Related to #1289 --------- Co-authored-by: Yulia Shanyrova --- CHANGELOG.md | 5 + engine/apps/api/tests/test_schedules.py | 129 +++++++++++++++++- engine/apps/api/views/schedule.py | 16 ++- .../components/Policy/EscalationPolicy.tsx | 2 + .../parts/EscalationVariantsPopup.tsx | 8 +- .../src/models/schedule/schedule.ts | 20 +-- grafana-plugin/src/models/user/user.ts | 6 +- .../src/pages/schedules/Schedules.tsx | 39 ++++-- .../src/plugin/GrafanaPluginRootPage.tsx | 2 +- grafana-plugin/src/utils/index.ts | 2 +- 10 files changed, 193 insertions(+), 36 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 16a4f339..d5db5e39 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Fixed importing of global grafana styles ([672](https://github.com/grafana/oncall/issues/672)) +### Changed + +- Add pagination to schedule listing + ## v1.1.29 (2023-02-23) ### Changed @@ -78,6 +82,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Incidents - Removed buttons column and replaced status with toggler ([#1237](https://github.com/grafana/oncall/issues/1237)) - Responsiveness changes across multiple pages (Incidents, Integrations, Schedules) ([#1237](https://github.com/grafana/oncall/issues/1237)) +- Add pagination to schedule listing - Link to source was added - Header of Incident page was reworked: clickable labels instead of just names, users section was deleted - "Go to Integration" button was deleted, because the functionality was moved to clickable labels diff --git a/engine/apps/api/tests/test_schedules.py b/engine/apps/api/tests/test_schedules.py index e3a9d633..79f6759a 100644 --- a/engine/apps/api/tests/test_schedules.py +++ b/engine/apps/api/tests/test_schedules.py @@ -19,6 +19,7 @@ from apps.schedules.models import ( OnCallScheduleICal, OnCallScheduleWeb, ) +from common.api_helpers.utils import create_engine_url ICAL_URL = "https://calendar.google.com/calendar/ical/amixr.io_37gttuakhrtr75ano72p69rt78%40group.calendar.google.com/private-1d00a680ba5be7426c3eb3ef1616e26d/basic.ics" @@ -74,7 +75,86 @@ def test_get_list_schedules( notify_schedule=web_schedule, ) - expected_payload = [ + expected_payload = { + "count": 3, + "next": None, + "previous": None, + "results": [ + { + "id": calendar_schedule.public_primary_key, + "type": 0, + "team": None, + "name": "test_calendar_schedule", + "time_zone": "UTC", + "slack_channel": None, + "user_group": None, + "warnings": [], + "ical_url_overrides": None, + "on_call_now": [], + "has_gaps": False, + "mention_oncall_next": False, + "mention_oncall_start": True, + "notify_empty_oncall": 0, + "notify_oncall_shift_freq": 1, + "number_of_escalation_chains": 0, + }, + { + "id": ical_schedule.public_primary_key, + "type": 1, + "team": None, + "name": "test_ical_schedule", + "ical_url_primary": ICAL_URL, + "ical_url_overrides": None, + "slack_channel": None, + "user_group": None, + "warnings": [], + "on_call_now": [], + "has_gaps": False, + "mention_oncall_next": False, + "mention_oncall_start": True, + "notify_empty_oncall": 0, + "notify_oncall_shift_freq": 1, + "number_of_escalation_chains": 0, + }, + { + "id": web_schedule.public_primary_key, + "type": 2, + "time_zone": "UTC", + "team": None, + "name": "test_web_schedule", + "slack_channel": None, + "user_group": None, + "warnings": [], + "on_call_now": [], + "has_gaps": False, + "mention_oncall_next": False, + "mention_oncall_start": True, + "notify_empty_oncall": 0, + "notify_oncall_shift_freq": 1, + "number_of_escalation_chains": 1, + }, + ], + } + response = client.get(url, format="json", **make_user_auth_headers(user, token)) + assert response.status_code == status.HTTP_200_OK + assert response.json() == expected_payload + + +@pytest.mark.django_db +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 + + # setup escalation chain linked to web schedule + escalation_chain = make_escalation_chain(user.organization) + make_escalation_policy( + escalation_chain=escalation_chain, + escalation_policy_step=EscalationPolicy.STEP_NOTIFY_SCHEDULE, + notify_schedule=web_schedule, + ) + + available_schedules = [ { "id": calendar_schedule.public_primary_key, "type": 0, @@ -129,9 +209,40 @@ def test_get_list_schedules( "number_of_escalation_chains": 1, }, ] - response = client.get(url, format="json", **make_user_auth_headers(user, token)) - assert response.status_code == status.HTTP_200_OK - assert response.json() == expected_payload + + client = APIClient() + + schedule_list_url = reverse("api-internal:schedule-list") + absolute_url = create_engine_url(schedule_list_url, override_base="http://testserver") + 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): + # only one schedule is passed here + assert qs.count() == 1 + return {} + + url = "{}?page={}&perpage=1".format(schedule_list_url, p) + with patch( + "apps.schedules.models.on_call_schedule.get_oncall_users_for_multiple_schedules", + side_effect=mock_oncall_now, + ): + response = client.get(url, format="json", **make_user_auth_headers(user, token)) + + assert response.status_code == status.HTTP_200_OK + previous_url = None + next_url = "{}?page={}&perpage=1".format(absolute_url, p + 1) + if p == 2: + previous_url = "{}?perpage=1".format(absolute_url) + elif p > 2: + previous_url = "{}?page={}&perpage=1".format(absolute_url, p - 1) + next_url = None + expected_payload = { + "count": 3, + "next": next_url, + "previous": previous_url, + "results": [schedule], + } + assert response.json() == expected_payload @pytest.mark.django_db @@ -149,7 +260,7 @@ def test_get_list_schedules_by_type( notify_schedule=web_schedule, ) - expected_payload = [ + available_schedules = [ { "id": calendar_schedule.public_primary_key, "type": 0, @@ -209,7 +320,13 @@ def test_get_list_schedules_by_type( url = reverse("api-internal:schedule-list") + "?type={}".format(schedule_type) response = client.get(url, format="json", **make_user_auth_headers(user, token)) assert response.status_code == status.HTTP_200_OK - assert response.json() == [expected_payload[schedule_type]] + expected_payload = { + "count": 1, + "next": None, + "previous": None, + "results": [available_schedules[schedule_type]], + } + assert response.json() == expected_payload @pytest.mark.django_db diff --git a/engine/apps/api/views/schedule.py b/engine/apps/api/views/schedule.py index 7d475f7f..44fa0024 100644 --- a/engine/apps/api/views/schedule.py +++ b/engine/apps/api/views/schedule.py @@ -4,10 +4,11 @@ from django.db.utils import IntegrityError from django.urls import reverse from django.utils import dateparse, timezone from django.utils.functional import cached_property -from rest_framework import status +from rest_framework import mixins, status from rest_framework.decorators import action from rest_framework.exceptions import NotFound from rest_framework.filters import SearchFilter +from rest_framework.pagination import PageNumberPagination from rest_framework.permissions import IsAuthenticated from rest_framework.views import Response from rest_framework.viewsets import ModelViewSet @@ -48,6 +49,13 @@ SCHEDULE_TYPE_TO_CLASS = { } +class SchedulePagination(PageNumberPagination): + page_size = 10 + page_query_param = "page" + page_size_query_param = "perpage" + max_page_size = 50 + + class ScheduleView( TeamFilteringMixin, PublicPrimaryKeyMixin, @@ -55,6 +63,7 @@ class ScheduleView( CreateSerializerMixin, UpdateSerializerMixin, ModelViewSet, + mixins.ListModelMixin, ): authentication_classes = (PluginAuthentication,) permission_classes = (IsAuthenticated, RBACPermission) @@ -86,6 +95,7 @@ class ScheduleView( create_serializer_class = PolymorphicScheduleCreateSerializer update_serializer_class = PolymorphicScheduleUpdateSerializer short_serializer_class = ScheduleFastSerializer + pagination_class = SchedulePagination @cached_property def can_update_user_groups(self): @@ -110,7 +120,9 @@ class ScheduleView( The result of this method is cached and is reused for the whole lifetime of a request, since self.get_serializer_context() is called multiple times for every instance in the queryset. """ - queryset = self.get_queryset() + current_page_schedules = self.paginate_queryset(self.get_queryset()) + pks = [schedule.pk for schedule in current_page_schedules] + queryset = OnCallSchedule.objects.filter(pk__in=pks) return queryset.get_oncall_users() def get_serializer_context(self): diff --git a/grafana-plugin/src/components/Policy/EscalationPolicy.tsx b/grafana-plugin/src/components/Policy/EscalationPolicy.tsx index 05eca27f..3f01158e 100644 --- a/grafana-plugin/src/components/Policy/EscalationPolicy.tsx +++ b/grafana-plugin/src/components/Policy/EscalationPolicy.tsx @@ -265,6 +265,8 @@ export class EscalationPolicy extends React.Component @@ -173,10 +173,10 @@ const EscalationVariantsPopup = observer((props: EscalationVariantsPopupProps) = onChange={setUsersSearchTerm} /> diff --git a/grafana-plugin/src/models/schedule/schedule.ts b/grafana-plugin/src/models/schedule/schedule.ts index a2dd6322..2be3e213 100644 --- a/grafana-plugin/src/models/schedule/schedule.ts +++ b/grafana-plugin/src/models/schedule/schedule.ts @@ -28,7 +28,7 @@ import { export class ScheduleStore extends BaseStore { @observable - searchResult: { results?: Array } = {}; + searchResult: { count?: number; results?: Array } = {}; @observable.shallow items: { [id: string]: Schedule } = {}; @@ -118,15 +118,14 @@ export class ScheduleStore extends BaseStore { } @action - async updateItems(f: any = { searchTerm: '', type: undefined }) { - // async updateItems(query = '') { + async updateItems(f: any = { searchTerm: '', type: undefined }, page = 1) { const filters = typeof f === 'string' ? { searchTerm: f } : f; const { searchTerm: search, type } = filters; - const result = await makeRequest(this.path, { method: 'GET', params: { search: search, type } }); + const { count, results } = await makeRequest(this.path, { method: 'GET', params: { search: search, type, page } }); this.items = { ...this.items, - ...result.reduce( + ...results.reduce( (acc: { [key: number]: Schedule }, item: Schedule) => ({ ...acc, [item.id]: item, @@ -135,8 +134,8 @@ export class ScheduleStore extends BaseStore { ), }; this.searchResult = { - ...this.searchResult, - results: result.map((item: Schedule) => item.id), + count, + results: results.map((item: Schedule) => item.id), }; } @@ -152,10 +151,13 @@ export class ScheduleStore extends BaseStore { } getSearchResult() { - if (!this.searchResult.results) { + if (!this.searchResult) { return undefined; } - return this.searchResult?.results?.map((scheduleId: Schedule['id']) => this.items[scheduleId]); + return { + count: this.searchResult.count, + results: this.searchResult.results?.map((scheduleId: Schedule['id']) => this.items[scheduleId]), + }; } @action diff --git a/grafana-plugin/src/models/user/user.ts b/grafana-plugin/src/models/user/user.ts index eb5900b9..1191449f 100644 --- a/grafana-plugin/src/models/user/user.ts +++ b/grafana-plugin/src/models/user/user.ts @@ -8,7 +8,7 @@ import { makeRequest } from 'network'; import { Mixpanel } from 'services/mixpanel'; import { RootStore } from 'state'; import { move } from 'state/helpers'; -import { trottlingError } from 'utils'; +import { throttlingError } from 'utils'; import { isUserActionAllowed, UserActions } from 'utils/authorization'; import { getTimezone, prepareForUpdate } from './user.helpers'; @@ -238,14 +238,14 @@ export class UserStore extends BaseStore { await makeRequest(`/users/${userPk}/get_verification_code/`, { method: 'GET', headers: { 'X-OnCall-Recaptcha': recaptchaToken }, - }).catch(trottlingError); + }).catch(throttlingError); } @action async verifyPhone(userPk: User['pk'], token: string) { return await makeRequest(`/users/${userPk}/verify_number/?token=${token}`, { method: 'PUT', - }).catch(trottlingError); + }).catch(throttlingError); } @action diff --git a/grafana-plugin/src/pages/schedules/Schedules.tsx b/grafana-plugin/src/pages/schedules/Schedules.tsx index 00c00dcc..c2f7dff8 100644 --- a/grafana-plugin/src/pages/schedules/Schedules.tsx +++ b/grafana-plugin/src/pages/schedules/Schedules.tsx @@ -27,16 +27,18 @@ import { Schedule, ScheduleType } from 'models/schedule/schedule.types'; import { getSlackChannelName } from 'models/slack_channel/slack_channel.helpers'; import { Timezone } from 'models/timezone/timezone.types'; import { getStartOfWeek } from 'pages/schedule/Schedule.helpers'; -import { WithStoreProps } from 'state/types'; +import { WithStoreProps, PageProps } from 'state/types'; import { withMobXProviderContext } from 'state/withStore'; +import LocationHelper from 'utils/LocationHelper'; import { UserActions } from 'utils/authorization'; import { PLUGIN_ROOT, TABLE_COLUMN_MAX_WIDTH } from 'utils/consts'; import styles from './Schedules.module.css'; const cx = cn.bind(styles); +const ITEMS_PER_PAGE = 10; -interface SchedulesPageProps extends WithStoreProps, RouteComponentProps {} +interface SchedulesPageProps extends WithStoreProps, RouteComponentProps, PageProps {} interface SchedulesPageState { startMoment: dayjs.Dayjs; @@ -44,6 +46,7 @@ interface SchedulesPageState { showNewScheduleSelector: boolean; expandedRowKeys: Array; scheduleIdToEdit?: Schedule['id']; + page: number; } @observer @@ -58,23 +61,36 @@ class SchedulesPage extends React.Component { + const { store } = this.props; + const { filters, page } = this.state; + + LocationHelper.update({ p: page }, 'partial'); + await store.scheduleStore.updateItems(filters, page); + }; + render() { const { store } = this.props; - const { filters, showNewScheduleSelector, expandedRowKeys, scheduleIdToEdit } = this.state; + const { filters, showNewScheduleSelector, expandedRowKeys, scheduleIdToEdit, page } = this.state; const { scheduleStore } = store; - const schedules = scheduleStore.getSearchResult(); + const { count, results } = scheduleStore.getSearchResult(); + const columns = [ { width: '10%', @@ -125,8 +141,8 @@ class SchedulesPage extends React.Component filters.status === 'all' || (filters.status === 'used' && schedule.number_of_escalation_chains) || @@ -158,7 +174,7 @@ class SchedulesPage extends React.Component {}; + handlePageChange = (page: number) => { + this.setState({ page }, this.updateSchedules); + this.setState({ expandedRowKeys: [] }); + }; update = () => { const { store } = this.props; diff --git a/grafana-plugin/src/plugin/GrafanaPluginRootPage.tsx b/grafana-plugin/src/plugin/GrafanaPluginRootPage.tsx index b9f3df12..7d3dd110 100644 --- a/grafana-plugin/src/plugin/GrafanaPluginRootPage.tsx +++ b/grafana-plugin/src/plugin/GrafanaPluginRootPage.tsx @@ -155,7 +155,7 @@ export const Root = observer((props: AppRootProps) => { - + diff --git a/grafana-plugin/src/utils/index.ts b/grafana-plugin/src/utils/index.ts index df1d9492..226ae7e3 100644 --- a/grafana-plugin/src/utils/index.ts +++ b/grafana-plugin/src/utils/index.ts @@ -47,7 +47,7 @@ export function refreshPageError(error: AxiosError) { throw error; } -export function trottlingError(error: AxiosError) { +export function throttlingError(error: AxiosError) { if (error.response?.status === 429) { const seconds = Number(error.response?.headers['retry-after']); const minutes = Math.floor(seconds / 60);