diff --git a/CHANGELOG.md b/CHANGELOG.md index 63630719b8..5241ac2e36 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Update the direct paging feature to page for acknowledged & silenced alert groups, and show a warning for resolved alert groups by @vadimkerr ([#2639](https://github.com/grafana/oncall/pull/2639)) +- Update checking on-call users to use schedule final events ([#2651](https://github.com/grafana/oncall/pull/2651)) ### Fixed diff --git a/engine/apps/alerts/escalation_snapshot/snapshot_classes/escalation_policy_snapshot.py b/engine/apps/alerts/escalation_snapshot/snapshot_classes/escalation_policy_snapshot.py index ef5c202ed5..8ff60c3056 100644 --- a/engine/apps/alerts/escalation_snapshot/snapshot_classes/escalation_policy_snapshot.py +++ b/engine/apps/alerts/escalation_snapshot/snapshot_classes/escalation_policy_snapshot.py @@ -275,7 +275,7 @@ def _escalation_step_notify_on_call_schedule(self, alert_group: "AlertGroup", re escalation_policy_step=self.step, ) else: - notify_to_users_list = list_users_to_notify_from_ical(on_call_schedule, include_viewers=True) + notify_to_users_list = list_users_to_notify_from_ical(on_call_schedule) if notify_to_users_list is None: log_record = AlertGroupLogRecord( type=AlertGroupLogRecord.TYPE_ESCALATION_FAILED, diff --git a/engine/apps/alerts/tests/test_escalation_policy_snapshot.py b/engine/apps/alerts/tests/test_escalation_policy_snapshot.py index 7ba39f00ce..bc79abc7b9 100644 --- a/engine/apps/alerts/tests/test_escalation_policy_snapshot.py +++ b/engine/apps/alerts/tests/test_escalation_policy_snapshot.py @@ -246,11 +246,8 @@ def test_escalation_step_notify_on_call_schedule_viewer_user( ) assert expected_eta + timezone.timedelta(seconds=15) > result.eta > expected_eta - timezone.timedelta(seconds=15) assert result == expected_result - assert notify_schedule_step.log_records.filter(type=AlertGroupLogRecord.TYPE_ESCALATION_TRIGGERED).exists() - assert list(escalation_policy_snapshot.notify_to_users_queue) == list( - list_users_to_notify_from_ical(schedule, include_viewers=True) - ) - assert list(escalation_policy_snapshot.notify_to_users_queue) == [viewer] + assert notify_schedule_step.log_records.filter(type=AlertGroupLogRecord.TYPE_ESCALATION_FAILED).exists() + assert list(escalation_policy_snapshot.notify_to_users_queue) == [] assert mocked_execute_tasks.called diff --git a/engine/apps/api/tests/test_schedules.py b/engine/apps/api/tests/test_schedules.py index 4cf8eb9c98..ad7f64c5a6 100644 --- a/engine/apps/api/tests/test_schedules.py +++ b/engine/apps/api/tests/test_schedules.py @@ -1227,7 +1227,7 @@ def test_filter_events_final_schedule( "is_gap": is_gap, "is_override": is_override, "priority_level": priority, - "start": start_date + timezone.timedelta(hours=start, milliseconds=1 if start == 0 else 0), + "start": start_date + timezone.timedelta(hours=start), "user": user, } for start, duration, user, priority, is_gap, is_override in expected diff --git a/engine/apps/api/views/on_call_shifts.py b/engine/apps/api/views/on_call_shifts.py index c760b53fc7..62c4e36fe2 100644 --- a/engine/apps/api/views/on_call_shifts.py +++ b/engine/apps/api/views/on_call_shifts.py @@ -1,3 +1,6 @@ +import datetime + +import pytz from django.db.models import Q from django_filters.rest_framework import DjangoFilterBackend from rest_framework import status @@ -106,8 +109,13 @@ def preview(self, request): updated_shift_pk = self.request.data.get("shift_pk") shift = CustomOnCallShift(**validated_data) schedule = shift.schedule + + pytz_tz = pytz.timezone(user_tz) + datetime_start = datetime.datetime.combine(starting_date, datetime.time.min, tzinfo=pytz_tz) + datetime_end = datetime_start + datetime.timedelta(days=days) + shift_events, final_events = schedule.preview_shift( - shift, user_tz, starting_date, days, updated_shift_pk=updated_shift_pk + shift, datetime_start, datetime_end, updated_shift_pk=updated_shift_pk ) data = { "rotation": shift_events, diff --git a/engine/apps/api/views/schedule.py b/engine/apps/api/views/schedule.py index d6960c9d39..7413920fcc 100644 --- a/engine/apps/api/views/schedule.py +++ b/engine/apps/api/views/schedule.py @@ -1,6 +1,8 @@ +import datetime import functools import operator +import pytz from django.core.exceptions import ObjectDoesNotExist from django.db.models import Count, OuterRef, Subquery from django.db.utils import IntegrityError @@ -274,12 +276,16 @@ def get_request_timezone(self): @action(detail=True, methods=["get"]) def events(self, request, pk): - user_tz, date = self.get_request_timezone() + user_tz, starting_date = self.get_request_timezone() with_empty = self.request.query_params.get("with_empty", False) == "true" with_gap = self.request.query_params.get("with_gap", False) == "true" schedule = self.get_object() - events = schedule.filter_events(user_tz, date, days=1, with_empty=with_empty, with_gap=with_gap) + + pytz_tz = pytz.timezone(user_tz) + datetime_start = datetime.datetime.combine(starting_date, datetime.time.min, tzinfo=pytz_tz) + datetime_end = datetime_start + datetime.timedelta(days=1) + events = schedule.filter_events(datetime_start, datetime_end, with_empty=with_empty, with_gap=with_gap) slack_channel = ( { @@ -312,19 +318,22 @@ def filter_events(self, request: Request, pk: str) -> Response: schedule = self.get_object() + pytz_tz = pytz.timezone(user_tz) + datetime_start = datetime.datetime.combine(starting_date, datetime.time.min, tzinfo=pytz_tz) + datetime_end = datetime_start + datetime.timedelta(days=days) + if filter_by is not None and filter_by != EVENTS_FILTER_BY_FINAL: filter_by = OnCallSchedule.PRIMARY if filter_by == EVENTS_FILTER_BY_ROTATION else OnCallSchedule.OVERRIDES events = schedule.filter_events( - user_tz, - starting_date, - days=days, + datetime_start, + datetime_end, with_empty=True, with_gap=resolve_schedule, filter_by=filter_by, all_day_datetime=True, ) else: # return final schedule - events = schedule.final_events(user_tz, starting_date, days) + events = schedule.final_events(datetime_start, datetime_end) result = { "id": schedule.public_primary_key, @@ -337,11 +346,11 @@ def filter_events(self, request: Request, pk: str) -> Response: @action(detail=True, methods=["get"]) def next_shifts_per_user(self, request, pk): """Return next shift for users in schedule.""" - user_tz, _ = self.get_request_timezone() now = timezone.now() - starting_date = now.date() + datetime_end = now + datetime.timedelta(days=30) schedule = self.get_object() - events = schedule.final_events(user_tz, starting_date, days=30) + + events = schedule.final_events(now, datetime_end) users = {u.public_primary_key: None for u in schedule.related_users()} for e in events: @@ -373,10 +382,11 @@ def quality(self, request, pk): schedule = self.get_object() _, date = self.get_request_timezone() + datetime_start = datetime.datetime.combine(date, datetime.time.min, tzinfo=pytz.UTC) days = self.request.query_params.get("days") days = int(days) if days else None - return Response(schedule.quality_report(date, days)) + return Response(schedule.quality_report(datetime_start, days)) @action(detail=False, methods=["get"]) def type_options(self, request): diff --git a/engine/apps/mobile_app/tasks.py b/engine/apps/mobile_app/tasks.py index 9810149462..41b36a3108 100644 --- a/engine/apps/mobile_app/tasks.py +++ b/engine/apps/mobile_app/tasks.py @@ -439,7 +439,8 @@ def conditionally_send_going_oncall_push_notifications_for_schedule(schedule_pk) return now = timezone.now() - schedule_final_events = schedule.final_events("UTC", now, days=7) + datetime_end = now + datetime.timedelta(days=7) + schedule_final_events = schedule.final_events(now, datetime_end) relevant_cache_keys = [ _generate_going_oncall_push_notification_cache_key(user["pk"], schedule_event) diff --git a/engine/apps/public_api/views/schedules.py b/engine/apps/public_api/views/schedules.py index 83aa3207f2..9c398f26cd 100644 --- a/engine/apps/public_api/views/schedules.py +++ b/engine/apps/public_api/views/schedules.py @@ -1,5 +1,7 @@ +import datetime import logging +import pytz from django_filters import rest_framework as filters from rest_framework import status from rest_framework.decorators import action @@ -147,8 +149,12 @@ def final_shifts(self, request, pk): end_date = serializer.validated_data["end_date"] days_between_start_and_end = (end_date - start_date).days - final_schedule_events: ScheduleEvents = schedule.final_events("UTC", start_date, days_between_start_and_end) + datetime_start = datetime.datetime.combine(start_date, datetime.time.min, tzinfo=pytz.UTC) + datetime_end = datetime_start + datetime.timedelta( + days=days_between_start_and_end - 1, hours=23, minutes=59, seconds=59 + ) + final_schedule_events: ScheduleEvents = schedule.final_events(datetime_start, datetime_end) logger.info( f"Exporting oncall shifts for schedule {pk} between dates {start_date} and {end_date}. {len(final_schedule_events)} shift events were found." ) diff --git a/engine/apps/schedules/ical_utils.py b/engine/apps/schedules/ical_utils.py index bbdda58114..72285e71c9 100644 --- a/engine/apps/schedules/ical_utils.py +++ b/engine/apps/schedules/ical_utils.py @@ -61,7 +61,6 @@ def users_in_ical( usernames_from_ical: typing.List[str], organization: "Organization", - include_viewers=False, users_to_filter: typing.Optional["UserQuerySet"] = None, ) -> typing.Sequence["User"]: """ @@ -94,11 +93,10 @@ def users_in_ical( } ) - users_found_in_ical = organization.users - if not include_viewers: - users_found_in_ical = users_found_in_ical.filter( - **User.build_permissions_query(RBACPermission.Permissions.SCHEDULES_WRITE, organization) - ) + # users_found_in_ical = organization.users + users_found_in_ical = organization.users.filter( + **User.build_permissions_query(RBACPermission.Permissions.SCHEDULES_WRITE, organization) + ) users_found_in_ical = users_found_in_ical.filter( (Q(username__in=usernames_from_ical) | Q(email__lower__in=emails_from_ical)) @@ -118,11 +116,10 @@ def memoized_users_in_ical( # used for display schedule events on web def list_of_oncall_shifts_from_ical( schedule: "OnCallSchedule", - date: datetime.date, - user_timezone: str = "UTC", + datetime_start: datetime.datetime, + datetime_end: datetime.datetime, with_empty_shifts: bool = False, with_gaps: bool = False, - days: int = 1, filter_by: str | None = None, from_cached_final: bool = False, ): @@ -152,16 +149,6 @@ def list_of_oncall_shifts_from_ical( else: calendars = schedule.get_icalendars() - # TODO: Review offset usage - pytz_tz = pytz.timezone(user_timezone) - - # utcoffset can technically return None, but we're confident it is a timedelta here - user_timezone_offset: datetime.timedelta = datetime.datetime.now().astimezone(pytz_tz).utcoffset() # type: ignore[assignment] - - datetime_min = datetime.datetime.combine(date, datetime.time.min) + datetime.timedelta(milliseconds=1) - datetime_start = (datetime_min - user_timezone_offset).astimezone(pytz.UTC) - datetime_end = datetime_start + datetime.timedelta(days=days - 1, hours=23, minutes=59, seconds=59) - result_datetime = [] result_date = [] @@ -204,6 +191,7 @@ def list_of_oncall_shifts_from_ical( ) def event_start_cmp_key(e): + pytz_tz = pytz.timezone("UTC") return ( datetime.datetime.combine(e["start"], datetime.datetime.min.time(), tzinfo=pytz_tz) if type(e["start"]) == datetime.date @@ -348,7 +336,6 @@ def list_of_empty_shifts_in_schedule( def list_users_to_notify_from_ical( schedule: "OnCallSchedule", events_datetime: typing.Optional[datetime.datetime] = None, - include_viewers: bool = False, users_to_filter: typing.Optional["UserQuerySet"] = None, ) -> typing.Sequence["User"]: """ @@ -359,7 +346,6 @@ def list_users_to_notify_from_ical( schedule, events_datetime, events_datetime, - include_viewers=include_viewers, users_to_filter=users_to_filter, ) @@ -368,35 +354,15 @@ def list_users_to_notify_from_ical_for_period( schedule: "OnCallSchedule", start_datetime: datetime.datetime, end_datetime: datetime.datetime, - include_viewers=False, users_to_filter=None, ) -> typing.Sequence["User"]: - # get list of iCalendars from current iCal files. If there is more than one calendar, primary calendar will always - # be the first - calendars = schedule.get_icalendars() - # reverse calendars to make overrides calendar the first, if schedule is iCal - calendars = calendars[::-1] users_found_in_ical: typing.Sequence["User"] = [] - # at first check overrides calendar and return users from it if it exists and on-call users are found - for calendar in calendars: - if calendar is None: - continue - events = ical_events.get_events_from_ical_between(calendar, start_datetime, end_datetime) - - parsed_ical_events: typing.Dict[int, typing.List[str]] = {} - for event in events: - current_usernames, current_priority = get_usernames_from_ical_event(event) - parsed_ical_events.setdefault(current_priority, []).extend(current_usernames) - # find users by usernames. if users are not found for shift, get users from lower priority - for _, usernames in sorted(parsed_ical_events.items(), reverse=True): - users_found_in_ical = users_in_ical( - usernames, schedule.organization, include_viewers=include_viewers, users_to_filter=users_to_filter - ) - if users_found_in_ical: - break - if users_found_in_ical: - # if users are found in the overrides calendar, there is no need to check primary calendar - break + events = schedule.final_events(start_datetime, end_datetime) + usernames = [] + for event in events: + usernames += [u["email"] for u in event.get("users", [])] + + users_found_in_ical = users_in_ical(usernames, schedule.organization, users_to_filter=users_to_filter) return users_found_in_ical diff --git a/engine/apps/schedules/models/on_call_schedule.py b/engine/apps/schedules/models/on_call_schedule.py index a63f46b09a..62cc061f34 100644 --- a/engine/apps/schedules/models/on_call_schedule.py +++ b/engine/apps/schedules/models/on_call_schedule.py @@ -307,9 +307,8 @@ def related_users(self): def filter_events( self, - user_timezone, - starting_date, - days, + datetime_start, + datetime_end, with_empty=False, with_gap=False, filter_by=None, @@ -320,11 +319,10 @@ def filter_events( shifts = ( list_of_oncall_shifts_from_ical( self, - starting_date, - user_timezone, + datetime_start, + datetime_end, with_empty, with_gap, - days=days, filter_by=filter_by, from_cached_final=from_cached_final, ) @@ -369,26 +367,23 @@ def filter_events( # combine multiple-users same-shift events into one return self._merge_events(events) - def final_events(self, user_tz, starting_date, days): + def final_events(self, datetime_start, datetime_end): """Return schedule final events, after resolving shifts and overrides.""" - events = self.filter_events( - user_tz, starting_date, days=days, with_empty=True, with_gap=True, all_day_datetime=True - ) - events = self._resolve_schedule(events) + events = self.filter_events(datetime_start, datetime_end, with_empty=True, with_gap=True, all_day_datetime=True) + events = self._resolve_schedule(events, datetime_start, datetime_end) return events def refresh_ical_final_schedule(self): - tz = "UTC" now = timezone.now() # window to consider: from now, -15 days + 6 months delta = EXPORT_WINDOW_DAYS_BEFORE - starting_datetime = now - datetime.timedelta(days=delta) - starting_date = starting_datetime.date() days = EXPORT_WINDOW_DAYS_AFTER + delta + datetime_start = now.replace(hour=0, minute=0, second=0, microsecond=0) - datetime.timedelta(days=delta) + datetime_end = datetime_start + datetime.timedelta(days=days - 1, hours=23, minutes=59, seconds=59) # setup calendar with final schedule shift events calendar = create_base_icalendar(self.name) - events = self.final_events(tz, starting_date, days) + events = self.final_events(datetime_start, datetime_end) updated_ids = set() for e in events: for u in e["users"]: @@ -417,12 +412,12 @@ def refresh_ical_final_schedule(self): dtend_datetime = datetime.datetime.combine( dtend.dt, datetime.datetime.min.time(), tzinfo=pytz.UTC ) - if dtend_datetime and dtend_datetime < starting_datetime: + if dtend_datetime and dtend_datetime < datetime_start: # event ended before window start continue is_cancelled = component.get(ICAL_STATUS) last_modified = component.get(ICAL_LAST_MODIFIED) - if is_cancelled and last_modified and last_modified.dt < starting_datetime: + if is_cancelled and last_modified and last_modified.dt < datetime_start: # drop already ended events older than the window we consider continue elif is_cancelled and not last_modified: @@ -441,17 +436,18 @@ def refresh_ical_final_schedule(self): self.save(update_fields=["cached_ical_final_schedule"]) def upcoming_shift_for_user(self, user, days=7): - user_tz = user.timezone or "UTC" now = timezone.now() # consider an extra day before to include events from UTC yesterday - starting_date = now.date() - datetime.timedelta(days=1) + datetime_start = now - datetime.timedelta(days=1) + datetime_end = datetime_start + datetime.timedelta(days=days) + current_shift = upcoming_shift = None if self.cached_ical_final_schedule is None: # no final schedule info available return None, None - events = self.filter_events(user_tz, starting_date, days=days, all_day_datetime=True, from_cached_final=True) + events = self.filter_events(datetime_start, datetime_end, all_day_datetime=True, from_cached_final=True) for e in events: if e["end"] < now: # shift is finished, ignore @@ -475,13 +471,13 @@ def quality_report(self, date: typing.Optional[datetime.datetime], days: typing. """ # get events to consider for calculation if date is None: - today = datetime.datetime.now(tz=timezone.utc) + today = timezone.now() date = today - datetime.timedelta(days=7 - today.weekday()) # start of next week in UTC if days is None: days = 52 * 7 # consider next 52 weeks (~1 year) + datetime_end = date + datetime.timedelta(days=days - 1, hours=23, minutes=59, seconds=59) - events = self.final_events(user_tz="UTC", starting_date=date, days=days) - + events = self.final_events(date, datetime_end) # an event is “good” if it's not a gap and not empty good_events: ScheduleEvents = [event for event in events if not event["is_gap"] and not event["is_empty"]] if not good_events: @@ -591,8 +587,13 @@ def get_balance_score_by_duration_map(dur_map: DurationMap) -> float: "overloaded_users": overloaded_users, } - def _resolve_schedule(self, events: ScheduleEvents) -> ScheduleEvents: - """Calculate final schedule shifts considering rotations and overrides.""" + def _resolve_schedule( + self, events: ScheduleEvents, datetime_start: datetime.datetime, datetime_end: datetime.datetime + ) -> ScheduleEvents: + """Calculate final schedule shifts considering rotations and overrides. + + Exclude events that after split/update are out of the requested (datetime_start, datetime_end) range. + """ if not events: return [] @@ -678,16 +679,20 @@ def _merge_intervals(evs: ScheduleEvents) -> ScheduleEventIntervals: # 1. add a split event copy to schedule the time before the already scheduled interval to_add = ev.copy() to_add["end"] = intervals[current_interval_idx][0] - resolved.append(to_add) + if to_add["end"] >= datetime_start: + # only include if updated event ends inside the requested time range + resolved.append(to_add) # 2. check if there is still time to be scheduled after the current scheduled interval ends if ev["end"] > intervals[current_interval_idx][1]: # event ends after current interval, update event start timestamp to match the interval end # and process the updated event as any other event ev["start"] = intervals[current_interval_idx][1] - # reorder pending events after updating current event start date - # (ie. insert the event where it should be to keep the order criteria) - # TODO: switch to bisect insert on python 3.10 (or consider heapq) - insort_event(pending, ev) + if ev["start"] < datetime_end: + # only include event if it is still inside the requested time range + # reorder pending events after updating current event start date + # (ie. insert the event where it should be to keep the order criteria) + # TODO: switch to bisect insert on python 3.10 (or consider heapq) + insort_event(pending, ev) # done, go to next event elif ev["start"] >= intervals[current_interval_idx][0] and ev["end"] <= intervals[current_interval_idx][1]: @@ -757,7 +762,7 @@ def _generate_ical_file_from_shifts(self, qs, extra_shifts=None, allow_empty_use ical += f"{end_line}\r\n" return ical - def preview_shift(self, custom_shift, user_tz, starting_date, days, updated_shift_pk=None): + def preview_shift(self, custom_shift, datetime_start, datetime_end, updated_shift_pk=None): """Return unsaved rotation and final schedule preview events.""" if custom_shift.type == CustomOnCallShift.TYPE_OVERRIDE: qs = self.custom_shifts.filter(type=CustomOnCallShift.TYPE_OVERRIDE) @@ -790,11 +795,12 @@ def _invalidate_cache(schedule, prop_name): setattr(self, ical_attr, ical_file) # filter events using a temporal overriden calendar including the not-yet-saved shift - events = self.filter_events(user_tz, starting_date, days=days, with_empty=True, with_gap=True) + events = self.filter_events(datetime_start, datetime_end, with_empty=True, with_gap=True) + # return preview events for affected shifts updated_shift_pks = {s.public_primary_key for s in extra_shifts} shift_events = [e.copy() for e in events if e["shift"]["pk"] in updated_shift_pks] - final_events = self._resolve_schedule(events) + final_events = self._resolve_schedule(events, datetime_start, datetime_end) _invalidate_cache(self, ical_property) setattr(self, ical_attr, original_value) @@ -993,11 +999,11 @@ def _generate_ical_file_primary(self): ical += f"{end_line}\r\n" return ical - def preview_shift(self, custom_shift, user_tz, starting_date, days, updated_shift_pk=None): + def preview_shift(self, custom_shift, datetime_start, datetime_end, updated_shift_pk=None): """Return unsaved rotation and final schedule preview events.""" if custom_shift.type != CustomOnCallShift.TYPE_OVERRIDE: raise ValueError("Invalid shift type") - return super().preview_shift(custom_shift, user_tz, starting_date, days, updated_shift_pk=updated_shift_pk) + return super().preview_shift(custom_shift, datetime_start, datetime_end, updated_shift_pk=updated_shift_pk) @property def insight_logs_type_verbal(self): diff --git a/engine/apps/schedules/tests/test_ical_utils.py b/engine/apps/schedules/tests/test_ical_utils.py index 822ac6adfc..fdd4b365d7 100644 --- a/engine/apps/schedules/tests/test_ical_utils.py +++ b/engine/apps/schedules/tests/test_ical_utils.py @@ -83,23 +83,18 @@ def test_users_in_ical_email_case_insensitive(make_organization_and_user, make_u @pytest.mark.django_db -@pytest.mark.parametrize("include_viewers", [True, False]) -def test_users_in_ical_viewers_inclusion(make_organization_and_user, make_user_for_organization, include_viewers): +def test_users_in_ical_viewers_inclusion(make_organization_and_user, make_user_for_organization): organization, user = make_organization_and_user() viewer = make_user_for_organization(organization, role=LegacyAccessControlRole.VIEWER) usernames = [user.username, viewer.username] - result = users_in_ical(usernames, organization, include_viewers=include_viewers) - if include_viewers: - assert set(result) == {user, viewer} - else: - assert set(result) == {user} + result = users_in_ical(usernames, organization) + assert set(result) == {user} @pytest.mark.django_db -@pytest.mark.parametrize("include_viewers", [True, False]) def test_list_users_to_notify_from_ical_viewers_inclusion( - make_organization_and_user, make_user_for_organization, make_schedule, make_on_call_shift, include_viewers + make_organization_and_user, make_user_for_organization, make_schedule, make_on_call_shift ): organization, user = make_organization_and_user() viewer = make_user_for_organization(organization, role=LegacyAccessControlRole.VIEWER) @@ -121,14 +116,10 @@ def test_list_users_to_notify_from_ical_viewers_inclusion( # get users on-call date = date + timezone.timedelta(minutes=5) - users_on_call = list_users_to_notify_from_ical(schedule, date, include_viewers=include_viewers) + users_on_call = list_users_to_notify_from_ical(schedule, date) - if include_viewers: - assert len(users_on_call) == 2 - assert set(users_on_call) == {user, viewer} - else: - assert len(users_on_call) == 1 - assert set(users_on_call) == {user} + assert len(users_on_call) == 1 + assert set(users_on_call) == {user} @pytest.mark.django_db @@ -161,7 +152,49 @@ def test_list_users_to_notify_from_ical_until_terminated_event( date = date + timezone.timedelta(minutes=5) # this should not raise despite the shift configuration (until < rotation start) users_on_call = list_users_to_notify_from_ical(schedule, date) - assert users_on_call == [] + assert list(users_on_call) == [] + + +@pytest.mark.django_db +def test_list_users_to_notify_from_ical_overlapping_events( + make_organization_and_user, make_user_for_organization, make_schedule, make_on_call_shift +): + organization, user = make_organization_and_user() + another_user = make_user_for_organization(organization) + + schedule = make_schedule(organization, schedule_class=OnCallScheduleWeb) + start = timezone.now() - timezone.timedelta(hours=1) + data = { + "start": start, + "rotation_start": start, + "duration": timezone.timedelta(hours=3), + "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]]) + + data = { + "start": start + timezone.timedelta(minutes=30), + "rotation_start": start + timezone.timedelta(minutes=30), + "duration": timezone.timedelta(hours=2), + "priority_level": 2, + "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([[another_user]]) + + # get users on-call now + users_on_call = list_users_to_notify_from_ical(schedule) + + assert len(users_on_call) == 1 + assert set(users_on_call) == {another_user} @pytest.mark.django_db @@ -173,17 +206,26 @@ def test_shifts_dict_all_day_middle_event(make_organization, make_schedule, get_ day_to_check_iso = "2021-01-27T15:27:14.448059+00:00" parsed_iso_day_to_check = datetime.datetime.fromisoformat(day_to_check_iso).replace(tzinfo=pytz.UTC) - requested_date = (parsed_iso_day_to_check - timezone.timedelta(days=1)).date() - shifts = list_of_oncall_shifts_from_ical(schedule, requested_date, days=3, with_empty_shifts=True) + requested_datetime = parsed_iso_day_to_check - timezone.timedelta(days=1) + datetime_end = requested_datetime + timezone.timedelta(days=2) + shifts = list_of_oncall_shifts_from_ical(schedule, requested_datetime, datetime_end, with_empty_shifts=True) assert len(shifts) == 5 for s in shifts: - start = s["start"].date() if isinstance(s["start"], datetime.datetime) else s["start"] - end = s["end"].date() if isinstance(s["end"], datetime.datetime) else s["end"] + start = ( + s["start"] + if isinstance(s["start"], datetime.datetime) + else datetime.datetime.combine(s["start"], datetime.time.min, tzinfo=pytz.UTC) + ) + end = ( + s["end"] + if isinstance(s["end"], datetime.datetime) + else datetime.datetime.combine(s["start"], datetime.time.max, tzinfo=pytz.UTC) + ) # event started in the given period, or ended in that period, or is happening during the period assert ( - requested_date <= start <= requested_date + timezone.timedelta(days=3) - or requested_date <= end <= requested_date + timezone.timedelta(days=3) - or start <= requested_date <= end + requested_datetime <= start <= requested_datetime + timezone.timedelta(days=2) + or requested_datetime <= end <= requested_datetime + timezone.timedelta(days=2) + or start <= requested_datetime <= end ) @@ -197,7 +239,8 @@ def test_shifts_dict_from_cached_final( organization = make_organization() u1 = make_user_for_organization(organization) - yesterday = timezone.now().replace(hour=0, minute=0, second=0, microsecond=0) - timezone.timedelta(days=1) + today = timezone.now().replace(hour=0, minute=0, second=0, microsecond=0) + yesterday = today - timezone.timedelta(days=1) schedule = make_schedule(organization, schedule_class=OnCallScheduleWeb) data = { "start": yesterday + timezone.timedelta(hours=10), @@ -227,7 +270,7 @@ def test_shifts_dict_from_cached_final( shifts = [ (s["calendar_type"], s["start"], list(s["users"])) - for s in list_of_oncall_shifts_from_ical(schedule, yesterday, days=1, from_cached_final=True) + for s in list_of_oncall_shifts_from_ical(schedule, yesterday, today, from_cached_final=True) ] expected_events = [ (OnCallSchedule.PRIMARY, on_call_shift.start, [u1]), diff --git a/engine/apps/schedules/tests/test_on_call_schedule.py b/engine/apps/schedules/tests/test_on_call_schedule.py index 8817b747dd..6b8e61efcc 100644 --- a/engine/apps/schedules/tests/test_on_call_schedule.py +++ b/engine/apps/schedules/tests/test_on_call_schedule.py @@ -81,7 +81,8 @@ def test_filter_events(make_organization, make_user_for_organization, make_sched override.add_rolling_users([[user]]) # filter primary non-empty shifts only - events = schedule.filter_events("UTC", start_date, days=3, filter_by=OnCallSchedule.TYPE_ICAL_PRIMARY) + end_date = start_date + timezone.timedelta(days=3) + events = schedule.filter_events(start_date, end_date, filter_by=OnCallSchedule.TYPE_ICAL_PRIMARY) expected = [ { "calendar_type": OnCallSchedule.TYPE_ICAL_PRIMARY, @@ -109,7 +110,8 @@ def test_filter_events(make_organization, make_user_for_organization, make_sched assert events == expected # filter overrides only - events = schedule.filter_events("UTC", start_date, days=3, filter_by=OnCallSchedule.TYPE_ICAL_OVERRIDES) + end_date = start_date + timezone.timedelta(days=3) + events = schedule.filter_events(start_date, end_date, filter_by=OnCallSchedule.TYPE_ICAL_OVERRIDES) expected_override = [ { "calendar_type": OnCallSchedule.TYPE_ICAL_OVERRIDES, @@ -136,7 +138,8 @@ def test_filter_events(make_organization, make_user_for_organization, make_sched assert events == expected_override # no type filter - events = schedule.filter_events("UTC", start_date, days=3) + end_date = start_date + timezone.timedelta(days=3) + events = schedule.filter_events(start_date, end_date) assert events == expected_override + expected @@ -165,13 +168,12 @@ def test_filter_events_include_gaps(make_organization, make_user_for_organizatio ) on_call_shift.add_rolling_users([[user]]) - events = schedule.filter_events( - "UTC", start_date, days=1, filter_by=OnCallSchedule.TYPE_ICAL_PRIMARY, with_gap=True - ) + end_date = start_date + timezone.timedelta(days=1) + events = schedule.filter_events(start_date, end_date, filter_by=OnCallSchedule.TYPE_ICAL_PRIMARY, with_gap=True) expected = [ { "calendar_type": None, - "start": start_date + timezone.timedelta(milliseconds=1), + "start": start_date, "end": on_call_shift.start, "all_day": False, "is_override": False, @@ -207,7 +209,7 @@ def test_filter_events_include_gaps(make_organization, make_user_for_organizatio { "calendar_type": None, "start": on_call_shift.start + on_call_shift.duration, - "end": on_call_shift.start + timezone.timedelta(hours=13, minutes=59, seconds=59, milliseconds=1), + "end": on_call_shift.start + timezone.timedelta(hours=14), "all_day": False, "is_override": False, "is_empty": False, @@ -247,9 +249,8 @@ def test_filter_events_include_empty(make_organization, make_user_for_organizati ) on_call_shift.add_rolling_users([[user]]) - events = schedule.filter_events( - "UTC", start_date, days=1, filter_by=OnCallSchedule.TYPE_ICAL_PRIMARY, with_empty=True - ) + end_date = start_date + timezone.timedelta(days=1) + events = schedule.filter_events(start_date, end_date, filter_by=OnCallSchedule.TYPE_ICAL_PRIMARY, with_empty=True) expected = [ { "calendar_type": OnCallSchedule.TYPE_ICAL_PRIMARY, @@ -282,9 +283,10 @@ def test_filter_events_ical_all_day(make_organization, make_user_for_organizatio day_to_check_iso = "2021-01-27T15:27:14.448059+00:00" parsed_iso_day_to_check = datetime.datetime.fromisoformat(day_to_check_iso).replace(tzinfo=pytz.UTC) - start_date = (parsed_iso_day_to_check - timezone.timedelta(days=1)).date() + datetime_start = parsed_iso_day_to_check - timezone.timedelta(days=1) + datetime_end = datetime_start + datetime.timedelta(days=1, hours=23, minutes=59, seconds=59) - events = schedule.final_events("UTC", start_date, days=2) + events = schedule.final_events(datetime_start, datetime_end) expected_events = [ # all_day, users, start, end ( @@ -311,6 +313,12 @@ def test_filter_events_ical_all_day(make_organization, make_user_for_organizatio datetime.datetime(2021, 1, 27, 8, 0, tzinfo=pytz.UTC), datetime.datetime(2021, 1, 27, 17, 0, tzinfo=pytz.UTC), ), + ( + False, + ["@Bernard Desruisseaux"], + datetime.datetime(2021, 1, 28, 8, 0, tzinfo=pytz.UTC), + datetime.datetime(2021, 1, 28, 17, 0, tzinfo=pytz.UTC), + ), ] expected = [ {"all_day": all_day, "users": users, "start": start, "end": end} @@ -388,7 +396,8 @@ def test_final_schedule_events(make_organization, make_user_for_organization, ma ) on_call_shift.add_rolling_users([[user]]) - returned_events = schedule.final_events("UTC", start_date, days=1) + datetime_end = start_date + timezone.timedelta(days=1) + returned_events = schedule.final_events(start_date, datetime_end) expected = ( # start (h), duration (H), user, priority, is_gap, is_override @@ -414,7 +423,7 @@ def test_final_schedule_events(make_organization, make_user_for_organization, ma "is_gap": is_gap, "is_override": is_override, "priority_level": priority, - "start": start_date + timezone.timedelta(hours=start, milliseconds=1 if start == 0 else 0), + "start": start_date + timezone.timedelta(hours=start), "user": user, } for start, duration, user, priority, is_gap, is_override in expected @@ -482,7 +491,8 @@ def test_final_schedule_override_no_priority_shift( ) override.add_rolling_users([[user_b]]) - returned_events = schedule.final_events("UTC", start_date, days=1) + datetime_end = start_date + timezone.timedelta(days=1) + returned_events = schedule.final_events(start_date, datetime_end) expected = ( # start (h), duration (H), user, priority, is_override @@ -552,7 +562,8 @@ def test_final_schedule_splitting_events( ) on_call_shift.add_rolling_users([[user]]) - returned_events = schedule.final_events("UTC", start_date, days=1) + datetime_end = start_date + timezone.timedelta(days=1) + returned_events = schedule.final_events(start_date, datetime_end) expected = ( # start (h), duration (H), user, priority @@ -621,7 +632,8 @@ def test_final_schedule_splitting_same_time_events( ) on_call_shift.add_rolling_users([[user]]) - returned_events = schedule.final_events("UTC", start_date, days=1) + datetime_end = start_date + timezone.timedelta(days=1) + returned_events = schedule.final_events(start_date, datetime_end) expected = ( # start (h), duration (H), user, priority @@ -695,7 +707,8 @@ def test_preview_shift(make_organization, make_user_for_organization, make_sched rolling_users=[{other_user.pk: other_user.public_primary_key}], ) - rotation_events, final_events = schedule.preview_shift(new_shift, "UTC", start_date, days=1) + datetime_end = start_date + timezone.timedelta(days=1) + rotation_events, final_events = schedule.preview_shift(new_shift, start_date, datetime_end) # check rotation events expected_rotation_events = [ @@ -796,7 +809,8 @@ def test_preview_shift_do_not_change_rotation_events( ) other_shift.add_rolling_users([[other_user]]) - rotation_events, final_events = schedule.preview_shift(on_call_shift, "UTC", start_date, days=1) + datetime_end = start_date + timezone.timedelta(days=1) + rotation_events, final_events = schedule.preview_shift(on_call_shift, start_date, datetime_end) # check rotation events expected_rotation_events = [ @@ -852,7 +866,8 @@ def test_preview_shift_no_user(make_organization, make_user_for_organization, ma rolling_users=[], ) - rotation_events, final_events = schedule.preview_shift(new_shift, "UTC", start_date, days=1) + datetime_end = start_date + timezone.timedelta(days=1) + rotation_events, final_events = schedule.preview_shift(new_shift, start_date, datetime_end) # check rotation events expected_rotation_events = [ @@ -930,7 +945,8 @@ def test_preview_override_shift(make_organization, make_user_for_organization, m rolling_users=[{other_user.pk: other_user.public_primary_key}], ) - rotation_events, final_events = schedule.preview_shift(new_shift, "UTC", start_date, days=1) + datetime_end = start_date + timezone.timedelta(days=1) + rotation_events, final_events = schedule.preview_shift(new_shift, start_date, datetime_end) # check rotation events expected_rotation_events = [ @@ -1089,7 +1105,8 @@ def test_filter_events_none_cache_unchanged( # schedule is removed from db schedule.delete() - events = schedule.filter_events("UTC", start_date, days=5, filter_by=OnCallSchedule.TYPE_ICAL_PRIMARY) + end_date = start_date + timezone.timedelta(days=5) + events = schedule.filter_events(start_date, end_date, filter_by=OnCallSchedule.TYPE_ICAL_PRIMARY) expected = [] assert events == expected @@ -1272,7 +1289,8 @@ def test_api_schedule_preview_requires_override(make_organization, make_schedule ) with pytest.raises(ValueError): - schedule.preview_shift(non_override_shift, "UTC", now, 1) + datetime_end = now + timezone.timedelta(days=1) + schedule.preview_shift(non_override_shift, now, datetime_end) @pytest.mark.django_db @@ -1817,4 +1835,5 @@ def test_event_until_non_utc(make_organization, make_schedule): now = timezone.now().replace(hour=0, minute=0, second=0, microsecond=0) # check this works without raising exception - schedule.final_events("UTC", now, days=7) + datetime_end = now + timezone.timedelta(days=7) + schedule.final_events(now, datetime_end) diff --git a/engine/apps/schedules/tests/test_quality_score.py b/engine/apps/schedules/tests/test_quality_score.py index 5926f9297f..bbcb794923 100644 --- a/engine/apps/schedules/tests/test_quality_score.py +++ b/engine/apps/schedules/tests/test_quality_score.py @@ -190,7 +190,7 @@ def test_get_schedule_score_weekdays( assert response.json() == { "total_score": 86, "comments": [ - {"type": "warning", "text": "Schedule has gaps (29% not covered)"}, + {"type": "warning", "text": "Schedule has gaps (28% not covered)"}, {"type": "info", "text": "Schedule is perfectly balanced"}, ], "overloaded_users": [], @@ -351,7 +351,7 @@ def test_get_schedule_score_all_week_imbalanced_weekends( { "id": user.public_primary_key, "username": user.username, - "score": 29, + "score": 28, } for user in users[:4] ],