Add schedule pagination to plugin API (#1309)
Related to #1289 --------- Co-authored-by: Yulia Shanyrova <yulia.shanyrova@grafana.com>
This commit is contained in:
parent
49946e6a4e
commit
98b3b918a5
10 changed files with 193 additions and 36 deletions
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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):
|
||||
|
|
|
|||
|
|
@ -265,6 +265,8 @@ export class EscalationPolicy extends React.Component<EscalationPolicyProps, any
|
|||
return (
|
||||
<WithPermissionControl key="notify_schedule" disableByPaywall userAction={UserActions.EscalationChainsWrite}>
|
||||
<GSelect
|
||||
showSearch
|
||||
allowClear
|
||||
modelName="scheduleStore"
|
||||
displayField="name"
|
||||
valueField="id"
|
||||
|
|
|
|||
|
|
@ -155,10 +155,10 @@ const EscalationVariantsPopup = observer((props: EscalationVariantsPopupProps) =
|
|||
onChange={setSchedulesSearchTerm}
|
||||
/>
|
||||
<GTable
|
||||
emptyText={store.scheduleStore.getSearchResult() ? 'No schedules found' : 'Loading...'}
|
||||
emptyText={store.scheduleStore.getSearchResult()?.results ? 'No schedules found' : 'Loading...'}
|
||||
rowKey="id"
|
||||
columns={scheduleColumns}
|
||||
data={store.scheduleStore.getSearchResult()}
|
||||
data={store.scheduleStore.getSearchResult()?.results}
|
||||
className={cx('schedule-table')}
|
||||
showHeader={false}
|
||||
/>
|
||||
|
|
@ -173,10 +173,10 @@ const EscalationVariantsPopup = observer((props: EscalationVariantsPopupProps) =
|
|||
onChange={setUsersSearchTerm}
|
||||
/>
|
||||
<GTable
|
||||
emptyText={store.userStore.getSearchResult().results ? 'No users found' : 'Loading...'}
|
||||
emptyText={store.userStore.getSearchResult()?.results ? 'No users found' : 'Loading...'}
|
||||
rowKey="id"
|
||||
columns={userColumns}
|
||||
data={store.userStore.getSearchResult().results}
|
||||
data={store.userStore.getSearchResult()?.results}
|
||||
className={cx('schedule-table')}
|
||||
showHeader={false}
|
||||
/>
|
||||
|
|
|
|||
|
|
@ -28,7 +28,7 @@ import {
|
|||
|
||||
export class ScheduleStore extends BaseStore {
|
||||
@observable
|
||||
searchResult: { results?: Array<Schedule['id']> } = {};
|
||||
searchResult: { count?: number; results?: Array<Schedule['id']> } = {};
|
||||
|
||||
@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
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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<Schedule['id']>;
|
||||
scheduleIdToEdit?: Schedule['id'];
|
||||
page: number;
|
||||
}
|
||||
|
||||
@observer
|
||||
|
|
@ -58,23 +61,36 @@ class SchedulesPage extends React.Component<SchedulesPageProps, SchedulesPageSta
|
|||
showNewScheduleSelector: false,
|
||||
expandedRowKeys: [],
|
||||
scheduleIdToEdit: undefined,
|
||||
page: 1,
|
||||
};
|
||||
}
|
||||
|
||||
async componentDidMount() {
|
||||
const { store } = this.props;
|
||||
const {
|
||||
store,
|
||||
query: { p },
|
||||
} = this.props;
|
||||
|
||||
store.userStore.updateItems();
|
||||
store.scheduleStore.updateItems();
|
||||
this.setState({ page: p ? Number(p) : 1 }, this.updateSchedules);
|
||||
}
|
||||
|
||||
updateSchedules = async () => {
|
||||
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<SchedulesPageProps, SchedulesPageSta
|
|||
|
||||
const users = store.userStore.getSearchResult().results;
|
||||
|
||||
const data = schedules
|
||||
? schedules.filter(
|
||||
const data = results
|
||||
? results.filter(
|
||||
(schedule) =>
|
||||
filters.status === 'all' ||
|
||||
(filters.status === 'used' && schedule.number_of_escalation_chains) ||
|
||||
|
|
@ -158,7 +174,7 @@ class SchedulesPage extends React.Component<SchedulesPageProps, SchedulesPageSta
|
|||
<Table
|
||||
columns={columns}
|
||||
data={data}
|
||||
pagination={{ page: 1, total: 1, onChange: this.handlePageChange }}
|
||||
pagination={{ page, total: Math.ceil((count || 0) / ITEMS_PER_PAGE), onChange: this.handlePageChange }}
|
||||
rowKey="id"
|
||||
expandable={{
|
||||
expandedRowKeys: expandedRowKeys,
|
||||
|
|
@ -407,7 +423,10 @@ class SchedulesPage extends React.Component<SchedulesPageProps, SchedulesPageSta
|
|||
|
||||
debouncedUpdateSchedules = debounce(this.applyFilters, 1000);
|
||||
|
||||
handlePageChange = (_page: number) => {};
|
||||
handlePageChange = (page: number) => {
|
||||
this.setState({ page }, this.updateSchedules);
|
||||
this.setState({ expandedRowKeys: [] });
|
||||
};
|
||||
|
||||
update = () => {
|
||||
const { store } = this.props;
|
||||
|
|
|
|||
|
|
@ -155,7 +155,7 @@ export const Root = observer((props: AppRootProps) => {
|
|||
<EscalationChains />
|
||||
</Route>
|
||||
<Route path={getRoutesForPage('schedules')} exact>
|
||||
<Schedules />
|
||||
<Schedules query={query} />
|
||||
</Route>
|
||||
<Route path={getRoutesForPage('schedule')} exact>
|
||||
<Schedule />
|
||||
|
|
|
|||
|
|
@ -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);
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue