Fix slack notification for shift end affected by taken swap

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
+
+- Fix slack notification for a shift which end is affected by a taken swap ([#3092](https://github.com/grafana/oncall/pull/3092))
+
 ## v1.3.40 (2023-08-28)
 
 ### Added

engine/apps/alerts/tasks/notify_ical_schedule_shift.py

@@ -22,6 +22,9 @@
     from apps.schedules.models import OnCallSchedule
 
 
+MIN_DAYS_TO_LOOKUP_FOR_THE_END_OF_EVENT = 3
+
+
 def convert_prev_shifts_to_new_format(prev_shifts: dict, schedule: "OnCallSchedule") -> list:
     new_prev_shifts = []
     user_ids = []
@@ -82,12 +85,6 @@ def notify_ical_schedule_shift(schedule_pk):
 
     task_logger.info(f"Notify ical schedule shift {schedule_pk}, organization {schedule.organization_id}")
 
-    MIN_DAYS_TO_LOOKUP_FOR_THE_END_OF_EVENT = 3
-
-    now = datetime.datetime.now(timezone.utc)
-
-    current_shifts = schedule.final_events(now, now, with_empty=False, with_gap=False, ignore_untaken_swaps=True)
-
     prev_shifts = json.loads(schedule.current_shifts) if not schedule.empty_oncall else []
     prev_shifts_updated = False
     # convert prev_shifts to new events format for compatibility with the previous version of this task
@@ -101,6 +98,33 @@ def notify_ical_schedule_shift(schedule_pk):
         prev_shift["start"] = datetime.datetime.strptime(prev_shift["start"], str_format)
         prev_shift["end"] = datetime.datetime.strptime(prev_shift["end"], str_format)
 
+    # get shifts in progress now
+    now = datetime.datetime.now(timezone.utc)
+    current_shifts = schedule.final_events(now, now, with_empty=False, with_gap=False, ignore_untaken_swaps=True)
+
+    # get days_to_lookup for next shifts (which may affect current shifts)
+    if len(current_shifts) != 0:
+        max_end_date = max([shift["end"].date() for shift in current_shifts])
+        days_to_lookup = (max_end_date - now.date()).days + 1
+        days_to_lookup = max([days_to_lookup, MIN_DAYS_TO_LOOKUP_FOR_THE_END_OF_EVENT])
+    else:
+        days_to_lookup = MIN_DAYS_TO_LOOKUP_FOR_THE_END_OF_EVENT
+
+    # get updated current and upcoming shifts
+    datetime_end = now + datetime.timedelta(days=days_to_lookup)
+    next_shifts_unfiltered = schedule.final_events(
+        now, datetime_end, with_empty=False, with_gap=False, ignore_untaken_swaps=True
+    )
+
+    # split current and next shifts
+    current_shifts = []
+    next_shifts = []
+    for shift in next_shifts_unfiltered:
+        if now < shift["start"]:
+            next_shifts.append(shift)
+        else:
+            current_shifts.append(shift)
+
     shift_changed, diff_shifts = calculate_shift_diff(current_shifts, prev_shifts)
 
     # Do not notify if there is no difference between current and previous shifts
@@ -114,25 +138,6 @@ def notify_ical_schedule_shift(schedule_pk):
 
     new_shifts = sorted(diff_shifts, key=lambda shift: shift["start"])
 
-    # get days_to_lookup for next shifts
-    if len(new_shifts) != 0:
-        max_end_date = max([shift["end"].date() for shift in new_shifts])
-        days_to_lookup = (max_end_date - now.date()).days + 1
-        days_to_lookup = max([days_to_lookup, MIN_DAYS_TO_LOOKUP_FOR_THE_END_OF_EVENT])
-    else:
-        days_to_lookup = MIN_DAYS_TO_LOOKUP_FOR_THE_END_OF_EVENT
-
-    datetime_end = now + datetime.timedelta(days=days_to_lookup)
-
-    next_shifts_unfiltered = schedule.final_events(
-        now, datetime_end, with_empty=False, with_gap=False, ignore_untaken_swaps=True
-    )
-    # drop events that already started
-    next_shifts = []
-    for next_shift in next_shifts_unfiltered:
-        if now < next_shift["start"]:
-            next_shifts.append(next_shift)
-
     upcoming_shifts = []
     # Add the earliest next_shift
     if len(next_shifts) > 0:

engine/apps/alerts/tests/test_notify_ical_schedule_shift.py

@@ -7,7 +7,10 @@
 import pytz
 from django.utils import timezone
 
-from apps.alerts.tasks.notify_ical_schedule_shift import notify_ical_schedule_shift
+from apps.alerts.tasks.notify_ical_schedule_shift import (
+    MIN_DAYS_TO_LOOKUP_FOR_THE_END_OF_EVENT,
+    notify_ical_schedule_shift,
+)
 from apps.schedules.ical_utils import memoized_users_in_ical
 from apps.schedules.models import CustomOnCallShift, OnCallScheduleCalendar, OnCallScheduleICal, OnCallScheduleWeb
 
@@ -371,6 +374,75 @@ def test_current_shift_changes_swap_split(
     assert "user2" in text_block if swap_taken else "user1" in text_block
 
 
+@pytest.mark.django_db
+def test_current_shift_changes_end_affected_by_swap(
+    make_organization_and_user_with_slack_identities,
+    make_user,
+    make_schedule,
+    make_on_call_shift,
+    make_shift_swap_request,
+):
+    organization, _, _, _ = make_organization_and_user_with_slack_identities()
+    user1 = make_user(organization=organization, username="user1")
+    user2 = make_user(organization=organization, username="user2")
+    # clear users pks <-> organization cache (persisting between tests)
+    memoized_users_in_ical.cache_clear()
+
+    schedule = make_schedule(
+        organization,
+        schedule_class=OnCallScheduleWeb,
+        name="test_schedule",
+        channel="channel",
+        prev_ical_file_overrides=None,
+        cached_ical_file_overrides=None,
+    )
+
+    today = timezone.now().replace(hour=0, minute=0, second=0, microsecond=0)
+    duration = timezone.timedelta(days=3)
+    data = {
+        "start": today,
+        "rotation_start": today,
+        "duration": duration,
+        "priority_level": 1,
+        "frequency": CustomOnCallShift.FREQUENCY_WEEKLY,
+        "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([[user1]])
+
+    # setup in progress swap request
+    swap_start = today + timezone.timedelta(days=1)
+    swap_end = today + timezone.timedelta(days=2)
+    make_shift_swap_request(
+        schedule,
+        user1,
+        swap_start=swap_start,
+        swap_end=swap_end,
+        benefactor=user2,
+    )
+
+    schedule.refresh_ical_file()
+
+    # setup empty current shifts before checking/triggering for notifications
+    schedule.current_shifts = json.dumps({}, default=str)
+    schedule.empty_oncall = False
+    schedule.save()
+
+    with patch("apps.slack.client.SlackClient.chat_postMessage") as mock_slack_api_call:
+        notify_ical_schedule_shift(schedule.pk)
+
+    current_text_block = mock_slack_api_call.call_args_list[0][1]["blocks"][1]["text"]["text"]
+    assert "user1" in current_text_block
+    assert today.strftime("%Y-%m-%d") in current_text_block
+    assert swap_start.strftime("%Y-%m-%d") in current_text_block
+    next_text_block = mock_slack_api_call.call_args_list[0][1]["blocks"][2]["text"]["text"]
+    assert "user2" in next_text_block
+    assert swap_start.strftime("%Y-%m-%d") in next_text_block
+    assert swap_end.strftime("%Y-%m-%d") in next_text_block
+
+
 @pytest.mark.django_db
 def test_next_shift_changes_no_triggering_notification(
     make_organization_and_user_with_slack_identities,
@@ -424,8 +496,11 @@ def test_next_shift_changes_no_triggering_notification(
 
     schedule.refresh_ical_file()
 
-    # setup empty current shifts before checking/triggering for notifications
-    current_shifts = schedule.final_events(now, now, False, False)
+    # setup current shifts before checking/triggering for notifications
+    next_shifts = schedule.final_events(
+        now, now + datetime.timedelta(days=MIN_DAYS_TO_LOOKUP_FOR_THE_END_OF_EVENT), False, False
+    )
+    current_shifts = [e for e in next_shifts if now > e["start"]]
     schedule.current_shifts = json.dumps(current_shifts, default=str)
     schedule.empty_oncall = False
     schedule.save()

engine/apps/schedules/ical_utils.py

@@ -419,29 +419,32 @@ def parse_event_uid(string: str, sequence: str = None, recurrence_id: str = None
     source = None
     source_verbal = None
 
-    match = RE_EVENT_UID_V2.match(string)
-    if match:
-        _, pk, _, _, source = match.groups()
+    match = None
+    if string.startswith("oncall"):
+        match = RE_EVENT_UID_V2.match(string)
+        if match:
+            _, pk, _, _, source = match.groups()
+    elif string.startswith("amixr"):
+        # eventually this path would be automatically deprecated
+        # once all ical representations are refreshed
+        match = RE_EVENT_UID_V1.match(string)
+        if match:
+            _, _, _, source = match.groups()
     else:
         match = RE_EVENT_UID_EXPORT.match(string)
         if match:
             pk, _, _ = match.groups()
-        else:
-            # eventually this path would be automatically deprecated
-            # once all ical representations are refreshed
-            match = RE_EVENT_UID_V1.match(string)
-            if match:
-                _, _, _, source = match.groups()
-            else:
-                # fallback to use the UID string as the rotation ID
-                pk = string
-                # in ical imported calendars, sequence and/or recurrence_id
-                # distinguish main recurring event vs instance modification
-                # (see https://icalendar.org/iCalendar-RFC-5545/3-8-4-4-recurrence-id.html)
-                if sequence:
-                    pk = f"{pk}_{sequence}"
-                if recurrence_id:
-                    pk = f"{pk}_{recurrence_id}"
+
+    if not match:
+        # fallback to use the UID string as the rotation ID
+        pk = string
+        # in ical imported calendars, sequence and/or recurrence_id
+        # distinguish main recurring event vs instance modification
+        # (see https://icalendar.org/iCalendar-RFC-5545/3-8-4-4-recurrence-id.html)
+        if sequence:
+            pk = f"{pk}_{sequence}"
+        if recurrence_id:
+            pk = f"{pk}_{recurrence_id}"
 
     if source is not None:
         source = int(source)