fix: update missing users / empty shifts check (#5322)

Related to https://github.com/grafana/oncall-private/issues/2950

- Represent missing users in schedule events (so they are displayed in
the web UI)
- Fix schedule checks for gaps/empty shifts so they send notifications

engine/apps/schedules/ical_utils.py

@@ -54,6 +54,19 @@ logger = logging.getLogger(__name__)
 logger.setLevel(logging.DEBUG)
 
 
+class MissingUser:
+    """Represent a missing user in a rolling users shift."""
+
+    DISPLAY_NAME = "(missing)"
+
+    def __init__(self, pk):
+        self.pk = pk
+
+    @property
+    def username(self):
+        return self.DISPLAY_NAME
+
+
 EmptyShift = namedtuple(
     "EmptyShift",
     ["start", "end", "summary", "description", "attendee", "all_day", "calendar_type", "calendar_tz", "shift_pk"],

engine/apps/schedules/models/custom_on_call_shift.py

@@ -18,6 +18,7 @@ from django.utils import timezone
 from django.utils.functional import cached_property
 from icalendar.cal import Event
 
+from apps.schedules.ical_utils import MissingUser
 from apps.schedules.tasks import (
     check_gaps_and_empty_shifts_in_schedule,
     drop_cached_ical_task,
@@ -645,10 +646,6 @@ class CustomOnCallShift(models.Model):
         all_users_pks = set()
         users_queue = []
         if self.rolling_users is not None:
-            # get all users pks from rolling_users field
-            for users_dict in self.rolling_users:
-                all_users_pks.update(users_dict.keys())
-            users = User.objects.filter(pk__in=all_users_pks)
             # generate users_queue list with user objects
             if self.start_rotation_from_user_index is not None:
                 rolling_users = (
@@ -657,10 +654,22 @@ class CustomOnCallShift(models.Model):
                 )
             else:
                 rolling_users = self.rolling_users
+
+            # get all users pks from rolling_users field
+            for users_dict in self.rolling_users:
+                all_users_pks.update(users_dict.keys())
+            users = User.objects.filter(pk__in=all_users_pks)
+            users_by_id = {user.pk: user for user in users}
             for users_dict in rolling_users:
-                users_list = list(users.filter(pk__in=users_dict.keys()))
-                if users_list:
-                    users_queue.append(users_list)
+                users_list = []
+                for user_pk in users_dict.keys():
+                    try:
+                        user_pk = int(user_pk)
+                        users_list.append(users_by_id.get(user_pk, MissingUser(user_pk)))
+                    except ValueError:
+                        users_list.append(MissingUser(user_pk))
+                users_queue.append(users_list)
+
         return users_queue
 
     def add_rolling_users(self, rolling_users_list):

engine/apps/schedules/tasks/__init__.py

@@ -1,17 +1,13 @@
 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,
     notify_about_empty_shifts_in_schedule_task,
     schedule_notify_about_empty_shifts_in_schedule,
-    start_check_empty_shifts_in_schedule,
     start_notify_about_empty_shifts_in_schedule,
 )
 from .notify_about_gaps_in_schedule import (  # noqa: F401
-    check_gaps_in_schedule,
     notify_about_gaps_in_schedule_task,
     schedule_notify_about_gaps_in_schedule,
-    start_check_gaps_in_schedule,
     start_notify_about_gaps_in_schedule,
 )
 from .refresh_ical_files import (  # noqa: F401

engine/apps/schedules/tasks/notify_about_empty_shifts_in_schedule.py

@@ -1,6 +1,7 @@
 import pytz
 from celery.utils.log import get_task_logger
 from django.core.cache import cache
+from django.db.models import Q
 from django.utils import timezone
 
 from apps.slack.utils import format_datetime_to_slack_with_time, post_message_to_channel
@@ -10,28 +11,16 @@ 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():
-    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):
-    return
-
-
 @shared_dedicated_queue_retry_task()
 def start_notify_about_empty_shifts_in_schedule():
-    from apps.schedules.models import OnCallScheduleICal
+    from apps.schedules.models import OnCallSchedule
 
     task_logger.info("Start start_notify_about_empty_shifts_in_schedule")
 
     today = timezone.now().date()
     week_ago = today - timezone.timedelta(days=7)
-    schedules = OnCallScheduleICal.objects.filter(
-        empty_shifts_report_sent_at__lte=week_ago,
+    schedules = OnCallSchedule.objects.filter(
+        Q(empty_shifts_report_sent_at__lte=week_ago) | Q(empty_shifts_report_sent_at__isnull=True),
         slack_channel__isnull=False,
         organization__deleted_at__isnull=True,
     )

engine/apps/schedules/tasks/notify_about_gaps_in_schedule.py

@@ -1,6 +1,7 @@
 import pytz
 from celery.utils.log import get_task_logger
 from django.core.cache import cache
+from django.db.models import Q
 from django.utils import timezone
 
 from apps.slack.utils import format_datetime_to_slack_with_time, post_message_to_channel
@@ -9,18 +10,6 @@ from common.custom_celery_tasks import shared_dedicated_queue_retry_task
 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():
-    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):
-    return
-
-
 @shared_dedicated_queue_retry_task()
 def start_notify_about_gaps_in_schedule():
     from apps.schedules.models import OnCallSchedule
@@ -30,7 +19,7 @@ def start_notify_about_gaps_in_schedule():
     today = timezone.now().date()
     week_ago = today - timezone.timedelta(days=7)
     schedules = OnCallSchedule.objects.filter(
-        gaps_report_sent_at__lte=week_ago,
+        Q(gaps_report_sent_at__lte=week_ago) | Q(gaps_report_sent_at__isnull=True),
         slack_channel__isnull=False,
         organization__deleted_at__isnull=True,
     )

engine/apps/schedules/tests/test_notify_about_empty_shifts_in_schedule.py

@@ -5,8 +5,8 @@ import pytest
 from django.utils import timezone
 
 from apps.api.permissions import LegacyAccessControlRole
-from apps.schedules.models import CustomOnCallShift, OnCallScheduleWeb
-from apps.schedules.tasks import notify_about_empty_shifts_in_schedule_task
+from apps.schedules.models import CustomOnCallShift, OnCallScheduleCalendar, OnCallScheduleICal, OnCallScheduleWeb
+from apps.schedules.tasks import notify_about_empty_shifts_in_schedule_task, start_notify_about_empty_shifts_in_schedule
 
 
 @pytest.mark.django_db
@@ -174,3 +174,45 @@ def test_empty_non_empty_shifts_trigger_notification(
     schedule.refresh_from_db()
     assert empty_shifts_report_sent_at != schedule.empty_shifts_report_sent_at
     assert schedule.has_empty_shifts
+
+
+@pytest.mark.parametrize(
+    "schedule_class",
+    [OnCallScheduleWeb, OnCallScheduleICal, OnCallScheduleCalendar],
+)
+@pytest.mark.parametrize(
+    "report_sent_days_ago,expected_call",
+    [(8, True), (6, False), (None, True)],
+)
+@pytest.mark.django_db
+def test_start_notify_about_empty_shifts(
+    make_slack_team_identity,
+    make_slack_channel,
+    make_organization,
+    make_schedule,
+    schedule_class,
+    report_sent_days_ago,
+    expected_call,
+):
+    slack_team_identity = make_slack_team_identity()
+    slack_channel = make_slack_channel(slack_team_identity)
+    organization = make_organization(slack_team_identity=slack_team_identity)
+
+    sent = timezone.now() - datetime.timedelta(days=report_sent_days_ago) if report_sent_days_ago else None
+    schedule = make_schedule(
+        organization,
+        schedule_class=schedule_class,
+        name="test_schedule",
+        slack_channel=slack_channel,
+        empty_shifts_report_sent_at=sent,
+    )
+
+    with patch(
+        "apps.schedules.tasks.notify_about_empty_shifts_in_schedule.notify_about_empty_shifts_in_schedule_task.apply_async"
+    ) as mock_notify:
+        start_notify_about_empty_shifts_in_schedule()
+
+    if expected_call:
+        mock_notify.assert_called_once_with((schedule.pk,))
+    else:
+        mock_notify.assert_not_called()

engine/apps/schedules/tests/test_notify_about_gaps_in_schedule.py

@@ -4,8 +4,8 @@ from unittest.mock import patch
 import pytest
 from django.utils import timezone
 
-from apps.schedules.models import CustomOnCallShift, OnCallScheduleWeb
-from apps.schedules.tasks import notify_about_gaps_in_schedule_task
+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
 
 
 @pytest.mark.django_db
@@ -286,3 +286,45 @@ def test_gaps_later_than_7_days_no_triggering_notification(
     schedule.refresh_from_db()
     assert gaps_report_sent_at != schedule.gaps_report_sent_at
     assert schedule.has_gaps is False
+
+
+@pytest.mark.parametrize(
+    "schedule_class",
+    [OnCallScheduleWeb, OnCallScheduleICal, OnCallScheduleCalendar],
+)
+@pytest.mark.parametrize(
+    "report_sent_days_ago,expected_call",
+    [(8, True), (6, False), (None, True)],
+)
+@pytest.mark.django_db
+def test_start_notify_about_gaps(
+    make_slack_team_identity,
+    make_slack_channel,
+    make_organization,
+    make_schedule,
+    schedule_class,
+    report_sent_days_ago,
+    expected_call,
+):
+    slack_team_identity = make_slack_team_identity()
+    slack_channel = make_slack_channel(slack_team_identity)
+    organization = make_organization(slack_team_identity=slack_team_identity)
+
+    sent = timezone.now() - datetime.timedelta(days=report_sent_days_ago) if report_sent_days_ago else None
+    schedule = make_schedule(
+        organization,
+        schedule_class=schedule_class,
+        name="test_schedule",
+        slack_channel=slack_channel,
+        gaps_report_sent_at=sent,
+    )
+
+    with patch(
+        "apps.schedules.tasks.notify_about_gaps_in_schedule.notify_about_gaps_in_schedule_task.apply_async"
+    ) as mock_notify:
+        start_notify_about_gaps_in_schedule()
+
+    if expected_call:
+        mock_notify.assert_called_once_with((schedule.pk,))
+    else:
+        mock_notify.assert_not_called()

engine/apps/schedules/tests/test_on_call_schedule.py

@@ -18,6 +18,7 @@ from apps.schedules.constants import (
     ICAL_STATUS_CANCELLED,
     ICAL_SUMMARY,
 )
+from apps.schedules.ical_utils import MissingUser
 from apps.schedules.models import (
     CustomOnCallShift,
     OnCallSchedule,
@@ -358,6 +359,57 @@ def test_filter_events_include_empty(make_organization, make_user_for_organizati
     assert events == expected
 
 
+@pytest.mark.django_db
+def test_filter_events_include_empty_if_deleted(
+    make_organization, make_user_for_organization, make_schedule, make_on_call_shift
+):
+    organization = make_organization()
+    schedule = make_schedule(
+        organization,
+        schedule_class=OnCallScheduleWeb,
+        name="test_web_schedule",
+    )
+    user = make_user_for_organization(organization)
+    now = timezone.now().replace(hour=0, minute=0, second=0, microsecond=0)
+    start_date = now - timezone.timedelta(days=7)
+
+    data = {
+        "start": start_date + timezone.timedelta(hours=10),
+        "rotation_start": start_date + timezone.timedelta(hours=10),
+        "duration": timezone.timedelta(hours=8),
+        "priority_level": 1,
+        "frequency": CustomOnCallShift.FREQUENCY_DAILY,
+        "schedule": schedule,
+    }
+    on_call_shift = make_on_call_shift(
+        organization=organization, shift_type=CustomOnCallShift.TYPE_ROLLING_USERS_EVENT, **data
+    )
+    on_call_shift.add_rolling_users([[user]])
+
+    # user is deleted, shift data still exists but the shift is empty
+    user.delete()
+
+    end_date = start_date + timezone.timedelta(days=1)
+    events = schedule.filter_events(start_date, end_date, filter_by=OnCallSchedule.TYPE_ICAL_PRIMARY, with_empty=True)
+    expected = [
+        {
+            "calendar_type": OnCallSchedule.TYPE_ICAL_PRIMARY,
+            "start": on_call_shift.start,
+            "end": on_call_shift.start + on_call_shift.duration,
+            "all_day": False,
+            "is_override": False,
+            "is_empty": True,
+            "is_gap": False,
+            "priority_level": on_call_shift.priority_level,
+            "missing_users": [MissingUser.DISPLAY_NAME],
+            "users": [],
+            "shift": {"pk": on_call_shift.public_primary_key},
+            "source": "api",
+        }
+    ]
+    assert events == expected
+
+
 @pytest.mark.django_db
 def test_filter_events_ical_all_day(make_organization, make_user_for_organization, make_schedule, get_ical):
     calendar = get_ical("calendar_with_all_day_event.ics")

engine/settings/celery_task_routes.py

@@ -33,13 +33,9 @@ CELERY_TASK_ROUTES = {
     "apps.schedules.tasks.refresh_ical_files.refresh_ical_final_schedule": {"queue": "default"},
     "apps.schedules.tasks.refresh_ical_files.start_refresh_ical_final_schedules": {"queue": "default"},
     "apps.schedules.tasks.check_gaps_and_empty_shifts.check_gaps_and_empty_shifts_in_schedule": {"queue": "default"},
-    "apps.schedules.tasks.notify_about_gaps_in_schedule.check_empty_shifts_in_schedule": {"queue": "default"},
     "apps.schedules.tasks.notify_about_gaps_in_schedule.start_notify_about_gaps_in_schedule": {"queue": "default"},
-    "apps.schedules.tasks.notify_about_gaps_in_schedule.check_gaps_in_schedule": {"queue": "default"},
     "apps.schedules.tasks.notify_about_gaps_in_schedule.notify_about_gaps_in_schedule_task": {"queue": "default"},
     "apps.schedules.tasks.notify_about_gaps_in_schedule.schedule_notify_about_gaps_in_schedule": {"queue": "default"},
-    "apps.schedules.tasks.notify_about_gaps_in_schedule.start_check_empty_shifts_in_schedule": {"queue": "default"},
-    "apps.schedules.tasks.notify_about_gaps_in_schedule.start_check_gaps_in_schedule": {"queue": "default"},
     "apps.schedules.tasks.notify_about_gaps_in_schedule.start_notify_about_empty_shifts_in_schedule": {
         "queue": "default"
     },