chore: update schedule checks notification period and improve wording (…

…#5412)

Related to grafana/oncall-private#2994

- Extend gaps/empty shift checks to consider 30 days (customizable via
param, eventually make it customizable per schedule?); ie. every week
(per beat schedule), check the schedule next 30 days
- Trigger checks via async task on schedule API updates (instead of a
sync call)
- Update notifications wording / link to schedule

engine/apps/api/serializers/schedule_base.py

@@ -2,8 +2,13 @@
 
 from apps.api.serializers.slack_channel import SlackChannelSerializer
 from apps.api.serializers.user_group import UserGroupSerializer
+from apps.schedules.constants import SCHEDULE_CHECK_NEXT_DAYS
 from apps.schedules.models import OnCallSchedule
-from apps.schedules.tasks import schedule_notify_about_empty_shifts_in_schedule, schedule_notify_about_gaps_in_schedule
+from apps.schedules.tasks import (
+    check_gaps_and_empty_shifts_in_schedule,
+    schedule_notify_about_empty_shifts_in_schedule,
+    schedule_notify_about_gaps_in_schedule,
+)
 from common.api_helpers.custom_fields import TeamPrimaryKeyRelatedField
 from common.api_helpers.mixins import EagerLoadingMixin
 from common.api_helpers.utils import CurrentOrganizationDefault
@@ -44,8 +49,8 @@ class Meta:
         "Cannot update the user group, make sure to grant user group modification rights to "
         "non-admin users in Slack workspace settings"
     )
-    SCHEDULE_HAS_GAPS_WARNING = "Schedule has unassigned time periods during next 7 days"
-    SCHEDULE_HAS_EMPTY_SHIFTS_WARNING = "Schedule has empty shifts during next 7 days"
+    SCHEDULE_HAS_GAPS_WARNING = f"Schedule has unassigned time periods during next {SCHEDULE_CHECK_NEXT_DAYS} days"
+    SCHEDULE_HAS_EMPTY_SHIFTS_WARNING = f"Schedule has empty shifts during next {SCHEDULE_CHECK_NEXT_DAYS} days"
 
     def get_warnings(self, obj):
         can_update_user_groups = self.context.get("can_update_user_groups", False)
@@ -81,7 +86,7 @@ def validate(self, attrs):
 
     def create(self, validated_data):
         created_schedule = super().create(validated_data)
-        created_schedule.check_gaps_and_empty_shifts_for_next_week()
+        check_gaps_and_empty_shifts_in_schedule.apply_async((created_schedule.pk,))
         schedule_notify_about_empty_shifts_in_schedule.apply_async((created_schedule.pk,))
         schedule_notify_about_gaps_in_schedule.apply_async((created_schedule.pk,))
         return created_schedule

engine/apps/api/serializers/schedule_calendar.py

@@ -2,7 +2,11 @@
 
 from apps.api.serializers.schedule_base import ScheduleBaseSerializer
 from apps.schedules.models import OnCallScheduleCalendar
-from apps.schedules.tasks import schedule_notify_about_empty_shifts_in_schedule, schedule_notify_about_gaps_in_schedule
+from apps.schedules.tasks import (
+    check_gaps_and_empty_shifts_in_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, TimeZoneField
 from common.api_helpers.utils import validate_ical_url
@@ -58,7 +62,7 @@ def update(self, instance, validated_data):
             or old_enable_web_overrides != updated_enable_web_overrides
         ):
             updated_schedule.drop_cached_ical()
-            updated_schedule.check_gaps_and_empty_shifts_for_next_week()
+            check_gaps_and_empty_shifts_in_schedule.apply_async((instance.pk,))
             schedule_notify_about_empty_shifts_in_schedule.apply_async((instance.pk,))
             schedule_notify_about_gaps_in_schedule.apply_async((instance.pk,))
         return updated_schedule

engine/apps/api/serializers/schedule_ical.py

@@ -1,6 +1,7 @@
 from apps.api.serializers.schedule_base import ScheduleBaseSerializer
 from apps.schedules.models import OnCallScheduleICal
 from apps.schedules.tasks import (
+    check_gaps_and_empty_shifts_in_schedule,
     refresh_ical_final_schedule,
     schedule_notify_about_empty_shifts_in_schedule,
     schedule_notify_about_gaps_in_schedule,
@@ -87,7 +88,7 @@ def update(self, instance, validated_data):
 
         if old_ical_url_primary != updated_ical_url_primary or old_ical_url_overrides != updated_ical_url_overrides:
             updated_schedule.drop_cached_ical()
-            updated_schedule.check_gaps_and_empty_shifts_for_next_week()
+            check_gaps_and_empty_shifts_in_schedule.apply_async((instance.pk,))
             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

engine/apps/api/serializers/schedule_web.py

@@ -1,6 +1,10 @@
 from apps.api.serializers.schedule_base import ScheduleBaseSerializer
 from apps.schedules.models import OnCallScheduleWeb
-from apps.schedules.tasks import schedule_notify_about_empty_shifts_in_schedule, schedule_notify_about_gaps_in_schedule
+from apps.schedules.tasks import (
+    check_gaps_and_empty_shifts_in_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, TimeZoneField
 
@@ -41,7 +45,7 @@ def update(self, instance, validated_data):
         updated_time_zone = updated_schedule.time_zone
         if old_time_zone != updated_time_zone:
             updated_schedule.drop_cached_ical()
-            updated_schedule.check_gaps_and_empty_shifts_for_next_week()
+            check_gaps_and_empty_shifts_in_schedule.apply_async((instance.pk,))
             schedule_notify_about_empty_shifts_in_schedule.apply_async((instance.pk,))
             schedule_notify_about_gaps_in_schedule.apply_async((instance.pk,))
         return updated_schedule

engine/apps/api/views/schedule.py

@@ -481,7 +481,7 @@ def type_options(self, request):
     def reload_ical(self, request, pk):
         schedule = self.get_object(annotate=False)
         schedule.drop_cached_ical()
-        schedule.check_gaps_and_empty_shifts_for_next_week()
+        schedule.check_gaps_and_empty_shifts_for_next_days()
 
         if schedule.user_group is not None:
             update_slack_user_group_for_schedules.apply_async((schedule.user_group.pk,))

engine/apps/schedules/constants.py

@@ -29,5 +29,6 @@
 
 SCHEDULE_ONCALL_CACHE_KEY_PREFIX = "schedule_oncall_users_"
 SCHEDULE_ONCALL_CACHE_TTL = 15 * 60  # 15 minutes in seconds
+SCHEDULE_CHECK_NEXT_DAYS = 30
 
 PREFETCHED_SHIFT_SWAPS = "prefetched_shift_swaps"

engine/apps/schedules/models/on_call_schedule.py

@@ -33,6 +33,7 @@
     ICAL_SUMMARY,
     ICAL_UID,
     PREFETCHED_SHIFT_SWAPS,
+    SCHEDULE_CHECK_NEXT_DAYS,
 )
 from apps.schedules.ical_utils import (
     EmptyShifts,
@@ -293,9 +294,9 @@ def get_prev_and_current_ical_files(self):
             (self.prev_ical_file_overrides, self.cached_ical_file_overrides),
         ]
 
-    def check_gaps_and_empty_shifts_for_next_week(self) -> None:
+    def check_gaps_and_empty_shifts_for_next_days(self, days=SCHEDULE_CHECK_NEXT_DAYS) -> None:
         datetime_start = timezone.now()
-        datetime_end = datetime_start + datetime.timedelta(days=7)
+        datetime_end = datetime_start + datetime.timedelta(days=days)
 
         # get empty shifts from all events and gaps from final events
         events = self.filter_events(
@@ -313,14 +314,14 @@ def check_gaps_and_empty_shifts_for_next_week(self) -> None:
             self.has_empty_shifts = has_empty_shifts
             self.save(update_fields=["has_gaps", "has_empty_shifts"])
 
-    def get_gaps_for_next_week(self) -> ScheduleEvents:
+    def get_gaps_for_next_days(self, days=SCHEDULE_CHECK_NEXT_DAYS) -> ScheduleEvents:
         today = timezone.now()
-        events = self.final_events(today, today + datetime.timedelta(days=7))
+        events = self.final_events(today, today + datetime.timedelta(days=days))
         return [event for event in events if event["is_gap"]]
 
-    def get_empty_shifts_for_next_week(self) -> EmptyShifts:
+    def get_empty_shifts_for_next_days(self, days=SCHEDULE_CHECK_NEXT_DAYS) -> EmptyShifts:
         today = timezone.now().date()
-        return list_of_empty_shifts_in_schedule(self, today, today + datetime.timedelta(days=7))
+        return list_of_empty_shifts_in_schedule(self, today, today + datetime.timedelta(days=days))
 
     def drop_cached_ical(self):
         self._drop_primary_ical_file()

engine/apps/schedules/tasks/check_gaps_and_empty_shifts.py

@@ -19,5 +19,5 @@ def check_gaps_and_empty_shifts_in_schedule(schedule_pk):
         task_logger.info(f"Tried to check_gaps_and_empty_shifts_in_schedule for non-existing schedule {schedule_pk}")
         return
 
-    schedule.check_gaps_and_empty_shifts_for_next_week()
+    schedule.check_gaps_and_empty_shifts_for_next_days()
     task_logger.info(f"Finish check_gaps_and_empty_shifts_in_schedule {schedule_pk}")

engine/apps/schedules/tasks/notify_about_empty_shifts_in_schedule.py

@@ -48,15 +48,15 @@ def notify_about_empty_shifts_in_schedule_task(schedule_pk):
         task_logger.info(f"Tried to notify_about_empty_shifts_in_schedule_task for non-existing schedule {schedule_pk}")
         return
 
-    empty_shifts = schedule.get_empty_shifts_for_next_week()
+    empty_shifts = schedule.get_empty_shifts_for_next_days()
     schedule.empty_shifts_report_sent_at = timezone.now().date()
 
     if len(empty_shifts) != 0:
         schedule.has_empty_shifts = True
         text = (
-            f'Tried to parse schedule *"{schedule.name}"* and found events without associated users.\n'
-            f"To ensure you don't miss any notifications, use a Grafana username as the event name in the calendar. "
-            f"The user should have Editor or Admin access.\n\n"
+            f"Reviewing *{schedule.slack_url}* on-call schedule found events without valid associated users.\n"
+            f"To ensure you don't miss any notifications, make sure user exists (or you provided a valid Grafana username). "
+            f"The user should have the right permissions, or be an Editor or Admin.\n\n"
         )
         for idx, empty_shift in enumerate(empty_shifts):
             start_timestamp = empty_shift.start.astimezone(pytz.UTC).timestamp()
@@ -80,7 +80,6 @@ def notify_about_empty_shifts_in_schedule_task(schedule_pk):
                 text += '*All-day* event in "UTC" TZ\n'
             else:
                 text += f"From {format_datetime_to_slack_with_time(start_timestamp)} to {format_datetime_to_slack_with_time(end_timestamp)} (your TZ)\n"
-            text += f"_From {OnCallSchedule.CALENDAR_TYPE_VERBAL[empty_shift.calendar_type]} calendar_\n"
             if idx != len(empty_shifts) - 1:
                 text += "\n\n"
         post_message_to_channel(schedule.organization, schedule.slack_channel_slack_id, text)

engine/apps/schedules/tasks/notify_about_gaps_in_schedule.py

@@ -48,13 +48,13 @@ def notify_about_gaps_in_schedule_task(schedule_pk):
         task_logger.info(f"Tried to notify_about_gaps_in_schedule_task for non-existing schedule {schedule_pk}")
         return
 
-    gaps = schedule.get_gaps_for_next_week()
+    gaps = schedule.get_gaps_for_next_days()
     schedule.gaps_report_sent_at = timezone.now().date()
 
     if len(gaps) != 0:
         schedule.has_gaps = True
-        text = f"There are time periods that are unassigned in *{schedule.name}* on-call schedule.\n"
-        for idx, gap in enumerate(gaps):
+        text = f"There are time periods that are unassigned in *{schedule.slack_url}* on-call schedule.\n"
+        for gap in gaps:
             if gap["start"]:
                 start_verbal = format_datetime_to_slack_with_time(gap["start"].astimezone(pytz.UTC).timestamp())
             else:
@@ -64,8 +64,7 @@ def notify_about_gaps_in_schedule_task(schedule_pk):
             else:
                 end_verbal = "..."
             text += f"From {start_verbal} to {end_verbal} (your TZ)\n"
-            if idx != len(gaps) - 1:
-                text += "\n\n"
+        text += "\n\n"
         post_message_to_channel(schedule.organization, schedule.slack_channel_slack_id, text)
     else:
         schedule.has_gaps = False

engine/apps/schedules/tests/test_check_gaps_and_empty_shifts.py

@@ -4,6 +4,7 @@
 from django.utils import timezone
 
 from apps.api.permissions import LegacyAccessControlRole
+from apps.schedules.constants import SCHEDULE_CHECK_NEXT_DAYS
 from apps.schedules.models import CustomOnCallShift, OnCallScheduleWeb
 
 
@@ -34,7 +35,7 @@ def test_no_empty_shifts_no_gaps(
     )
     on_call_shift.add_rolling_users([[user1]])
     schedule.refresh_ical_file()
-    schedule.check_gaps_and_empty_shifts_for_next_week()
+    schedule.check_gaps_and_empty_shifts_for_next_days()
     schedule.refresh_from_db()
 
     assert schedule.has_gaps is False
@@ -73,7 +74,7 @@ def test_no_empty_shifts_but_gaps_now(
     assert schedule.has_gaps is False
     assert schedule.has_empty_shifts is False
 
-    schedule.check_gaps_and_empty_shifts_for_next_week()
+    schedule.check_gaps_and_empty_shifts_for_next_days()
     schedule.refresh_from_db()
 
     assert schedule.has_gaps is True
@@ -111,7 +112,7 @@ def test_empty_shifts_no_gaps(
     assert schedule.has_gaps is False
     assert schedule.has_empty_shifts is False
 
-    schedule.check_gaps_and_empty_shifts_for_next_week()
+    schedule.check_gaps_and_empty_shifts_for_next_days()
     schedule.refresh_from_db()
 
     assert schedule.has_gaps is False
@@ -150,7 +151,7 @@ def test_empty_shifts_and_gaps(
     assert schedule.has_gaps is False
     assert schedule.has_empty_shifts is False
 
-    schedule.check_gaps_and_empty_shifts_for_next_week()
+    schedule.check_gaps_and_empty_shifts_for_next_days()
     schedule.refresh_from_db()
 
     assert schedule.has_gaps is True
@@ -206,7 +207,7 @@ def test_empty_shifts_and_gaps_in_the_past(
     assert schedule.has_gaps is False
     assert schedule.has_empty_shifts is False
 
-    schedule.check_gaps_and_empty_shifts_for_next_week()
+    schedule.check_gaps_and_empty_shifts_for_next_days()
     schedule.refresh_from_db()
 
     assert schedule.has_gaps is False
@@ -225,9 +226,9 @@ def test_empty_shifts_and_gaps_in_the_future(
     user2 = make_user(organization=organization, username="user2", role=LegacyAccessControlRole.ADMIN)
 
     schedule = make_schedule(organization, schedule_class=OnCallScheduleWeb, name="test_schedule")
-    # empty shift with gaps starts in 7 days 1 min
+    # empty shift with gaps starts in SCHEDULE_CHECK_NEXT_DAYS days 1 min
     now = timezone.now().replace(microsecond=0)
-    start_date = now + datetime.timedelta(days=7, minutes=1)
+    start_date = now + datetime.timedelta(days=SCHEDULE_CHECK_NEXT_DAYS, minutes=1)
     data = {
         "start": start_date,
         "rotation_start": start_date,
@@ -241,9 +242,9 @@ def test_empty_shifts_and_gaps_in_the_future(
         organization=organization, shift_type=CustomOnCallShift.TYPE_ROLLING_USERS_EVENT, **data
     )
     on_call_shift.add_rolling_users([[user1]])
-    # normal shift ends in 7 days 1 min
-    start_date2 = now - datetime.timedelta(days=7, minutes=1)
-    until = now + datetime.timedelta(days=7, minutes=1)
+    # normal shift ends in SCHEDULE_CHECK_NEXT_DAYS days 1 min
+    start_date2 = now - datetime.timedelta(days=SCHEDULE_CHECK_NEXT_DAYS, minutes=1)
+    until = now + datetime.timedelta(days=SCHEDULE_CHECK_NEXT_DAYS, minutes=1)
     data2 = {
         "start": start_date2,
         "rotation_start": start_date2,
@@ -262,8 +263,8 @@ def test_empty_shifts_and_gaps_in_the_future(
     assert schedule.has_gaps is False
     assert schedule.has_empty_shifts is False
 
-    schedule.check_gaps_and_empty_shifts_for_next_week()
+    schedule.check_gaps_and_empty_shifts_for_next_days()
     schedule.refresh_from_db()
-    # no gaps and empty shifts in the next 7 days
+    # no gaps and empty shifts in the next SCHEDULE_CHECK_NEXT_DAYS days
     assert schedule.has_gaps is False
     assert schedule.has_empty_shifts is False

engine/apps/schedules/tests/test_notify_about_gaps_in_schedule.py

@@ -4,6 +4,7 @@
 import pytest
 from django.utils import timezone
 
+from apps.schedules.constants import SCHEDULE_CHECK_NEXT_DAYS
 from apps.schedules.models import CustomOnCallShift, OnCallScheduleCalendar, OnCallScheduleICal, OnCallScheduleWeb
 from apps.schedules.tasks import notify_about_gaps_in_schedule_task, start_notify_about_gaps_in_schedule
 
@@ -236,7 +237,7 @@ def test_gaps_near_future_trigger_notification(
 
 
 @pytest.mark.django_db
-def test_gaps_later_than_7_days_no_triggering_notification(
+def test_gaps_later_than_days_no_triggering_notification(
     make_slack_team_identity,
     make_slack_channel,
     make_organization,
@@ -259,8 +260,8 @@ def test_gaps_later_than_7_days_no_triggering_notification(
         prev_ical_file_overrides=None,
         cached_ical_file_overrides=None,
     )
-    start_date = now - datetime.timedelta(days=7, minutes=1)
-    until_date = now + datetime.timedelta(days=8)
+    start_date = now - datetime.timedelta(days=SCHEDULE_CHECK_NEXT_DAYS, minutes=1)
+    until_date = now + datetime.timedelta(days=SCHEDULE_CHECK_NEXT_DAYS + 1)
     data = {
         "start": start_date,
         "rotation_start": start_date,