Refactoring schedule final events (#2651)

Update `list_users_to_notify_from_ical` to use schedule final events

CHANGELOG.md

@@ -13,6 +13,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 - Update the direct paging feature to page for acknowledged & silenced alert groups,
   and show a warning for resolved alert groups by @vadimkerr ([#2639](https://github.com/grafana/oncall/pull/2639))
+- Update checking on-call users to use schedule final events ([#2651](https://github.com/grafana/oncall/pull/2651))
 
 ### Fixed
 

engine/apps/alerts/escalation_snapshot/snapshot_classes/escalation_policy_snapshot.py

@@ -275,7 +275,7 @@ def _escalation_step_notify_on_call_schedule(self, alert_group: "AlertGroup", re
                 escalation_policy_step=self.step,
             )
         else:
-            notify_to_users_list = list_users_to_notify_from_ical(on_call_schedule, include_viewers=True)
+            notify_to_users_list = list_users_to_notify_from_ical(on_call_schedule)
             if notify_to_users_list is None:
                 log_record = AlertGroupLogRecord(
                     type=AlertGroupLogRecord.TYPE_ESCALATION_FAILED,

engine/apps/alerts/tests/test_escalation_policy_snapshot.py

@@ -246,11 +246,8 @@ def test_escalation_step_notify_on_call_schedule_viewer_user(
     )
     assert expected_eta + timezone.timedelta(seconds=15) > result.eta > expected_eta - timezone.timedelta(seconds=15)
     assert result == expected_result
-    assert notify_schedule_step.log_records.filter(type=AlertGroupLogRecord.TYPE_ESCALATION_TRIGGERED).exists()
-    assert list(escalation_policy_snapshot.notify_to_users_queue) == list(
-        list_users_to_notify_from_ical(schedule, include_viewers=True)
-    )
-    assert list(escalation_policy_snapshot.notify_to_users_queue) == [viewer]
+    assert notify_schedule_step.log_records.filter(type=AlertGroupLogRecord.TYPE_ESCALATION_FAILED).exists()
+    assert list(escalation_policy_snapshot.notify_to_users_queue) == []
     assert mocked_execute_tasks.called
 
 

engine/apps/api/tests/test_schedules.py

@@ -1227,7 +1227,7 @@ def test_filter_events_final_schedule(
             "is_gap": is_gap,
             "is_override": is_override,
             "priority_level": priority,
-            "start": start_date + timezone.timedelta(hours=start, milliseconds=1 if start == 0 else 0),
+            "start": start_date + timezone.timedelta(hours=start),
             "user": user,
         }
         for start, duration, user, priority, is_gap, is_override in expected

engine/apps/api/views/on_call_shifts.py

@@ -1,3 +1,6 @@
+import datetime
+
+import pytz
 from django.db.models import Q
 from django_filters.rest_framework import DjangoFilterBackend
 from rest_framework import status
@@ -106,8 +109,13 @@ def preview(self, request):
         updated_shift_pk = self.request.data.get("shift_pk")
         shift = CustomOnCallShift(**validated_data)
         schedule = shift.schedule
+
+        pytz_tz = pytz.timezone(user_tz)
+        datetime_start = datetime.datetime.combine(starting_date, datetime.time.min, tzinfo=pytz_tz)
+        datetime_end = datetime_start + datetime.timedelta(days=days)
+
         shift_events, final_events = schedule.preview_shift(
-            shift, user_tz, starting_date, days, updated_shift_pk=updated_shift_pk
+            shift, datetime_start, datetime_end, updated_shift_pk=updated_shift_pk
         )
         data = {
             "rotation": shift_events,

engine/apps/api/views/schedule.py

@@ -1,6 +1,8 @@
+import datetime
 import functools
 import operator
 
+import pytz
 from django.core.exceptions import ObjectDoesNotExist
 from django.db.models import Count, OuterRef, Subquery
 from django.db.utils import IntegrityError
@@ -274,12 +276,16 @@ def get_request_timezone(self):
 
     @action(detail=True, methods=["get"])
     def events(self, request, pk):
-        user_tz, date = self.get_request_timezone()
+        user_tz, starting_date = self.get_request_timezone()
         with_empty = self.request.query_params.get("with_empty", False) == "true"
         with_gap = self.request.query_params.get("with_gap", False) == "true"
 
         schedule = self.get_object()
-        events = schedule.filter_events(user_tz, date, days=1, with_empty=with_empty, with_gap=with_gap)
+
+        pytz_tz = pytz.timezone(user_tz)
+        datetime_start = datetime.datetime.combine(starting_date, datetime.time.min, tzinfo=pytz_tz)
+        datetime_end = datetime_start + datetime.timedelta(days=1)
+        events = schedule.filter_events(datetime_start, datetime_end, with_empty=with_empty, with_gap=with_gap)
 
         slack_channel = (
             {
@@ -312,19 +318,22 @@ def filter_events(self, request: Request, pk: str) -> Response:
 
         schedule = self.get_object()
 
+        pytz_tz = pytz.timezone(user_tz)
+        datetime_start = datetime.datetime.combine(starting_date, datetime.time.min, tzinfo=pytz_tz)
+        datetime_end = datetime_start + datetime.timedelta(days=days)
+
         if filter_by is not None and filter_by != EVENTS_FILTER_BY_FINAL:
             filter_by = OnCallSchedule.PRIMARY if filter_by == EVENTS_FILTER_BY_ROTATION else OnCallSchedule.OVERRIDES
             events = schedule.filter_events(
-                user_tz,
-                starting_date,
-                days=days,
+                datetime_start,
+                datetime_end,
                 with_empty=True,
                 with_gap=resolve_schedule,
                 filter_by=filter_by,
                 all_day_datetime=True,
             )
         else:  # return final schedule
-            events = schedule.final_events(user_tz, starting_date, days)
+            events = schedule.final_events(datetime_start, datetime_end)
 
         result = {
             "id": schedule.public_primary_key,
@@ -337,11 +346,11 @@ def filter_events(self, request: Request, pk: str) -> Response:
     @action(detail=True, methods=["get"])
     def next_shifts_per_user(self, request, pk):
         """Return next shift for users in schedule."""
-        user_tz, _ = self.get_request_timezone()
         now = timezone.now()
-        starting_date = now.date()
+        datetime_end = now + datetime.timedelta(days=30)
         schedule = self.get_object()
-        events = schedule.final_events(user_tz, starting_date, days=30)
+
+        events = schedule.final_events(now, datetime_end)
 
         users = {u.public_primary_key: None for u in schedule.related_users()}
         for e in events:
@@ -373,10 +382,11 @@ def quality(self, request, pk):
         schedule = self.get_object()
 
         _, date = self.get_request_timezone()
+        datetime_start = datetime.datetime.combine(date, datetime.time.min, tzinfo=pytz.UTC)
         days = self.request.query_params.get("days")
         days = int(days) if days else None
 
-        return Response(schedule.quality_report(date, days))
+        return Response(schedule.quality_report(datetime_start, days))
 
     @action(detail=False, methods=["get"])
     def type_options(self, request):

engine/apps/mobile_app/tasks.py

@@ -439,7 +439,8 @@ def conditionally_send_going_oncall_push_notifications_for_schedule(schedule_pk)
         return
 
     now = timezone.now()
-    schedule_final_events = schedule.final_events("UTC", now, days=7)
+    datetime_end = now + datetime.timedelta(days=7)
+    schedule_final_events = schedule.final_events(now, datetime_end)
 
     relevant_cache_keys = [
         _generate_going_oncall_push_notification_cache_key(user["pk"], schedule_event)

engine/apps/public_api/views/schedules.py

@@ -1,5 +1,7 @@
+import datetime
 import logging
 
+import pytz
 from django_filters import rest_framework as filters
 from rest_framework import status
 from rest_framework.decorators import action
@@ -147,8 +149,12 @@ def final_shifts(self, request, pk):
         end_date = serializer.validated_data["end_date"]
         days_between_start_and_end = (end_date - start_date).days
 
-        final_schedule_events: ScheduleEvents = schedule.final_events("UTC", start_date, days_between_start_and_end)
+        datetime_start = datetime.datetime.combine(start_date, datetime.time.min, tzinfo=pytz.UTC)
+        datetime_end = datetime_start + datetime.timedelta(
+            days=days_between_start_and_end - 1, hours=23, minutes=59, seconds=59
+        )
 
+        final_schedule_events: ScheduleEvents = schedule.final_events(datetime_start, datetime_end)
         logger.info(
             f"Exporting oncall shifts for schedule {pk} between dates {start_date} and {end_date}. {len(final_schedule_events)} shift events were found."
         )

engine/apps/schedules/ical_utils.py

@@ -61,7 +61,6 @@
 def users_in_ical(
     usernames_from_ical: typing.List[str],
     organization: "Organization",
-    include_viewers=False,
     users_to_filter: typing.Optional["UserQuerySet"] = None,
 ) -> typing.Sequence["User"]:
     """
@@ -94,11 +93,10 @@ def users_in_ical(
             }
         )
 
-    users_found_in_ical = organization.users
-    if not include_viewers:
-        users_found_in_ical = users_found_in_ical.filter(
-            **User.build_permissions_query(RBACPermission.Permissions.SCHEDULES_WRITE, organization)
-        )
+    # users_found_in_ical = organization.users
+    users_found_in_ical = organization.users.filter(
+        **User.build_permissions_query(RBACPermission.Permissions.SCHEDULES_WRITE, organization)
+    )
 
     users_found_in_ical = users_found_in_ical.filter(
         (Q(username__in=usernames_from_ical) | Q(email__lower__in=emails_from_ical))
@@ -118,11 +116,10 @@ def memoized_users_in_ical(
 # used for display schedule events on web
 def list_of_oncall_shifts_from_ical(
     schedule: "OnCallSchedule",
-    date: datetime.date,
-    user_timezone: str = "UTC",
+    datetime_start: datetime.datetime,
+    datetime_end: datetime.datetime,
     with_empty_shifts: bool = False,
     with_gaps: bool = False,
-    days: int = 1,
     filter_by: str | None = None,
     from_cached_final: bool = False,
 ):
@@ -152,16 +149,6 @@ def list_of_oncall_shifts_from_ical(
     else:
         calendars = schedule.get_icalendars()
 
-    # TODO: Review offset usage
-    pytz_tz = pytz.timezone(user_timezone)
-
-    # utcoffset can technically return None, but we're confident it is a timedelta here
-    user_timezone_offset: datetime.timedelta = datetime.datetime.now().astimezone(pytz_tz).utcoffset()  # type: ignore[assignment]
-
-    datetime_min = datetime.datetime.combine(date, datetime.time.min) + datetime.timedelta(milliseconds=1)
-    datetime_start = (datetime_min - user_timezone_offset).astimezone(pytz.UTC)
-    datetime_end = datetime_start + datetime.timedelta(days=days - 1, hours=23, minutes=59, seconds=59)
-
     result_datetime = []
     result_date = []
 
@@ -204,6 +191,7 @@ def list_of_oncall_shifts_from_ical(
             )
 
     def event_start_cmp_key(e):
+        pytz_tz = pytz.timezone("UTC")
         return (
             datetime.datetime.combine(e["start"], datetime.datetime.min.time(), tzinfo=pytz_tz)
             if type(e["start"]) == datetime.date
@@ -348,7 +336,6 @@ def list_of_empty_shifts_in_schedule(
 def list_users_to_notify_from_ical(
     schedule: "OnCallSchedule",
     events_datetime: typing.Optional[datetime.datetime] = None,
-    include_viewers: bool = False,
     users_to_filter: typing.Optional["UserQuerySet"] = None,
 ) -> typing.Sequence["User"]:
     """
@@ -359,7 +346,6 @@ def list_users_to_notify_from_ical(
         schedule,
         events_datetime,
         events_datetime,
-        include_viewers=include_viewers,
         users_to_filter=users_to_filter,
     )
 
@@ -368,35 +354,15 @@ def list_users_to_notify_from_ical_for_period(
     schedule: "OnCallSchedule",
     start_datetime: datetime.datetime,
     end_datetime: datetime.datetime,
-    include_viewers=False,
     users_to_filter=None,
 ) -> typing.Sequence["User"]:
-    # get list of iCalendars from current iCal files. If there is more than one calendar, primary calendar will always
-    # be the first
-    calendars = schedule.get_icalendars()
-    # reverse calendars to make overrides calendar the first, if schedule is iCal
-    calendars = calendars[::-1]
     users_found_in_ical: typing.Sequence["User"] = []
-    # at first check overrides calendar and return users from it if it exists and on-call users are found
-    for calendar in calendars:
-        if calendar is None:
-            continue
-        events = ical_events.get_events_from_ical_between(calendar, start_datetime, end_datetime)
-
-        parsed_ical_events: typing.Dict[int, typing.List[str]] = {}
-        for event in events:
-            current_usernames, current_priority = get_usernames_from_ical_event(event)
-            parsed_ical_events.setdefault(current_priority, []).extend(current_usernames)
-        # find users by usernames. if users are not found for shift, get users from lower priority
-        for _, usernames in sorted(parsed_ical_events.items(), reverse=True):
-            users_found_in_ical = users_in_ical(
-                usernames, schedule.organization, include_viewers=include_viewers, users_to_filter=users_to_filter
-            )
-            if users_found_in_ical:
-                break
-        if users_found_in_ical:
-            # if users are found in the overrides calendar, there is no need to check primary calendar
-            break
+    events = schedule.final_events(start_datetime, end_datetime)
+    usernames = []
+    for event in events:
+        usernames += [u["email"] for u in event.get("users", [])]
+
+    users_found_in_ical = users_in_ical(usernames, schedule.organization, users_to_filter=users_to_filter)
     return users_found_in_ical