diff --git a/CHANGELOG.md b/CHANGELOG.md index 1f9c1cc8..e20b8e5f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added - Use shift data from event object +- Update shifts public API to improve web shifts support ([#3165](https://github.com/grafana/oncall/pull/3165)) ### Fixed diff --git a/engine/apps/public_api/serializers/on_call_shifts.py b/engine/apps/public_api/serializers/on_call_shifts.py index ffdf2cb1..7b9dfb83 100644 --- a/engine/apps/public_api/serializers/on_call_shifts.py +++ b/engine/apps/public_api/serializers/on_call_shifts.py @@ -5,6 +5,7 @@ from rest_framework import fields, serializers from apps.schedules.models import CustomOnCallShift from apps.user_management.models import User from common.api_helpers.custom_fields import ( + OrganizationFilteredPrimaryKeyRelatedField, RollingUsersField, TeamPrimaryKeyRelatedField, TimeZoneField, @@ -70,6 +71,7 @@ class CustomOnCallShiftSerializer(EagerLoadingMixin, serializers.ModelSerializer id = serializers.CharField(read_only=True, source="public_primary_key") organization = serializers.HiddenField(default=CurrentOrganizationDefault()) team_id = TeamPrimaryKeyRelatedField(required=False, allow_null=True, source="team") + schedule = OrganizationFilteredPrimaryKeyRelatedField(read_only=True) type = CustomOnCallShiftTypeField() time_zone = TimeZoneField(required=False, allow_null=True) users = UsersFilteredByOrganizationField(queryset=User.objects, required=False) @@ -92,6 +94,7 @@ class CustomOnCallShiftSerializer(EagerLoadingMixin, serializers.ModelSerializer "id", "organization", "team_id", + "schedule", "name", "type", "time_zone", @@ -116,7 +119,8 @@ class CustomOnCallShiftSerializer(EagerLoadingMixin, serializers.ModelSerializer "source": {"required": False, "write_only": True}, } - PREFETCH_RELATED = ["users"] + SELECT_RELATED = ["schedule"] + PREFETCH_RELATED = ["schedules", "users"] def create(self, validated_data): self._validate_frequency_and_week_start( @@ -244,6 +248,9 @@ class CustomOnCallShiftSerializer(EagerLoadingMixin, serializers.ModelSerializer def to_representation(self, instance): result = super().to_representation(instance) + if result["schedule"] is None: + related_schedules = instance.schedules.all() + result["schedule"] = related_schedules[0].public_primary_key if related_schedules else None result["duration"] = int(instance.duration.total_seconds()) result["start"] = instance.start.strftime("%Y-%m-%dT%H:%M:%S") result["rotation_start"] = instance.rotation_start.strftime("%Y-%m-%dT%H:%M:%S") @@ -377,4 +384,7 @@ class CustomOnCallShiftUpdateSerializer(CustomOnCallShiftSerializer): result = super().update(instance, validated_data) for schedule in instance.schedules.all(): instance.start_drop_ical_and_check_schedule_tasks(schedule) + if instance.schedule: + # web-schedule shifts use FK instead + instance.start_drop_ical_and_check_schedule_tasks(instance.schedule) return result diff --git a/engine/apps/public_api/tests/test_on_call_shifts.py b/engine/apps/public_api/tests/test_on_call_shifts.py index 1a66fa09..ce531e06 100644 --- a/engine/apps/public_api/tests/test_on_call_shifts.py +++ b/engine/apps/public_api/tests/test_on_call_shifts.py @@ -1,3 +1,5 @@ +from unittest.mock import patch + import pytest from django.urls import reverse from django.utils import timezone @@ -48,6 +50,43 @@ invalid_field_data_10 = { } +@pytest.mark.django_db +def test_filter_on_call_shift_schedule(make_organization_and_user_with_token, make_on_call_shift, make_schedule): + organization, user, token = make_organization_and_user_with_token() + client = APIClient() + + schedule_1 = make_schedule(organization, schedule_class=OnCallScheduleWeb) + schedule_2 = make_schedule(organization, schedule_class=OnCallScheduleWeb) + start_date = timezone.now().replace(microsecond=0) + shifts = [] + for schedule in (schedule_1, schedule_2): + data = { + "start": start_date, + "rotation_start": start_date, + "duration": timezone.timedelta(seconds=7200), + "schedule": schedule, + } + on_call_shift = make_on_call_shift( + organization=organization, shift_type=CustomOnCallShift.TYPE_SINGLE_EVENT, **data + ) + on_call_shift.users.add(user) + shifts.append(on_call_shift) + + url = reverse("api-public:on_call_shifts-list") + response = client.get(url, format="json", HTTP_AUTHORIZATION=f"{token}") + assert response.status_code == status.HTTP_200_OK + expected = sorted([s.public_primary_key for s in shifts]) + returned = sorted([s["id"] for s in response.json()["results"]]) + assert returned == expected + + url += f"?schedule_id={schedule_1.public_primary_key}" + response = client.get(url, format="json", HTTP_AUTHORIZATION=f"{token}") + assert response.status_code == status.HTTP_200_OK + expected = [shifts[0].public_primary_key] + returned = [s["id"] for s in response.json()["results"]] + assert returned == expected + + @pytest.mark.django_db def test_get_on_call_shift(make_organization_and_user_with_token, make_on_call_shift, make_schedule): organization, user, token = make_organization_and_user_with_token() @@ -73,6 +112,7 @@ def test_get_on_call_shift(make_organization_and_user_with_token, make_on_call_s result = { "id": on_call_shift.public_primary_key, "team_id": None, + "schedule": schedule.public_primary_key, "name": on_call_shift.name, "type": "single_event", "time_zone": None, @@ -111,6 +151,7 @@ def test_get_override_on_call_shift(make_organization_and_user_with_token, make_ result = { "id": on_call_shift.public_primary_key, "team_id": None, + "schedule": schedule.public_primary_key, "name": on_call_shift.name, "type": "override", "time_zone": None, @@ -155,6 +196,7 @@ def test_create_on_call_shift(make_organization_and_user_with_token): result = { "id": on_call_shift.public_primary_key, "team_id": None, + "schedule": None, "name": data["name"], "type": "recurrent_event", "time_zone": None, @@ -206,6 +248,7 @@ def test_create_on_call_shift_using_default_interval(make_organization_and_user_ expected = { "id": on_call_shift.public_primary_key, "team_id": None, + "schedule": None, "name": data["name"], "type": "recurrent_event", "time_zone": None, @@ -282,6 +325,7 @@ def test_create_override_on_call_shift(make_organization_and_user_with_token): result = { "id": on_call_shift.public_primary_key, "team_id": None, + "schedule": None, "name": data["name"], "type": "override", "time_zone": None, @@ -360,11 +404,13 @@ def test_update_on_call_shift(make_organization_and_user_with_token, make_on_cal assert on_call_shift.by_day != data_to_update["by_day"] assert len(on_call_shift.users.filter(public_primary_key=user.public_primary_key)) == 0 - response = client.put(url, data=data_to_update, format="json", HTTP_AUTHORIZATION=f"{token}") + with patch("apps.schedules.models.CustomOnCallShift.start_drop_ical_and_check_schedule_tasks") as mock_drop_ical: + response = client.put(url, data=data_to_update, format="json", HTTP_AUTHORIZATION=f"{token}") result = { "id": on_call_shift.public_primary_key, "team_id": None, + "schedule": schedule.public_primary_key, "name": on_call_shift.name, "type": "recurrent_event", "time_zone": None, @@ -389,6 +435,69 @@ def test_update_on_call_shift(make_organization_and_user_with_token, make_on_cal assert on_call_shift.by_day == data_to_update["by_day"] assert len(on_call_shift.users.filter(public_primary_key=user.public_primary_key)) == 1 assert response.data == result + mock_drop_ical.assert_called_once_with(schedule) + + +@pytest.mark.django_db +def test_update_on_call_shift_web_schedule(make_organization_and_user_with_token, make_on_call_shift, make_schedule): + organization, user, token = make_organization_and_user_with_token() + client = APIClient() + + start_date = timezone.now().replace(microsecond=0) + schedule = make_schedule(organization, schedule_class=OnCallScheduleWeb) + data = { + "start": start_date, + "rotation_start": start_date, + "duration": timezone.timedelta(seconds=7200), + "frequency": CustomOnCallShift.FREQUENCY_WEEKLY, + "interval": 2, + "by_day": ["MO", "FR"], + "schedule": schedule, + } + on_call_shift = make_on_call_shift( + organization=organization, shift_type=CustomOnCallShift.TYPE_ROLLING_USERS_EVENT, **data + ) + on_call_shift.add_rolling_users([[user]]) + + url = reverse("api-public:on_call_shifts-detail", kwargs={"pk": on_call_shift.public_primary_key}) + data_to_update = { + "duration": 14400, + "by_day": ["MO", "WE", "FR"], + } + assert int(on_call_shift.duration.total_seconds()) != data_to_update["duration"] + assert on_call_shift.by_day != data_to_update["by_day"] + + with patch("apps.schedules.models.CustomOnCallShift.start_drop_ical_and_check_schedule_tasks") as mock_drop_ical: + response = client.put(url, data=data_to_update, format="json", HTTP_AUTHORIZATION=f"{token}") + + result = { + "id": on_call_shift.public_primary_key, + "team_id": None, + "schedule": schedule.public_primary_key, + "name": on_call_shift.name, + "type": "rolling_users", + "time_zone": None, + "level": 0, + "start": on_call_shift.start.strftime("%Y-%m-%dT%H:%M:%S"), + "rotation_start": on_call_shift.rotation_start.strftime("%Y-%m-%dT%H:%M:%S"), + "duration": data_to_update["duration"], + "frequency": "weekly", + "interval": on_call_shift.interval, + "until": None, + "week_start": "SU", + "by_day": data_to_update["by_day"], + "rolling_users": [[user.public_primary_key]], + "start_rotation_from_user_index": None, + "by_month": None, + "by_monthday": None, + } + + assert response.status_code == status.HTTP_200_OK + on_call_shift.refresh_from_db() + assert int(on_call_shift.duration.total_seconds()) == data_to_update["duration"] + assert on_call_shift.by_day == data_to_update["by_day"] + assert response.data == result + mock_drop_ical.assert_called_once_with(schedule) @pytest.mark.django_db @@ -482,6 +591,7 @@ def test_create_web_override(make_organization_and_user_with_token, make_on_call expected_response = { "id": shift.public_primary_key, "team_id": None, + "schedule": None, "name": "test web override", "type": "override", "start": start_str, diff --git a/engine/apps/public_api/views/on_call_shifts.py b/engine/apps/public_api/views/on_call_shifts.py index 7b904b6a..af944b00 100644 --- a/engine/apps/public_api/views/on_call_shifts.py +++ b/engine/apps/public_api/views/on_call_shifts.py @@ -1,3 +1,4 @@ +from django.db.models import Q from django_filters.rest_framework import DjangoFilterBackend from rest_framework.exceptions import NotFound from rest_framework.permissions import IsAuthenticated @@ -35,7 +36,9 @@ class CustomOnCallShiftView(RateLimitHeadersMixin, UpdateSerializerMixin, ModelV queryset = CustomOnCallShift.objects.filter(organization=self.request.auth.organization) if schedule_id: - queryset = queryset.filter(schedules__public_primary_key=schedule_id) + queryset = queryset.filter( + Q(schedules__public_primary_key=schedule_id) | Q(schedule__public_primary_key=schedule_id) + ) if name: queryset = queryset.filter(name=name) return queryset.order_by("schedules")