From e2ea77d56519de3c034159bbecd38c03a4d9c835 Mon Sep 17 00:00:00 2001 From: Joey Orlando Date: Mon, 29 May 2023 14:53:59 -0400 Subject: [PATCH 01/19] WIP --- .../apps/public_api/tests/test_schedules.py | 17 +++++++++++ engine/apps/public_api/views/schedules.py | 28 +++++++++++++++++++ 2 files changed, 45 insertions(+) diff --git a/engine/apps/public_api/tests/test_schedules.py b/engine/apps/public_api/tests/test_schedules.py index b4a4027930..9e95e342b1 100644 --- a/engine/apps/public_api/tests/test_schedules.py +++ b/engine/apps/public_api/tests/test_schedules.py @@ -781,3 +781,20 @@ def test_create_ical_schedule_without_ical_url(make_organization_and_user_with_t } response = client.post(url, data=data, format="json", HTTP_AUTHORIZATION=f"{token}") assert response.status_code == status.HTTP_400_BAD_REQUEST + + +@pytest.mark.django_db +@pytest.only +def test_oncall_shifts_export_doesnt_work_for_ical_schedules( + make_organization_and_user_with_token, + make_schedule, +): + organization, _, token = make_organization_and_user_with_token() + schedule = make_schedule(organization, schedule_class=OnCallScheduleICal) + + client = APIClient() + + url = reverse("api-public:schedules-oncall_shifts_export", kwargs={"pk": schedule.public_primary_key}) + + response = client.get(url, format="json", HTTP_AUTHORIZATION=token) + assert response.status_code == status.HTTP_400_BAD_REQUEST diff --git a/engine/apps/public_api/views/schedules.py b/engine/apps/public_api/views/schedules.py index 39d5d26de4..eedb3ee05d 100644 --- a/engine/apps/public_api/views/schedules.py +++ b/engine/apps/public_api/views/schedules.py @@ -1,3 +1,7 @@ +import csv + +from django.http import HttpResponse +from django.utils import timezone from django_filters import rest_framework as filters from rest_framework import status from rest_framework.decorators import action @@ -12,6 +16,7 @@ from apps.public_api.throttlers.user_throttle import UserThrottle from apps.schedules.ical_utils import ical_export_from_schedule from apps.schedules.models import OnCallSchedule, OnCallScheduleWeb +from apps.schedules.models.on_call_schedule import ScheduleEvents from apps.slack.tasks import update_slack_user_group_for_schedules from common.api_helpers.exceptions import BadRequest from common.api_helpers.filters import ByTeamFilter @@ -120,3 +125,26 @@ def export(self, request, pk): # Not using existing get_object method because it requires access to the organization user attribute export = ical_export_from_schedule(self.request.auth.schedule) return Response(export, status=status.HTTP_200_OK) + + @action(methods=["get"], detail=True) + def oncall_shifts_export(self, request, pk): + schedule = self.get_object() + + response = HttpResponse(content_type="text/csv") + response["Content-Disposition"] = "attachment; filename='export.csv'" + writer = csv.DictWriter(response, fieldnames=["user_pk", "start", "end"]) + writer.writeheader() + + thirty_days_ago = timezone.now() - timezone.timedelta(days=30) + final_schedule_events: ScheduleEvents = schedule.final_events("UTC", thirty_days_ago, 30) + + # TODO: drop the request if it's not a web calendar + # TODO: handle query param filters + # TODO: pull dates from query params + for event in final_schedule_events: + shift_start = event["start"] + shift_end = event["end"] + for user in event["users"]: + writer.writerow({"user_pk": user["pk"], "shift_start": shift_start, "shift_end": shift_end}) + + return response From 4482eb942892a3ac6b71b76685722f5fa5295dae Mon Sep 17 00:00:00 2001 From: Joey Orlando Date: Mon, 29 May 2023 14:56:03 -0400 Subject: [PATCH 02/19] update changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index b6554cdba6..5faf6ab9e6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Helm chart: configuration of `uwsgi` using environment variables by @alexintech ([#2045](https://github.com/grafana/oncall/pull/2045)) - Much expanded/improved docs for mobile app ([2026](https://github.com/grafana/oncall/pull/2026>)) - Enable by-day selection when defining monthly and hourly rotations ([2037](https://github.com/grafana/oncall/pull/2037)) +- Add public API endpoint to export a schedule's oncall-shifts as a `.csv` by @joeyorlando ([2047](https://github.com/grafana/oncall/pull/2047)) ### Fixed From ad287061071cc2e3016140211468b153450f2fb7 Mon Sep 17 00:00:00 2001 From: Joey Orlando Date: Fri, 2 Jun 2023 10:03:31 +0200 Subject: [PATCH 03/19] WIP --- engine/apps/public_api/views/schedules.py | 63 ++++++++++++++++++++--- 1 file changed, 57 insertions(+), 6 deletions(-) diff --git a/engine/apps/public_api/views/schedules.py b/engine/apps/public_api/views/schedules.py index eedb3ee05d..4ad3d88059 100644 --- a/engine/apps/public_api/views/schedules.py +++ b/engine/apps/public_api/views/schedules.py @@ -1,7 +1,8 @@ import csv +import logging +from datetime import date from django.http import HttpResponse -from django.utils import timezone from django_filters import rest_framework as filters from rest_framework import status from rest_framework.decorators import action @@ -24,6 +25,8 @@ from common.api_helpers.paginators import FiftyPageSizePaginator from common.insight_log import EntityEvent, write_resource_insight_log +logger = logging.getLogger(__name__) + class OnCallScheduleChannelView(RateLimitHeadersMixin, UpdateSerializerMixin, ModelViewSet): authentication_classes = (ApiTokenAuthentication,) @@ -130,17 +133,65 @@ def export(self, request, pk): def oncall_shifts_export(self, request, pk): schedule = self.get_object() + if not isinstance(schedule, OnCallScheduleWeb): + return Response( + "OnCall shifts exports are currently only available for web calendars", + status=status.HTTP_400_BAD_REQUEST, + ) + + start_date_field_name = "start_date" + end_date_field_name = "end_date" + + def _field_is_required(field_name): + return Response(f"{field_name} is required", status=status.HTTP_400_BAD_REQUEST) + + def _field_is_invalid_date(field_name): + return Response( + f"{field_name} is not a valid date, must be in any valid ISO 8601 format", + status=status.HTTP_400_BAD_REQUEST, + ) + + def _convert_date(value): + if not value: + return None + + try: + return date.fromisoformat(value) + except ValueError: + return None + + start_date_str = request.query_params.get(start_date_field_name, None) + start_date = _convert_date(start_date_str) + + end_date_str = request.query_params.get(end_date_field_name, None) + end_date = _convert_date(end_date_str) + + if start_date_str is None: + return _field_is_required(start_date_field_name) + elif start_date is None: + return _field_is_invalid_date(start_date_field_name) + elif end_date_str is None: + return _field_is_required(end_date_field_name) + elif end_date is None: + return _field_is_invalid_date(end_date_field_name) + + if start_date > end_date: + return Response( + f"{start_date_field_name} must be less than or equal to {end_date_field_name}", + status=status.HTTP_400_BAD_REQUEST, + ) + response = HttpResponse(content_type="text/csv") response["Content-Disposition"] = "attachment; filename='export.csv'" writer = csv.DictWriter(response, fieldnames=["user_pk", "start", "end"]) writer.writeheader() - thirty_days_ago = timezone.now() - timezone.timedelta(days=30) - final_schedule_events: ScheduleEvents = schedule.final_events("UTC", thirty_days_ago, 30) + final_schedule_events: ScheduleEvents = schedule.final_events("UTC", start_date, (end_date - start_date).days) + + logger.info( + f"Exporting oncall shifts for schedule {pk} between dates {start_date_str} and {end_date_str}. {len(final_schedule_events)} shift events were found." + ) - # TODO: drop the request if it's not a web calendar - # TODO: handle query param filters - # TODO: pull dates from query params for event in final_schedule_events: shift_start = event["start"] shift_end = event["end"] From 878228be415277a9c7a56d9587f32b8913e013be Mon Sep 17 00:00:00 2001 From: Joey Orlando Date: Fri, 2 Jun 2023 11:01:49 +0200 Subject: [PATCH 04/19] work on tests --- .../apps/public_api/tests/test_schedules.py | 83 +++++++++++++++++-- 1 file changed, 78 insertions(+), 5 deletions(-) diff --git a/engine/apps/public_api/tests/test_schedules.py b/engine/apps/public_api/tests/test_schedules.py index 9e95e342b1..5ea84b359d 100644 --- a/engine/apps/public_api/tests/test_schedules.py +++ b/engine/apps/public_api/tests/test_schedules.py @@ -1,3 +1,4 @@ +import csv from unittest.mock import patch import pytest @@ -784,17 +785,89 @@ def test_create_ical_schedule_without_ical_url(make_organization_and_user_with_t @pytest.mark.django_db -@pytest.only -def test_oncall_shifts_export_doesnt_work_for_ical_schedules( +def test_oncall_shifts_request_validation( make_organization_and_user_with_token, make_schedule, ): organization, _, token = make_organization_and_user_with_token() - schedule = make_schedule(organization, schedule_class=OnCallScheduleICal) + ical_schedule = make_schedule(organization, schedule_class=OnCallScheduleICal) + terraform_schedule = make_schedule(organization, schedule_class=OnCallScheduleCalendar) + web_schedule = make_schedule(organization, schedule_class=OnCallScheduleWeb) + + schedule_type_validation_msg = "OnCall shifts exports are currently only available for web calendars" + valid_date_msg = "is not a valid date, must be in any valid ISO 8601 format" client = APIClient() - url = reverse("api-public:schedules-oncall_shifts_export", kwargs={"pk": schedule.public_primary_key}) + def _make_request(schedule, query_params=""): + url = reverse("api-public:schedules-oncall-shifts-export", kwargs={"pk": schedule.public_primary_key}) + return client.get(f"{url}{query_params}", format="json", HTTP_AUTHORIZATION=token) + + # only web schedules are allowed for now + response = _make_request(ical_schedule) + assert response.status_code == status.HTTP_400_BAD_REQUEST + assert response.data == schedule_type_validation_msg + + response = _make_request(terraform_schedule) + assert response.status_code == status.HTTP_400_BAD_REQUEST + assert response.data == schedule_type_validation_msg + + # query param validation + response = _make_request(web_schedule, "?start_date=2021-01-01") + assert response.status_code == status.HTTP_400_BAD_REQUEST + assert response.data == "end_date is required" - response = client.get(url, format="json", HTTP_AUTHORIZATION=token) + response = _make_request(web_schedule, "?start_date=asdfasdf") assert response.status_code == status.HTTP_400_BAD_REQUEST + assert response.data == f"start_date {valid_date_msg}" + + response = _make_request(web_schedule, "?end_date=2021-01-01") + assert response.status_code == status.HTTP_400_BAD_REQUEST + assert response.data == "start_date is required" + + response = _make_request(web_schedule, "?start_date=2021-01-01&end_date=asdfasdf") + assert response.status_code == status.HTTP_400_BAD_REQUEST + assert response.data == f"end_date {valid_date_msg}" + + response = _make_request(web_schedule, "?end_date=2021-01-01&start_date=2022-01-01") + assert response.status_code == status.HTTP_400_BAD_REQUEST + assert response.data == "start_date must be less than or equal to end_date" + + +@pytest.mark.django_db +def test_oncall_shifts_export( + make_organization_and_user_with_token, + make_schedule, + make_on_call_shift, +): + organization, user, token = make_organization_and_user_with_token() + schedule = make_schedule(organization, schedule_class=OnCallScheduleWeb) + + start_date = timezone.datetime(2023, 1, 1, 9, 0, 0) + make_on_call_shift( + organization=organization, + schedule=schedule, + shift_type=CustomOnCallShift.TYPE_ROLLING_USERS_EVENT, + frequency=CustomOnCallShift.FREQUENCY_DAILY, + priority_level=1, + by_day=["MO", "WE", "FR"], + start=start_date, + until=start_date + timezone.timedelta(days=28), + rolling_users=[{user.pk: user.public_primary_key}], + rotation_start=start_date, + duration=timezone.timedelta(hours=8), + ) + + client = APIClient() + + url = reverse("api-public:schedules-oncall-shifts-export", kwargs={"pk": schedule.public_primary_key}) + response = client.get(f"{url}?start_date=2023-01-01&end_date=2023-02-01", format="json", HTTP_AUTHORIZATION=token) + + assert response.status_code == status.HTTP_200_OK + + total_time_on_call = 0 + for row in csv.DictReader(response.content): + total_time_on_call += row["shift_end"] - row["shift_start"] + + # 3 shifts per week x 4 weeks x 8 hours per shift = 96 + assert total_time_on_call == 96 From 88a20f805eb8d1412ebe825e04517301700a9213 Mon Sep 17 00:00:00 2001 From: Joey Orlando Date: Fri, 2 Jun 2023 11:02:57 +0200 Subject: [PATCH 05/19] tests --- engine/apps/public_api/tests/test_schedules.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/engine/apps/public_api/tests/test_schedules.py b/engine/apps/public_api/tests/test_schedules.py index 5ea84b359d..7724900ea1 100644 --- a/engine/apps/public_api/tests/test_schedules.py +++ b/engine/apps/public_api/tests/test_schedules.py @@ -867,7 +867,9 @@ def test_oncall_shifts_export( total_time_on_call = 0 for row in csv.DictReader(response.content): - total_time_on_call += row["shift_end"] - row["shift_start"] + end = timezone.datetime.strptime(row["shift_end"]) + start = timezone.datetime.strptime(row["shift_start"]) + total_time_on_call += (end - start).hours # 3 shifts per week x 4 weeks x 8 hours per shift = 96 assert total_time_on_call == 96 From 76d417de8772d4776c9aa731accc24ec0741b805 Mon Sep 17 00:00:00 2001 From: Joey Orlando Date: Fri, 2 Jun 2023 11:15:10 +0200 Subject: [PATCH 06/19] add public docs --- .../sources/oncall-api-reference/schedules.md | 22 +++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/docs/sources/oncall-api-reference/schedules.md b/docs/sources/oncall-api-reference/schedules.md index 4be429b7b8..8d58b40357 100644 --- a/docs/sources/oncall-api-reference/schedules.md +++ b/docs/sources/oncall-api-reference/schedules.md @@ -197,3 +197,25 @@ curl "{{API_URL}}/api/v1/schedules/SBM7DV7BKFUYU/" \ **HTTP request** `DELETE {{API_URL}}/api/v1/schedules//` + +# Export a schedule's shifts + +**HTTP request** + +```shell +curl "{{API_URL}}/api/v1/schedules/SBM7DV7BKFUYU/oncall_shifts_export?start_date=2023-01-01&end_date=2023-02-01" \ + --request GET \ + --header "Authorization: meowmeowmeow" +``` + +The response body will contain `text/csv` content as such: + +```csv +user_pk,start,end +UBM7DV7BKFUYU,2023-06-02 09:00:00,2023-06-02 17:00:00 +UAHG785BNVMCD,2023-06-02 09:00:00,2023-06-02 17:00:00 +... +``` + +_Note_: `start_date` and `end_date` are both required query parameters, that should represent ISO 8601 formatted dates. +`end_date` must be greater than or equal to `start_date`. From 7aa3277c266a12cfd9c8aa98e24683a6d33861fb Mon Sep 17 00:00:00 2001 From: Joey Orlando Date: Fri, 2 Jun 2023 11:17:22 +0200 Subject: [PATCH 07/19] update tests --- engine/apps/public_api/tests/test_schedules.py | 1 + 1 file changed, 1 insertion(+) diff --git a/engine/apps/public_api/tests/test_schedules.py b/engine/apps/public_api/tests/test_schedules.py index 7724900ea1..59a37f6564 100644 --- a/engine/apps/public_api/tests/test_schedules.py +++ b/engine/apps/public_api/tests/test_schedules.py @@ -850,6 +850,7 @@ def test_oncall_shifts_export( shift_type=CustomOnCallShift.TYPE_ROLLING_USERS_EVENT, frequency=CustomOnCallShift.FREQUENCY_DAILY, priority_level=1, + interval=1, by_day=["MO", "WE", "FR"], start=start_date, until=start_date + timezone.timedelta(days=28), From f00bfbd0bf4b3b04ca3328887a05b4b6cd376e18 Mon Sep 17 00:00:00 2001 From: Joey Orlando Date: Fri, 2 Jun 2023 11:18:20 +0200 Subject: [PATCH 08/19] update changelog --- CHANGELOG.md | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index bd003a733b..2f9fac6f7e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,12 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## Unreleased + +### Added + +- Add public API endpoint to export a schedule's oncall-shifts as a `.csv` by @joeyorlando ([2047](https://github.com/grafana/oncall/pull/2047)) + ## v1.2.35 (2023-06-01) ### Fixed @@ -40,7 +46,6 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Helm chart: configuration of `uwsgi` using environment variables by @alexintech ([#2045](https://github.com/grafana/oncall/pull/2045)) - Much expanded/improved docs for mobile app ([2026](https://github.com/grafana/oncall/pull/2026>)) - Enable by-day selection when defining monthly and hourly rotations ([2037](https://github.com/grafana/oncall/pull/2037)) -- Add public API endpoint to export a schedule's oncall-shifts as a `.csv` by @joeyorlando ([2047](https://github.com/grafana/oncall/pull/2047)) ### Fixed From e78d8d5b1b7969ff7289a1cf80f4fbd7ae03bd74 Mon Sep 17 00:00:00 2001 From: Joey Orlando Date: Fri, 2 Jun 2023 11:45:37 +0200 Subject: [PATCH 09/19] update tests --- engine/apps/public_api/tests/test_schedules.py | 13 +++++++++---- engine/apps/public_api/views/schedules.py | 14 ++++++++++++-- 2 files changed, 21 insertions(+), 6 deletions(-) diff --git a/engine/apps/public_api/tests/test_schedules.py b/engine/apps/public_api/tests/test_schedules.py index 59a37f6564..d4d784420b 100644 --- a/engine/apps/public_api/tests/test_schedules.py +++ b/engine/apps/public_api/tests/test_schedules.py @@ -1,4 +1,5 @@ import csv +import io from unittest.mock import patch import pytest @@ -866,11 +867,15 @@ def test_oncall_shifts_export( assert response.status_code == status.HTTP_200_OK + print(response.content) + total_time_on_call = 0 - for row in csv.DictReader(response.content): - end = timezone.datetime.strptime(row["shift_end"]) - start = timezone.datetime.strptime(row["shift_start"]) - total_time_on_call += (end - start).hours + for row in csv.DictReader(io.StringIO(response.content.decode())): + if row["user_pk"] == user.public_primary_key: + end = timezone.datetime.fromisoformat(row["shift_end"]) + start = timezone.datetime.fromisoformat(row["shift_start"]) + shift_time_in_seconds = (end - start).total_seconds() + total_time_on_call += shift_time_in_seconds / (60 * 60) # 3 shifts per week x 4 weeks x 8 hours per shift = 96 assert total_time_on_call == 96 diff --git a/engine/apps/public_api/views/schedules.py b/engine/apps/public_api/views/schedules.py index 4ad3d88059..0bc1d815dc 100644 --- a/engine/apps/public_api/views/schedules.py +++ b/engine/apps/public_api/views/schedules.py @@ -183,7 +183,11 @@ def _convert_date(value): response = HttpResponse(content_type="text/csv") response["Content-Disposition"] = "attachment; filename='export.csv'" - writer = csv.DictWriter(response, fieldnames=["user_pk", "start", "end"]) + + user_pk_field_name = "user_pk" + shift_start_field_name = "shift_start" + shift_end_field_name = "shift_end" + writer = csv.DictWriter(response, fieldnames=[user_pk_field_name, shift_start_field_name, shift_end_field_name]) writer.writeheader() final_schedule_events: ScheduleEvents = schedule.final_events("UTC", start_date, (end_date - start_date).days) @@ -196,6 +200,12 @@ def _convert_date(value): shift_start = event["start"] shift_end = event["end"] for user in event["users"]: - writer.writerow({"user_pk": user["pk"], "shift_start": shift_start, "shift_end": shift_end}) + writer.writerow( + { + user_pk_field_name: user["pk"], + shift_start_field_name: shift_start, + shift_end_field_name: shift_end, + } + ) return response From ad75ed0bd67985baad56e03c2186055011fca3f4 Mon Sep 17 00:00:00 2001 From: Joey Orlando Date: Fri, 2 Jun 2023 11:45:50 +0200 Subject: [PATCH 10/19] update tests --- engine/apps/public_api/tests/test_schedules.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/engine/apps/public_api/tests/test_schedules.py b/engine/apps/public_api/tests/test_schedules.py index d4d784420b..523b789164 100644 --- a/engine/apps/public_api/tests/test_schedules.py +++ b/engine/apps/public_api/tests/test_schedules.py @@ -867,8 +867,6 @@ def test_oncall_shifts_export( assert response.status_code == status.HTTP_200_OK - print(response.content) - total_time_on_call = 0 for row in csv.DictReader(io.StringIO(response.content.decode())): if row["user_pk"] == user.public_primary_key: From a52db0a751746f67d97ceff7e92fae7df853981c Mon Sep 17 00:00:00 2001 From: Joey Orlando Date: Fri, 2 Jun 2023 11:50:17 +0200 Subject: [PATCH 11/19] update documentation --- docs/sources/oncall-api-reference/schedules.md | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/docs/sources/oncall-api-reference/schedules.md b/docs/sources/oncall-api-reference/schedules.md index 8d58b40357..aaaaf885ff 100644 --- a/docs/sources/oncall-api-reference/schedules.md +++ b/docs/sources/oncall-api-reference/schedules.md @@ -211,10 +211,19 @@ curl "{{API_URL}}/api/v1/schedules/SBM7DV7BKFUYU/oncall_shifts_export?start_date The response body will contain `text/csv` content as such: ```csv -user_pk,start,end -UBM7DV7BKFUYU,2023-06-02 09:00:00,2023-06-02 17:00:00 -UAHG785BNVMCD,2023-06-02 09:00:00,2023-06-02 17:00:00 -... +user_pk,shift_start,shift_end +UQENVYDBP3IFE,2023-01-02 09:00:00+00:00,2023-01-02 17:00:00+00:00 +UQENVYDBP3IFE,2023-01-04 09:00:00+00:00,2023-01-04 17:00:00+00:00 +UQENVYDBP3IFE,2023-01-06 09:00:00+00:00,2023-01-06 17:00:00+00:00 +UQENVYDBP3IFE,2023-01-09 09:00:00+00:00,2023-01-09 17:00:00+00:00 +UQENVYDBP3IFE,2023-01-11 09:00:00+00:00,2023-01-11 17:00:00+00:00 +UQENVYDBP3IFE,2023-01-13 09:00:00+00:00,2023-01-13 17:00:00+00:00 +UQENVYDBP3IFE,2023-01-16 09:00:00+00:00,2023-01-16 17:00:00+00:00 +UQENVYDBP3IFE,2023-01-18 09:00:00+00:00,2023-01-18 17:00:00+00:00 +UQENVYDBP3IFE,2023-01-20 09:00:00+00:00,2023-01-20 17:00:00+00:00 +UQENVYDBP3IFE,2023-01-23 09:00:00+00:00,2023-01-23 17:00:00+00:00 +UQENVYDBP3IFE,2023-01-25 09:00:00+00:00,2023-01-25 17:00:00+00:00 +UQENVYDBP3IFE,2023-01-27 09:00:00+00:00,2023-01-27 17:00:00+00:00 ``` _Note_: `start_date` and `end_date` are both required query parameters, that should represent ISO 8601 formatted dates. From 9ae7e88601998c3ab32931487334e8d8dd4ce1b1 Mon Sep 17 00:00:00 2001 From: Joey Orlando Date: Fri, 2 Jun 2023 14:44:02 +0200 Subject: [PATCH 12/19] updates per discussions --- .../sources/oncall-api-reference/schedules.md | 144 +++++++++++++++--- .../apps/public_api/tests/test_schedules.py | 47 ++++-- engine/apps/public_api/views/schedules.py | 57 +++---- .../apps/schedules/models/on_call_schedule.py | 7 + 4 files changed, 194 insertions(+), 61 deletions(-) diff --git a/docs/sources/oncall-api-reference/schedules.md b/docs/sources/oncall-api-reference/schedules.md index aaaaf885ff..0cac4fe13b 100644 --- a/docs/sources/oncall-api-reference/schedules.md +++ b/docs/sources/oncall-api-reference/schedules.md @@ -198,33 +198,137 @@ curl "{{API_URL}}/api/v1/schedules/SBM7DV7BKFUYU/" \ `DELETE {{API_URL}}/api/v1/schedules//` -# Export a schedule's shifts +# Export a schedule's final shifts **HTTP request** ```shell -curl "{{API_URL}}/api/v1/schedules/SBM7DV7BKFUYU/oncall_shifts_export?start_date=2023-01-01&end_date=2023-02-01" \ +curl "{{API_URL}}/api/v1/schedules/SBM7DV7BKFUYU/final_shifts?start_date=2023-01-01&end_date=2023-02-01" \ --request GET \ --header "Authorization: meowmeowmeow" ``` -The response body will contain `text/csv` content as such: - -```csv -user_pk,shift_start,shift_end -UQENVYDBP3IFE,2023-01-02 09:00:00+00:00,2023-01-02 17:00:00+00:00 -UQENVYDBP3IFE,2023-01-04 09:00:00+00:00,2023-01-04 17:00:00+00:00 -UQENVYDBP3IFE,2023-01-06 09:00:00+00:00,2023-01-06 17:00:00+00:00 -UQENVYDBP3IFE,2023-01-09 09:00:00+00:00,2023-01-09 17:00:00+00:00 -UQENVYDBP3IFE,2023-01-11 09:00:00+00:00,2023-01-11 17:00:00+00:00 -UQENVYDBP3IFE,2023-01-13 09:00:00+00:00,2023-01-13 17:00:00+00:00 -UQENVYDBP3IFE,2023-01-16 09:00:00+00:00,2023-01-16 17:00:00+00:00 -UQENVYDBP3IFE,2023-01-18 09:00:00+00:00,2023-01-18 17:00:00+00:00 -UQENVYDBP3IFE,2023-01-20 09:00:00+00:00,2023-01-20 17:00:00+00:00 -UQENVYDBP3IFE,2023-01-23 09:00:00+00:00,2023-01-23 17:00:00+00:00 -UQENVYDBP3IFE,2023-01-25 09:00:00+00:00,2023-01-25 17:00:00+00:00 -UQENVYDBP3IFE,2023-01-27 09:00:00+00:00,2023-01-27 17:00:00+00:00 +The above command returns JSON structured in the following way: + +```json +{ + "count": 12, + "next": null, + "previous": null, + "results": [ + { + "user_pk": "UC2CHRT5SD34X", + "shift_start": "2023-01-02T09:00:00Z", + "shift_end": "2023-01-02T17:00:00Z" + }, + { + "user_pk": "U7S8H84ARFTGN", + "shift_start": "2023-01-04T09:00:00Z", + "shift_end": "2023-01-04T17:00:00Z" + }, + { + "user_pk": "UC2CHRT5SD34X", + "shift_start": "2023-01-06T09:00:00Z", + "shift_end": "2023-01-06T17:00:00Z" + }, + { + "user_pk": "U7S8H84ARFTGN", + "shift_start": "2023-01-09T09:00:00Z", + "shift_end": "2023-01-09T17:00:00Z" + }, + { + "user_pk": "UC2CHRT5SD34X", + "shift_start": "2023-01-11T09:00:00Z", + "shift_end": "2023-01-11T17:00:00Z" + }, + { + "user_pk": "U7S8H84ARFTGN", + "shift_start": "2023-01-13T09:00:00Z", + "shift_end": "2023-01-13T17:00:00Z" + }, + { + "user_pk": "UC2CHRT5SD34X", + "shift_start": "2023-01-16T09:00:00Z", + "shift_end": "2023-01-16T17:00:00Z" + }, + { + "user_pk": "U7S8H84ARFTGN", + "shift_start": "2023-01-18T09:00:00Z", + "shift_end": "2023-01-18T17:00:00Z" + }, + { + "user_pk": "UC2CHRT5SD34X", + "shift_start": "2023-01-20T09:00:00Z", + "shift_end": "2023-01-20T17:00:00Z" + }, + { + "user_pk": "U7S8H84ARFTGN", + "shift_start": "2023-01-23T09:00:00Z", + "shift_end": "2023-01-23T17:00:00Z" + }, + { + "user_pk": "UC2CHRT5SD34X", + "shift_start": "2023-01-25T09:00:00Z", + "shift_end": "2023-01-25T17:00:00Z" + }, + { + "user_pk": "U7S8H84ARFTGN", + "shift_start": "2023-01-27T09:00:00Z", + "shift_end": "2023-01-27T17:00:00Z" + } + ] +} ``` -_Note_: `start_date` and `end_date` are both required query parameters, that should represent ISO 8601 formatted dates. -`end_date` must be greater than or equal to `start_date`. +## Caveats + +Some notes on the `start_date` and `end_date` query parameters: + +- they are both required and should represent ISO 8601 formatted dates +- `end_date` must be greater than or equal to `start_date` +- `end_date` cannot be more than 365 days in the future from `start_date` + +Lastly, this endpoint is currently only active for web schedules. It will return HTTP 400 for schedules +defined via Terraform or iCal. + +## Example script to transform data to .csv for all of your schedules + +The following Python script will generate a `.csv` file, `oncall-report-2023-01-01-to-2023-01-31.csv`. This file will +contain two columns, `user_pk` and `hours_on_call`, which represents how many hours each user was on call during the +period starting January 1, 2023 to January 31, 2023 (inclusive). + +```python +import collections +import csv +import requests +from datetime import datetime + +# CUSTOMIZE THE FOLLOWING VARIABLES +START_DATE = "2023-01-01" +END_DATE = "2023-01-31" +OUTPUT_FILE_NAME = f"oncall-report-{START_DATE}-to-{END_DATE}.csv" +MY_ONCALL_API_BASE_URL = "https://oncall-prod-us-central-0.grafana.net/oncall/api/v1/schedules" +MY_ONCALL_API_KEY = "meowmeowwoofwoof" + +headers = {"Authorization": MY_ONCALL_API_KEY} +schedule_ids = [schedule["id"] for schedule in requests.get(MY_ONCALL_API_BASE_URL, headers=headers).json()["results"]] +user_on_call_hours = collections.defaultdict(int) + +for schedule_id in schedule_ids: + response = requests.get( + f"{MY_ONCALL_API_BASE_URL}/{schedule_id}/final_shifts?start_date={START_DATE}&end_date={END_DATE}", + headers=headers) + + for final_shift in response.json()["results"]: + end = datetime.fromisoformat(final_shift["shift_end"]) + start = datetime.fromisoformat(final_shift["shift_start"]) + shift_time_in_seconds = (end - start).total_seconds() + user_on_call_hours[final_shift["user_pk"]] += shift_time_in_seconds / (60 * 60) + +with open(OUTPUT_FILE_NAME, "w") as fp: + csv_writer = csv.DictWriter(fp, ["user_pk", "hours_on_call"]) + csv_writer.writeheader() + + for user_pk, hours_on_call in user_on_call_hours.items(): + csv_writer.writerow({"user_pk": user_pk, "hours_on_call": hours_on_call}) +``` diff --git a/engine/apps/public_api/tests/test_schedules.py b/engine/apps/public_api/tests/test_schedules.py index 523b789164..a9153c3cf1 100644 --- a/engine/apps/public_api/tests/test_schedules.py +++ b/engine/apps/public_api/tests/test_schedules.py @@ -1,5 +1,4 @@ -import csv -import io +import collections from unittest.mock import patch import pytest @@ -801,7 +800,7 @@ def test_oncall_shifts_request_validation( client = APIClient() def _make_request(schedule, query_params=""): - url = reverse("api-public:schedules-oncall-shifts-export", kwargs={"pk": schedule.public_primary_key}) + url = reverse("api-public:schedules-final-shifts", kwargs={"pk": schedule.public_primary_key}) return client.get(f"{url}{query_params}", format="json", HTTP_AUTHORIZATION=token) # only web schedules are allowed for now @@ -834,14 +833,20 @@ def _make_request(schedule, query_params=""): assert response.status_code == status.HTTP_400_BAD_REQUEST assert response.data == "start_date must be less than or equal to end_date" + response = _make_request(web_schedule, "?end_date=2021-01-01&start_date=2019-12-31") + assert response.status_code == status.HTTP_400_BAD_REQUEST + assert response.data == "The difference between start_date and end_date must be less than one year (365 days)" + @pytest.mark.django_db def test_oncall_shifts_export( make_organization_and_user_with_token, + make_user, make_schedule, make_on_call_shift, ): - organization, user, token = make_organization_and_user_with_token() + organization, user1, token = make_organization_and_user_with_token() + user2 = make_user(organization=organization) schedule = make_schedule(organization, schedule_class=OnCallScheduleWeb) start_date = timezone.datetime(2023, 1, 1, 9, 0, 0) @@ -855,25 +860,37 @@ def test_oncall_shifts_export( by_day=["MO", "WE", "FR"], start=start_date, until=start_date + timezone.timedelta(days=28), - rolling_users=[{user.pk: user.public_primary_key}], + rolling_users=[{user1.pk: user1.public_primary_key}, {user2.pk: user2.public_primary_key}], rotation_start=start_date, duration=timezone.timedelta(hours=8), ) client = APIClient() - url = reverse("api-public:schedules-oncall-shifts-export", kwargs={"pk": schedule.public_primary_key}) + url = reverse("api-public:schedules-final-shifts", kwargs={"pk": schedule.public_primary_key}) response = client.get(f"{url}?start_date=2023-01-01&end_date=2023-02-01", format="json", HTTP_AUTHORIZATION=token) + response_json = response.json() + shifts = response_json["results"] + + total_time_on_call = collections.defaultdict(int) + for row in shifts: + end = timezone.datetime.fromisoformat(row["shift_end"]) + start = timezone.datetime.fromisoformat(row["shift_start"]) + shift_time_in_seconds = (end - start).total_seconds() + total_time_on_call[row["user_pk"]] += shift_time_in_seconds / (60 * 60) assert response.status_code == status.HTTP_200_OK - total_time_on_call = 0 - for row in csv.DictReader(io.StringIO(response.content.decode())): - if row["user_pk"] == user.public_primary_key: - end = timezone.datetime.fromisoformat(row["shift_end"]) - start = timezone.datetime.fromisoformat(row["shift_start"]) - shift_time_in_seconds = (end - start).total_seconds() - total_time_on_call += shift_time_in_seconds / (60 * 60) + # 3 shifts per week x 4 weeks x 8 hours per shift = 96 / 2 users = 48h per user for this period + expected_time_on_call = 48 + assert total_time_on_call[user1.public_primary_key] == expected_time_on_call + 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) + + print(response_json["results"]) - # 3 shifts per week x 4 weeks x 8 hours per shift = 96 - assert total_time_on_call == 96 + assert True is False diff --git a/engine/apps/public_api/views/schedules.py b/engine/apps/public_api/views/schedules.py index 0bc1d815dc..9e34cdb1de 100644 --- a/engine/apps/public_api/views/schedules.py +++ b/engine/apps/public_api/views/schedules.py @@ -1,8 +1,6 @@ -import csv import logging from datetime import date -from django.http import HttpResponse from django_filters import rest_framework as filters from rest_framework import status from rest_framework.decorators import action @@ -17,7 +15,7 @@ from apps.public_api.throttlers.user_throttle import UserThrottle from apps.schedules.ical_utils import ical_export_from_schedule from apps.schedules.models import OnCallSchedule, OnCallScheduleWeb -from apps.schedules.models.on_call_schedule import ScheduleEvents +from apps.schedules.models.on_call_schedule import ScheduleEvents, ScheduleFinalShifts from apps.slack.tasks import update_slack_user_group_for_schedules from common.api_helpers.exceptions import BadRequest from common.api_helpers.filters import ByTeamFilter @@ -130,7 +128,7 @@ def export(self, request, pk): return Response(export, status=status.HTTP_200_OK) @action(methods=["get"], detail=True) - def oncall_shifts_export(self, request, pk): + def final_shifts(self, request, pk): schedule = self.get_object() if not isinstance(schedule, OnCallScheduleWeb): @@ -181,31 +179,38 @@ def _convert_date(value): status=status.HTTP_400_BAD_REQUEST, ) - response = HttpResponse(content_type="text/csv") - response["Content-Disposition"] = "attachment; filename='export.csv'" - - user_pk_field_name = "user_pk" - shift_start_field_name = "shift_start" - shift_end_field_name = "shift_end" - writer = csv.DictWriter(response, fieldnames=[user_pk_field_name, shift_start_field_name, shift_end_field_name]) - writer.writeheader() + days_between_start_and_end = (end_date - start_date).days + if days_between_start_and_end > 365: + return Response( + f"The difference between {start_date_field_name} and {end_date_field_name} must be less than one year (365 days)", + status=status.HTTP_400_BAD_REQUEST, + ) - final_schedule_events: ScheduleEvents = schedule.final_events("UTC", start_date, (end_date - start_date).days) + final_schedule_events: ScheduleEvents = schedule.final_events("UTC", start_date, days_between_start_and_end) logger.info( f"Exporting oncall shifts for schedule {pk} between dates {start_date_str} and {end_date_str}. {len(final_schedule_events)} shift events were found." ) - for event in final_schedule_events: - shift_start = event["start"] - shift_end = event["end"] - for user in event["users"]: - writer.writerow( - { - user_pk_field_name: user["pk"], - shift_start_field_name: shift_start, - shift_end_field_name: shift_end, - } - ) - - return response + data: ScheduleFinalShifts = [ + { + "user_pk": user["pk"], + "shift_start": event["start"], + "shift_end": event["end"], + } + for event in final_schedule_events + for user in event["users"] + ] + + # right now we'll "mock out" the pagination related parameters (next and previous) + # rather than use a Pagination class from drf (as currently it operates on querysets). We've decided on this + # to make this response schema consistent with the rest of the public API + make it easy to add pagination + # here in the future (should we decide to migrate "final_shifts" to an actual model) + return Response( + { + "count": len(data), + "next": None, + "previous": None, + "results": data, + } + ) diff --git a/engine/apps/schedules/models/on_call_schedule.py b/engine/apps/schedules/models/on_call_schedule.py index 816784c866..5d76248425 100644 --- a/engine/apps/schedules/models/on_call_schedule.py +++ b/engine/apps/schedules/models/on_call_schedule.py @@ -97,8 +97,15 @@ class ScheduleEvent(TypedDict): shift: ScheduleEventShift +class ScheduleFinalShift(TypedDict): + user_pk: str + shift_start: str + shift_end: str + + ScheduleEvents = List[ScheduleEvent] ScheduleEventIntervals = List[List[datetime.datetime]] +ScheduleFinalShifts = List[ScheduleFinalShift] def generate_public_primary_key_for_oncall_schedule_channel(): From ba09a8c7aa70e5f900e1aac699620cd731065bf7 Mon Sep 17 00:00:00 2001 From: Joey Orlando Date: Fri, 2 Jun 2023 14:55:55 +0200 Subject: [PATCH 13/19] update changelog --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 66d503cfb1..71838bb83f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,7 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added -- Add public API endpoint to export a schedule's oncall-shifts as a `.csv` by @joeyorlando ([2047](https://github.com/grafana/oncall/pull/2047)) +- Add public API endpoint to export a schedule's final shifts by @joeyorlando ([2047](https://github.com/grafana/oncall/pull/2047)) ### Fixed From f9accf5b400d0b9ee636386b72391e23fe263446 Mon Sep 17 00:00:00 2001 From: Joey Orlando Date: Mon, 5 Jun 2023 11:18:43 +0200 Subject: [PATCH 14/19] export user email as well --- .../sources/oncall-api-reference/schedules.md | 37 +++++++++++++++---- .../apps/public_api/tests/test_schedules.py | 28 +++++++++----- engine/apps/public_api/views/schedules.py | 1 + .../apps/schedules/models/on_call_schedule.py | 3 ++ 4 files changed, 52 insertions(+), 17 deletions(-) diff --git a/docs/sources/oncall-api-reference/schedules.md b/docs/sources/oncall-api-reference/schedules.md index 0cac4fe13b..38d0c749b2 100644 --- a/docs/sources/oncall-api-reference/schedules.md +++ b/docs/sources/oncall-api-reference/schedules.md @@ -218,61 +218,73 @@ The above command returns JSON structured in the following way: "results": [ { "user_pk": "UC2CHRT5SD34X", + "user_email": "spongebob@grafana.com", "shift_start": "2023-01-02T09:00:00Z", "shift_end": "2023-01-02T17:00:00Z" }, { "user_pk": "U7S8H84ARFTGN", + "user_email": "squarepants@grafana.com", "shift_start": "2023-01-04T09:00:00Z", "shift_end": "2023-01-04T17:00:00Z" }, { "user_pk": "UC2CHRT5SD34X", + "user_email": "spongebob@grafana.com", "shift_start": "2023-01-06T09:00:00Z", "shift_end": "2023-01-06T17:00:00Z" }, { "user_pk": "U7S8H84ARFTGN", + "user_email": "squarepants@grafana.com", "shift_start": "2023-01-09T09:00:00Z", "shift_end": "2023-01-09T17:00:00Z" }, { "user_pk": "UC2CHRT5SD34X", + "user_email": "spongebob@grafana.com", "shift_start": "2023-01-11T09:00:00Z", "shift_end": "2023-01-11T17:00:00Z" }, { "user_pk": "U7S8H84ARFTGN", + "user_email": "squarepants@grafana.com", "shift_start": "2023-01-13T09:00:00Z", "shift_end": "2023-01-13T17:00:00Z" }, { "user_pk": "UC2CHRT5SD34X", + "user_email": "spongebob@grafana.com", "shift_start": "2023-01-16T09:00:00Z", "shift_end": "2023-01-16T17:00:00Z" }, { "user_pk": "U7S8H84ARFTGN", + "user_email": "squarepants@grafana.com", "shift_start": "2023-01-18T09:00:00Z", "shift_end": "2023-01-18T17:00:00Z" }, { "user_pk": "UC2CHRT5SD34X", + "user_email": "spongebob@grafana.com", "shift_start": "2023-01-20T09:00:00Z", "shift_end": "2023-01-20T17:00:00Z" }, { "user_pk": "U7S8H84ARFTGN", + "user_email": "squarepants@grafana.com", "shift_start": "2023-01-23T09:00:00Z", "shift_end": "2023-01-23T17:00:00Z" }, { "user_pk": "UC2CHRT5SD34X", + "user_email": "spongebob@grafana.com", "shift_start": "2023-01-25T09:00:00Z", "shift_end": "2023-01-25T17:00:00Z" }, { "user_pk": "U7S8H84ARFTGN", + "user_email": "squarepants@grafana.com", "shift_start": "2023-01-27T09:00:00Z", "shift_end": "2023-01-27T17:00:00Z" } @@ -294,11 +306,10 @@ defined via Terraform or iCal. ## Example script to transform data to .csv for all of your schedules The following Python script will generate a `.csv` file, `oncall-report-2023-01-01-to-2023-01-31.csv`. This file will -contain two columns, `user_pk` and `hours_on_call`, which represents how many hours each user was on call during the -period starting January 1, 2023 to January 31, 2023 (inclusive). +contain three columns, `user_pk`, `user_email`, and `hours_on_call`, which represents how many hours each user was +on call during the period starting January 1, 2023 to January 31, 2023 (inclusive). ```python -import collections import csv import requests from datetime import datetime @@ -312,7 +323,7 @@ MY_ONCALL_API_KEY = "meowmeowwoofwoof" headers = {"Authorization": MY_ONCALL_API_KEY} schedule_ids = [schedule["id"] for schedule in requests.get(MY_ONCALL_API_BASE_URL, headers=headers).json()["results"]] -user_on_call_hours = collections.defaultdict(int) +user_on_call_hours = {} for schedule_id in schedule_ids: response = requests.get( @@ -320,15 +331,25 @@ for schedule_id in schedule_ids: headers=headers) for final_shift in response.json()["results"]: + user_pk = final_shift["user_pk"] end = datetime.fromisoformat(final_shift["shift_end"]) start = datetime.fromisoformat(final_shift["shift_start"]) shift_time_in_seconds = (end - start).total_seconds() - user_on_call_hours[final_shift["user_pk"]] += shift_time_in_seconds / (60 * 60) + shift_time_in_hours = shift_time_in_seconds / (60 * 60) + + if user_pk in user_on_call_hours: + user_on_call_hours[user_pk]["hours_on_call"] += shift_time_in_hours + else: + user_on_call_hours[user_pk] = { + "email": final_shift["user_email"], + "hours_on_call": shift_time_in_hours, + } with open(OUTPUT_FILE_NAME, "w") as fp: - csv_writer = csv.DictWriter(fp, ["user_pk", "hours_on_call"]) + csv_writer = csv.DictWriter(fp, ["user_pk", "user_email", "hours_on_call"]) csv_writer.writeheader() - for user_pk, hours_on_call in user_on_call_hours.items(): - csv_writer.writerow({"user_pk": user_pk, "hours_on_call": hours_on_call}) + for user_pk, user_info in user_on_call_hours.items(): + csv_writer.writerow({ + "user_pk": user_pk, "user_email": user_info["email"], "hours_on_call": user_info["hours_on_call"]}) ``` diff --git a/engine/apps/public_api/tests/test_schedules.py b/engine/apps/public_api/tests/test_schedules.py index a9153c3cf1..5fccbdfe06 100644 --- a/engine/apps/public_api/tests/test_schedules.py +++ b/engine/apps/public_api/tests/test_schedules.py @@ -845,8 +845,16 @@ def test_oncall_shifts_export( make_schedule, make_on_call_shift, ): - organization, user1, token = make_organization_and_user_with_token() - user2 = make_user(organization=organization) + organization, _, token = make_organization_and_user_with_token() + + user1_email = "spongebob@grafana.com" + user2_email = "squarepants@grafana.com" + + user1 = make_user(organization=organization, email=user1_email) + user2 = make_user(organization=organization, email=user2_email) + + user1_public_primary_key = user1.public_primary_key + user2_public_primary_key = user2.public_primary_key schedule = make_schedule(organization, schedule_class=OnCallScheduleWeb) start_date = timezone.datetime(2023, 1, 1, 9, 0, 0) @@ -860,7 +868,7 @@ def test_oncall_shifts_export( by_day=["MO", "WE", "FR"], start=start_date, until=start_date + timezone.timedelta(days=28), - rolling_users=[{user1.pk: user1.public_primary_key}, {user2.pk: user2.public_primary_key}], + rolling_users=[{user1.pk: user1_public_primary_key}, {user2.pk: user2_public_primary_key}], rotation_start=start_date, duration=timezone.timedelta(hours=8), ) @@ -873,7 +881,13 @@ def test_oncall_shifts_export( shifts = response_json["results"] total_time_on_call = collections.defaultdict(int) + pk_to_email_mapping = {user1_public_primary_key: user1_email, user2_public_primary_key: user2_email} for row in shifts: + user_pk = row["user_pk"] + + # make sure we're exporting email as well + assert pk_to_email_mapping[user_pk] == row["user_email"] + end = timezone.datetime.fromisoformat(row["shift_end"]) start = timezone.datetime.fromisoformat(row["shift_start"]) shift_time_in_seconds = (end - start).total_seconds() @@ -883,14 +897,10 @@ def test_oncall_shifts_export( # 3 shifts per week x 4 weeks x 8 hours per shift = 96 / 2 users = 48h per user for this period expected_time_on_call = 48 - assert total_time_on_call[user1.public_primary_key] == expected_time_on_call - assert total_time_on_call[user2.public_primary_key] == expected_time_on_call + assert total_time_on_call[user1_public_primary_key] == expected_time_on_call + 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) - - print(response_json["results"]) - - assert True is False diff --git a/engine/apps/public_api/views/schedules.py b/engine/apps/public_api/views/schedules.py index 9e34cdb1de..4947933d8d 100644 --- a/engine/apps/public_api/views/schedules.py +++ b/engine/apps/public_api/views/schedules.py @@ -195,6 +195,7 @@ def _convert_date(value): data: ScheduleFinalShifts = [ { "user_pk": user["pk"], + "user_email": user["email"], "shift_start": event["start"], "shift_end": event["end"], } diff --git a/engine/apps/schedules/models/on_call_schedule.py b/engine/apps/schedules/models/on_call_schedule.py index 5d76248425..f678a2425b 100644 --- a/engine/apps/schedules/models/on_call_schedule.py +++ b/engine/apps/schedules/models/on_call_schedule.py @@ -76,6 +76,7 @@ class QualityReport(TypedDict): class ScheduleEventUser(TypedDict): display_name: str pk: str + email: str class ScheduleEventShift(TypedDict): @@ -99,6 +100,7 @@ class ScheduleEvent(TypedDict): class ScheduleFinalShift(TypedDict): user_pk: str + user_email: str shift_start: str shift_end: str @@ -330,6 +332,7 @@ def filter_events( "users": [ { "display_name": user.username, + "email": user.email, "pk": user.public_primary_key, } for user in shift["users"] From ee70083a4711d85a3fd26d6734f5a692393fb304 Mon Sep 17 00:00:00 2001 From: Joey Orlando Date: Mon, 5 Jun 2023 12:05:22 +0200 Subject: [PATCH 15/19] update failing unit test --- engine/apps/api/tests/test_oncall_shift.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/engine/apps/api/tests/test_oncall_shift.py b/engine/apps/api/tests/test_oncall_shift.py index d92f4c4f29..a265f82956 100644 --- a/engine/apps/api/tests/test_oncall_shift.py +++ b/engine/apps/api/tests/test_oncall_shift.py @@ -1363,7 +1363,9 @@ def test_on_call_shift_preview( "is_gap": False, "priority_level": 2, "missing_users": [], - "users": [{"display_name": other_user.username, "pk": other_user.public_primary_key}], + "users": [ + {"display_name": other_user.username, "pk": other_user.public_primary_key, "email": other_user.email} + ], "source": "web", } ] From 3d97983dcbc24988ada2ca78f28cfa044f51234c Mon Sep 17 00:00:00 2001 From: Joey Orlando Date: Mon, 5 Jun 2023 13:23:39 +0200 Subject: [PATCH 16/19] PR comments --- .../sources/oncall-api-reference/schedules.md | 36 ++++++++---- engine/apps/api/tests/test_oncall_shift.py | 8 ++- .../public_api/serializers/schedules_base.py | 14 +++++ .../apps/public_api/tests/test_schedules.py | 49 ++++++++++++----- engine/apps/public_api/views/schedules.py | 55 +++---------------- .../apps/schedules/models/on_call_schedule.py | 1 + 6 files changed, 87 insertions(+), 76 deletions(-) diff --git a/docs/sources/oncall-api-reference/schedules.md b/docs/sources/oncall-api-reference/schedules.md index 70d0c3d917..9f9410a915 100644 --- a/docs/sources/oncall-api-reference/schedules.md +++ b/docs/sources/oncall-api-reference/schedules.md @@ -216,73 +216,85 @@ The above command returns JSON structured in the following way: "results": [ { "user_pk": "UC2CHRT5SD34X", - "user_email": "spongebob@grafana.com", + "user_email": "alice@example.com", + "user_username": "alice", "shift_start": "2023-01-02T09:00:00Z", "shift_end": "2023-01-02T17:00:00Z" }, { "user_pk": "U7S8H84ARFTGN", - "user_email": "squarepants@grafana.com", + "user_email": "bob@example.com", + "user_username": "bob", "shift_start": "2023-01-04T09:00:00Z", "shift_end": "2023-01-04T17:00:00Z" }, { "user_pk": "UC2CHRT5SD34X", - "user_email": "spongebob@grafana.com", + "user_email": "alice@example.com", + "user_username": "alice", "shift_start": "2023-01-06T09:00:00Z", "shift_end": "2023-01-06T17:00:00Z" }, { "user_pk": "U7S8H84ARFTGN", - "user_email": "squarepants@grafana.com", + "user_email": "bob@example.com", + "user_username": "bob", "shift_start": "2023-01-09T09:00:00Z", "shift_end": "2023-01-09T17:00:00Z" }, { "user_pk": "UC2CHRT5SD34X", - "user_email": "spongebob@grafana.com", + "user_email": "alice@example.com", + "user_username": "alice", "shift_start": "2023-01-11T09:00:00Z", "shift_end": "2023-01-11T17:00:00Z" }, { "user_pk": "U7S8H84ARFTGN", - "user_email": "squarepants@grafana.com", + "user_email": "bob@example.com", + "user_username": "bob", "shift_start": "2023-01-13T09:00:00Z", "shift_end": "2023-01-13T17:00:00Z" }, { "user_pk": "UC2CHRT5SD34X", - "user_email": "spongebob@grafana.com", + "user_email": "alice@example.com", + "user_username": "alice", "shift_start": "2023-01-16T09:00:00Z", "shift_end": "2023-01-16T17:00:00Z" }, { "user_pk": "U7S8H84ARFTGN", - "user_email": "squarepants@grafana.com", + "user_email": "bob@example.com", + "user_username": "bob", "shift_start": "2023-01-18T09:00:00Z", "shift_end": "2023-01-18T17:00:00Z" }, { "user_pk": "UC2CHRT5SD34X", - "user_email": "spongebob@grafana.com", + "user_email": "alice@example.com", + "user_username": "alice", "shift_start": "2023-01-20T09:00:00Z", "shift_end": "2023-01-20T17:00:00Z" }, { "user_pk": "U7S8H84ARFTGN", - "user_email": "squarepants@grafana.com", + "user_email": "bob@example.com", + "user_username": "bob", "shift_start": "2023-01-23T09:00:00Z", "shift_end": "2023-01-23T17:00:00Z" }, { "user_pk": "UC2CHRT5SD34X", - "user_email": "spongebob@grafana.com", + "user_email": "alice@example.com", + "user_username": "alice", "shift_start": "2023-01-25T09:00:00Z", "shift_end": "2023-01-25T17:00:00Z" }, { "user_pk": "U7S8H84ARFTGN", - "user_email": "squarepants@grafana.com", + "user_email": "bob@example.com", + "user_username": "bob", "shift_start": "2023-01-27T09:00:00Z", "shift_end": "2023-01-27T17:00:00Z" } diff --git a/engine/apps/api/tests/test_oncall_shift.py b/engine/apps/api/tests/test_oncall_shift.py index a265f82956..d7fd687def 100644 --- a/engine/apps/api/tests/test_oncall_shift.py +++ b/engine/apps/api/tests/test_oncall_shift.py @@ -1655,7 +1655,9 @@ def test_on_call_shift_preview_update( "is_gap": False, "priority_level": 1, "missing_users": [], - "users": [{"display_name": other_user.username, "pk": other_user.public_primary_key}], + "users": [ + {"display_name": other_user.username, "pk": other_user.public_primary_key, "email": other_user.email} + ], "source": "web", } assert rotation_events[-1] == expected_shift_preview @@ -1766,7 +1768,9 @@ def test_on_call_shift_preview_update_not_started_reuse_pk( "is_gap": False, "priority_level": 1, "missing_users": [], - "users": [{"display_name": other_user.username, "pk": other_user.public_primary_key}], + "users": [ + {"display_name": other_user.username, "pk": other_user.public_primary_key, "email": other_user.email} + ], "source": "web", }, ] diff --git a/engine/apps/public_api/serializers/schedules_base.py b/engine/apps/public_api/serializers/schedules_base.py index d69c620cf3..42742cb42d 100644 --- a/engine/apps/public_api/serializers/schedules_base.py +++ b/engine/apps/public_api/serializers/schedules_base.py @@ -71,3 +71,17 @@ def to_representation(self, instance): } return result + + +class FinalShiftQueryParamsSerializer(serializers.Serializer): + start_date = serializers.DateField(required=True) + end_date = serializers.DateField(required=True) + + def validate(self, attrs): + if attrs["start_date"] > attrs["end_date"]: + raise serializers.ValidationError("start_date must be less than or equal to end_date") + if attrs["end_date"] - attrs["start_date"] > timezone.timedelta(days=365): + raise serializers.ValidationError( + "The difference between start_date and end_date must be less than one year (365 days)" + ) + return attrs diff --git a/engine/apps/public_api/tests/test_schedules.py b/engine/apps/public_api/tests/test_schedules.py index 5fccbdfe06..904cac3e69 100644 --- a/engine/apps/public_api/tests/test_schedules.py +++ b/engine/apps/public_api/tests/test_schedules.py @@ -795,7 +795,7 @@ def test_oncall_shifts_request_validation( web_schedule = make_schedule(organization, schedule_class=OnCallScheduleWeb) schedule_type_validation_msg = "OnCall shifts exports are currently only available for web calendars" - valid_date_msg = "is not a valid date, must be in any valid ISO 8601 format" + valid_date_msg = "Date has wrong format. Use one of these formats instead: YYYY-MM-DD." client = APIClient() @@ -815,27 +815,35 @@ def _make_request(schedule, query_params=""): # query param validation response = _make_request(web_schedule, "?start_date=2021-01-01") assert response.status_code == status.HTTP_400_BAD_REQUEST - assert response.data == "end_date is required" + assert response.json()["end_date"][0] == "This field is required." response = _make_request(web_schedule, "?start_date=asdfasdf") assert response.status_code == status.HTTP_400_BAD_REQUEST - assert response.data == f"start_date {valid_date_msg}" + assert response.json()["start_date"][0] == valid_date_msg response = _make_request(web_schedule, "?end_date=2021-01-01") assert response.status_code == status.HTTP_400_BAD_REQUEST - assert response.data == "start_date is required" + assert response.json()["start_date"][0] == "This field is required." response = _make_request(web_schedule, "?start_date=2021-01-01&end_date=asdfasdf") assert response.status_code == status.HTTP_400_BAD_REQUEST - assert response.data == f"end_date {valid_date_msg}" + assert response.json()["end_date"][0] == valid_date_msg response = _make_request(web_schedule, "?end_date=2021-01-01&start_date=2022-01-01") assert response.status_code == status.HTTP_400_BAD_REQUEST - assert response.data == "start_date must be less than or equal to end_date" + assert response.json() == { + "non_field_errors": [ + "start_date must be less than or equal to end_date", + ] + } response = _make_request(web_schedule, "?end_date=2021-01-01&start_date=2019-12-31") assert response.status_code == status.HTTP_400_BAD_REQUEST - assert response.data == "The difference between start_date and end_date must be less than one year (365 days)" + assert response.json() == { + "non_field_errors": [ + "The difference between start_date and end_date must be less than one year (365 days)", + ] + } @pytest.mark.django_db @@ -847,11 +855,13 @@ def test_oncall_shifts_export( ): organization, _, token = make_organization_and_user_with_token() - user1_email = "spongebob@grafana.com" - user2_email = "squarepants@grafana.com" + user1_email = "alice@example.com" + user2_email = "bob@example.com" + user1_username = "alice" + user2_username = "bob" - user1 = make_user(organization=organization, email=user1_email) - user2 = make_user(organization=organization, email=user2_email) + user1 = make_user(organization=organization, email=user1_email, username=user1_username) + user2 = make_user(organization=organization, email=user2_email, username=user2_username) user1_public_primary_key = user1.public_primary_key user2_public_primary_key = user2.public_primary_key @@ -881,12 +891,23 @@ def test_oncall_shifts_export( shifts = response_json["results"] total_time_on_call = collections.defaultdict(int) - pk_to_email_mapping = {user1_public_primary_key: user1_email, user2_public_primary_key: user2_email} + pk_to_user_mapping = { + user1_public_primary_key: { + "email": user1_email, + "username": user1_username, + }, + user2_public_primary_key: { + "email": user2_email, + "username": user2_username, + }, + } + for row in shifts: user_pk = row["user_pk"] - # make sure we're exporting email as well - assert pk_to_email_mapping[user_pk] == row["user_email"] + # make sure we're exporting email and username as well + assert pk_to_user_mapping[user_pk]["email"] == row["user_email"] + assert pk_to_user_mapping[user_pk]["username"] == row["user_username"] end = timezone.datetime.fromisoformat(row["shift_end"]) start = timezone.datetime.fromisoformat(row["shift_start"]) diff --git a/engine/apps/public_api/views/schedules.py b/engine/apps/public_api/views/schedules.py index 4947933d8d..ab3aec8e58 100644 --- a/engine/apps/public_api/views/schedules.py +++ b/engine/apps/public_api/views/schedules.py @@ -1,5 +1,4 @@ import logging -from datetime import date from django_filters import rest_framework as filters from rest_framework import status @@ -12,6 +11,7 @@ from apps.auth_token.auth import ApiTokenAuthentication, ScheduleExportAuthentication from apps.public_api.custom_renderers import CalendarRenderer from apps.public_api.serializers import PolymorphicScheduleSerializer, PolymorphicScheduleUpdateSerializer +from apps.public_api.serializers.schedules_base import FinalShiftQueryParamsSerializer from apps.public_api.throttlers.user_throttle import UserThrottle from apps.schedules.ical_utils import ical_export_from_schedule from apps.schedules.models import OnCallSchedule, OnCallScheduleWeb @@ -137,65 +137,24 @@ def final_shifts(self, request, pk): status=status.HTTP_400_BAD_REQUEST, ) - start_date_field_name = "start_date" - end_date_field_name = "end_date" - - def _field_is_required(field_name): - return Response(f"{field_name} is required", status=status.HTTP_400_BAD_REQUEST) - - def _field_is_invalid_date(field_name): - return Response( - f"{field_name} is not a valid date, must be in any valid ISO 8601 format", - status=status.HTTP_400_BAD_REQUEST, - ) - - def _convert_date(value): - if not value: - return None - - try: - return date.fromisoformat(value) - except ValueError: - return None - - start_date_str = request.query_params.get(start_date_field_name, None) - start_date = _convert_date(start_date_str) - - end_date_str = request.query_params.get(end_date_field_name, None) - end_date = _convert_date(end_date_str) - - if start_date_str is None: - return _field_is_required(start_date_field_name) - elif start_date is None: - return _field_is_invalid_date(start_date_field_name) - elif end_date_str is None: - return _field_is_required(end_date_field_name) - elif end_date is None: - return _field_is_invalid_date(end_date_field_name) - - if start_date > end_date: - return Response( - f"{start_date_field_name} must be less than or equal to {end_date_field_name}", - status=status.HTTP_400_BAD_REQUEST, - ) + serializer = FinalShiftQueryParamsSerializer(data=request.query_params) + serializer.is_valid(raise_exception=True) + start_date = serializer.validated_data["start_date"] + end_date = serializer.validated_data["end_date"] days_between_start_and_end = (end_date - start_date).days - if days_between_start_and_end > 365: - return Response( - f"The difference between {start_date_field_name} and {end_date_field_name} must be less than one year (365 days)", - status=status.HTTP_400_BAD_REQUEST, - ) final_schedule_events: ScheduleEvents = schedule.final_events("UTC", start_date, days_between_start_and_end) logger.info( - f"Exporting oncall shifts for schedule {pk} between dates {start_date_str} and {end_date_str}. {len(final_schedule_events)} shift events were found." + f"Exporting oncall shifts for schedule {pk} between dates {start_date} and {end_date}. {len(final_schedule_events)} shift events were found." ) data: ScheduleFinalShifts = [ { "user_pk": user["pk"], "user_email": user["email"], + "user_username": user["display_name"], "shift_start": event["start"], "shift_end": event["end"], } diff --git a/engine/apps/schedules/models/on_call_schedule.py b/engine/apps/schedules/models/on_call_schedule.py index f678a2425b..0f1f99de7a 100644 --- a/engine/apps/schedules/models/on_call_schedule.py +++ b/engine/apps/schedules/models/on_call_schedule.py @@ -101,6 +101,7 @@ class ScheduleEvent(TypedDict): class ScheduleFinalShift(TypedDict): user_pk: str user_email: str + user_username: str shift_start: str shift_end: str From c3ec7efca63c5287718e86c8e9b467861e3100b3 Mon Sep 17 00:00:00 2001 From: Joey Orlando Date: Mon, 5 Jun 2023 13:52:43 +0200 Subject: [PATCH 17/19] update failing unit tests --- engine/apps/api/tests/test_schedules.py | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/engine/apps/api/tests/test_schedules.py b/engine/apps/api/tests/test_schedules.py index 7a00c7ef9d..5a5748944c 100644 --- a/engine/apps/api/tests/test_schedules.py +++ b/engine/apps/api/tests/test_schedules.py @@ -812,7 +812,7 @@ def test_events_calendar( "all_day": False, "start": on_call_shift.start, "end": on_call_shift.start + on_call_shift.duration, - "users": [{"display_name": user.username, "pk": user.public_primary_key}], + "users": [{"display_name": user.username, "pk": user.public_primary_key, "email": user.email}], "missing_users": [], "priority_level": on_call_shift.priority_level, "source": "api", @@ -878,7 +878,7 @@ def test_filter_events_calendar( "all_day": False, "start": mon_start, "end": mon_start + on_call_shift.duration, - "users": [{"display_name": user.username, "pk": user.public_primary_key}], + "users": [{"display_name": user.username, "pk": user.public_primary_key, "email": user.email}], "missing_users": [], "priority_level": on_call_shift.priority_level, "source": "api", @@ -894,7 +894,7 @@ def test_filter_events_calendar( "all_day": False, "start": fri_start, "end": fri_start + on_call_shift.duration, - "users": [{"display_name": user.username, "pk": user.public_primary_key}], + "users": [{"display_name": user.username, "pk": user.public_primary_key, "email": user.email}], "missing_users": [], "priority_level": on_call_shift.priority_level, "source": "api", @@ -977,7 +977,7 @@ def test_filter_events_range_calendar( "all_day": False, "start": fri_start, "end": fri_start + on_call_shift.duration, - "users": [{"display_name": user.username, "pk": user.public_primary_key}], + "users": [{"display_name": user.username, "pk": user.public_primary_key, "email": user.email}], "missing_users": [], "priority_level": on_call_shift.priority_level, "source": "api", @@ -1059,7 +1059,13 @@ def test_filter_events_overrides( "all_day": False, "start": override_start, "end": override_start + override.duration, - "users": [{"display_name": other_user.username, "pk": other_user.public_primary_key}], + "users": [ + { + "display_name": other_user.username, + "pk": other_user.public_primary_key, + "email": other_user.email, + } + ], "missing_users": [], "priority_level": None, "source": "api", From d71ee6c981706b868cf94db228da86253499cc46 Mon Sep 17 00:00:00 2001 From: Joey Orlando Date: Mon, 5 Jun 2023 14:12:13 +0200 Subject: [PATCH 18/19] and some more unit tests.. --- .../apps/schedules/tests/test_on_call_schedule.py | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/engine/apps/schedules/tests/test_on_call_schedule.py b/engine/apps/schedules/tests/test_on_call_schedule.py index 3e7e7121d9..a61c7432fb 100644 --- a/engine/apps/schedules/tests/test_on_call_schedule.py +++ b/engine/apps/schedules/tests/test_on_call_schedule.py @@ -93,7 +93,7 @@ def test_filter_events(make_organization, make_user_for_organization, make_sched "is_gap": False, "priority_level": on_call_shift.priority_level, "missing_users": [], - "users": [{"display_name": user.username, "pk": user.public_primary_key}], + "users": [{"display_name": user.username, "pk": user.public_primary_key, "email": user.email}], "shift": {"pk": on_call_shift.public_primary_key}, "source": "api", } @@ -114,7 +114,7 @@ def test_filter_events(make_organization, make_user_for_organization, make_sched "is_gap": False, "priority_level": None, "missing_users": [], - "users": [{"display_name": user.username, "pk": user.public_primary_key}], + "users": [{"display_name": user.username, "pk": user.public_primary_key, "email": user.email}], "shift": {"pk": override.public_primary_key}, "source": "api", } @@ -179,7 +179,7 @@ def test_filter_events_include_gaps(make_organization, make_user_for_organizatio "is_gap": False, "priority_level": on_call_shift.priority_level, "missing_users": [], - "users": [{"display_name": user.username, "pk": user.public_primary_key}], + "users": [{"display_name": user.username, "pk": user.public_primary_key, "email": user.email}], "shift": {"pk": on_call_shift.public_primary_key}, "source": "api", }, @@ -688,7 +688,9 @@ def test_preview_shift(make_organization, make_user_for_organization, make_sched "is_gap": False, "priority_level": new_shift.priority_level, "missing_users": [], - "users": [{"display_name": other_user.username, "pk": other_user.public_primary_key}], + "users": [ + {"display_name": other_user.username, "pk": other_user.public_primary_key, "email": other_user.email} + ], "shift": {"pk": new_shift.public_primary_key}, "source": "api", } @@ -846,7 +848,9 @@ def test_preview_override_shift(make_organization, make_user_for_organization, m "is_gap": False, "priority_level": None, "missing_users": [], - "users": [{"display_name": other_user.username, "pk": other_user.public_primary_key}], + "users": [ + {"display_name": other_user.username, "pk": other_user.public_primary_key, "email": other_user.email} + ], "shift": {"pk": new_shift.public_primary_key}, "source": "api", } From 06583dae76524097a8f77f74f174cc694290510e Mon Sep 17 00:00:00 2001 From: Joey Orlando Date: Mon, 5 Jun 2023 15:55:39 +0200 Subject: [PATCH 19/19] update test --- engine/apps/public_api/tests/test_schedules.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/engine/apps/public_api/tests/test_schedules.py b/engine/apps/public_api/tests/test_schedules.py index 904cac3e69..1e5a2ba525 100644 --- a/engine/apps/public_api/tests/test_schedules.py +++ b/engine/apps/public_api/tests/test_schedules.py @@ -855,8 +855,8 @@ def test_oncall_shifts_export( ): organization, _, token = make_organization_and_user_with_token() - user1_email = "alice@example.com" - user2_email = "bob@example.com" + user1_email = "alice909450945045@example.com" + user2_email = "bob123123123123123@example.com" user1_username = "alice" user2_username = "bob"