From 241283959a234096141b4cc0d56d1c0b9ad01f6a Mon Sep 17 00:00:00 2001 From: Matias Bordese Date: Wed, 26 Oct 2022 16:25:50 -0300 Subject: [PATCH 1/6] Enable daily/by-day shifts option for web schedules --- engine/apps/api/serializers/on_call_shifts.py | 6 +- engine/apps/api/tests/test_oncall_shift.py | 25 +++++- engine/apps/api/views/schedule.py | 8 +- .../public_api/serializers/on_call_shifts.py | 10 ++- .../public_api/tests/test_on_call_shifts.py | 5 ++ .../amixr_recurring_ical_events_adapter.py | 2 + .../schedules/models/custom_on_call_shift.py | 53 +++++++++++- .../apps/schedules/models/on_call_schedule.py | 12 ++- .../tests/test_custom_on_call_shift.py | 61 ++++++++++++++ .../schedules/tests/test_on_call_schedule.py | 80 +++++++++++++++++++ 10 files changed, 249 insertions(+), 13 deletions(-) diff --git a/engine/apps/api/serializers/on_call_shifts.py b/engine/apps/api/serializers/on_call_shifts.py index be5660d3..7b561c38 100644 --- a/engine/apps/api/serializers/on_call_shifts.py +++ b/engine/apps/api/serializers/on_call_shifts.py @@ -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 > 1: + raise serializers.ValidationError( + {"interval": ["Cannot set interval > 1 if a days value is set for a daily frequency"]} + ) def _validate_rotation_start(self, shift_start, rotation_start): if rotation_start < shift_start: diff --git a/engine/apps/api/tests/test_oncall_shift.py b/engine/apps/api/tests/test_oncall_shift.py index efa2fb96..807eb8a6 100644 --- a/engine/apps/api/tests/test_oncall_shift.py +++ b/engine/apps/api/tests/test_oncall_shift.py @@ -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 > 1 + 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] == "Cannot set interval > 1 if a days value is set for a daily frequency" + @pytest.mark.django_db def test_create_on_call_shift_invalid_data_shift_end(on_call_shift_internal_api_setup, make_user_auth_headers): diff --git a/engine/apps/api/views/schedule.py b/engine/apps/api/views/schedule.py index 04de4393..6f828a54 100644 --- a/engine/apps/api/views/schedule.py +++ b/engine/apps/api/views/schedule.py @@ -1,3 +1,5 @@ +import datetime + import pytz from django.core.exceptions import ObjectDoesNotExist from django.db.models import Count, OuterRef, Subquery @@ -289,7 +291,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} diff --git a/engine/apps/public_api/serializers/on_call_shifts.py b/engine/apps/public_api/serializers/on_call_shifts.py index d6592803..3bbce50d 100644 --- a/engine/apps/public_api/serializers/on_call_shifts.py +++ b/engine/apps/public_api/serializers/on_call_shifts.py @@ -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 != 1: + raise BadRequest(detail="Day limits and frequency 'daily' require 'interval' to be 1") 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) diff --git a/engine/apps/public_api/tests/test_on_call_shifts.py b/engine/apps/public_api/tests/test_on_call_shifts.py index 2934c6c7..236ae479 100644 --- a/engine/apps/public_api/tests/test_on_call_shifts.py +++ b/engine/apps/public_api/tests/test_on_call_shifts.py @@ -40,6 +40,10 @@ invalid_field_data_8 = { "until": "not-a-date", } +invalid_field_data_9 = { + "frequency": CustomOnCallShift.FREQUENCY_DAILY, +} + @pytest.mark.django_db def test_get_on_call_shift(make_organization_and_user_with_token, make_on_call_shift, make_schedule): @@ -284,6 +288,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): diff --git a/engine/apps/schedules/ical_events/adapter/amixr_recurring_ical_events_adapter.py b/engine/apps/schedules/ical_events/adapter/amixr_recurring_ical_events_adapter.py index 24f56ee5..a49a8b31 100644 --- a/engine/apps/schedules/ical_events/adapter/amixr_recurring_ical_events_adapter.py +++ b/engine/apps/schedules/ical_events/adapter/amixr_recurring_ical_events_adapter.py @@ -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)) diff --git a/engine/apps/schedules/models/custom_on_call_shift.py b/engine/apps/schedules/models/custom_on_call_shift.py index db9ca5d3..569f5ef1 100644 --- a/engine/apps/schedules/models/custom_on_call_shift.py +++ b/engine/apps/schedules/models/custom_on_call_shift.py @@ -1,3 +1,5 @@ +import copy +import itertools import logging import random import string @@ -274,6 +276,47 @@ 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.""" + 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 + cycle_users = itertools.cycle(users_queue) + orig_start = last_start = start + for users in cycle_users: + if not start: # means that rotation ends before next event starts + break + last_start = start + day = CustomOnCallShift.ICAL_WEEKDAY_MAP[start.weekday()] + if (users, day) in combinations: + break + + starting_dates.append(start) + combinations.append((users, day)) + # get next event date following the original rule + event_ical = self.generate_ical(start, 1, None, 1, time_zone) + start = self.get_rotation_date(event_ical, get_next_date=True) + + # number of weeks used to cover all combinations + week_interval = ((last_start - orig_start).days // 7) or 1 + counter = 1 + for ((users, day), start) in zip(combinations, starting_dates): + 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 +342,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 +372,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 +380,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") diff --git a/engine/apps/schedules/models/on_call_schedule.py b/engine/apps/schedules/models/on_call_schedule.py index 24aa8d0d..0181cbb7 100644 --- a/engine/apps/schedules/models/on_call_schedule.py +++ b/engine/apps/schedules/models/on_call_schedule.py @@ -320,13 +320,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 @@ -390,8 +392,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 diff --git a/engine/apps/schedules/tests/test_custom_on_call_shift.py b/engine/apps/schedules/tests/test_custom_on_call_shift.py index 4dbf6029..fe79e220 100644 --- a/engine/apps/schedules/tests/test_custom_on_call_shift.py +++ b/engine/apps/schedules/tests/test_custom_on_call_shift.py @@ -293,6 +293,67 @@ def test_rolling_users_event_with_interval_daily( 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": 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_weekly( make_organization_and_user, make_user_for_organization, make_on_call_shift, make_schedule diff --git a/engine/apps/schedules/tests/test_on_call_schedule.py b/engine/apps/schedules/tests/test_on_call_schedule.py index 8bb079f4..0a45f93e 100644 --- a/engine/apps/schedules/tests/test_on_call_schedule.py +++ b/engine/apps/schedules/tests/test_on_call_schedule.py @@ -357,6 +357,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 From 1d4f03630343fa92759194ceb95abdc07b773ba4 Mon Sep 17 00:00:00 2001 From: Matias Bordese Date: Wed, 26 Oct 2022 16:26:16 -0300 Subject: [PATCH 2/6] Update Makefile to use .env.dev --- Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile b/Makefile index 3f72d5a6..736428f1 100644 --- a/Makefile +++ b/Makefile @@ -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): From 4bc3ed17a8b1841fa72bf484eb593a8bb40900a9 Mon Sep 17 00:00:00 2001 From: Matias Bordese Date: Wed, 26 Oct 2022 16:35:59 -0300 Subject: [PATCH 3/6] Make interval check more explicit --- engine/apps/api/serializers/on_call_shifts.py | 4 ++-- engine/apps/api/tests/test_oncall_shift.py | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/engine/apps/api/serializers/on_call_shifts.py b/engine/apps/api/serializers/on_call_shifts.py index 7b561c38..9ef9130b 100644 --- a/engine/apps/api/serializers/on_call_shifts.py +++ b/engine/apps/api/serializers/on_call_shifts.py @@ -116,9 +116,9 @@ class OnCallShiftSerializer(EagerLoadingMixin, serializers.ModelSerializer): ) 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 > 1: + if frequency == CustomOnCallShift.FREQUENCY_DAILY and by_day and interval != 1: raise serializers.ValidationError( - {"interval": ["Cannot set interval > 1 if a days value is set for a daily frequency"]} + {"interval": ["Interval must be 1 if a days value is set for a daily frequency"]} ) def _validate_rotation_start(self, shift_start, rotation_start): diff --git a/engine/apps/api/tests/test_oncall_shift.py b/engine/apps/api/tests/test_oncall_shift.py index 807eb8a6..945ecf50 100644 --- a/engine/apps/api/tests/test_oncall_shift.py +++ b/engine/apps/api/tests/test_oncall_shift.py @@ -789,7 +789,7 @@ 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 > 1 + # by_day, daily, interval != 1 data = { "title": "Test Shift 2", "type": CustomOnCallShift.TYPE_ROLLING_USERS_EVENT, @@ -808,7 +808,7 @@ def test_create_on_call_shift_invalid_data_interval(on_call_shift_internal_api_s 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] == "Cannot set interval > 1 if a days value is set for a daily frequency" + assert response.data["interval"][0] == "Interval must be 1 if a days value is set for a daily frequency" @pytest.mark.django_db From bc2244da434297e99bf2116818ed1c8e356d7f89 Mon Sep 17 00:00:00 2001 From: Matias Bordese Date: Thu, 27 Oct 2022 09:46:04 -0300 Subject: [PATCH 4/6] Enable day selection for daily shifts in web schedule --- grafana-plugin/src/containers/RotationForm/RotationForm.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/grafana-plugin/src/containers/RotationForm/RotationForm.tsx b/grafana-plugin/src/containers/RotationForm/RotationForm.tsx index 1242abca..82b1e707 100644 --- a/grafana-plugin/src/containers/RotationForm/RotationForm.tsx +++ b/grafana-plugin/src/containers/RotationForm/RotationForm.tsx @@ -154,7 +154,7 @@ const RotationForm: FC = 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 = observer((props) => { /> - {repeatEveryPeriod === 1 && ( + {(repeatEveryPeriod === 1 || repeatEveryPeriod === 0) && ( Date: Fri, 28 Oct 2022 14:54:33 -0300 Subject: [PATCH 5/6] Add support for daily/by-day and custom interval --- engine/apps/api/serializers/on_call_shifts.py | 4 +- engine/apps/api/tests/test_oncall_shift.py | 4 +- .../public_api/serializers/on_call_shifts.py | 4 +- .../public_api/tests/test_on_call_shifts.py | 1 + .../schedules/models/custom_on_call_shift.py | 38 +++++---- .../tests/test_custom_on_call_shift.py | 81 ++++++++++++++++++- 6 files changed, 111 insertions(+), 21 deletions(-) diff --git a/engine/apps/api/serializers/on_call_shifts.py b/engine/apps/api/serializers/on_call_shifts.py index 9ef9130b..5d8d409a 100644 --- a/engine/apps/api/serializers/on_call_shifts.py +++ b/engine/apps/api/serializers/on_call_shifts.py @@ -116,9 +116,9 @@ class OnCallShiftSerializer(EagerLoadingMixin, serializers.ModelSerializer): ) 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 != 1: + if frequency == CustomOnCallShift.FREQUENCY_DAILY and by_day and interval > len(by_day): raise serializers.ValidationError( - {"interval": ["Interval must be 1 if a days value is set for a daily frequency"]} + {"interval": ["Interval must be less than or equal to the number of selected days"]} ) def _validate_rotation_start(self, shift_start, rotation_start): diff --git a/engine/apps/api/tests/test_oncall_shift.py b/engine/apps/api/tests/test_oncall_shift.py index 945ecf50..8d5db17f 100644 --- a/engine/apps/api/tests/test_oncall_shift.py +++ b/engine/apps/api/tests/test_oncall_shift.py @@ -789,7 +789,7 @@ 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 != 1 + # by_day, daily, interval > len(by_day) data = { "title": "Test Shift 2", "type": CustomOnCallShift.TYPE_ROLLING_USERS_EVENT, @@ -808,7 +808,7 @@ def test_create_on_call_shift_invalid_data_interval(on_call_shift_internal_api_s 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 1 if a days value is set for a daily frequency" + assert response.data["interval"][0] == "Interval must be less than or equal to the number of selected days" @pytest.mark.django_db diff --git a/engine/apps/public_api/serializers/on_call_shifts.py b/engine/apps/public_api/serializers/on_call_shifts.py index 3bbce50d..15b0d271 100644 --- a/engine/apps/public_api/serializers/on_call_shifts.py +++ b/engine/apps/public_api/serializers/on_call_shifts.py @@ -210,8 +210,8 @@ class CustomOnCallShiftSerializer(EagerLoadingMixin, serializers.ModelSerializer detail="Day limits are temporarily disabled for on-call shifts with type 'rolling_users' " "and frequency 'daily'" ) - if by_day and interval != 1: - raise BadRequest(detail="Day limits and frequency 'daily' require 'interval' to be 1") + 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: diff --git a/engine/apps/public_api/tests/test_on_call_shifts.py b/engine/apps/public_api/tests/test_on_call_shifts.py index 236ae479..82dac03d 100644 --- a/engine/apps/public_api/tests/test_on_call_shifts.py +++ b/engine/apps/public_api/tests/test_on_call_shifts.py @@ -42,6 +42,7 @@ invalid_field_data_8 = { invalid_field_data_9 = { "frequency": CustomOnCallShift.FREQUENCY_DAILY, + "interval": 5, } diff --git a/engine/apps/schedules/models/custom_on_call_shift.py b/engine/apps/schedules/models/custom_on_call_shift.py index 569f5ef1..153ec1dd 100644 --- a/engine/apps/schedules/models/custom_on_call_shift.py +++ b/engine/apps/schedules/models/custom_on_call_shift.py @@ -285,24 +285,33 @@ class CustomOnCallShift(models.Model): # we may need to iterate several times over users until we get a seen combination cycle_users = itertools.cycle(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 users in cycle_users: - if not start: # means that rotation ends before next event starts - break - last_start = start - day = CustomOnCallShift.ICAL_WEEKDAY_MAP[start.weekday()] - if (users, day) in combinations: - break + 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 (users, day, i) in combinations: + all_rotations_checked = True + break - starting_dates.append(start) - combinations.append((users, day)) - # get next event date following the original rule - event_ical = self.generate_ical(start, 1, None, 1, time_zone) - start = self.get_rotation_date(event_ical, get_next_date=True) + starting_dates.append(start) + combinations.append((users, 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 ((users, day), start) in zip(combinations, starting_dates): + for ((users, day, _), start) in zip(combinations, starting_dates): 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 @@ -398,7 +407,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 @@ -412,7 +421,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) diff --git a/engine/apps/schedules/tests/test_custom_on_call_shift.py b/engine/apps/schedules/tests/test_custom_on_call_shift.py index fe79e220..a7945765 100644 --- a/engine/apps/schedules/tests/test_custom_on_call_shift.py +++ b/engine/apps/schedules/tests/test_custom_on_call_shift.py @@ -294,7 +294,7 @@ def test_rolling_users_event_with_interval_daily( @pytest.mark.django_db -def test_rolling_users_event_with_interval_daily_by_day( +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() @@ -354,6 +354,85 @@ def test_rolling_users_event_with_interval_daily_by_day( 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 From 4f6734dcf50b90db107c331f886222411f44ab2b Mon Sep 17 00:00:00 2001 From: Matias Bordese Date: Thu, 3 Nov 2022 15:26:46 -0300 Subject: [PATCH 6/6] Update daily/by-day to user group id when generating weekly shifts --- .../schedules/models/custom_on_call_shift.py | 26 ++++++++++++++----- 1 file changed, 20 insertions(+), 6 deletions(-) diff --git a/engine/apps/schedules/models/custom_on_call_shift.py b/engine/apps/schedules/models/custom_on_call_shift.py index 153ec1dd..3026325e 100644 --- a/engine/apps/schedules/models/custom_on_call_shift.py +++ b/engine/apps/schedules/models/custom_on_call_shift.py @@ -277,31 +277,44 @@ 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.""" + """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 - cycle_users = itertools.cycle(users_queue) + # 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 users in cycle_users: + 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 (users, day, i) in combinations: + if (user_group_id, day, i) in combinations: all_rotations_checked = True break starting_dates.append(start) - combinations.append((users, day, i)) + 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) @@ -311,7 +324,8 @@ class CustomOnCallShift(models.Model): # number of weeks used to cover all combinations week_interval = ((last_start - orig_start).days // 7) or 1 counter = 1 - for ((users, day, _), start) in zip(combinations, starting_dates): + 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