From 52871b08e691aaabf9f7a7ba31948b822a321c78 Mon Sep 17 00:00:00 2001 From: Matias Bordese Date: Wed, 31 Jan 2024 11:06:54 -0300 Subject: [PATCH] Fix interval validation when creating shift via public API (#3775) Related to https://github.com/grafana/support-escalations/issues/9142. --- CHANGELOG.md | 1 + .../public_api/serializers/on_call_shifts.py | 8 ++-- .../public_api/tests/test_on_call_shifts.py | 47 +++++++++++++++++++ 3 files changed, 52 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3c849c88..63a349db 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed +- Fix interval validation when creating shift via public API ([#3775](https://github.com/grafana/oncall/pull/3775)) - Fix list user serializer logic refactoring ([3793](https://github.com/grafana/oncall/pull/3793)) ## v1.3.94 (2024-01-30) diff --git a/engine/apps/public_api/serializers/on_call_shifts.py b/engine/apps/public_api/serializers/on_call_shifts.py index 796755b1..d1b84025 100644 --- a/engine/apps/public_api/serializers/on_call_shifts.py +++ b/engine/apps/public_api/serializers/on_call_shifts.py @@ -123,13 +123,13 @@ class CustomOnCallShiftSerializer(EagerLoadingMixin, serializers.ModelSerializer PREFETCH_RELATED = ["schedules", "users"] def create(self, validated_data): + validated_data = self._correct_validated_data(validated_data["type"], validated_data) self._validate_frequency_and_week_start( validated_data["type"], validated_data.get("frequency"), - validated_data.get("interval", 1), # if field is missing, the default value will be used + validated_data.get("interval"), validated_data.get("week_start"), ) - validated_data = self._correct_validated_data(validated_data["type"], validated_data) self._validate_start_rotation_from_user_index( validated_data["type"], validated_data.get("start_rotation_from_user_index"), @@ -346,7 +346,7 @@ class CustomOnCallShiftSerializer(EagerLoadingMixin, serializers.ModelSerializer validated_data[field] = None if validated_data.get("start") is not None: validated_data["start"] = validated_data["start"].replace(tzinfo=None) - if validated_data.get("frequency") and validated_data.get("interval") is None: + if validated_data.get("frequency") is not None and "interval" not in validated_data: # if there is frequency but no interval is given, default to 1 validated_data["interval"] = 1 @@ -374,6 +374,7 @@ class CustomOnCallShiftUpdateSerializer(CustomOnCallShiftSerializer): def update(self, instance, validated_data): event_type = validated_data.get("type", instance.type) + validated_data = self._correct_validated_data(event_type, validated_data) frequency = validated_data.get("frequency", instance.frequency) start_rotation_from_user_index = validated_data.get( "start_rotation_from_user_index", instance.start_rotation_from_user_index @@ -389,7 +390,6 @@ class CustomOnCallShiftUpdateSerializer(CustomOnCallShiftSerializer): if start_rotation_from_user_index != instance.start_rotation_from_user_index: self._validate_start_rotation_from_user_index(event_type, start_rotation_from_user_index) - validated_data = self._correct_validated_data(event_type, validated_data) result = super().update(instance, validated_data) for schedule in instance.schedules.all(): instance.start_drop_ical_and_check_schedule_tasks(schedule) 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 c3312493..81d7c91b 100644 --- a/engine/apps/public_api/tests/test_on_call_shifts.py +++ b/engine/apps/public_api/tests/test_on_call_shifts.py @@ -346,6 +346,53 @@ def test_create_on_call_shift_using_none_interval_fails(make_organization_and_us assert response.json() == {"detail": "Field 'interval' must be a positive integer"} +@pytest.mark.django_db +def test_create_on_call_shift_no_interval_fallback(make_organization_and_user_with_token): + _, user, token = make_organization_and_user_with_token() + client = APIClient() + + url = reverse("api-public:on_call_shifts-list") + + start = timezone.now() + until = start + timezone.timedelta(days=30) + data = { + "team_id": None, + "name": "test name", + "type": "recurrent_event", + "level": 1, + "start": start.strftime("%Y-%m-%dT%H:%M:%S"), + "rotation_start": start.strftime("%Y-%m-%dT%H:%M:%S"), + "duration": 10800, + "users": [user.public_primary_key], + "frequency": "daily", + "until": until.strftime("%Y-%m-%dT%H:%M:%S"), + } + response = client.post(url, data=data, format="json", HTTP_AUTHORIZATION=f"{token}") + + on_call_shift = CustomOnCallShift.objects.get(public_primary_key=response.data["id"]) + expected = { + "id": on_call_shift.public_primary_key, + "team_id": None, + "schedule": None, + "name": data["name"], + "type": "recurrent_event", + "time_zone": None, + "level": data["level"], + "start": data["start"], + "rotation_start": data["rotation_start"], + "duration": data["duration"], + "frequency": data["frequency"], + "interval": 1, + "until": data["until"], + "by_day": None, + "users": [user.public_primary_key], + "by_month": None, + "by_monthday": None, + } + assert response.status_code == status.HTTP_201_CREATED + assert response.data == expected + + @pytest.mark.django_db def test_create_override_on_call_shift(make_organization_and_user_with_token): _, user, token = make_organization_and_user_with_token()