From 1801cecaff651f75cd875fc683d202ddf8868e98 Mon Sep 17 00:00:00 2001 From: Joey Orlando Date: Thu, 6 Jul 2023 21:43:04 +0200 Subject: [PATCH] add user (full) avatar to schedule filter events internal API endpoint (#2459) # What this PR does ## Checklist - [x] Unit, integration, and e2e (if applicable) tests updated - [ ] Documentation added (or `pr:no public docs` PR label added if not required) (N/A) - [x] `CHANGELOG.md` updated (or `pr:no changelog` PR label added if not required) --- CHANGELOG.md | 7 +++ engine/apps/api/tests/test_oncall_shift.py | 21 ++++++-- engine/apps/api/tests/test_schedules.py | 37 ++++++++++++-- engine/apps/api/views/schedule.py | 9 ++-- .../apps/schedules/models/on_call_schedule.py | 2 + .../schedules/tests/test_on_call_schedule.py | 50 ++++++++++++++++--- engine/common/api_helpers/utils.py | 6 ++- 7 files changed, 113 insertions(+), 19 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f6651ead..8fc1e169 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,13 @@ 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 `event.users.avatar_full` field to `GET /api/internal/v1/schedules/{schedule_id}/filter_events` + payload by @joeyorlando ([#2459](https://github.com/grafana/oncall/pull/2459)) + ## v1.3.7 (2023-07-06) ### Changed diff --git a/engine/apps/api/tests/test_oncall_shift.py b/engine/apps/api/tests/test_oncall_shift.py index 2256b150..7e708daf 100644 --- a/engine/apps/api/tests/test_oncall_shift.py +++ b/engine/apps/api/tests/test_oncall_shift.py @@ -1502,7 +1502,12 @@ def test_on_call_shift_preview( "priority_level": 2, "missing_users": [], "users": [ - {"display_name": other_user.username, "pk": other_user.public_primary_key, "email": other_user.email} + { + "display_name": other_user.username, + "pk": other_user.public_primary_key, + "email": other_user.email, + "avatar_full": other_user.avatar_full_url, + }, ], "source": "web", } @@ -1791,7 +1796,12 @@ def test_on_call_shift_preview_update( "priority_level": 1, "missing_users": [], "users": [ - {"display_name": other_user.username, "pk": other_user.public_primary_key, "email": other_user.email} + { + "display_name": other_user.username, + "pk": other_user.public_primary_key, + "email": other_user.email, + "avatar_full": other_user.avatar_full_url, + }, ], "source": "web", } @@ -1901,7 +1911,12 @@ def test_on_call_shift_preview_update_not_started_reuse_pk( "priority_level": 1, "missing_users": [], "users": [ - {"display_name": other_user.username, "pk": other_user.public_primary_key, "email": other_user.email} + { + "display_name": other_user.username, + "pk": other_user.public_primary_key, + "email": other_user.email, + "avatar_full": other_user.avatar_full_url, + }, ], "source": "web", }, diff --git a/engine/apps/api/tests/test_schedules.py b/engine/apps/api/tests/test_schedules.py index 69ae02dc..6a239f68 100644 --- a/engine/apps/api/tests/test_schedules.py +++ b/engine/apps/api/tests/test_schedules.py @@ -824,7 +824,14 @@ def test_events_calendar( "all_day": False, "start": on_call_shift.start, "end": on_call_shift.start + on_call_shift.duration, - "users": [{"display_name": user.username, "pk": user.public_primary_key, "email": user.email}], + "users": [ + { + "display_name": user.username, + "pk": user.public_primary_key, + "email": user.email, + "avatar_full": user.avatar_full_url, + }, + ], "missing_users": [], "priority_level": on_call_shift.priority_level, "source": "api", @@ -890,7 +897,14 @@ def test_filter_events_calendar( "all_day": False, "start": mon_start, "end": mon_start + on_call_shift.duration, - "users": [{"display_name": user.username, "pk": user.public_primary_key, "email": user.email}], + "users": [ + { + "display_name": user.username, + "pk": user.public_primary_key, + "email": user.email, + "avatar_full": user.avatar_full_url, + }, + ], "missing_users": [], "priority_level": on_call_shift.priority_level, "source": "api", @@ -906,7 +920,14 @@ def test_filter_events_calendar( "all_day": False, "start": fri_start, "end": fri_start + on_call_shift.duration, - "users": [{"display_name": user.username, "pk": user.public_primary_key, "email": user.email}], + "users": [ + { + "display_name": user.username, + "pk": user.public_primary_key, + "email": user.email, + "avatar_full": user.avatar_full_url, + } + ], "missing_users": [], "priority_level": on_call_shift.priority_level, "source": "api", @@ -989,7 +1010,14 @@ def test_filter_events_range_calendar( "all_day": False, "start": fri_start, "end": fri_start + on_call_shift.duration, - "users": [{"display_name": user.username, "pk": user.public_primary_key, "email": user.email}], + "users": [ + { + "display_name": user.username, + "pk": user.public_primary_key, + "email": user.email, + "avatar_full": user.avatar_full_url, + }, + ], "missing_users": [], "priority_level": on_call_shift.priority_level, "source": "api", @@ -1076,6 +1104,7 @@ def test_filter_events_overrides( "display_name": other_user.username, "pk": other_user.public_primary_key, "email": other_user.email, + "avatar_full": other_user.avatar_full_url, } ], "missing_users": [], diff --git a/engine/apps/api/views/schedule.py b/engine/apps/api/views/schedule.py index 57d25668..7e958f9e 100644 --- a/engine/apps/api/views/schedule.py +++ b/engine/apps/api/views/schedule.py @@ -15,6 +15,7 @@ from rest_framework.fields import BooleanField from rest_framework.filters import SearchFilter from rest_framework.pagination import PageNumberPagination from rest_framework.permissions import IsAuthenticated +from rest_framework.request import Request from rest_framework.views import Response from rest_framework.viewsets import ModelViewSet @@ -232,10 +233,10 @@ class ScheduleView( if instance.user_group is not None: update_slack_user_group_for_schedules.apply_async((instance.user_group.pk,)) - def get_object(self): + def get_object(self) -> OnCallSchedule: # get the object from the whole organization if there is a flag `get_from_organization=true` # otherwise get the object from the current team - get_from_organization = self.request.query_params.get("from_organization", "false") == "true" + get_from_organization: bool = self.request.query_params.get("from_organization", "false") == "true" if get_from_organization: return self.get_object_from_organization() return super().get_object() @@ -307,10 +308,10 @@ class ScheduleView( return Response(result, status=status.HTTP_200_OK) @action(detail=True, methods=["get"]) - def filter_events(self, request, pk): + def filter_events(self, request: Request, pk: str) -> Response: user_tz, starting_date, days = get_date_range_from_request(self.request) - filter_by = self.request.query_params.get("type") + filter_by: str | None = self.request.query_params.get("type") valid_filters = (EVENTS_FILTER_BY_ROTATION, EVENTS_FILTER_BY_OVERRIDE, EVENTS_FILTER_BY_FINAL) if filter_by is not None and filter_by not in valid_filters: raise BadRequest(detail="Invalid type value") diff --git a/engine/apps/schedules/models/on_call_schedule.py b/engine/apps/schedules/models/on_call_schedule.py index eefc6c47..fdcf4657 100644 --- a/engine/apps/schedules/models/on_call_schedule.py +++ b/engine/apps/schedules/models/on_call_schedule.py @@ -88,6 +88,7 @@ class ScheduleEventUser(typing.TypedDict): display_name: str pk: str email: str + avatar_full: str class ScheduleEventShift(typing.TypedDict): @@ -349,6 +350,7 @@ class OnCallSchedule(PolymorphicModel): "display_name": user.username, "email": user.email, "pk": user.public_primary_key, + "avatar_full": user.avatar_full_url, } for user in shift["users"] ], diff --git a/engine/apps/schedules/tests/test_on_call_schedule.py b/engine/apps/schedules/tests/test_on_call_schedule.py index cf696be8..8817b747 100644 --- a/engine/apps/schedules/tests/test_on_call_schedule.py +++ b/engine/apps/schedules/tests/test_on_call_schedule.py @@ -93,7 +93,14 @@ def test_filter_events(make_organization, make_user_for_organization, make_sched "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}], + "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}, "source": "api", } @@ -114,7 +121,14 @@ def test_filter_events(make_organization, make_user_for_organization, make_sched "is_gap": False, "priority_level": None, "missing_users": [], - "users": [{"display_name": user.username, "pk": user.public_primary_key, "email": user.email}], + "users": [ + { + "display_name": user.username, + "pk": user.public_primary_key, + "email": user.email, + "avatar_full": user.avatar_full_url, + }, + ], "shift": {"pk": override.public_primary_key}, "source": "api", } @@ -179,7 +193,14 @@ def test_filter_events_include_gaps(make_organization, make_user_for_organizatio "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}], + "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}, "source": "api", }, @@ -689,7 +710,12 @@ def test_preview_shift(make_organization, make_user_for_organization, make_sched "priority_level": new_shift.priority_level, "missing_users": [], "users": [ - {"display_name": other_user.username, "pk": other_user.public_primary_key, "email": other_user.email} + { + "display_name": other_user.username, + "pk": other_user.public_primary_key, + "email": other_user.email, + "avatar_full": other_user.avatar_full_url, + }, ], "shift": {"pk": new_shift.public_primary_key}, "source": "api", @@ -784,7 +810,14 @@ def test_preview_shift_do_not_change_rotation_events( "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}], + "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}, "source": "api", } @@ -912,7 +945,12 @@ def test_preview_override_shift(make_organization, make_user_for_organization, m "priority_level": None, "missing_users": [], "users": [ - {"display_name": other_user.username, "pk": other_user.public_primary_key, "email": other_user.email} + { + "display_name": other_user.username, + "pk": other_user.public_primary_key, + "email": other_user.email, + "avatar_full": other_user.avatar_full_url, + }, ], "shift": {"pk": new_shift.public_primary_key}, "source": "api", diff --git a/engine/common/api_helpers/utils.py b/engine/common/api_helpers/utils.py index da57b13d..c3be4c58 100644 --- a/engine/common/api_helpers/utils.py +++ b/engine/common/api_helpers/utils.py @@ -1,5 +1,6 @@ import datetime import re +import typing from urllib.parse import urljoin import requests @@ -9,6 +10,7 @@ from django.utils import dateparse, timezone from django.utils.regex_helper import _lazy_re_compile from icalendar import Calendar from rest_framework import serializers +from rest_framework.request import Request from apps.schedules.ical_utils import fetch_ical_file from common.api_helpers.exceptions import BadRequest @@ -125,12 +127,12 @@ def create_engine_url(path, override_base=None): return urljoin(base, trimmed_path) -def get_date_range_from_request(request): +def get_date_range_from_request(request: Request) -> typing.Tuple[str, datetime.date, int]: """Extract timezone, starting date and number of days params from request. Used mainly for schedules and shifts API. """ - user_tz = request.query_params.get("user_tz", "UTC") + user_tz: str = request.query_params.get("user_tz", "UTC") raise_exception_if_not_valid_timezone(user_tz) date = timezone.now().date()