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

Fix for user appearing in shift rotation when they should not be (#5064)

Browse source 
# What this PR does
Fix calculation for interval when building ical events for schedules
with end dates and masked days. Add a test which makes it easier to test
different cases that can occur.

## Which issue(s) this PR closes

Related to https://github.com/grafana/support-escalations/issues/12388

<!--
*Note*: If you want the issue to be auto-closed once the PR is merged,
change "Related to" to "Closes" in the line above.
If you have more than one GitHub issue that this PR closes, be sure to
preface
each issue link with a [closing
keyword](https://docs.github.com/en/get-started/writing-on-github/working-with-advanced-formatting/using-keywords-in-issues-and-pull-requests#linking-a-pull-request-to-an-issue).
This ensures that the issue(s) are auto-closed once the PR has been
merged.
-->

## Checklist

- [x] Unit, integration, and e2e (if applicable) tests updated
- [x] Documentation added (or `pr:no public docs` PR label added if not
required)
- [x] Added the relevant release notes label (see labels prefixed w/
`release:`). These labels dictate how your PR will
    show up in the autogenerated release notes.

---------

Co-authored-by: Matias Bordese <mbordese@gmail.com>
This commit is contained in:
Michael Derynck 2024-10-03 14:04:55 -06:00 • committed by GitHub
parent ee2ae50f27
commit c65a3c9cea
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
3 changed files with 581 additions and 18 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

51
engine/apps/schedules/models/custom_on_call_shift.py
View file

@ -323,38 +323,53 @@ class CustomOnCallShift(models.Model):
# we may need to iterate several times over users until we get a seen combination
# use the group index as reference since user groups could repeat in the queue
cycle_user_groups = itertools.cycle(range(len(users_queue)))
orig_start = last_start = start
previous_day = None
all_rotations_checked = False
# we need to go through each individual day
day_by_day_rrule = copy.deepcopy(self.event_ical_rules)
day_by_day_rrule["interval"] = 1
for user_group_id in cycle_user_groups:
for i in range(self.interval):
if not start: # means that rotation ends before next event starts
all_rotations_checked = True
break
last_start = start
day = CustomOnCallShift.ICAL_WEEKDAY_MAP[start.weekday()]
# double-check day is valid (when until is set, we may get unexpected days)
if day in self.by_day:
if not start:
# means that rotation ended before next event starts
# keep iterating to track missing combinations and get the right week_interval
if previous_day is None:
day = self.by_day[0]
else:
previous_index = self.by_day.index(previous_day)
day = self.by_day[(previous_index + 1) % len(self.by_day)]
if (user_group_id, day, i) in combinations:
all_rotations_checked = True
break
starting_dates.append(start)
combinations.append((user_group_id, day, i))
# get next event date following the original rule
event_ical = self.generate_ical(start, 1, None, 1, time_zone, custom_rrule=day_by_day_rrule)
start = self.get_rotation_date(event_ical, get_next_date=True, interval=1)
previous_day = day
else:
day = CustomOnCallShift.ICAL_WEEKDAY_MAP[start.weekday()]
# double-check day is valid (when until is set, we may get unexpected days)
if day in self.by_day:
if (user_group_id, day, i) in combinations:
all_rotations_checked = True
break
starting_dates.append(start)
combinations.append((user_group_id, day, i))
previous_day = day
# get next event date following the original rule
event_ical = self.generate_ical(start, 1, None, 1, time_zone, custom_rrule=day_by_day_rrule)
start = self.get_rotation_date(event_ical, get_next_date=True, interval=1)
if all_rotations_checked:
break
week_interval = 1
if orig_start and last_start:
# number of weeks used to cover all combinations
week_interval = ((last_start - orig_start).days // 7) or 1
# interval is given by the number of weeks required to cover all combinations
week_interval = (len(combinations) // len(self.by_day)) or 1
counter = 1
for (user_group_id, day, _), start in zip(combinations, starting_dates):
for (user_group_id, day, _), start in itertools.zip_longest(combinations, starting_dates, fillvalue=None):
if not start:
# means that rotation ended before next event starts, no more events to generate
break
users = users_queue[user_group_id]
for user_counter, user in enumerate(users, start=1):
# setup weekly events, for each user group/day combinations,

474
engine/apps/schedules/tests/custom_shift_test_cases.py Normal file
View file

@ -0,0 +1,474 @@
from typing import List, Tuple
from apps.schedules.models import CustomOnCallShift
"""
Simplified shift definition to test that shifts contain the correct users.
This test is only checking the start date of the shift is landing on the correct day.
If you need a test for precise hours that should be a different test so that these cases
can stay easy to define.
Define custom shift test case parameters:
users_per_group - Recurrence groups, generates users for each group based on number, Users are named A-Z
shift_start - Date to start shift
day_mask - Which days are included, None to include all
total_days - Length of shift (end = start + total_days)
frequency - Recurrence frequency
interval - Recurrence interval
expected_result - Dict where each day displays the users that should be scheduled
"""
CUSTOM_SHIFT_TEST_CASES: List[Tuple] = [
(
# 0 - Original Test Case
[1, 1, 1],
"2024-08-01",
["FR"],
21,
CustomOnCallShift.FREQUENCY_DAILY,
1,
{
"2024-08-02": "A",
"2024-08-09": "B",
"2024-08-16": "C",
},
),
(
# 1 - Weekdays Start Monday Daily
[1, 1, 1, 1, 1],
"2024-08-05",
["MO", "TU", "WE", "TH", "FR"],
14,
CustomOnCallShift.FREQUENCY_DAILY,
1,
{
"2024-08-05": "A",
"2024-08-06": "B",
"2024-08-07": "C",
"2024-08-08": "D",
"2024-08-09": "E",
"2024-08-12": "A",
"2024-08-13": "B",
"2024-08-14": "C",
"2024-08-15": "D",
"2024-08-16": "E",
},
),
(
# 2 - Weekdays Start Monday Daily Interval 2
[1, 1, 1, 1],
"2024-08-05",
["MO", "TU", "WE", "TH", "FR"],
14,
CustomOnCallShift.FREQUENCY_DAILY,
2,
{
"2024-08-05": "A",
"2024-08-06": "A",
"2024-08-07": "B",
"2024-08-08": "B",
"2024-08-09": "C",
"2024-08-12": "C",
"2024-08-13": "D",
"2024-08-14": "D",
"2024-08-15": "A",
"2024-08-16": "A",
},
),
(
# 3 - Weekdays Start Monday Weekly
[1, 1, 1, 1, 1],
"2024-08-05",
["MO", "TU", "WE", "TH", "FR"],
14,
CustomOnCallShift.FREQUENCY_WEEKLY,
1,
{
"2024-08-05": "A",
"2024-08-06": "A",
"2024-08-07": "A",
"2024-08-08": "A",
"2024-08-09": "A",
"2024-08-12": "B",
"2024-08-13": "B",
"2024-08-14": "B",
"2024-08-15": "B",
"2024-08-16": "B",
},
),
(
# 4 - Weekdays Start Monday Monthly
[1, 1, 1, 1, 1],
"2024-08-26",
["MO", "TU", "WE", "TH", "FR"],
14,
CustomOnCallShift.FREQUENCY_MONTHLY,
1,
{
"2024-08-26": "A",
"2024-08-27": "A",
"2024-08-28": "A",
"2024-08-29": "A",
"2024-08-30": "A",
"2024-09-02": "B",
"2024-09-03": "B",
"2024-09-04": "B",
"2024-09-05": "B",
"2024-09-06": "B",
},
),
(
# 5 - Weekdays Start Thursday Daily
[1, 1, 1, 1, 1],
"2024-08-08",
["MO", "TU", "WE", "TH", "FR"],
14,
CustomOnCallShift.FREQUENCY_DAILY,
1,
{
"2024-08-08": "A",
"2024-08-09": "B",
"2024-08-12": "C",
"2024-08-13": "D",
"2024-08-14": "E",
"2024-08-15": "A",
"2024-08-16": "B",
"2024-08-19": "C",
"2024-08-20": "D",
"2024-08-21": "E",
},
),
(
# 6 - Weekdays Start Thursday Weekly
[1, 1, 1, 1, 1],
"2024-08-08",
["MO", "TU", "WE", "TH", "FR"],
14,
CustomOnCallShift.FREQUENCY_WEEKLY,
1,
{
"2024-08-08": "A",
"2024-08-09": "A",
"2024-08-12": "B",
"2024-08-13": "B",
"2024-08-14": "B",
"2024-08-15": "B",
"2024-08-16": "B",
"2024-08-19": "C",
"2024-08-20": "C",
"2024-08-21": "C",
},
),
(
# 7 - Weekdays Start Thursday Monthly
[1, 1, 1, 1, 1],
"2024-08-29",
["MO", "TU", "WE", "TH", "FR"],
14,
CustomOnCallShift.FREQUENCY_MONTHLY,
1,
{
"2024-08-29": "A",
"2024-08-30": "A",
"2024-09-02": "B",
"2024-09-03": "B",
"2024-09-04": "B",
"2024-09-05": "B",
"2024-09-06": "B",
"2024-09-09": "B",
"2024-09-10": "B",
"2024-09-11": "B",
},
),
(
# 8 - All Days uneven groups
[2, 1],
"2024-08-14",
None,
9,
CustomOnCallShift.FREQUENCY_DAILY,
1,
{
"2024-08-14": "AB",
"2024-08-15": "C",
"2024-08-16": "AB",
"2024-08-17": "C",
"2024-08-18": "AB",
"2024-08-19": "C",
"2024-08-20": "AB",
"2024-08-21": "C",
"2024-08-22": "AB",
},
),
(
# 9 - Weekends Start Saturday
[1, 1, 1],
"2024-08-03",
["SA", "SU"],
15,
CustomOnCallShift.FREQUENCY_DAILY,
1,
{
"2024-08-03": "A",
"2024-08-04": "B",
"2024-08-10": "C",
"2024-08-11": "A",
"2024-08-17": "B",
},
),
(
# 10 - Weekends Start Saturday Users > Shifts
[1, 1, 1, 1, 1],
"2024-08-03",
["SA", "SU"],
14,
CustomOnCallShift.FREQUENCY_DAILY,
1,
{
"2024-08-03": "A",
"2024-08-04": "B",
"2024-08-10": "C",
"2024-08-11": "D",
},
),
(
# 11 - Weekends Start Saturday Users > Shifts
[1, 1, 1, 1, 1],
"2024-08-03",
["SA", "SU"],
14,
CustomOnCallShift.FREQUENCY_DAILY,
1,
{
"2024-08-03": "A",
"2024-08-04": "B",
"2024-08-10": "C",
"2024-08-11": "D",
},
),
(
# 12 - Weekends Start Thursday
[1],
"2024-08-01",
["SA", "SU"],
5,
CustomOnCallShift.FREQUENCY_DAILY,
1,
{
"2024-08-03": "A",
"2024-08-04": "A",
},
),
(
# 13 - Weekends Start Thursday
[1, 1, 1],
"2024-08-01",
["SA", "SU"],
17,
CustomOnCallShift.FREQUENCY_DAILY,
1,
{
"2024-08-03": "A",
"2024-08-04": "B",
"2024-08-10": "C",
"2024-08-11": "A",
"2024-08-17": "B",
},
),
(
# 14 - Weekends Start Thursday Users > Shifts
[1, 1, 1, 1, 1, 1],
"2024-08-01",
["SA", "SU"],
17,
CustomOnCallShift.FREQUENCY_DAILY,
1,
{
"2024-08-03": "A",
"2024-08-04": "B",
"2024-08-10": "C",
"2024-08-11": "D",
"2024-08-17": "E",
},
),
(
# 15 - Weekends Start Thursday Interval 2
[1, 1, 1],
"2024-08-01",
["SA", "SU"],
17,
CustomOnCallShift.FREQUENCY_DAILY,
2,
{
"2024-08-03": "A",
"2024-08-04": "A",
"2024-08-10": "B",
"2024-08-11": "B",
"2024-08-17": "C",
},
),
(
# 16 - Weekends Start Saturday Weekly
[1, 1, 1],
"2024-08-01",
["SA", "SU"],
18,
CustomOnCallShift.FREQUENCY_WEEKLY,
1,
{
"2024-08-03": "A",
"2024-08-04": "A",
"2024-08-10": "B",
"2024-08-11": "B",
"2024-08-17": "C",
"2024-08-18": "C",
},
),
(
# 17 - Weekends Start Saturday Weekly Interval 2
[1, 1, 1],
"2024-08-01",
["SA", "SU"],
18,
CustomOnCallShift.FREQUENCY_WEEKLY,
2,
{
"2024-08-03": "A",
"2024-08-04": "A",
"2024-08-17": "B",
"2024-08-18": "B",
},
),
(
# 18 - Weekends Start Thursday Monthly
[1, 1, 1],
"2024-08-29",
["SA", "SU"],
18,
CustomOnCallShift.FREQUENCY_MONTHLY,
1,
{
"2024-08-31": "A",
"2024-09-01": "B",
"2024-09-07": "B",
"2024-09-08": "B",
"2024-09-14": "B",
"2024-09-15": "B",
},
),
(
# 19 - No days (==All)
[1, 1, 1, 1, 1, 1, 1, 1, 1, 1],
"2024-09-30",
[],
18,
CustomOnCallShift.FREQUENCY_DAILY,
1,
{
"2024-09-30": "A",
"2024-10-01": "B",
"2024-10-02": "C",
"2024-10-03": "D",
"2024-10-04": "E",
"2024-10-05": "F",
"2024-10-06": "G",
"2024-10-07": "H",
"2024-10-08": "I",
"2024-10-09": "J",
"2024-10-10": "A",
"2024-10-11": "B",
"2024-10-12": "C",
"2024-10-13": "D",
"2024-10-14": "E",
"2024-10-15": "F",
"2024-10-16": "G",
"2024-10-17": "H",
},
),
(
# 20 - All days
[1, 1, 1, 1, 1, 1, 1, 1, 1, 1],
"2024-09-30",
["MO", "TU", "WE", "TH", "FR", "SA", "SU"],
18,
CustomOnCallShift.FREQUENCY_DAILY,
1,
{
"2024-09-30": "A",
"2024-10-01": "B",
"2024-10-02": "C",
"2024-10-03": "D",
"2024-10-04": "E",
"2024-10-05": "F",
"2024-10-06": "G",
"2024-10-07": "H",
"2024-10-08": "I",
"2024-10-09": "J",
"2024-10-10": "A",
"2024-10-11": "B",
"2024-10-12": "C",
"2024-10-13": "D",
"2024-10-14": "E",
"2024-10-15": "F",
"2024-10-16": "G",
"2024-10-17": "H",
},
),
(
# 21 - No days (==All) 7 Interval
# Note: This test verifies expected behavior not the correct result,
# it *should* be equivalent to #22, but it is not using the daily_by_day logic
[1, 1, 1, 1, 1, 1, 1, 1, 1, 1],
"2024-09-30",
[],
18,
CustomOnCallShift.FREQUENCY_DAILY,
7,
{
"2024-09-30": "A",
"2024-10-07": "B",
"2024-10-14": "C",
"2024-10-15": "D",
"2024-10-16": "E",
"2024-10-17": "F",
},
),
(
# 22 - All days 7 Interval
[1, 1, 1, 1, 1, 1, 1, 1, 1, 1],
"2024-09-30",
["MO", "TU", "WE", "TH", "FR", "SA", "SU"],
18,
CustomOnCallShift.FREQUENCY_DAILY,
7,
{
"2024-09-30": "A",
"2024-10-01": "A",
"2024-10-02": "A",
"2024-10-03": "A",
"2024-10-04": "A",
"2024-10-05": "A",
"2024-10-06": "A",
"2024-10-07": "B",
"2024-10-08": "B",
"2024-10-09": "B",
"2024-10-10": "B",
"2024-10-11": "B",
"2024-10-12": "B",
"2024-10-13": "B",
"2024-10-14": "C",
"2024-10-15": "C",
"2024-10-16": "C",
"2024-10-17": "C",
},
),
(
# 23 - Single User 1 day
[1],
"2024-08-01",
["MO"],
14,
CustomOnCallShift.FREQUENCY_DAILY,
1,
{"2024-08-05": "A", "2024-08-12": "A"},
),
]

74
engine/apps/schedules/tests/test_custom_on_call_shift.py
View file

@ -1,12 +1,17 @@
import datetime
from calendar import monthrange
from collections import defaultdict
from unittest.mock import patch
from zoneinfo import ZoneInfo
import icalendar
import pytest
from django.utils import timezone
from recurring_ical_events import UnfoldableCalendar
from apps.schedules.ical_utils import list_users_to_notify_from_ical
from apps.schedules.models import CustomOnCallShift, OnCallSchedule, OnCallScheduleCalendar, OnCallScheduleWeb
from apps.schedules.tests.custom_shift_test_cases import CUSTOM_SHIFT_TEST_CASES
@pytest.mark.django_db
@ -1826,3 +1831,72 @@ def test_refresh_schedule(make_organization_and_user, make_schedule, make_on_cal
assert mock_refresh_final.apply_async.called
assert schedule.cached_ical_file_primary is not None
assert schedule.cached_ical_file_overrides is not None
@pytest.mark.parametrize(
"users_per_group, shift_start, day_mask, total_days, frequency, interval, expected_result", CUSTOM_SHIFT_TEST_CASES
)
@pytest.mark.django_db
def test_ical_shift_generation(
make_organization,
make_user_for_organization,
make_schedule,
make_on_call_shift,
users_per_group,
shift_start,
day_mask,
total_days,
frequency,
interval,
expected_result,
):
organization = make_organization()
schedule = make_schedule(
organization,
schedule_class=OnCallScheduleWeb,
name="test_web_schedule",
)
total_users = sum(users_per_group)
users = [make_user_for_organization(organization, username=chr(i + 64)) for i in range(1, total_users + 1)]
start = datetime.datetime.strptime(shift_start, "%Y-%m-%d").replace(tzinfo=ZoneInfo("UTC"))
data = {
"start": start,
"rotation_start": start,
"until": start + timezone.timedelta(days=total_days),
"duration": timezone.timedelta(hours=12),
"frequency": frequency,
"by_day": day_mask,
"schedule": schedule,
"interval": interval,
"priority_level": 1,
"week_start": CustomOnCallShift.MONDAY,
}
on_call_shift = make_on_call_shift(
organization=organization, shift_type=CustomOnCallShift.TYPE_ROLLING_USERS_EVENT, **data
)
rolling_users = []
start_user = 0
for count in users_per_group:
end = start_user + count
rolling_users.append(users[start_user:end])
start_user = end
on_call_shift.add_rolling_users(rolling_users)
query_start = start
query_end = data["until"]
calendar = icalendar.Calendar.from_ical(schedule._ical_file_primary)
events = UnfoldableCalendar(calendar).between(query_start, query_end)
day_events = defaultdict(str)
for event in events:
event_start = event["DTSTART"].dt
event_summary = event["SUMMARY"].strip()[-1]
event_date = event_start.date().strftime("%Y-%m-%d")
day_events[event_date] += event_summary
for k, v in day_events.items():
day_events[k] = "".join(sorted(v))
assert day_events == expected_result