diff --git a/CHANGELOG.md b/CHANGELOG.md index 03508f5f..7e6e81ee 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 + +### Fixed + +- Update ical schedule creation/update to trigger final schedule refresh ([#3156](https://github.com/grafana/oncall/pull/3156)) + ## v1.3.44 (2023-10-16) ### Added diff --git a/engine/apps/api/serializers/schedule_ical.py b/engine/apps/api/serializers/schedule_ical.py index 3852f662..a668b140 100644 --- a/engine/apps/api/serializers/schedule_ical.py +++ b/engine/apps/api/serializers/schedule_ical.py @@ -1,6 +1,10 @@ from apps.api.serializers.schedule_base import ScheduleBaseSerializer from apps.schedules.models import OnCallScheduleICal -from apps.schedules.tasks import schedule_notify_about_empty_shifts_in_schedule, schedule_notify_about_gaps_in_schedule +from apps.schedules.tasks import ( + refresh_ical_final_schedule, + schedule_notify_about_empty_shifts_in_schedule, + schedule_notify_about_gaps_in_schedule, +) from apps.slack.models import SlackChannel, SlackUserGroup from common.api_helpers.custom_fields import OrganizationFilteredPrimaryKeyRelatedField from common.api_helpers.utils import validate_ical_url @@ -37,6 +41,12 @@ class ScheduleICalCreateSerializer(ScheduleICalSerializer): allow_null=True, ) + def create(self, validated_data): + created_schedule = super().create(validated_data) + # for iCal-based schedules we need to refresh final schedule information + refresh_ical_final_schedule.apply_async((created_schedule.pk,)) + return created_schedule + class Meta: model = OnCallScheduleICal fields = [ @@ -80,4 +90,6 @@ class ScheduleICalUpdateSerializer(ScheduleICalCreateSerializer): updated_schedule.check_gaps_for_next_week() schedule_notify_about_empty_shifts_in_schedule.apply_async((instance.pk,)) schedule_notify_about_gaps_in_schedule.apply_async((instance.pk,)) + # for iCal-based schedules we need to refresh final schedule information + refresh_ical_final_schedule.apply_async((instance.pk,)) return updated_schedule diff --git a/engine/apps/api/tests/test_schedules.py b/engine/apps/api/tests/test_schedules.py index 24983191..756d8285 100644 --- a/engine/apps/api/tests/test_schedules.py +++ b/engine/apps/api/tests/test_schedules.py @@ -601,25 +601,25 @@ def test_create_ical_schedule(schedule_internal_api_setup, make_user_auth_header user, token, _, _, _, _ = schedule_internal_api_setup client = APIClient() url = reverse("api-internal:schedule-list") + data = { + "ical_url_primary": ICAL_URL, + "ical_url_overrides": None, + "name": "created_ical_schedule", + "type": 1, + "slack_channel_id": None, + "user_group": None, + "team": 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, + } with patch( "apps.api.serializers.schedule_ical.ScheduleICalSerializer.validate_ical_url_primary", return_value=ICAL_URL - ): - data = { - "ical_url_primary": ICAL_URL, - "ical_url_overrides": None, - "name": "created_ical_schedule", - "type": 1, - "slack_channel_id": None, - "user_group": None, - "team": 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, - } + ), patch("apps.schedules.tasks.refresh_ical_final_schedule.apply_async") as mock_refresh_final: response = client.post(url, data, format="json", **make_user_auth_headers(user, token)) # modify initial data by adding id and None for optional fields schedule = OnCallSchedule.objects.get(public_primary_key=response.data["id"]) @@ -628,6 +628,8 @@ def test_create_ical_schedule(schedule_internal_api_setup, make_user_auth_header data["enable_web_overrides"] = False assert response.status_code == status.HTTP_201_CREATED assert response.data == data + # check final schedule refresh triggered + mock_refresh_final.assert_called_once_with((schedule.pk,)) @pytest.mark.django_db @@ -736,12 +738,40 @@ def test_update_ical_schedule(schedule_internal_api_setup, make_user_auth_header "type": 1, "team": None, } - response = client.put( - url, data=json.dumps(data), content_type="application/json", **make_user_auth_headers(user, token) - ) + with patch("apps.schedules.tasks.refresh_ical_final_schedule.apply_async") as mock_refresh_final: + response = client.put( + url, data=json.dumps(data), content_type="application/json", **make_user_auth_headers(user, token) + ) updated_instance = OnCallSchedule.objects.get(public_primary_key=ical_schedule.public_primary_key) assert response.status_code == status.HTTP_200_OK assert updated_instance.name == "updated_ical_schedule" + # check refresh final is not triggered (url unchanged) + assert not mock_refresh_final.called + + +@pytest.mark.django_db +def test_update_ical_schedule_url(schedule_internal_api_setup, make_user_auth_headers): + user, token, _, ical_schedule, _, _ = schedule_internal_api_setup + client = APIClient() + + url = reverse("api-internal:schedule-detail", kwargs={"pk": ical_schedule.public_primary_key}) + + updated_url = "another-url" + data = { + "name": ical_schedule.name, + "type": 1, + "ical_url_primary": updated_url, + } + with patch( + "apps.api.serializers.schedule_ical.ScheduleICalSerializer.validate_ical_url_primary", return_value=updated_url + ), patch("apps.schedules.tasks.refresh_ical_final_schedule.apply_async") as mock_refresh_final: + response = client.put( + url, data=json.dumps(data), content_type="application/json", **make_user_auth_headers(user, token) + ) + updated_instance = OnCallSchedule.objects.get(public_primary_key=ical_schedule.public_primary_key) + assert response.status_code == status.HTTP_200_OK + # check refresh final triggered (changing url) + mock_refresh_final.assert_called_once_with((updated_instance.pk,)) @pytest.mark.django_db diff --git a/engine/apps/public_api/serializers/schedules_ical.py b/engine/apps/public_api/serializers/schedules_ical.py index 458265c4..51a66dca 100644 --- a/engine/apps/public_api/serializers/schedules_ical.py +++ b/engine/apps/public_api/serializers/schedules_ical.py @@ -2,6 +2,7 @@ from apps.public_api.serializers.schedules_base import ScheduleBaseSerializer from apps.schedules.models import OnCallScheduleICal from apps.schedules.tasks import ( drop_cached_ical_task, + refresh_ical_final_schedule, schedule_notify_about_empty_shifts_in_schedule, schedule_notify_about_gaps_in_schedule, ) @@ -32,6 +33,12 @@ class ScheduleICalSerializer(ScheduleBaseSerializer): def validate_ical_url_overrides(self, url): return validate_ical_url(url) + def create(self, validated_data): + created_schedule = super().create(validated_data) + # for iCal-based schedules we need to refresh final schedule information + refresh_ical_final_schedule.apply_async((created_schedule.pk,)) + return created_schedule + class ScheduleICalUpdateSerializer(ScheduleICalSerializer): team_id = TeamPrimaryKeyRelatedField(required=False, allow_null=True, source="team") @@ -70,4 +77,5 @@ class ScheduleICalUpdateSerializer(ScheduleICalSerializer): ) schedule_notify_about_empty_shifts_in_schedule.apply_async((instance.pk,)) schedule_notify_about_gaps_in_schedule.apply_async((instance.pk,)) + refresh_ical_final_schedule.apply_async((instance.pk,)) return super().update(instance, validated_data) diff --git a/engine/apps/public_api/tests/test_schedules.py b/engine/apps/public_api/tests/test_schedules.py index 2e1ed13e..45a78dd1 100644 --- a/engine/apps/public_api/tests/test_schedules.py +++ b/engine/apps/public_api/tests/test_schedules.py @@ -393,24 +393,24 @@ def test_update_ical_url_overrides_calendar_schedule( with patch("common.api_helpers.utils.validate_ical_url", return_value=ICAL_URL): response = client.put(url, data=data, format="json", HTTP_AUTHORIZATION=f"{token}") - result = { - "id": schedule.public_primary_key, - "team_id": None, - "name": schedule.name, - "type": "calendar", - "time_zone": schedule.time_zone, - "on_call_now": [], - "shifts": [], - "slack": { - "channel_id": "SLACKCHANNELID", - "user_group_id": None, - }, - "ical_url_overrides": ICAL_URL, - "enable_web_overrides": False, - } + result = { + "id": schedule.public_primary_key, + "team_id": None, + "name": schedule.name, + "type": "calendar", + "time_zone": schedule.time_zone, + "on_call_now": [], + "shifts": [], + "slack": { + "channel_id": "SLACKCHANNELID", + "user_group_id": None, + }, + "ical_url_overrides": ICAL_URL, + "enable_web_overrides": False, + } - assert response.status_code == status.HTTP_200_OK - assert response.json() == result + assert response.status_code == status.HTTP_200_OK + assert response.json() == result @pytest.mark.django_db @@ -633,7 +633,7 @@ def test_create_ical_schedule(make_organization_and_user_with_token): with patch( "apps.public_api.serializers.schedules_ical.ScheduleICalSerializer.validate_ical_url_primary", return_value=ICAL_URL, - ): + ), patch("apps.schedules.tasks.refresh_ical_final_schedule.apply_async") as mock_refresh_final: response = client.post(url, data=data, format="json", HTTP_AUTHORIZATION=f"{token}") schedule = OnCallSchedule.objects.get(public_primary_key=response.data["id"]) @@ -653,6 +653,7 @@ def test_create_ical_schedule(make_organization_and_user_with_token): assert response.status_code == status.HTTP_201_CREATED assert response.json() == result + mock_refresh_final.assert_called_once_with((schedule.pk,)) @pytest.mark.django_db @@ -680,7 +681,8 @@ def test_update_ical_schedule( assert schedule.name != data["name"] - response = client.put(url, data=data, format="json", HTTP_AUTHORIZATION=f"{token}") + with patch("apps.schedules.tasks.refresh_ical_final_schedule.apply_async") as mock_refresh_final: + response = client.put(url, data=data, format="json", HTTP_AUTHORIZATION=f"{token}") result = { "id": schedule.public_primary_key, @@ -700,6 +702,7 @@ def test_update_ical_schedule( schedule.refresh_from_db() assert schedule.name == data["name"] assert response.json() == result + assert not mock_refresh_final.called @pytest.mark.django_db