diff --git a/CHANGELOG.md b/CHANGELOG.md index 7b3738cf..5a7e4b7e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,12 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## Unreleased + +### Added + +- Add additional shift info in schedule filter_events internal API ([#3110](https://github.com/grafana/oncall/pull/3110)) + ## v1.3.41 (2023-10-04) ### Added diff --git a/engine/apps/api/serializers/on_call_shifts.py b/engine/apps/api/serializers/on_call_shifts.py index 2b585f9e..def64076 100644 --- a/engine/apps/api/serializers/on_call_shifts.py +++ b/engine/apps/api/serializers/on_call_shifts.py @@ -63,6 +63,7 @@ class OnCallShiftSerializer(EagerLoadingMixin, serializers.ModelSerializer): } SELECT_RELATED = ["schedule", "updated_shift"] + PREFETCH_RELATED = ["schedules"] def get_shift_end(self, obj): return obj.start + obj.duration @@ -70,6 +71,11 @@ class OnCallShiftSerializer(EagerLoadingMixin, serializers.ModelSerializer): def to_representation(self, instance): ret = super().to_representation(instance) ret["week_start"] = CustomOnCallShift.ICAL_WEEKDAY_MAP[instance.week_start] + if ret["schedule"] is None: + # for terraform based schedules, related schedule comes from M2M field + # TODO: migrate terraform schedules to use FK instead + related_schedules = instance.schedules.all() + ret["schedule"] = related_schedules[0].public_primary_key if related_schedules else None return ret def to_internal_value(self, data): diff --git a/engine/apps/api/tests/test_oncall_shift.py b/engine/apps/api/tests/test_oncall_shift.py index be796a56..ab962707 100644 --- a/engine/apps/api/tests/test_oncall_shift.py +++ b/engine/apps/api/tests/test_oncall_shift.py @@ -215,6 +215,55 @@ def test_get_on_call_shift( assert response.json() == expected_payload +@pytest.mark.django_db +def test_get_calendar_on_call_shift( + on_call_shift_internal_api_setup, + make_schedule, + make_on_call_shift, + make_user_auth_headers, +): + token, user1, user2, organization, _ = on_call_shift_internal_api_setup + schedule = make_schedule(organization, schedule_class=OnCallScheduleCalendar) + + client = APIClient() + start_date = timezone.now().replace(microsecond=0) + + name = "Test Shift Rotation" + on_call_shift = make_on_call_shift( + schedule.organization, + shift_type=CustomOnCallShift.TYPE_ROLLING_USERS_EVENT, + name=name, + start=start_date, + duration=timezone.timedelta(hours=1), + rotation_start=start_date, + rolling_users=[{user1.pk: user1.public_primary_key}, {user2.pk: user2.public_primary_key}], + ) + on_call_shift.schedules.add(schedule) + url = reverse("api-internal:oncall_shifts-detail", kwargs={"pk": on_call_shift.public_primary_key}) + + response = client.get(url, format="json", **make_user_auth_headers(user1, token)) + expected_payload = { + "id": response.data["id"], + "name": name, + "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=1)).strftime("%Y-%m-%dT%H:%M:%SZ"), + "rotation_start": start_date.strftime("%Y-%m-%dT%H:%M:%SZ"), + "until": None, + "frequency": None, + "interval": None, + "by_day": None, + "week_start": CustomOnCallShift.ICAL_WEEKDAY_MAP[CustomOnCallShift.SUNDAY], + "rolling_users": [[user1.public_primary_key], [user2.public_primary_key]], + "updated_shift": None, + } + + assert response.status_code == status.HTTP_200_OK + assert response.json() == expected_payload + + @pytest.mark.django_db def test_list_on_call_shift( on_call_shift_internal_api_setup, diff --git a/engine/apps/api/tests/test_schedules.py b/engine/apps/api/tests/test_schedules.py index 6f703bda..24983191 100644 --- a/engine/apps/api/tests/test_schedules.py +++ b/engine/apps/api/tests/test_schedules.py @@ -926,6 +926,8 @@ def test_filter_events_calendar( "is_override": False, "shift": { "pk": on_call_shift.public_primary_key, + "name": on_call_shift.name, + "type": on_call_shift.type, }, }, { @@ -949,6 +951,8 @@ def test_filter_events_calendar( "is_override": False, "shift": { "pk": on_call_shift.public_primary_key, + "name": on_call_shift.name, + "type": on_call_shift.type, }, }, ], @@ -1039,6 +1043,8 @@ def test_filter_events_range_calendar( "is_override": False, "shift": { "pk": on_call_shift.public_primary_key, + "name": on_call_shift.name, + "type": on_call_shift.type, }, } ], @@ -1128,6 +1134,8 @@ def test_filter_events_overrides( "is_override": True, "shift": { "pk": override.public_primary_key, + "name": override.name, + "type": override.type, }, } ], @@ -2147,8 +2155,12 @@ def test_current_user_events( assert result["schedules"][0]["name"] == schedule_with_current_user.name assert len(result["schedules"][0]["events"]) > 0 for event in result["schedules"][0]["events"]: - # check the current user shift pk is set in the event - assert event["shift"]["pk"] == on_call_shift.public_primary_key + # check the current user shift is populated + assert event["shift"] == { + "pk": on_call_shift.public_primary_key, + "name": on_call_shift.name, + "type": on_call_shift.type, + } @pytest.mark.django_db diff --git a/engine/apps/api/views/schedule.py b/engine/apps/api/views/schedule.py index b46ac785..eb300ee2 100644 --- a/engine/apps/api/views/schedule.py +++ b/engine/apps/api/views/schedule.py @@ -337,9 +337,10 @@ class ScheduleView( with_gap=resolve_schedule, filter_by=filter_by, all_day_datetime=True, + include_shift_info=True, ) else: # return final schedule - events = schedule.final_events(datetime_start, datetime_end) + events = schedule.final_events(datetime_start, datetime_end, include_shift_info=True) result = { "id": schedule.public_primary_key, diff --git a/engine/apps/schedules/models/on_call_schedule.py b/engine/apps/schedules/models/on_call_schedule.py index e1f1103c..f96517df 100644 --- a/engine/apps/schedules/models/on_call_schedule.py +++ b/engine/apps/schedules/models/on_call_schedule.py @@ -346,6 +346,7 @@ class OnCallSchedule(PolymorphicModel): all_day_datetime: bool = False, ignore_untaken_swaps: bool = False, from_cached_final: bool = False, + include_shift_info: bool = False, ) -> ScheduleEvents: """Return filtered events from schedule.""" shifts = ( @@ -360,6 +361,13 @@ class OnCallSchedule(PolymorphicModel): ) or [] ) + shifts_data = {} + if include_shift_info: + pks = set(shift["shift_pk"] for shift in shifts) + shifts_from_db = CustomOnCallShift.objects.filter( + organization=self.organization, public_primary_key__in=pks + ) + shifts_data = {s.public_primary_key: {"name": s.name, "type": s.type} for s in shifts_from_db} events: ScheduleEvents = [] for shift in shifts: start = shift["start"] @@ -396,6 +404,15 @@ class OnCallSchedule(PolymorphicModel): "pk": shift["shift_pk"], }, } + if include_shift_info and not is_gap: + no_data = { + "name": None, + "type": CustomOnCallShift.TYPE_OVERRIDE + if shift["calendar_type"] == OnCallSchedule.TYPE_ICAL_OVERRIDES + else None, + } + shift_data = shifts_data.get(shift["shift_pk"], no_data) + shift_json["shift"].update(shift_data) events.append(shift_json) # combine multiple-users same-shift events into one @@ -415,6 +432,7 @@ class OnCallSchedule(PolymorphicModel): with_empty: bool = True, with_gap: bool = True, ignore_untaken_swaps: bool = False, + include_shift_info: bool = False, ) -> ScheduleEvents: """Return schedule final events, after resolving shifts and overrides.""" events = self.filter_events( @@ -424,6 +442,7 @@ class OnCallSchedule(PolymorphicModel): with_gap=with_gap, all_day_datetime=True, ignore_untaken_swaps=ignore_untaken_swaps, + include_shift_info=include_shift_info, ) events = self._resolve_schedule(events, datetime_start, datetime_end) return events @@ -518,7 +537,9 @@ class OnCallSchedule(PolymorphicModel): # no final schedule info available return passed_shifts, current_shifts, upcoming_shifts - events = self.filter_events(datetime_start, datetime_end, all_day_datetime=True, from_cached_final=True) + events = self.filter_events( + datetime_start, datetime_end, all_day_datetime=True, from_cached_final=True, include_shift_info=True + ) events.sort(key=lambda e: e["start"]) for event in events: users = {u["pk"] for u in event["users"]} diff --git a/engine/apps/schedules/tests/test_on_call_schedule.py b/engine/apps/schedules/tests/test_on_call_schedule.py index f6599dee..d9fd5cdb 100644 --- a/engine/apps/schedules/tests/test_on_call_schedule.py +++ b/engine/apps/schedules/tests/test_on_call_schedule.py @@ -224,6 +224,95 @@ def test_filter_events_include_gaps(make_organization, make_user_for_organizatio assert events == expected +@pytest.mark.django_db +def test_filter_events_include_shift_info( + make_organization, make_user_for_organization, make_schedule, make_on_call_shift +): + organization = make_organization() + schedule = make_schedule( + organization, + schedule_class=OnCallScheduleWeb, + name="test_web_schedule", + ) + user = make_user_for_organization(organization) + now = timezone.now().replace(hour=0, minute=0, second=0, microsecond=0) + start_date = now - timezone.timedelta(days=7) + + data = { + "start": start_date + timezone.timedelta(hours=10), + "rotation_start": start_date + timezone.timedelta(hours=10), + "duration": timezone.timedelta(hours=8), + "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]]) + + 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, include_shift_info=True + ) + expected = [ + { + "calendar_type": None, + "start": start_date, + "end": on_call_shift.start, + "all_day": False, + "is_override": False, + "is_empty": False, + "is_gap": True, + "priority_level": None, + "missing_users": [], + "users": [], + "shift": {"pk": None}, + "source": None, + }, + { + "calendar_type": OnCallSchedule.TYPE_ICAL_PRIMARY, + "start": on_call_shift.start, + "end": on_call_shift.start + on_call_shift.duration, + "all_day": False, + "is_override": False, + "is_empty": False, + "is_gap": False, + "priority_level": on_call_shift.priority_level, + "missing_users": [], + "users": [ + { + "display_name": user.username, + "pk": user.public_primary_key, + "email": user.email, + "avatar_full": user.avatar_full_url, + }, + ], + "shift": { + "pk": on_call_shift.public_primary_key, + "name": on_call_shift.name, + "type": on_call_shift.type, + }, + "source": "api", + }, + { + "calendar_type": None, + "start": on_call_shift.start + on_call_shift.duration, + "end": on_call_shift.start + timezone.timedelta(hours=14), + "all_day": False, + "is_override": False, + "is_empty": False, + "is_gap": True, + "priority_level": None, + "missing_users": [], + "users": [], + "shift": {"pk": None}, + "source": None, + }, + ] + assert events == expected + + @pytest.mark.django_db def test_filter_events_include_empty(make_organization, make_user_for_organization, make_schedule, make_on_call_shift): organization = make_organization() @@ -337,7 +426,10 @@ def test_filter_events_ical_all_day(make_organization, make_user_for_organizatio @pytest.mark.django_db -def test_final_schedule_events(make_organization, make_user_for_organization, make_on_call_shift, make_schedule): +@pytest.mark.parametrize("include_shift_info", [False, True]) +def test_final_schedule_events( + make_organization, make_user_for_organization, make_on_call_shift, make_schedule, include_shift_info +): organization = make_organization() schedule = make_schedule( organization, @@ -364,6 +456,7 @@ def test_final_schedule_events(make_organization, make_user_for_organization, ma (user_d, 2, 17, 1), # r2-3: 17-18 / D (user_d, 2, 20, 3), # r2-4: 20-23 / D ) + oncall_shifts = [] for user, priority, start_h, duration in shifts: data = { "start": start_date + timezone.timedelta(hours=start_h), @@ -377,6 +470,7 @@ def test_final_schedule_events(make_organization, make_user_for_organization, ma organization=organization, shift_type=CustomOnCallShift.TYPE_ROLLING_USERS_EVENT, **data ) on_call_shift.add_rolling_users([[user]]) + oncall_shifts.append(on_call_shift) overrides = ( # user, priority, start time (h), duration (hs) @@ -395,26 +489,27 @@ def test_final_schedule_events(make_organization, make_user_for_organization, ma organization=organization, shift_type=CustomOnCallShift.TYPE_OVERRIDE, **data ) on_call_shift.add_rolling_users([[user]]) + oncall_shifts.append(on_call_shift) datetime_end = start_date + timezone.timedelta(days=1) - returned_events = schedule.final_events(start_date, datetime_end) + returned_events = schedule.final_events(start_date, datetime_end, include_shift_info=include_shift_info) expected = ( - # start (h), duration (H), user, priority, is_gap, is_override - (0, 10, None, None, True, False), # 0-10 gap - (10, 2, "A", 1, False, False), # 10-12 A - (11, 1, "B", 1, False, False), # 11-12 B - (12, 2, "C", 2, False, False), # 12-14 C - (14, 1, "D", 2, False, False), # 14-15 D - (15, 1, None, None, True, False), # 15-16 gap - (16, 1, "A", 1, False, False), # 16-17 A - (17, 1, "D", 2, False, False), # 17-18 D - (18, 1, "A", 1, False, False), # 18-19 A - (19, 1, None, None, True, False), # 19-20 gap - (20, 2, "D", 2, False, False), # 20-22 D - (22, 0.5, "A", 1, False, True), # 22-22:30 A (override the override) - (22.5, 0.5, "E", None, False, True), # 22:30-23 E (override) - (23, 1, "B", 1, False, False), # 23-00 B + # start (h), duration (H), user, priority, is_gap, is_override, shift + (0, 10, None, None, True, False, None), # 0-10 gap + (10, 2, "A", 1, False, False, oncall_shifts[0]), # 10-12 A + (11, 1, "B", 1, False, False, oncall_shifts[1]), # 11-12 B + (12, 2, "C", 2, False, False, oncall_shifts[5]), # 12-14 C + (14, 1, "D", 2, False, False, oncall_shifts[6]), # 14-15 D + (15, 1, None, None, True, False, None), # 15-16 gap + (16, 1, "A", 1, False, False, oncall_shifts[2]), # 16-17 A + (17, 1, "D", 2, False, False, oncall_shifts[7]), # 17-18 D + (18, 1, "A", 1, False, False, oncall_shifts[2]), # 18-19 A + (19, 1, None, None, True, False, None), # 19-20 gap + (20, 2, "D", 2, False, False, oncall_shifts[8]), # 20-22 D + (22, 0.5, "A", 1, False, True, oncall_shifts[10]), # 22-22:30 A (override the override) + (22.5, 0.5, "E", None, False, True, oncall_shifts[9]), # 22:30-23 E (override) + (23, 1, "B", 1, False, False, oncall_shifts[4]), # 23-00 B ) expected_events = [ { @@ -425,8 +520,15 @@ def test_final_schedule_events(make_organization, make_user_for_organization, ma "priority_level": priority, "start": start_date + timezone.timedelta(hours=start), "user": user, + "shift": ( + {"pk": shift.public_primary_key, "name": shift.name, "type": shift.type} + if include_shift_info + else {"pk": shift.public_primary_key} + ) + if not is_gap + else {"pk": None}, } - for start, duration, user, priority, is_gap, is_override in expected + for start, duration, user, priority, is_gap, is_override, shift in expected ] returned_events = [ { @@ -437,6 +539,7 @@ def test_final_schedule_events(make_organization, make_user_for_organization, ma "priority_level": e["priority_level"], "start": e["start"], "user": e["users"][0]["display_name"] if e["users"] else None, + "shift": e["shift"], } for e in returned_events ]