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

Merge pull request #720 from grafana/matiasb/daily-shifts-by-day

Browse source 
Enable daily/by-day shifts option for web schedules
This commit is contained in:
Maxim Mordasov 2022-11-04 14:23:09 +00:00 • committed by GitHub
commit 387b334d2a
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
12 changed files with 358 additions and 18 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

2
Makefile
View file

@ -11,7 +11,7 @@ PYTEST = $(ENV)/bin/pytest
DOCKER_FILE ?= docker-compose-developer.yml
define setup_engine_env
export `grep -v '^#' .env | xargs -0` && cd engine
export `grep -v '^#' .env.dev | xargs -0` && cd engine
endef
$(ENV):

6
engine/apps/api/serializers/on_call_shifts.py
View file

@ -114,8 +114,12 @@ class OnCallShiftSerializer(EagerLoadingMixin, serializers.ModelSerializer):
raise serializers.ValidationError(
{"frequency": ["Cannot set 'frequency' for shifts with type 'override'"]}
)
if frequency != CustomOnCallShift.FREQUENCY_WEEKLY and by_day:
if frequency not in (CustomOnCallShift.FREQUENCY_WEEKLY, CustomOnCallShift.FREQUENCY_DAILY) and by_day:
raise serializers.ValidationError({"by_day": ["Cannot set days value for this frequency type"]})
if frequency == CustomOnCallShift.FREQUENCY_DAILY and by_day and interval > len(by_day):
raise serializers.ValidationError(
{"interval": ["Interval must be less than or equal to the number of selected days"]}
)
def _validate_rotation_start(self, shift_start, rotation_start):
if rotation_start < shift_start:

25
engine/apps/api/tests/test_oncall_shift.py
View file

@ -739,7 +739,7 @@ def test_create_on_call_shift_invalid_data_by_day(on_call_shift_internal_api_set
assert response.status_code == status.HTTP_400_BAD_REQUEST
assert response.data["by_day"][0] == "Cannot set days value for non-recurrent shifts"
# by_day with non-weekly frequency
# by_day with non-weekly/non-daily frequency
data = {
"title": "Test Shift 2",
"type": CustomOnCallShift.TYPE_ROLLING_USERS_EVENT,
@ -749,7 +749,7 @@ def test_create_on_call_shift_invalid_data_by_day(on_call_shift_internal_api_set
"shift_end": (start_date + timezone.timedelta(hours=2)).strftime("%Y-%m-%dT%H:%M:%SZ"),
"rotation_start": start_date.strftime("%Y-%m-%dT%H:%M:%SZ"),
"until": None,
"frequency": CustomOnCallShift.FREQUENCY_DAILY,
"frequency": CustomOnCallShift.FREQUENCY_MONTHLY,
"interval": None,
"by_day": [CustomOnCallShift.ICAL_WEEKDAY_MAP[CustomOnCallShift.MONDAY]],
"rolling_users": [[user1.public_primary_key]],
@ -789,6 +789,27 @@ def test_create_on_call_shift_invalid_data_interval(on_call_shift_internal_api_s
assert response.status_code == status.HTTP_400_BAD_REQUEST
assert response.data["interval"][0] == "Cannot set interval for non-recurrent shifts"
# by_day, daily, interval > len(by_day)
data = {
"title": "Test Shift 2",
"type": CustomOnCallShift.TYPE_ROLLING_USERS_EVENT,
"schedule": schedule.public_primary_key,
"priority_level": 0,
"shift_start": start_date.strftime("%Y-%m-%dT%H:%M:%SZ"),
"shift_end": (start_date + timezone.timedelta(hours=2)).strftime("%Y-%m-%dT%H:%M:%SZ"),
"rotation_start": start_date.strftime("%Y-%m-%dT%H:%M:%SZ"),
"until": None,
"frequency": CustomOnCallShift.FREQUENCY_DAILY,
"interval": 2,
"by_day": [CustomOnCallShift.ICAL_WEEKDAY_MAP[CustomOnCallShift.MONDAY]],
"rolling_users": [[user1.public_primary_key]],
}
response = client.post(url, data, format="json", **make_user_auth_headers(user1, token))
assert response.status_code == status.HTTP_400_BAD_REQUEST
assert response.data["interval"][0] == "Interval must be less than or equal to the number of selected days"
@pytest.mark.django_db
def test_create_on_call_shift_invalid_data_shift_end(on_call_shift_internal_api_setup, make_user_auth_headers):

8
engine/apps/api/views/schedule.py
View file

@ -1,3 +1,5 @@
import datetime
import pytz
from django.core.exceptions import ObjectDoesNotExist
from django.db.models import Count, OuterRef, Subquery
@ -295,7 +297,11 @@ class ScheduleView(
users = {u: None for u in schedule.related_users()}
for e in events:
user = e["users"][0]["pk"] if e["users"] else None
if user is not None and users.get(user) is None and e["end"] > now:
event_end = e["end"]
if not isinstance(event_end, datetime.datetime):
# all day events end is a date, make it a datetime for comparison
event_end = datetime.datetime.combine(event_end, datetime.datetime.min.time(), tzinfo=pytz.UTC)
if user is not None and users.get(user) is None and event_end > now:
users[user] = e
result = {"users": users}

10
engine/apps/public_api/serializers/on_call_shifts.py
View file

@ -129,6 +129,7 @@ class CustomOnCallShiftSerializer(EagerLoadingMixin, serializers.ModelSerializer
self._validate_frequency_daily(
validated_data["type"],
validated_data.get("frequency"),
validated_data.get("interval"),
validated_data.get("by_day"),
validated_data.get("by_monthday"),
)
@ -201,14 +202,16 @@ class CustomOnCallShiftSerializer(EagerLoadingMixin, serializers.ModelSerializer
elif frequency == CustomOnCallShift.FREQUENCY_WEEKLY and week_start is None:
raise BadRequest(detail="Field 'week_start' is required for frequency type 'weekly'")
def _validate_frequency_daily(self, event_type, frequency, by_day, by_monthday):
def _validate_frequency_daily(self, event_type, frequency, interval, by_day, by_monthday):
if event_type == CustomOnCallShift.TYPE_ROLLING_USERS_EVENT:
if frequency == CustomOnCallShift.FREQUENCY_DAILY:
if by_day or by_monthday:
if by_monthday:
raise BadRequest(
detail="Day limits are temporarily disabled for on-call shifts with type 'rolling_users' "
"and frequency 'daily'"
)
if by_day and interval > len(by_day):
raise BadRequest(detail="Interval must be less than or equal to the number of selected days")
def _validate_start_rotation_from_user_index(self, type, index):
if type == CustomOnCallShift.TYPE_ROLLING_USERS_EVENT and index is None:
@ -354,9 +357,10 @@ class CustomOnCallShiftUpdateSerializer(CustomOnCallShiftSerializer):
if frequency != instance.frequency:
self._validate_frequency_and_week_start(event_type, frequency, week_start)
interval = validated_data.get("interval", instance.interval)
by_day = validated_data.get("by_day", instance.by_day)
by_monthday = validated_data.get("by_monthday", instance.by_monthday)
self._validate_frequency_daily(event_type, frequency, by_day, by_monthday)
self._validate_frequency_daily(event_type, frequency, interval, by_day, by_monthday)
if start_rotation_from_user_index != instance.start_rotation_from_user_index:
self._validate_start_rotation_from_user_index(event_type, start_rotation_from_user_index)

6
engine/apps/public_api/tests/test_on_call_shifts.py
View file

@ -40,6 +40,11 @@ invalid_field_data_8 = {
"until": "not-a-date",
}
invalid_field_data_9 = {
"frequency": CustomOnCallShift.FREQUENCY_DAILY,
"interval": 5,
}
@pytest.mark.django_db
def test_get_on_call_shift(make_organization_and_user_with_token, make_on_call_shift, make_schedule):
@ -284,6 +289,7 @@ def test_update_on_call_shift(make_organization_and_user_with_token, make_on_cal
invalid_field_data_6,
invalid_field_data_7,
invalid_field_data_8,
invalid_field_data_9,
],
)
def test_update_on_call_shift_invalid_field(make_organization_and_user_with_token, make_on_call_shift, data_to_update):

2
engine/apps/schedules/ical_events/adapter/amixr_recurring_ical_events_adapter.py
View file

@ -106,6 +106,8 @@ class AmixrRecurringIcalEventsAdapter(IcalService):
def filter_extra_days(event):
event_start, event_end = self.get_start_and_end_with_respect_to_event_type(event)
if event_start > event_end:
return False
return time_span_contains_event(start_date, end_date, event_start, event_end)
return list(filter(filter_extra_days, events))

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

@ -1,3 +1,5 @@
import copy
import itertools
import logging
import random
import string
@ -274,6 +276,70 @@ class CustomOnCallShift(models.Model):
return is_finished
def _daily_by_day_to_ical(self, time_zone, start, users_queue):
"""Create ical weekly shifts to distribute user groups combining daily + by_day.
e.g.
by_day: [WED, FRI]
users_queue: [user_group_1, user_group_2, user_group_3]
will result in the following ical shift rules:
user_group_1, weekly WED interval 3
user_group_2, weekly FRI interval 3
user_group_3, weekly WED interval 3
user_group_1, weekly FRI interval 3
user_group_2, weekly WED interval 3
user_group_3, weekly FRI interval 3
"""
result = ""
# keep tracking of (users, day) combinations, and starting dates for each
combinations = []
starting_dates = []
# 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
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()]
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)
if all_rotations_checked:
break
# number of weeks used to cover all combinations
week_interval = ((last_start - orig_start).days // 7) or 1
counter = 1
for ((user_group_id, day, _), start) in zip(combinations, starting_dates):
users = users_queue[user_group_id]
for user_counter, user in enumerate(users, start=1):
# setup weekly events, for each user group/day combinations,
# setting the right interval and the corresponding day
custom_rrule = copy.deepcopy(self.event_ical_rules)
custom_rrule["freq"] = ["WEEKLY"]
custom_rrule["interval"] = [week_interval]
custom_rrule["byday"] = [day]
custom_event_ical = self.generate_ical(
start, user_counter, user, counter, time_zone, custom_rrule=custom_rrule
)
result += custom_event_ical
counter += 1
return result
def convert_to_ical(self, time_zone="UTC", allow_empty_users=False):
result = ""
# use shift time_zone if it exists, otherwise use schedule or default time_zone
@ -299,6 +365,10 @@ class CustomOnCallShift(models.Model):
else:
start = self.get_rotation_date(event_ical)
if self.frequency == CustomOnCallShift.FREQUENCY_DAILY and self.by_day:
result = self._daily_by_day_to_ical(time_zone, start, users_queue)
all_rotation_checked = True
while not all_rotation_checked:
for counter, users in enumerate(users_queue, start=1):
if not start: # means that rotation ends before next event starts
@ -325,7 +395,7 @@ class CustomOnCallShift(models.Model):
result += self.generate_ical(self.start, user_counter, user, time_zone=time_zone)
return result
def generate_ical(self, start, user_counter=0, user=None, counter=1, time_zone="UTC"):
def generate_ical(self, start, user_counter=0, user=None, counter=1, time_zone="UTC", custom_rrule=None):
event = Event()
event["uid"] = f"oncall-{self.uuid}-PK{self.public_primary_key}-U{user_counter}-E{counter}-S{self.source}"
if user:
@ -333,7 +403,9 @@ class CustomOnCallShift(models.Model):
event.add("dtstart", self.convert_dt_to_schedule_timezone(start, time_zone))
event.add("dtend", self.convert_dt_to_schedule_timezone(start + self.duration, time_zone))
event.add("dtstamp", self.rotation_start)
if self.event_ical_rules:
if custom_rrule:
event.add("rrule", custom_rrule)
elif self.event_ical_rules:
event.add("rrule", self.event_ical_rules)
try:
event_in_ical = event.to_ical().decode("utf-8")
@ -349,7 +421,7 @@ class CustomOnCallShift(models.Model):
summary += f"{user.username} "
return summary
def get_rotation_date(self, event_ical, get_next_date=False):
def get_rotation_date(self, event_ical, get_next_date=False, interval=None):
"""Get date of the next event (for rolling_users shifts)"""
ONE_DAY = 1
ONE_HOUR = 1
@ -363,7 +435,8 @@ class CustomOnCallShift(models.Model):
current_event = Event.from_ical(event_ical)
# take shift interval, not event interval. For rolling_users shift it is not the same.
interval = self.interval or 1
if interval is None:
interval = self.interval or 1
if "rrule" in current_event:
# when triggering shift previews, there could be no rrule information yet
# (e.g. initial empty weekly rotation has no rrule set)

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

@ -336,13 +336,15 @@ class OnCallSchedule(PolymorphicModel):
resolved.append(ev)
continue
if ev["priority_level"] != current_priority:
# api/terraform shifts could be missing a priority; assume None means 0
priority = ev["priority_level"] or 0
if priority != current_priority:
# update scheduled intervals on priority change
# and start from the beginning for the new priority level
resolved.sort(key=event_start_cmp_key)
intervals = _merge_intervals(resolved)
current_interval_idx = 0
current_priority = ev["priority_level"]
current_priority = priority
if current_interval_idx >= len(intervals):
# event outside scheduled intervals, add to resolved
@ -406,8 +408,10 @@ class OnCallSchedule(PolymorphicModel):
and current["shift"]["pk"] is not None
and current["shift"]["pk"] == next_event["shift"]["pk"]
):
current["users"] += next_event["users"]
current["missing_users"] += next_event["missing_users"]
current["users"] += [u for u in next_event["users"] if u not in current["users"]]
current["missing_users"] += [
u for u in next_event["missing_users"] if u not in current["missing_users"]
]
else:
merged.append(next_event)
current = next_event

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

@ -293,6 +293,146 @@ def test_rolling_users_event_with_interval_daily(
assert len(users_on_call) == 0
@pytest.mark.django_db
def test_rolling_users_event_daily_by_day(
make_organization_and_user, make_user_for_organization, make_on_call_shift, make_schedule
):
organization, user_1 = make_organization_and_user()
user_2 = make_user_for_organization(organization)
schedule = make_schedule(organization, schedule_class=OnCallScheduleWeb)
now = timezone.now().replace(hour=0, minute=0, second=0, microsecond=0)
today_weekday = now.weekday()
delta_days = (0 - today_weekday) % 7 + (7 if today_weekday == 0 else 0)
next_week_monday = now + timezone.timedelta(days=delta_days)
# MO, WE, FR
weekdays = [0, 2, 4]
by_day = [CustomOnCallShift.ICAL_WEEKDAY_MAP[day] for day in weekdays]
data = {
"priority_level": 1,
"start": next_week_monday,
"rotation_start": next_week_monday,
"duration": timezone.timedelta(seconds=10800),
"frequency": CustomOnCallShift.FREQUENCY_DAILY,
"interval": 1,
"by_day": by_day,
"schedule": schedule,
}
rolling_users = [[user_1], [user_2]]
on_call_shift = make_on_call_shift(
organization=organization, shift_type=CustomOnCallShift.TYPE_ROLLING_USERS_EVENT, **data
)
on_call_shift.add_rolling_users(rolling_users)
date = next_week_monday + timezone.timedelta(minutes=5)
user_1_on_call_dates = [date, date + timezone.timedelta(days=4), date + timezone.timedelta(days=9)]
user_2_on_call_dates = [date + timezone.timedelta(days=2), date + timezone.timedelta(days=7)]
nobody_on_call_dates = [
date + timezone.timedelta(days=1), # TU
date + timezone.timedelta(days=3), # TH
date + timezone.timedelta(days=5), # SAT
date + timezone.timedelta(days=6), # SUN
date + timezone.timedelta(days=8), # TU
date + timezone.timedelta(days=10), # TH
date + timezone.timedelta(days=12), # SAT
]
for dt in user_1_on_call_dates:
users_on_call = list_users_to_notify_from_ical(schedule, dt)
assert len(users_on_call) == 1
assert user_1 in users_on_call
for dt in user_2_on_call_dates:
users_on_call = list_users_to_notify_from_ical(schedule, dt)
assert len(users_on_call) == 1
assert user_2 in users_on_call
for dt in nobody_on_call_dates:
users_on_call = list_users_to_notify_from_ical(schedule, dt)
assert len(users_on_call) == 0
@pytest.mark.django_db
def test_rolling_users_event_with_interval_daily_by_day(
make_organization_and_user, make_user_for_organization, make_on_call_shift, make_schedule
):
organization, user_1 = make_organization_and_user()
user_2 = make_user_for_organization(organization)
schedule = make_schedule(organization, schedule_class=OnCallScheduleWeb)
now = timezone.now().replace(hour=0, minute=0, second=0, microsecond=0)
today_weekday = now.weekday()
delta_days = (0 - today_weekday) % 7 + (7 if today_weekday == 0 else 0)
next_week_monday = now + timezone.timedelta(days=delta_days)
# MO, WE, FR
weekdays = [0, 2, 4]
by_day = [CustomOnCallShift.ICAL_WEEKDAY_MAP[day] for day in weekdays]
data = {
"priority_level": 1,
"start": next_week_monday,
"rotation_start": next_week_monday,
"duration": timezone.timedelta(seconds=10800),
"frequency": CustomOnCallShift.FREQUENCY_DAILY,
"interval": 2,
"by_day": by_day,
"schedule": schedule,
}
rolling_users = [[user_1], [user_2]]
on_call_shift = make_on_call_shift(
organization=organization, shift_type=CustomOnCallShift.TYPE_ROLLING_USERS_EVENT, **data
)
on_call_shift.add_rolling_users(rolling_users)
date = next_week_monday + timezone.timedelta(minutes=5)
user_1_on_call_dates = [
date, # MO
date + timezone.timedelta(days=2), # WE
date + timezone.timedelta(days=9), # WE
date + timezone.timedelta(days=11), # FR
date + timezone.timedelta(days=18), # FR
date + timezone.timedelta(days=21), # MO
date + timezone.timedelta(days=28), # MO
date + timezone.timedelta(days=30), # WE
]
user_2_on_call_dates = [
date + timezone.timedelta(days=4), # FR
date + timezone.timedelta(days=7), # MO
date + timezone.timedelta(days=14), # MO
date + timezone.timedelta(days=16), # WE
date + timezone.timedelta(days=23), # WE
date + timezone.timedelta(days=25), # FR
date + timezone.timedelta(days=32), # FR
date + timezone.timedelta(days=35), # MO
]
nobody_on_call_dates = [
date + timezone.timedelta(days=1), # TU
date + timezone.timedelta(days=3), # TH
date + timezone.timedelta(days=5), # SAT
date + timezone.timedelta(days=6), # SUN
date + timezone.timedelta(days=8), # TU
date + timezone.timedelta(days=10), # TH
date + timezone.timedelta(days=12), # SAT
]
for dt in user_1_on_call_dates:
users_on_call = list_users_to_notify_from_ical(schedule, dt)
assert len(users_on_call) == 1
assert user_1 in users_on_call
for dt in user_2_on_call_dates:
users_on_call = list_users_to_notify_from_ical(schedule, dt)
assert len(users_on_call) == 1
assert user_2 in users_on_call
for dt in nobody_on_call_dates:
users_on_call = list_users_to_notify_from_ical(schedule, dt)
assert len(users_on_call) == 0
@pytest.mark.django_db
def test_rolling_users_event_with_interval_weekly(
make_organization_and_user, make_user_for_organization, make_on_call_shift, make_schedule

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

@ -387,6 +387,86 @@ def test_final_schedule_events(make_organization, make_user_for_organization, ma
assert returned_events == expected_events
@pytest.mark.django_db
def test_final_schedule_override_no_priority_shift(
make_organization, make_user_for_organization, make_on_call_shift, make_schedule
):
organization = make_organization()
schedule = make_schedule(
organization,
schedule_class=OnCallScheduleWeb,
name="test_web_schedule",
)
now = timezone.now().replace(hour=0, minute=0, second=0, microsecond=0)
start_date = now - timezone.timedelta(days=7)
user_a, user_b = (make_user_for_organization(organization, username=i) for i in "AB")
# clear users pks <-> organization cache (persisting between tests)
memoized_users_in_ical.cache_clear()
shifts = (
# user, priority, start time (h), duration (hs)
(user_a, 0, 10, 5), # 10-15 / A
)
for user, priority, start_h, duration in shifts:
data = {
"start": start_date + timezone.timedelta(hours=start_h),
"rotation_start": start_date + timezone.timedelta(hours=start_h),
"duration": timezone.timedelta(hours=duration),
"priority_level": priority,
"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]])
# override: 10-15 / B
override_data = {
"start": start_date + timezone.timedelta(hours=10),
"rotation_start": start_date + timezone.timedelta(hours=5),
"duration": timezone.timedelta(hours=5),
"schedule": schedule,
}
override = make_on_call_shift(
organization=organization, shift_type=CustomOnCallShift.TYPE_OVERRIDE, **override_data
)
override.add_rolling_users([[user_b]])
returned_events = schedule.final_events("UTC", start_date, days=1)
expected = (
# start (h), duration (H), user, priority, is_override
(10, 5, "B", None, True), # 10-15 B
)
expected_events = [
{
"calendar_type": 1 if is_override else 0,
"end": start_date + timezone.timedelta(hours=start + duration),
"is_override": is_override,
"priority_level": priority,
"start": start_date + timezone.timedelta(hours=start, milliseconds=1 if start == 0 else 0),
"user": user,
}
for start, duration, user, priority, is_override in expected
]
returned_events = [
{
"calendar_type": e["calendar_type"],
"end": e["end"],
"is_override": e["is_override"],
"priority_level": e["priority_level"],
"start": e["start"],
"user": e["users"][0]["display_name"] if e["users"] else None,
}
for e in returned_events
if not e["is_gap"]
]
assert returned_events == expected_events
@pytest.mark.django_db
def test_final_schedule_splitting_events(
make_organization, make_user_for_organization, make_on_call_shift, make_schedule

4
grafana-plugin/src/containers/RotationForm/RotationForm.tsx
View file

@ -154,7 +154,7 @@ const RotationForm: FC<RotationFormProps> = observer((props) => {
rolling_users: userGroups,
interval: repeatEveryValue,
frequency: repeatEveryPeriod,
by_day: repeatEveryPeriod === 1 ? selectedDays : null,
by_day: repeatEveryPeriod === 1 || repeatEveryPeriod === 0 ? selectedDays : null,
priority_level: shiftId === 'new' ? layerPriority : shift?.priority_level,
}),
[
@ -320,7 +320,7 @@ const RotationForm: FC<RotationFormProps> = observer((props) => {
/>
</Field>
</HorizontalGroup>
{repeatEveryPeriod === 1 && (
{(repeatEveryPeriod === 1 || repeatEveryPeriod === 0) && (
<Field label="Select days to repeat">
<DaysSelector
options={store.scheduleStore.byDayOptions}