From 65cdcf93ba388ab25fe0b865c5f0257cc7009314 Mon Sep 17 00:00:00 2001 From: Matias Bordese Date: Mon, 29 Jan 2024 14:41:20 -0300 Subject: [PATCH] Add is_currently_oncall information to internal user details API (#3765) Related to https://github.com/grafana/oncall/issues/3164 --- CHANGELOG.md | 1 + engine/apps/api/serializers/user.py | 31 +++++--- engine/apps/api/tests/test_user.py | 37 +++++++++- engine/apps/api/views/user.py | 108 +++++++++++++++++----------- 4 files changed, 127 insertions(+), 50 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3329f979..12da05ae 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added - Improved logging during plugin sync and install with Grafana @mderynck ([#3730](https://github.com/grafana/oncall/pull/3730)) +- Added `is_currently_oncall` information to internal user details API ([#3765](https://github.com/grafana/oncall/pull/3765)) - Add a modal for autoresolve and grouping templates for Alertmanager-based integrations ([#3764](https://github.com/grafana/oncall/pull/3764)) ### Fixed diff --git a/engine/apps/api/serializers/user.py b/engine/apps/api/serializers/user.py index c399fe26..9babaa5b 100644 --- a/engine/apps/api/serializers/user.py +++ b/engine/apps/api/serializers/user.py @@ -51,7 +51,7 @@ class WorkingHoursSerializer(serializers.Serializer): sunday = serializers.ListField(child=WorkingHoursPeriodSerializer()) -class UserSerializer(DynamicFieldsModelSerializer, EagerLoadingMixin): +class ListUserSerializer(DynamicFieldsModelSerializer, EagerLoadingMixin): pk = serializers.CharField(read_only=True, source="public_primary_key") slack_user_identity = SlackUserIdentitySerializer(read_only=True) @@ -165,6 +165,24 @@ class UserSerializer(DynamicFieldsModelSerializer, EagerLoadingMixin): return f"{HIDE_SYMBOL * (len(number) - SHOW_LAST_SYMBOLS)}{number[-SHOW_LAST_SYMBOLS:]}" +class UserSerializer(ListUserSerializer): + context: UserSerializerContext + + is_currently_oncall = serializers.SerializerMethodField() + + class Meta(ListUserSerializer.Meta): + fields = ListUserSerializer.Meta.fields + [ + "is_currently_oncall", + ] + read_only_fields = ListUserSerializer.Meta.read_only_fields + [ + "is_currently_oncall", + ] + + def get_is_currently_oncall(self, obj: User) -> bool: + # Serializer context is set here: apps.api.views.user.UserView.get_serializer_context. + return any(obj in users for users in self.context.get("schedules_with_oncall_users", {}).values()) + + class CurrentUserSerializer(UserSerializer): rbac_permissions = UserPermissionSerializer(read_only=True, many=True, source="permissions") @@ -176,7 +194,7 @@ class CurrentUserSerializer(UserSerializer): read_only_fields = UserSerializer.Meta.read_only_fields -class UserHiddenFieldsSerializer(UserSerializer): +class UserHiddenFieldsSerializer(ListUserSerializer): fields_available_for_all_users = [ "pk", "organization", @@ -198,7 +216,7 @@ class UserHiddenFieldsSerializer(UserSerializer): return ret -class ScheduleUserSerializer(UserSerializer): +class ScheduleUserSerializer(ListUserSerializer): fields_to_keep = [ "pk", "organization", @@ -214,7 +232,7 @@ class ScheduleUserSerializer(UserSerializer): ] def to_representation(self, instance): - serialized = super(UserSerializer, self).to_representation(instance) + serialized = super(ListUserSerializer, self).to_representation(instance) ret = {field: value for field, value in serialized.items() if field in self.fields_to_keep} return ret @@ -288,10 +306,7 @@ class UserIsCurrentlyOnCallSerializer(UserShortSerializer, EagerLoadingMixin): def get_is_currently_oncall(self, obj: User) -> bool: # Serializer context is set here: apps.api.views.user.UserView.get_serializer_context. - for users in self.context.get("schedules_with_oncall_users", {}).values(): - if obj in users: - return True - return False + return any(obj in users for users in self.context.get("schedules_with_oncall_users", {}).values()) class PagedUserSerializer(serializers.Serializer): diff --git a/engine/apps/api/tests/test_user.py b/engine/apps/api/tests/test_user.py index 0d853861..6a8eaf29 100644 --- a/engine/apps/api/tests/test_user.py +++ b/engine/apps/api/tests/test_user.py @@ -24,7 +24,12 @@ def clear_cache(): @pytest.mark.django_db -def test_current_user(make_organization_and_user_with_plugin_token, make_user_auth_headers): +def test_current_user( + make_organization_and_user_with_plugin_token, + make_user_auth_headers, + make_schedule, + make_on_call_shift, +): organization, user, token = make_organization_and_user_with_plugin_token() client = APIClient() @@ -42,6 +47,7 @@ def test_current_user(make_organization_and_user_with_plugin_token, make_user_au "rbac_permissions": user.permissions, "timezone": None, "working_hours": default_working_hours(), + "is_currently_oncall": False, "unverified_phone_number": None, "verified_phone_number": None, "telegram_configuration": None, @@ -61,6 +67,28 @@ def test_current_user(make_organization_and_user_with_plugin_token, make_user_au assert response.status_code == status.HTTP_200_OK assert response.json() == expected_response + # current user is on-call + today = timezone.now().replace(hour=0, minute=0, second=0, microsecond=0) + schedule = make_schedule(organization, schedule_class=OnCallScheduleWeb) + on_call_shift = make_on_call_shift( + organization=organization, + shift_type=CustomOnCallShift.TYPE_ROLLING_USERS_EVENT, + start=today, + rotation_start=today, + duration=timezone.timedelta(seconds=24 * 60 * 60), + priority_level=1, + frequency=CustomOnCallShift.FREQUENCY_DAILY, + schedule=schedule, + ) + on_call_shift.add_rolling_users([[user]]) + schedule.refresh_ical_file() + schedule.refresh_ical_final_schedule() + + response = client.get(url, format="json", **make_user_auth_headers(user, token)) + assert response.status_code == status.HTTP_200_OK + expected_response["is_currently_oncall"] = True + assert response.json() == expected_response + data_to_update = {"hide_phone_number": True} response = client.put(url, data=data_to_update, format="json", **make_user_auth_headers(user, token)) @@ -127,6 +155,7 @@ def test_update_user_cant_change_email_and_username( "role": admin.role, "timezone": None, "working_hours": default_working_hours(), + "is_currently_oncall": False, "unverified_phone_number": phone_number, "verified_phone_number": None, "telegram_configuration": None, @@ -2017,6 +2046,12 @@ def test_users_is_currently_oncall_attribute_works_properly( assert user["teams"] == [] assert user["is_currently_oncall"] == oncall_statuses[user["pk"]] + # getting specific user details include currently on-call info + url = reverse("api-internal:user-detail", kwargs={"pk": user1.public_primary_key}) + response = client.get(url, format="json", **make_user_auth_headers(user1, token)) + + assert response.json()["is_currently_oncall"] + @pytest.mark.django_db def test_list_users_filtered_by_is_currently_oncall( diff --git a/engine/apps/api/views/user.py b/engine/apps/api/views/user.py index fe8e9cb6..ee26f73d 100644 --- a/engine/apps/api/views/user.py +++ b/engine/apps/api/views/user.py @@ -31,6 +31,7 @@ from apps.api.serializers.team import TeamSerializer from apps.api.serializers.user import ( CurrentUserSerializer, FilterUserSerializer, + ListUserSerializer, UserHiddenFieldsSerializer, UserIsCurrentlyOnCallSerializer, UserSerializer, @@ -101,12 +102,40 @@ class UpcomingShift(typing.TypedDict): UpcomingShifts = list[UpcomingShift] -class CurrentUserView(APIView): +class CachedSchedulesContextMixin: + @cached_property + def schedules_with_oncall_users(self): + """ + The result of this method is cached and is reused for the whole lifetime of a request, + since self.get_serializer_context() is called multiple times for every instance in the queryset. + """ + return get_cached_oncall_users_for_multiple_schedules(self.request.user.organization.oncall_schedules.all()) + + def _populate_schedules_oncall_cache(self): + return False + + def get_serializer_context(self): + context = getattr(super(), "get_serializer_context", lambda: {})() + context.update( + { + "schedules_with_oncall_users": self.schedules_with_oncall_users + if self._populate_schedules_oncall_cache() + else {} + } + ) + return context + + +class CurrentUserView(APIView, CachedSchedulesContextMixin): authentication_classes = (MobileAppAuthTokenAuthentication, PluginAuthentication) permission_classes = (IsAuthenticated,) + def _populate_schedules_oncall_cache(self): + return True + def get(self, request): - context = {"request": self.request, "format": self.format_kwarg, "view": self} + context = self.get_serializer_context() + context.update({"request": self.request, "format": self.format_kwarg, "view": self}) if settings.IS_OPEN_SOURCE and live_settings.GRAFANA_CLOUD_NOTIFICATIONS_ENABLED: from apps.oss_installation.models import CloudConnector, CloudUserIdentity @@ -122,8 +151,10 @@ class CurrentUserView(APIView): return Response(serializer.data) def put(self, request): + context = self.get_serializer_context() + context.update({"request": self.request}) data = self.request.data - serializer = CurrentUserSerializer(request.user, data=data, context={"request": self.request}) + serializer = CurrentUserSerializer(request.user, data=data, context=context) serializer.is_valid(raise_exception=True) serializer.save() return Response(serializer.data) @@ -158,6 +189,7 @@ class UserFilter(ByTeamModelFieldFilterMixin, filters.FilterSet): class UserView( PublicPrimaryKeyMixin, + CachedSchedulesContextMixin, mixins.RetrieveModelMixin, mixins.UpdateModelMixin, mixins.ListModelMixin, @@ -247,51 +279,49 @@ class UserView( filterset_class = UserFilter - @cached_property - def schedules_with_oncall_users(self): - """ - The result of this method is cached and is reused for the whole lifetime of a request, - since self.get_serializer_context() is called multiple times for every instance in the queryset. - """ - return get_cached_oncall_users_for_multiple_schedules(self.request.user.organization.oncall_schedules.all()) - def _get_is_currently_oncall_query_param(self) -> str: return self.request.query_params.get("is_currently_oncall", "").lower() - def _is_currently_oncall_request(self) -> bool: - return self._get_is_currently_oncall_query_param() in ["true", "false", "all"] - - def get_serializer_context(self): - context = super().get_serializer_context() - context.update( - { - "schedules_with_oncall_users": self.schedules_with_oncall_users - if self._is_currently_oncall_request() - else {} - } + def _populate_schedules_oncall_cache(self): + return ( + # admin or owner can see on-call schedule information for a user + (self.is_owner_or_admin() and self.action != "list") + or + # list requests need to explicitly request on-call information + self._get_is_currently_oncall_query_param() in ["true", "false", "all"] ) - return context - def get_serializer_class(self): + def is_owner_or_admin(self): request = self.request user = request.user kwargs = self.kwargs - query_params = request.query_params - - is_list_request = self.action in ["list"] - is_filters_request = query_params.get("filters", "false") == "true" - - if is_list_request and is_filters_request: - return FilterUserSerializer - elif is_list_request and self._is_currently_oncall_request(): - return UserIsCurrentlyOnCallSerializer is_users_own_data = kwargs.get("pk") is not None and kwargs.get("pk") == user.public_primary_key has_admin_permission = user_is_authorized(user, [RBACPermission.Permissions.USER_SETTINGS_ADMIN]) - if is_users_own_data or has_admin_permission: - return UserSerializer - return UserHiddenFieldsSerializer + return is_users_own_data or has_admin_permission + + def get_serializer_class(self): + request = self.request + query_params = request.query_params + + is_list_request = self.action == "list" + is_filters_request = query_params.get("filters", "false") == "true" + + if is_list_request: + serializer = ListUserSerializer + if is_filters_request: + serializer = FilterUserSerializer + elif self._populate_schedules_oncall_cache(): + serializer = UserIsCurrentlyOnCallSerializer + return serializer + + # non-list requests + serializer = UserHiddenFieldsSerializer + if self.is_owner_or_admin(): + serializer = UserSerializer + + return serializer def get_queryset(self): slack_identity = self.request.query_params.get("slack_identity", None) == "true" @@ -308,7 +338,7 @@ class UserView( @extend_schema( responses=PolymorphicProxySerializer( component_name="UserPolymorphic", - serializers=[FilterUserSerializer, UserIsCurrentlyOnCallSerializer, UserSerializer], + serializers=[FilterUserSerializer, UserIsCurrentlyOnCallSerializer, ListUserSerializer], resource_type_field_name=None, ) ) @@ -401,10 +431,6 @@ class UserView( status=status.HTTP_403_FORBIDDEN, ) - def current(self, request) -> Response: - serializer = UserSerializer(self.get_queryset().get(pk=self.request.user.pk)) - return Response(serializer.data) - @extend_schema(responses={status.HTTP_200_OK: resolve_type_hint(typing.List[str])}) @action(detail=False, methods=["get"]) def timezone_options(self, request) -> Response: