Update ical schedule creation/update to trigger final schedule refresh (#3156)

Otherwise iCal schedules only refresh their cached final representation once a day (via periodic task). Related to https://github.com/grafana/oncall/issues/3154
2023-10-18 13:41:25 -03:00 · 2023-10-18 13:41:25 -03:00 · 5c85ced4a9
commit 5c85ced4a9
parent f23ae99998
5 changed files with 99 additions and 40 deletions
--- 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
--- 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
--- 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
--- 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)
--- 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