diff --git a/CHANGELOG.md b/CHANGELOG.md index 87b53a30..dc2ca270 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -23,10 +23,6 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 parsed, when they were space-delimited. This fix allows them to be _either_ space or comma-delimited. by @joeyorlando ([#2623](https://github.com/grafana/oncall/pull/2623)) -### Changed - -- Update checking on-call users to use schedule final events ([#2625](https://github.com/grafana/oncall/pull/2625)) - ## v1.3.16 (2023-07-21) ### Added 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 8ff60c30..ef5c202e 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 @@ class EscalationPolicySnapshot: escalation_policy_step=self.step, ) else: - notify_to_users_list = list_users_to_notify_from_ical(on_call_schedule) + notify_to_users_list = list_users_to_notify_from_ical(on_call_schedule, include_viewers=True) 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 bc79abc7..7ba39f00 100644 --- a/engine/apps/alerts/tests/test_escalation_policy_snapshot.py +++ b/engine/apps/alerts/tests/test_escalation_policy_snapshot.py @@ -246,8 +246,11 @@ 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_FAILED).exists() - assert list(escalation_policy_snapshot.notify_to_users_queue) == [] + 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 mocked_execute_tasks.called diff --git a/engine/apps/api/tests/test_schedules.py b/engine/apps/api/tests/test_schedules.py index ad7f64c5..4cf8eb9c 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), + "start": start_date + timezone.timedelta(hours=start, milliseconds=1 if start == 0 else 0), "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 62c4e36f..c760b53f 100644 --- a/engine/apps/api/views/on_call_shifts.py +++ b/engine/apps/api/views/on_call_shifts.py @@ -1,6 +1,3 @@ -import datetime - -import pytz from django.db.models import Q from django_filters.rest_framework import DjangoFilterBackend from rest_framework import status @@ -109,13 +106,8 @@ class OnCallShiftView(TeamFilteringMixin, PublicPrimaryKeyMixin, UpdateSerialize 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, datetime_start, datetime_end, updated_shift_pk=updated_shift_pk + shift, user_tz, starting_date, days, 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 7413920f..d6960c9d 100644 --- a/engine/apps/api/views/schedule.py +++ b/engine/apps/api/views/schedule.py @@ -1,8 +1,6 @@ -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 @@ -276,16 +274,12 @@ class ScheduleView( @action(detail=True, methods=["get"]) def events(self, request, pk): - user_tz, starting_date = self.get_request_timezone() + user_tz, 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() - - 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) + events = schedule.filter_events(user_tz, date, days=1, with_empty=with_empty, with_gap=with_gap) slack_channel = ( { @@ -318,22 +312,19 @@ class ScheduleView( 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( - datetime_start, - datetime_end, + user_tz, + starting_date, + days=days, with_empty=True, with_gap=resolve_schedule, filter_by=filter_by, all_day_datetime=True, ) else: # return final schedule - events = schedule.final_events(datetime_start, datetime_end) + events = schedule.final_events(user_tz, starting_date, days) result = { "id": schedule.public_primary_key, @@ -346,11 +337,11 @@ class ScheduleView( @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() - datetime_end = now + datetime.timedelta(days=30) + starting_date = now.date() schedule = self.get_object() - - events = schedule.final_events(now, datetime_end) + events = schedule.final_events(user_tz, starting_date, days=30) users = {u.public_primary_key: None for u in schedule.related_users()} for e in events: @@ -382,11 +373,10 @@ class ScheduleView( 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(datetime_start, days)) + return Response(schedule.quality_report(date, 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 8e7d0af1..9b16baae 100644 --- a/engine/apps/mobile_app/tasks.py +++ b/engine/apps/mobile_app/tasks.py @@ -439,8 +439,7 @@ def conditionally_send_going_oncall_push_notifications_for_schedule(schedule_pk) return now = timezone.now() - datetime_end = now + datetime.timedelta(days=7) - schedule_final_events = schedule.final_events(now, datetime_end) + schedule_final_events = schedule.final_events("UTC", now, days=7) 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 9c398f26..83aa3207 100644 --- a/engine/apps/public_api/views/schedules.py +++ b/engine/apps/public_api/views/schedules.py @@ -1,7 +1,5 @@ -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 @@ -149,12 +147,8 @@ class OnCallScheduleChannelView(RateLimitHeadersMixin, UpdateSerializerMixin, Mo end_date = serializer.validated_data["end_date"] days_between_start_and_end = (end_date - start_date).days - 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("UTC", start_date, days_between_start_and_end) - 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 72285e71..bbdda581 100644 --- a/engine/apps/schedules/ical_utils.py +++ b/engine/apps/schedules/ical_utils.py @@ -61,6 +61,7 @@ IcalEvents = typing.List[IcalEvent] 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"]: """ @@ -93,10 +94,11 @@ def users_in_ical( } ) - # 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 = 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 = users_found_in_ical.filter( (Q(username__in=usernames_from_ical) | Q(email__lower__in=emails_from_ical)) @@ -116,10 +118,11 @@ def memoized_users_in_ical( # used for display schedule events on web def list_of_oncall_shifts_from_ical( schedule: "OnCallSchedule", - datetime_start: datetime.datetime, - datetime_end: datetime.datetime, + date: datetime.date, + user_timezone: str = "UTC", with_empty_shifts: bool = False, with_gaps: bool = False, + days: int = 1, filter_by: str | None = None, from_cached_final: bool = False, ): @@ -149,6 +152,16 @@ 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 = [] @@ -191,7 +204,6 @@ 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 @@ -336,6 +348,7 @@ 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"]: """ @@ -346,6 +359,7 @@ def list_users_to_notify_from_ical( schedule, events_datetime, events_datetime, + include_viewers=include_viewers, users_to_filter=users_to_filter, ) @@ -354,15 +368,35 @@ 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"] = [] - events = schedule.final_events(start_datetime, end_datetime) - usernames = [] - for event in events: - usernames += [u["email"] for u in event.get("users", [])] + # 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) - users_found_in_ical = users_in_ical(usernames, schedule.organization, users_to_filter=users_to_filter) + 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 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 68fa401e..a63f46b0 100644 --- a/engine/apps/schedules/models/on_call_schedule.py +++ b/engine/apps/schedules/models/on_call_schedule.py @@ -307,8 +307,9 @@ class OnCallSchedule(PolymorphicModel): def filter_events( self, - datetime_start, - datetime_end, + user_timezone, + starting_date, + days, with_empty=False, with_gap=False, filter_by=None, @@ -319,10 +320,11 @@ class OnCallSchedule(PolymorphicModel): shifts = ( list_of_oncall_shifts_from_ical( self, - datetime_start, - datetime_end, + starting_date, + user_timezone, with_empty, with_gap, + days=days, filter_by=filter_by, from_cached_final=from_cached_final, ) @@ -367,23 +369,26 @@ class OnCallSchedule(PolymorphicModel): # combine multiple-users same-shift events into one return self._merge_events(events) - def final_events(self, datetime_start, datetime_end): + def final_events(self, user_tz, starting_date, days): """Return schedule final events, after resolving shifts and overrides.""" - events = self.filter_events(datetime_start, datetime_end, with_empty=True, with_gap=True, all_day_datetime=True) + 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) 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(datetime_start, datetime_end) + events = self.final_events(tz, starting_date, days) updated_ids = set() for e in events: for u in e["users"]: @@ -412,12 +417,12 @@ class OnCallSchedule(PolymorphicModel): dtend_datetime = datetime.datetime.combine( dtend.dt, datetime.datetime.min.time(), tzinfo=pytz.UTC ) - if dtend_datetime and dtend_datetime < datetime_start: + if dtend_datetime and dtend_datetime < starting_datetime: # 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 < datetime_start: + if is_cancelled and last_modified and last_modified.dt < starting_datetime: # drop already ended events older than the window we consider continue elif is_cancelled and not last_modified: @@ -436,18 +441,17 @@ class OnCallSchedule(PolymorphicModel): 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 - datetime_start = now - datetime.timedelta(days=1) - datetime_end = datetime_start + datetime.timedelta(days=days) - + starting_date = now.date() - datetime.timedelta(days=1) 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(datetime_start, datetime_end, all_day_datetime=True, from_cached_final=True) + events = self.filter_events(user_tz, starting_date, days=days, all_day_datetime=True, from_cached_final=True) for e in events: if e["end"] < now: # shift is finished, ignore @@ -471,13 +475,13 @@ class OnCallSchedule(PolymorphicModel): """ # get events to consider for calculation if date is None: - today = timezone.now() + today = datetime.datetime.now(tz=timezone.utc) 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(date, datetime_end) + events = self.final_events(user_tz="UTC", starting_date=date, days=days) + # 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: @@ -753,7 +757,7 @@ class OnCallSchedule(PolymorphicModel): ical += f"{end_line}\r\n" return ical - def preview_shift(self, custom_shift, datetime_start, datetime_end, updated_shift_pk=None): + def preview_shift(self, custom_shift, user_tz, starting_date, days, 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) @@ -786,8 +790,7 @@ class OnCallSchedule(PolymorphicModel): setattr(self, ical_attr, ical_file) # filter events using a temporal overriden calendar including the not-yet-saved shift - events = self.filter_events(datetime_start, datetime_end, with_empty=True, with_gap=True) - + events = self.filter_events(user_tz, starting_date, days=days, 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] @@ -990,11 +993,11 @@ class OnCallScheduleCalendar(OnCallSchedule): ical += f"{end_line}\r\n" return ical - def preview_shift(self, custom_shift, datetime_start, datetime_end, updated_shift_pk=None): + def preview_shift(self, custom_shift, user_tz, starting_date, days, 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, datetime_start, datetime_end, updated_shift_pk=updated_shift_pk) + return super().preview_shift(custom_shift, user_tz, starting_date, days, 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 41c06266..822ac6ad 100644 --- a/engine/apps/schedules/tests/test_ical_utils.py +++ b/engine/apps/schedules/tests/test_ical_utils.py @@ -83,18 +83,23 @@ def test_users_in_ical_email_case_insensitive(make_organization_and_user, make_u @pytest.mark.django_db -def test_users_in_ical_viewers_inclusion(make_organization_and_user, make_user_for_organization): +@pytest.mark.parametrize("include_viewers", [True, False]) +def test_users_in_ical_viewers_inclusion(make_organization_and_user, make_user_for_organization, include_viewers): 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) - assert set(result) == {user} + result = users_in_ical(usernames, organization, include_viewers=include_viewers) + if include_viewers: + assert set(result) == {user, viewer} + else: + 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 + make_organization_and_user, make_user_for_organization, make_schedule, make_on_call_shift, include_viewers ): organization, user = make_organization_and_user() viewer = make_user_for_organization(organization, role=LegacyAccessControlRole.VIEWER) @@ -116,10 +121,14 @@ 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) + users_on_call = list_users_to_notify_from_ical(schedule, date, include_viewers=include_viewers) - assert len(users_on_call) == 1 - assert set(users_on_call) == {user} + 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} @pytest.mark.django_db @@ -152,7 +161,7 @@ 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 list(users_on_call) == [] + assert users_on_call == [] @pytest.mark.django_db @@ -164,26 +173,17 @@ 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_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) + 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) assert len(shifts) == 5 for s in shifts: - 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) - ) + 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"] # event started in the given period, or ended in that period, or is happening during the period assert ( - requested_datetime <= start <= requested_datetime + timezone.timedelta(days=2) - or requested_datetime <= end <= requested_datetime + timezone.timedelta(days=2) - or start <= requested_datetime <= end + requested_date <= start <= requested_date + timezone.timedelta(days=3) + or requested_date <= end <= requested_date + timezone.timedelta(days=3) + or start <= requested_date <= end ) @@ -197,8 +197,7 @@ def test_shifts_dict_from_cached_final( organization = make_organization() u1 = make_user_for_organization(organization) - today = timezone.now().replace(hour=0, minute=0, second=0, microsecond=0) - yesterday = today - timezone.timedelta(days=1) + yesterday = timezone.now().replace(hour=0, minute=0, second=0, microsecond=0) - timezone.timedelta(days=1) schedule = make_schedule(organization, schedule_class=OnCallScheduleWeb) data = { "start": yesterday + timezone.timedelta(hours=10), @@ -228,7 +227,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, today, from_cached_final=True) + for s in list_of_oncall_shifts_from_ical(schedule, yesterday, days=1, 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 6b8e61ef..8817b747 100644 --- a/engine/apps/schedules/tests/test_on_call_schedule.py +++ b/engine/apps/schedules/tests/test_on_call_schedule.py @@ -81,8 +81,7 @@ def test_filter_events(make_organization, make_user_for_organization, make_sched override.add_rolling_users([[user]]) # filter primary non-empty shifts only - end_date = start_date + timezone.timedelta(days=3) - events = schedule.filter_events(start_date, end_date, filter_by=OnCallSchedule.TYPE_ICAL_PRIMARY) + events = schedule.filter_events("UTC", start_date, days=3, filter_by=OnCallSchedule.TYPE_ICAL_PRIMARY) expected = [ { "calendar_type": OnCallSchedule.TYPE_ICAL_PRIMARY, @@ -110,8 +109,7 @@ def test_filter_events(make_organization, make_user_for_organization, make_sched assert events == expected # filter overrides only - end_date = start_date + timezone.timedelta(days=3) - events = schedule.filter_events(start_date, end_date, filter_by=OnCallSchedule.TYPE_ICAL_OVERRIDES) + events = schedule.filter_events("UTC", start_date, days=3, filter_by=OnCallSchedule.TYPE_ICAL_OVERRIDES) expected_override = [ { "calendar_type": OnCallSchedule.TYPE_ICAL_OVERRIDES, @@ -138,8 +136,7 @@ def test_filter_events(make_organization, make_user_for_organization, make_sched assert events == expected_override # no type filter - end_date = start_date + timezone.timedelta(days=3) - events = schedule.filter_events(start_date, end_date) + events = schedule.filter_events("UTC", start_date, days=3) assert events == expected_override + expected @@ -168,12 +165,13 @@ def test_filter_events_include_gaps(make_organization, make_user_for_organizatio ) on_call_shift.add_rolling_users([[user]]) - 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) + events = schedule.filter_events( + "UTC", start_date, days=1, filter_by=OnCallSchedule.TYPE_ICAL_PRIMARY, with_gap=True + ) expected = [ { "calendar_type": None, - "start": start_date, + "start": start_date + timezone.timedelta(milliseconds=1), "end": on_call_shift.start, "all_day": False, "is_override": False, @@ -209,7 +207,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=14), + "end": on_call_shift.start + timezone.timedelta(hours=13, minutes=59, seconds=59, milliseconds=1), "all_day": False, "is_override": False, "is_empty": False, @@ -249,8 +247,9 @@ def test_filter_events_include_empty(make_organization, make_user_for_organizati ) on_call_shift.add_rolling_users([[user]]) - 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) + events = schedule.filter_events( + "UTC", start_date, days=1, filter_by=OnCallSchedule.TYPE_ICAL_PRIMARY, with_empty=True + ) expected = [ { "calendar_type": OnCallSchedule.TYPE_ICAL_PRIMARY, @@ -283,10 +282,9 @@ 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) - 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) + start_date = (parsed_iso_day_to_check - timezone.timedelta(days=1)).date() - events = schedule.final_events(datetime_start, datetime_end) + events = schedule.final_events("UTC", start_date, days=2) expected_events = [ # all_day, users, start, end ( @@ -313,12 +311,6 @@ 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} @@ -396,8 +388,7 @@ def test_final_schedule_events(make_organization, make_user_for_organization, ma ) on_call_shift.add_rolling_users([[user]]) - datetime_end = start_date + timezone.timedelta(days=1) - returned_events = schedule.final_events(start_date, datetime_end) + returned_events = schedule.final_events("UTC", start_date, days=1) expected = ( # start (h), duration (H), user, priority, is_gap, is_override @@ -423,7 +414,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), + "start": start_date + timezone.timedelta(hours=start, milliseconds=1 if start == 0 else 0), "user": user, } for start, duration, user, priority, is_gap, is_override in expected @@ -491,8 +482,7 @@ def test_final_schedule_override_no_priority_shift( ) override.add_rolling_users([[user_b]]) - datetime_end = start_date + timezone.timedelta(days=1) - returned_events = schedule.final_events(start_date, datetime_end) + returned_events = schedule.final_events("UTC", start_date, days=1) expected = ( # start (h), duration (H), user, priority, is_override @@ -562,8 +552,7 @@ def test_final_schedule_splitting_events( ) on_call_shift.add_rolling_users([[user]]) - datetime_end = start_date + timezone.timedelta(days=1) - returned_events = schedule.final_events(start_date, datetime_end) + returned_events = schedule.final_events("UTC", start_date, days=1) expected = ( # start (h), duration (H), user, priority @@ -632,8 +621,7 @@ def test_final_schedule_splitting_same_time_events( ) on_call_shift.add_rolling_users([[user]]) - datetime_end = start_date + timezone.timedelta(days=1) - returned_events = schedule.final_events(start_date, datetime_end) + returned_events = schedule.final_events("UTC", start_date, days=1) expected = ( # start (h), duration (H), user, priority @@ -707,8 +695,7 @@ def test_preview_shift(make_organization, make_user_for_organization, make_sched rolling_users=[{other_user.pk: other_user.public_primary_key}], ) - datetime_end = start_date + timezone.timedelta(days=1) - rotation_events, final_events = schedule.preview_shift(new_shift, start_date, datetime_end) + rotation_events, final_events = schedule.preview_shift(new_shift, "UTC", start_date, days=1) # check rotation events expected_rotation_events = [ @@ -809,8 +796,7 @@ def test_preview_shift_do_not_change_rotation_events( ) other_shift.add_rolling_users([[other_user]]) - datetime_end = start_date + timezone.timedelta(days=1) - rotation_events, final_events = schedule.preview_shift(on_call_shift, start_date, datetime_end) + rotation_events, final_events = schedule.preview_shift(on_call_shift, "UTC", start_date, days=1) # check rotation events expected_rotation_events = [ @@ -866,8 +852,7 @@ def test_preview_shift_no_user(make_organization, make_user_for_organization, ma rolling_users=[], ) - datetime_end = start_date + timezone.timedelta(days=1) - rotation_events, final_events = schedule.preview_shift(new_shift, start_date, datetime_end) + rotation_events, final_events = schedule.preview_shift(new_shift, "UTC", start_date, days=1) # check rotation events expected_rotation_events = [ @@ -945,8 +930,7 @@ def test_preview_override_shift(make_organization, make_user_for_organization, m rolling_users=[{other_user.pk: other_user.public_primary_key}], ) - datetime_end = start_date + timezone.timedelta(days=1) - rotation_events, final_events = schedule.preview_shift(new_shift, start_date, datetime_end) + rotation_events, final_events = schedule.preview_shift(new_shift, "UTC", start_date, days=1) # check rotation events expected_rotation_events = [ @@ -1105,8 +1089,7 @@ def test_filter_events_none_cache_unchanged( # schedule is removed from db schedule.delete() - end_date = start_date + timezone.timedelta(days=5) - events = schedule.filter_events(start_date, end_date, filter_by=OnCallSchedule.TYPE_ICAL_PRIMARY) + events = schedule.filter_events("UTC", start_date, days=5, filter_by=OnCallSchedule.TYPE_ICAL_PRIMARY) expected = [] assert events == expected @@ -1289,8 +1272,7 @@ def test_api_schedule_preview_requires_override(make_organization, make_schedule ) with pytest.raises(ValueError): - datetime_end = now + timezone.timedelta(days=1) - schedule.preview_shift(non_override_shift, now, datetime_end) + schedule.preview_shift(non_override_shift, "UTC", now, 1) @pytest.mark.django_db @@ -1835,5 +1817,4 @@ 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 - datetime_end = now + timezone.timedelta(days=7) - schedule.final_events(now, datetime_end) + schedule.final_events("UTC", now, days=7) diff --git a/engine/apps/schedules/tests/test_quality_score.py b/engine/apps/schedules/tests/test_quality_score.py index bbcb7949..5926f929 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 (28% not covered)"}, + {"type": "warning", "text": "Schedule has gaps (29% 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": 28, + "score": 29, } for user in users[:4] ],