centralcloud/oncall-engine
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Fix slack notification for shift end affected by taken swap (#3092)

Browse source 
Related to https://github.com/grafana/oncall/issues/3096
This commit is contained in:
Matias Bordese 2023-10-02 09:56:07 -03:00 • committed by GitHub
parent 6d7fc16fbd
commit 29eae9b2c6
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
4 changed files with 136 additions and 47 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

6
CHANGELOG.md
View file

@ -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

55
engine/apps/alerts/tasks/notify_ical_schedule_shift.py
View file

@ -22,6 +22,9 @@ if TYPE_CHECKING:
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:

81
engine/apps/alerts/tests/test_notify_ical_schedule_shift.py
View file

@ -7,7 +7,10 @@ import pytest
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()

41
engine/apps/schedules/ical_utils.py
View file

@ -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)