Refactor gaps and empty shift checks

CHANGELOG.md

@@ -14,6 +14,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 ### Changed
 
 - Remove `/oncall` Slack slash command (ie. manual alert group creation command) by @joeyorlando ([#3790](https://github.com/grafana/oncall/pull/3790))
+- Increase frequency of checking for gaps and empty shifts in schedules by @Ferril ([#3785](https://github.com/grafana/oncall/pull/3785))
 
 ### Fixed
 

engine/apps/api/serializers/schedule_base.py

@@ -88,9 +88,8 @@ def validate(self, attrs):
 
     def create(self, validated_data):
         created_schedule = super().create(validated_data)
-        created_schedule.check_empty_shifts_for_next_week()
+        created_schedule.check_gaps_and_empty_shifts_for_next_week()
         schedule_notify_about_empty_shifts_in_schedule.apply_async((created_schedule.pk,))
-        created_schedule.check_gaps_for_next_week()
         schedule_notify_about_gaps_in_schedule.apply_async((created_schedule.pk,))
         return created_schedule
 

engine/apps/api/serializers/schedule_calendar.py

@@ -57,8 +57,7 @@ def update(self, instance, validated_data):
             or old_enable_web_overrides != updated_enable_web_overrides
         ):
             updated_schedule.drop_cached_ical()
-            updated_schedule.check_empty_shifts_for_next_week()
-            updated_schedule.check_gaps_for_next_week()
+            updated_schedule.check_gaps_and_empty_shifts_for_next_week()
             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

@@ -86,8 +86,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_empty_shifts_for_next_week()
-            updated_schedule.check_gaps_for_next_week()
+            updated_schedule.check_gaps_and_empty_shifts_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

engine/apps/api/serializers/schedule_web.py

@@ -40,8 +40,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_empty_shifts_for_next_week()
-            updated_schedule.check_gaps_for_next_week()
+            updated_schedule.check_gaps_and_empty_shifts_for_next_week()
             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

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

engine/apps/schedules/models/custom_on_call_shift.py

@@ -19,6 +19,7 @@
 from icalendar.cal import Event
 
 from apps.schedules.tasks import (
+    check_gaps_and_empty_shifts_in_schedule,
     drop_cached_ical_task,
     refresh_ical_final_schedule,
     schedule_notify_about_empty_shifts_in_schedule,
@@ -692,6 +693,7 @@ def refresh_schedule(self):
         schedule = self.schedule.get_real_instance()
         schedule.refresh_ical_file()
         refresh_ical_final_schedule.apply_async((schedule.pk,))
+        check_gaps_and_empty_shifts_in_schedule.apply_async((schedule.pk,))
 
     def start_drop_ical_and_check_schedule_tasks(self, schedule):
         drop_cached_ical_task.apply_async((schedule.pk,))

engine/apps/schedules/models/on_call_schedule.py

@@ -34,6 +34,7 @@
     ICAL_UID,
 )
 from apps.schedules.ical_utils import (
+    EmptyShifts,
     create_base_icalendar,
     fetch_ical_file_or_get_error,
     get_oncall_users_for_multiple_schedules,
@@ -278,22 +279,34 @@ def get_prev_and_current_ical_files(self):
             (self.prev_ical_file_overrides, self.cached_ical_file_overrides),
         ]
 
-    def check_gaps_for_next_week(self) -> bool:
+    def check_gaps_and_empty_shifts_for_next_week(self) -> None:
+        datetime_start = timezone.now()
+        datetime_end = datetime_start + datetime.timedelta(days=7)
+
+        # get empty shifts from all events and gaps from final events
+        events = self.filter_events(
+            datetime_start,
+            datetime_end,
+            with_empty=True,
+            with_gap=True,
+            all_day_datetime=True,
+        )
+        has_empty_shifts = len([event for event in events if event["is_empty"]]) != 0
+        final_events = self._resolve_schedule(events, datetime_start, datetime_end)
+        has_gaps = len([final_event for final_event in final_events if final_event["is_gap"]]) != 0
+        if has_gaps != self.has_gaps or has_empty_shifts != self.has_empty_shifts:
+            self.has_gaps = has_gaps
+            self.has_empty_shifts = has_empty_shifts
+            self.save(update_fields=["has_gaps", "has_empty_shifts"])
+
+    def get_gaps_for_next_week(self) -> ScheduleEvents:
         today = timezone.now()
         events = self.final_events(today, today + datetime.timedelta(days=7))
-        gaps = [event for event in events if event["is_gap"] and not event["is_empty"]]
-        has_gaps = len(gaps) != 0
-        self.has_gaps = has_gaps
-        self.save(update_fields=["has_gaps"])
-        return has_gaps
+        return [event for event in events if event["is_gap"]]
 
-    def check_empty_shifts_for_next_week(self):
+    def get_empty_shifts_for_next_week(self) -> EmptyShifts:
         today = timezone.now().date()
-        empty_shifts = list_of_empty_shifts_in_schedule(self, today, today + datetime.timedelta(days=7))
-        has_empty_shifts = len(empty_shifts) != 0
-        self.has_empty_shifts = has_empty_shifts
-        self.save(update_fields=["has_empty_shifts"])
-        return has_empty_shifts
+        return list_of_empty_shifts_in_schedule(self, today, today + datetime.timedelta(days=7))
 
     def drop_cached_ical(self):
         self._drop_primary_ical_file()

engine/apps/schedules/tasks/__init__.py

@@ -1,3 +1,4 @@
+from .check_gaps_and_empty_shifts import check_gaps_and_empty_shifts_in_schedule  # noqa: F401
 from .drop_cached_ical import drop_cached_ical_for_custom_events_for_organization, drop_cached_ical_task  # noqa: F401
 from .notify_about_empty_shifts_in_schedule import (  # noqa: F401
     check_empty_shifts_in_schedule,

engine/apps/schedules/tasks/check_gaps_and_empty_shifts.py

@@ -0,0 +1,23 @@
+from celery.utils.log import get_task_logger
+
+from common.custom_celery_tasks import shared_dedicated_queue_retry_task
+
+task_logger = get_task_logger(__name__)
+
+
+@shared_dedicated_queue_retry_task()
+def check_gaps_and_empty_shifts_in_schedule(schedule_pk):
+    from apps.schedules.models import OnCallSchedule
+
+    task_logger.info(f"Start check_gaps_and_empty_shifts_in_schedule {schedule_pk}")
+
+    try:
+        schedule = OnCallSchedule.objects.get(
+            pk=schedule_pk,
+        )
+    except OnCallSchedule.DoesNotExist:
+        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()
+    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

@@ -3,44 +3,23 @@
 from django.core.cache import cache
 from django.utils import timezone
 
-from apps.schedules.ical_utils import list_of_empty_shifts_in_schedule
 from apps.slack.utils import format_datetime_to_slack_with_time, post_message_to_channel
 from common.custom_celery_tasks import shared_dedicated_queue_retry_task
 from common.utils import trim_if_needed
 
 task_logger = get_task_logger(__name__)
 
 
+# deprecated # todo: delete this task from here and from task routes after the next release
 @shared_dedicated_queue_retry_task()
 def start_check_empty_shifts_in_schedule():
-    from apps.schedules.models import OnCallSchedule
-
-    task_logger.info("Start start_notify_about_empty_shifts_in_schedule")
-
-    schedules = OnCallSchedule.objects.all()
-
-    for schedule in schedules:
-        check_empty_shifts_in_schedule.apply_async((schedule.pk,))
-
-    task_logger.info("Finish start_notify_about_empty_shifts_in_schedule")
+    return
 
 
+# deprecated # todo: delete this task from here and from task routes after the next release
 @shared_dedicated_queue_retry_task()
 def check_empty_shifts_in_schedule(schedule_pk):
-    from apps.schedules.models import OnCallSchedule
-
-    task_logger.info(f"Start check_empty_shifts_in_schedule {schedule_pk}")
-
-    try:
-        schedule = OnCallSchedule.objects.get(
-            pk=schedule_pk,
-        )
-    except OnCallSchedule.DoesNotExist:
-        task_logger.info(f"Tried to check_empty_shifts_in_schedule for non-existing schedule {schedule_pk}")
-        return
-
-    schedule.check_empty_shifts_for_next_week()
-    task_logger.info(f"Finish check_empty_shifts_in_schedule {schedule_pk}")
+    return
 
 
 @shared_dedicated_queue_retry_task()
@@ -54,6 +33,7 @@ def start_notify_about_empty_shifts_in_schedule():
     schedules = OnCallScheduleICal.objects.filter(
         empty_shifts_report_sent_at__lte=week_ago,
         channel__isnull=False,
+        organization__deleted_at__isnull=True,
     )
 
     for schedule in schedules:
@@ -79,9 +59,8 @@ 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
 
-    today = timezone.now().date()
-    empty_shifts = list_of_empty_shifts_in_schedule(schedule, today, today + timezone.timedelta(days=7))
-    schedule.empty_shifts_report_sent_at = today
+    empty_shifts = schedule.get_empty_shifts_for_next_week()
+    schedule.empty_shifts_report_sent_at = timezone.now().date()
 
     if len(empty_shifts) != 0:
         schedule.has_empty_shifts = True

engine/apps/schedules/tasks/notify_about_gaps_in_schedule.py

@@ -1,5 +1,3 @@
-import datetime
-
 import pytz
 from celery.utils.log import get_task_logger
 from django.core.cache import cache
@@ -11,36 +9,16 @@
 task_logger = get_task_logger(__name__)
 
 
+# deprecated # todo: delete this task from here and from task routes after the next release
 @shared_dedicated_queue_retry_task()
 def start_check_gaps_in_schedule():
-    from apps.schedules.models import OnCallSchedule
-
-    task_logger.info("Start start_check_gaps_in_schedule")
-
-    schedules = OnCallSchedule.objects.all()
-
-    for schedule in schedules:
-        check_gaps_in_schedule.apply_async((schedule.pk,))
-
-    task_logger.info("Finish start_check_gaps_in_schedule")
+    return
 
 
+# deprecated # todo: delete this task from here and from task routes after the next release
 @shared_dedicated_queue_retry_task()
 def check_gaps_in_schedule(schedule_pk):
-    from apps.schedules.models import OnCallSchedule
-
-    task_logger.info(f"Start check_gaps_in_schedule {schedule_pk}")
-
-    try:
-        schedule = OnCallSchedule.objects.get(
-            pk=schedule_pk,
-        )
-    except OnCallSchedule.DoesNotExist:
-        task_logger.info(f"Tried to check_gaps_in_schedule for non-existing schedule {schedule_pk}")
-        return
-
-    schedule.check_gaps_for_next_week()
-    task_logger.info(f"Finish check_gaps_in_schedule {schedule_pk}")
+    return
 
 
 @shared_dedicated_queue_retry_task()
@@ -54,6 +32,7 @@ def start_notify_about_gaps_in_schedule():
     schedules = OnCallSchedule.objects.filter(
         gaps_report_sent_at__lte=week_ago,
         channel__isnull=False,
+        organization__deleted_at__isnull=True,
     )
 
     for schedule in schedules:
@@ -80,10 +59,8 @@ 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
 
-    now = timezone.now()
-    events = schedule.final_events(now, now + datetime.timedelta(days=7))
-    gaps = [event for event in events if event["is_gap"] and not event["is_empty"]]
-    schedule.gaps_report_sent_at = now.date()
+    gaps = schedule.get_gaps_for_next_week()
+    schedule.gaps_report_sent_at = timezone.now().date()
 
     if len(gaps) != 0:
         schedule.has_gaps = True

engine/apps/schedules/tasks/refresh_ical_files.py

@@ -2,7 +2,11 @@
 
 from apps.alerts.tasks import notify_ical_schedule_shift  # type: ignore[no-redef]
 from apps.schedules.ical_utils import is_icals_equal, update_cached_oncall_users_for_schedule
-from apps.schedules.tasks import notify_about_empty_shifts_in_schedule_task, notify_about_gaps_in_schedule_task
+from apps.schedules.tasks import (
+    check_gaps_and_empty_shifts_in_schedule,
+    notify_about_empty_shifts_in_schedule_task,
+    notify_about_gaps_in_schedule_task,
+)
 from apps.slack.tasks import start_update_slack_user_group_for_schedules
 from common.custom_celery_tasks import shared_dedicated_queue_retry_task
 
@@ -84,6 +88,9 @@ def refresh_ical_file(schedule_pk):
     # update cached schedule on-call users
     update_cached_oncall_users_for_schedule(schedule)
 
+    check_gaps_and_empty_shifts_in_schedule.apply_async((schedule_pk,))
+    # todo: refactor tasks below to unify checking and notifying about gaps and empty shifts to avoid doing the same
+    # todo: work twice.
     if run_task:
         notify_about_empty_shifts_in_schedule_task.apply_async((schedule_pk,))
         notify_about_gaps_in_schedule_task.apply_async((schedule_pk,))