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

Filter out untaken swaps from final schedule and shift notifications (#2748)

Browse source 
Avoid creating (or notifying) about potential event splits resulting
from untaken swap requests.
This commit is contained in:
Matias Bordese 2023-08-04 14:43:54 -03:00 • committed by GitHub
parent bc78fc29e5
commit bb9f647608
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
7 changed files with 205 additions and 24 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

1
CHANGELOG.md
View file

@ -15,6 +15,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
### Changed
- Add stack slug to organization options for direct paging Slash command by @vadimkerr ([#2743](https://github.com/grafana/oncall/pull/2743))
- Avoid creating (or notifying about) potential event splits resulting from untaken swap requests ([#2748](https://github.com/grafana/oncall/pull/2748))
### Fixed

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

@ -81,7 +81,7 @@ def notify_ical_schedule_shift(schedule_pk):
now = datetime.datetime.now(timezone.utc)
current_shifts = schedule.final_events(now, now, with_empty=False, with_gap=False)
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
@ -119,7 +119,9 @@ def notify_ical_schedule_shift(schedule_pk):
datetime_end = now + datetime.timedelta(days=days_to_lookup)
next_shifts_unfiltered = schedule.final_events(now, datetime_end, with_empty=False, with_gap=False)
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:

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

@ -305,6 +305,71 @@ def test_current_shift_changes_trigger_notification(
assert mock_slack_api_call.called
@pytest.mark.django_db
@pytest.mark.parametrize("swap_taken", [False, True])
def test_current_shift_changes_swap_split(
make_organization_and_user_with_slack_identities,
make_user,
make_schedule,
make_on_call_shift,
make_shift_swap_request,
swap_taken,
):
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(hours=23, minutes=59, seconds=59)
data = {
"start": today,
"rotation_start": today,
"duration": duration,
"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([[user1]])
# setup in progress swap request
swap_request = make_shift_swap_request(
schedule,
user1,
swap_start=today,
swap_end=today + timezone.timedelta(days=2),
)
if swap_taken:
swap_request.benefactor = user2
swap_request.save()
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.slack_client.SlackClientWithErrorHandling.api_call") as mock_slack_api_call:
notify_ical_schedule_shift(schedule.pk)
text_block = mock_slack_api_call.call_args_list[0][1]["blocks"][0]["text"]["text"]
assert "user2" in text_block if swap_taken else "user1" in text_block
@pytest.mark.django_db
def test_next_shift_changes_no_triggering_notification(
make_organization_and_user_with_slack_identities,

25
engine/apps/schedules/models/on_call_schedule.py
View file

@ -344,6 +344,7 @@ class OnCallSchedule(PolymorphicModel):
with_gap: bool = False,
filter_by: str | None = None,
all_day_datetime: bool = False,
ignore_untaken_swaps: bool = False,
from_cached_final: bool = False,
) -> ScheduleEvents:
"""Return filtered events from schedule."""
@ -401,7 +402,9 @@ class OnCallSchedule(PolymorphicModel):
events = self._merge_events(events)
# annotate events with swap request details swapping users as needed
events = self._apply_swap_requests(events, datetime_start, datetime_end)
events = self._apply_swap_requests(
events, datetime_start, datetime_end, ignore_untaken_swaps=ignore_untaken_swaps
)
return events
@ -411,10 +414,16 @@ class OnCallSchedule(PolymorphicModel):
datetime_end: datetime.datetime,
with_empty: bool = True,
with_gap: bool = True,
ignore_untaken_swaps: bool = False,
) -> ScheduleEvents:
"""Return schedule final events, after resolving shifts and overrides."""
events = self.filter_events(
datetime_start, datetime_end, with_empty=with_empty, with_gap=with_gap, all_day_datetime=True
datetime_start,
datetime_end,
with_empty=with_empty,
with_gap=with_gap,
all_day_datetime=True,
ignore_untaken_swaps=ignore_untaken_swaps,
)
events = self._resolve_schedule(events, datetime_start, datetime_end)
return events
@ -442,7 +451,7 @@ class OnCallSchedule(PolymorphicModel):
# setup calendar with final schedule shift events
calendar = create_base_icalendar(self.name)
events = self.final_events(datetime_start, datetime_end)
events = self.final_events(datetime_start, datetime_end, ignore_untaken_swaps=True)
updated_ids = set()
for e in events:
for u in e["users"]:
@ -647,7 +656,11 @@ class OnCallSchedule(PolymorphicModel):
}
def _apply_swap_requests(
self, events: ScheduleEvents, datetime_start: datetime.datetime, datetime_end: datetime.datetime
self,
events: ScheduleEvents,
datetime_start: datetime.datetime,
datetime_end: datetime.datetime,
ignore_untaken_swaps: bool = False,
) -> ScheduleEvents:
"""Apply swap requests details to schedule events."""
# get swaps requests affecting this schedule / time range
@ -663,8 +676,8 @@ class OnCallSchedule(PolymorphicModel):
# apply swaps sequentially
for swap in swaps:
if swap.is_past_due:
# ignore untaken expired requests
if swap.is_past_due or (ignore_untaken_swaps and not swap.is_taken):
# ignore expired requests, or untaken if specified
continue
i = 0
while i < len(events):

2
engine/apps/schedules/models/shift_swap_request.py
View file

@ -127,7 +127,7 @@ class ShiftSwapRequest(models.Model):
@property
def is_past_due(self) -> bool:
return timezone.now() > self.swap_start
return not self.is_taken and timezone.now() > self.swap_start
@property
def is_open(self) -> bool:

122
engine/apps/schedules/tests/test_on_call_schedule.py
View file

@ -2169,21 +2169,36 @@ def test_swap_request_split_both(
with patch("apps.schedules.models.on_call_schedule.EXPORT_WINDOW_DAYS_BEFORE", 0):
schedule.refresh_ical_final_schedule()
assert schedule.cached_ical_final_schedule
expected_events = [
# start, end, user
(start, start + duration, user.username), # today shift unchanged
(start + timezone.timedelta(days=1), start + timezone.timedelta(days=1, hours=1), user.username), # first split
(
start + timezone.timedelta(days=1, hours=1),
start + timezone.timedelta(days=1, hours=2),
other_user.username if swap_taken else user.username,
), # second split
(
start + timezone.timedelta(days=1, hours=2),
start + timezone.timedelta(days=1, hours=3),
user.username,
), # third split
]
if swap_taken:
expected_events = [
# start, end, user
(start, start + duration, user.username), # today shift unchanged
(
start + timezone.timedelta(days=1),
start + timezone.timedelta(days=1, hours=1),
user.username,
), # first split
(
start + timezone.timedelta(days=1, hours=1),
start + timezone.timedelta(days=1, hours=2),
other_user.username if swap_taken else user.username,
), # second split
(
start + timezone.timedelta(days=1, hours=2),
start + timezone.timedelta(days=1, hours=3),
user.username,
), # third split
]
else:
expected_events = [
# start, end, user
(start, start + duration, user.username), # today shift unchanged
(
start + timezone.timedelta(days=1),
start + timezone.timedelta(days=1) + duration,
user.username,
), # no split
]
calendar = icalendar.Calendar.from_ical(schedule.cached_ical_final_schedule)
for component in calendar.walk():
if component.name == ICAL_COMPONENT_VEVENT:
@ -2195,6 +2210,83 @@ def test_swap_request_split_both(
assert event in expected_events
@pytest.mark.django_db
@pytest.mark.parametrize("swap_taken", [False, True])
def test_swap_request_ignore_untaken(
make_organization,
make_user_for_organization,
make_schedule,
make_on_call_shift,
make_shift_swap_request,
swap_taken,
):
organization = make_organization()
user = make_user_for_organization(organization)
other_user = make_user_for_organization(organization)
schedule = make_schedule(organization, schedule_class=OnCallScheduleWeb)
today = timezone.now().replace(hour=0, minute=0, second=0, microsecond=0)
start = today + timezone.timedelta(hours=12)
duration = timezone.timedelta(hours=3)
data = {
"start": start,
"rotation_start": start,
"duration": duration,
"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]])
tomorrow = today + timezone.timedelta(days=1)
# setup swap request
swap_request = make_shift_swap_request(
schedule,
user,
swap_start=tomorrow + timezone.timedelta(hours=13),
swap_end=tomorrow + timezone.timedelta(hours=14),
)
if swap_taken:
swap_request.take(other_user)
# set flag to ignore untaken swaps
events = schedule.filter_events(today, today + timezone.timedelta(days=2), ignore_untaken_swaps=True)
if swap_taken:
expected = [
# start, end, swap requested
(start, start + duration, False), # today shift unchanged
(start + timezone.timedelta(days=1), start + timezone.timedelta(days=1, hours=1), False), # first split
(
start + timezone.timedelta(days=1, hours=1),
start + timezone.timedelta(days=1, hours=2),
True,
), # second split
(
start + timezone.timedelta(days=1, hours=2),
start + timezone.timedelta(days=1, hours=3),
False,
), # third split
]
else:
expected = [
# start, end, swap requested
(start, start + duration, False), # today shift unchanged
(start + timezone.timedelta(days=1), start + timezone.timedelta(days=1) + duration, False), # no split
]
returned = [(e["start"], e["end"], bool(e["users"][0].get("swap_request", False))) for e in events]
assert returned == expected
# check swap request details
if swap_taken:
assert events[2]["users"][0]["swap_request"]["pk"] == swap_request.public_primary_key
assert events[2]["users"][0]["pk"] == other_user.public_primary_key
assert events[2]["users"][0]["swap_request"]["user"]["pk"] == user.public_primary_key
@pytest.mark.django_db
@pytest.mark.parametrize("swap_taken", [False, True])
def test_swap_request_whole_shift(

8
engine/apps/schedules/tests/test_shift_swap_request.py
View file

@ -46,6 +46,14 @@ def test_status_taken(shift_swap_request_setup) -> None:
assert ssr.status == ShiftSwapRequest.Statuses.TAKEN
assert ssr.is_taken is True
# taken in the past it's still taken
now = timezone.now()
ssr.swap_start = now - timezone.timedelta(days=2)
ssr.save()
assert ssr.status == ShiftSwapRequest.Statuses.TAKEN
assert ssr.is_taken is True
assert ssr.is_past_due is False
@pytest.mark.django_db
def test_status_past_due(shift_swap_request_setup) -> None: