From df32f9099f5643833ac74a51749d04114f7eb418 Mon Sep 17 00:00:00 2001 From: Vadim Stepanov Date: Mon, 11 Jul 2022 13:16:56 +0100 Subject: [PATCH] Add API support for user timezone and working hours (#201) * add API support for user timezone and working hours * add tests --- engine/apps/api/serializers/user.py | 55 +++++++++ engine/apps/api/tests/test_user.py | 107 ++++++++++++++++++ engine/apps/api/views/user.py | 7 +- .../migrations/0002_auto_20220705_1214.py | 24 ++++ engine/apps/user_management/models/user.py | 30 ++++- 5 files changed, 217 insertions(+), 6 deletions(-) create mode 100644 engine/apps/user_management/migrations/0002_auto_20220705_1214.py diff --git a/engine/apps/api/serializers/user.py b/engine/apps/api/serializers/user.py index 51418885..73466515 100644 --- a/engine/apps/api/serializers/user.py +++ b/engine/apps/api/serializers/user.py @@ -1,3 +1,6 @@ +import time + +import pytz from django.conf import settings from rest_framework import serializers @@ -9,6 +12,7 @@ from apps.base.utils import live_settings from apps.oss_installation.utils import cloud_user_identity_status from apps.twilioapp.utils import check_phone_number_is_valid from apps.user_management.models import User +from apps.user_management.models.user import default_working_hours from common.api_helpers.custom_fields import TeamPrimaryKeyRelatedField from common.api_helpers.mixins import EagerLoadingMixin from common.constants.role import Role @@ -29,6 +33,7 @@ class UserSerializer(DynamicFieldsModelSerializer, EagerLoadingMixin): organization = FastOrganizationSerializer(read_only=True) current_team = TeamPrimaryKeyRelatedField(allow_null=True, required=False) + timezone = serializers.CharField(allow_null=True, required=False) avatar = serializers.URLField(source="avatar_url", read_only=True) permissions = serializers.SerializerMethodField() @@ -47,6 +52,8 @@ class UserSerializer(DynamicFieldsModelSerializer, EagerLoadingMixin): "username", "role", "avatar", + "timezone", + "working_hours", "unverified_phone_number", "verified_phone_number", "slack_user_identity", @@ -63,6 +70,52 @@ class UserSerializer(DynamicFieldsModelSerializer, EagerLoadingMixin): "verified_phone_number", ] + def validate_timezone(self, tz): + if tz is None: + return tz + + try: + pytz.timezone(tz) + except pytz.UnknownTimeZoneError: + raise serializers.ValidationError("not a valid timezone") + + return tz + + def validate_working_hours(self, working_hours): + if not isinstance(working_hours, dict): + raise serializers.ValidationError("must be dict") + + # check that all days are present + if sorted(working_hours.keys()) != sorted(default_working_hours().keys()): + raise serializers.ValidationError("missing some days") + + for day in working_hours: + periods = working_hours[day] + + if not isinstance(periods, list): + raise serializers.ValidationError("periods must be list") + + for period in periods: + if not isinstance(period, dict): + raise serializers.ValidationError("period must be dict") + + if sorted(period.keys()) != sorted(["start", "end"]): + raise serializers.ValidationError("'start' and 'end' fields must be present") + + if not isinstance(period["start"], str) or not isinstance(period["end"], str): + raise serializers.ValidationError("'start' and 'end' fields must be str") + + try: + start = time.strptime(period["start"], "%H:%M:%S") + end = time.strptime(period["end"], "%H:%M:%S") + except ValueError: + raise serializers.ValidationError("'start' and 'end' fields must be in '%H:%M:%S' format") + + if start >= end: + raise serializers.ValidationError("'start' must be less than 'end'") + + return working_hours + def validate_unverified_phone_number(self, value): if value: if check_phone_number_is_valid(value): @@ -110,6 +163,8 @@ class UserHiddenFieldsSerializer(UserSerializer): "current_team", "username", "avatar", + "timezone", + "working_hours", "notification_chain_verbal", "permissions", ] diff --git a/engine/apps/api/tests/test_user.py b/engine/apps/api/tests/test_user.py index dd23feb5..7c064616 100644 --- a/engine/apps/api/tests/test_user.py +++ b/engine/apps/api/tests/test_user.py @@ -10,6 +10,7 @@ from rest_framework.test import APIClient from apps.base.constants import ADMIN_PERMISSIONS, EDITOR_PERMISSIONS from apps.base.models import UserNotificationPolicy +from apps.user_management.models.user import default_working_hours from common.constants.role import Role @@ -67,6 +68,8 @@ def test_update_user_cant_change_email_and_username( "email": admin.email, "username": admin.username, "role": admin.role, + "timezone": None, + "working_hours": default_working_hours(), "unverified_phone_number": phone_number, "verified_phone_number": None, "telegram_configuration": None, @@ -113,6 +116,8 @@ def test_list_users( "email": admin.email, "username": admin.username, "role": admin.role, + "timezone": None, + "working_hours": default_working_hours(), "unverified_phone_number": None, "verified_phone_number": None, "telegram_configuration": None, @@ -134,6 +139,8 @@ def test_list_users( "email": editor.email, "username": editor.username, "role": editor.role, + "timezone": None, + "working_hours": default_working_hours(), "unverified_phone_number": None, "verified_phone_number": None, "telegram_configuration": None, @@ -1485,3 +1492,103 @@ def test_viewer_cant_unlink_backend_another_user( response = client.post(url, format="json", **make_user_auth_headers(second_user, token)) assert response.status_code == status.HTTP_403_FORBIDDEN + + +@pytest.mark.django_db +def test_change_timezone( + make_organization, make_user_for_organization, make_token_for_organization, make_user_auth_headers +): + organization = make_organization() + user = make_user_for_organization(organization, role=Role.EDITOR) + _, token = make_token_for_organization(organization) + + client = APIClient() + url = reverse("api-internal:user-detail", kwargs={"pk": user.public_primary_key}) + + data = {"timezone": "Europe/London"} + + response = client.put(f"{url}", data, format="json", **make_user_auth_headers(user, token)) + assert response.status_code == status.HTTP_200_OK + assert "timezone" in response.json() + assert response.json()["timezone"] == "Europe/London" + + +@pytest.mark.django_db +@pytest.mark.parametrize("timezone", ["", 1, "NotATimezone"]) +def test_invalid_timezone( + make_organization, make_user_for_organization, make_token_for_organization, make_user_auth_headers, timezone +): + organization = make_organization() + user = make_user_for_organization(organization, role=Role.EDITOR) + _, token = make_token_for_organization(organization) + + client = APIClient() + url = reverse("api-internal:user-detail", kwargs={"pk": user.public_primary_key}) + + data = {"timezone": timezone} + + response = client.put(f"{url}", data, format="json", **make_user_auth_headers(user, token)) + assert response.status_code == status.HTTP_400_BAD_REQUEST + + +@pytest.mark.django_db +def test_change_working_hours( + make_organization, make_user_for_organization, make_token_for_organization, make_user_auth_headers +): + organization = make_organization() + user = make_user_for_organization(organization, role=Role.EDITOR) + _, token = make_token_for_organization(organization) + + client = APIClient() + url = reverse("api-internal:user-detail", kwargs={"pk": user.public_primary_key}) + + periods = [{"start": "05:00:00", "end": "23:00:00"}] + working_hours = { + day: periods for day in ["monday", "tuesday", "wednesday", "thursday", "friday", "saturday", "sunday"] + } + + data = {"working_hours": working_hours} + + response = client.put(f"{url}", data, format="json", **make_user_auth_headers(user, token)) + assert response.status_code == status.HTTP_200_OK + assert "working_hours" in response.json() + assert response.json()["working_hours"] == working_hours + + +@pytest.mark.django_db +@pytest.mark.parametrize( + "working_hours_extra", + [ + {}, + {"sunday": 1}, + {"sunday": ""}, + {"sunday": {"start": "18:00:00"}}, + {"sunday": {"start": "", "end": ""}}, + {"sunday": {"start": "18:00:00", "end": None}}, + {"sunday": {"start": "18:00:00", "end": "18:00:00"}}, + {"sunday": {"start": "18:00:00", "end": "9:00:00"}}, + {"sunday": {"start": "18:00:00", "end": "9:00:00", "extra": 1}}, + ], +) +def test_invalid_working_hours( + make_organization, + make_user_for_organization, + make_token_for_organization, + make_user_auth_headers, + working_hours_extra, +): + organization = make_organization() + user = make_user_for_organization(organization, role=Role.EDITOR) + _, token = make_token_for_organization(organization) + + client = APIClient() + url = reverse("api-internal:user-detail", kwargs={"pk": user.public_primary_key}) + + periods = [{"start": "05:00:00", "end": "23:00:00"}] + working_hours = {day: periods for day in ["monday", "tuesday", "wednesday", "thursday", "friday", "saturday"]} + working_hours.update(working_hours_extra) + + data = {"working_hours": working_hours} + response = client.put(f"{url}", data, format="json", **make_user_auth_headers(user, token)) + + assert response.status_code == status.HTTP_400_BAD_REQUEST diff --git a/engine/apps/api/views/user.py b/engine/apps/api/views/user.py index e7d20a32..b79be50f 100644 --- a/engine/apps/api/views/user.py +++ b/engine/apps/api/views/user.py @@ -1,6 +1,7 @@ import logging from urllib.parse import urljoin +import pytz from django.apps import apps from django.conf import settings from django.core.exceptions import ObjectDoesNotExist @@ -123,7 +124,7 @@ class UserView( "mobile_app_verification_token", "mobile_app_auth_token", ), - AnyRole: ("retrieve",), + AnyRole: ("retrieve", "timezone_options"), } action_object_permissions = { @@ -236,6 +237,10 @@ class UserView( serializer = UserSerializer(self.get_queryset().get(pk=self.request.user.pk)) return Response(serializer.data) + @action(detail=False, methods=["get"]) + def timezone_options(self, request): + return Response(pytz.common_timezones) + @action(detail=True, methods=["get"]) def get_verification_code(self, request, pk): user = self.get_object() diff --git a/engine/apps/user_management/migrations/0002_auto_20220705_1214.py b/engine/apps/user_management/migrations/0002_auto_20220705_1214.py new file mode 100644 index 00000000..976bfe7a --- /dev/null +++ b/engine/apps/user_management/migrations/0002_auto_20220705_1214.py @@ -0,0 +1,24 @@ +# Generated by Django 3.2.13 on 2022-07-05 12:14 + +import apps.user_management.models.user +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('user_management', '0001_squashed_initial'), + ] + + operations = [ + migrations.AddField( + model_name='user', + name='_timezone', + field=models.CharField(default=None, max_length=50, null=True), + ), + migrations.AddField( + model_name='user', + name='working_hours', + field=models.JSONField(default=apps.user_management.models.user.default_working_hours, null=True), + ), + ] diff --git a/engine/apps/user_management/models/user.py b/engine/apps/user_management/models/user.py index ecdc46e0..ca769243 100644 --- a/engine/apps/user_management/models/user.py +++ b/engine/apps/user_management/models/user.py @@ -30,6 +30,16 @@ def generate_public_primary_key_for_user(): return new_public_primary_key +def default_working_hours(): + weekdays = ["monday", "tuesday", "wednesday", "thursday", "friday"] + weekends = ["saturday", "sunday"] + + working_hours = {day: [{"start": "09:00:00", "end": "17:00:00"}] for day in weekdays} + working_hours |= {day: [] for day in weekends} + + return working_hours + + class UserManager(models.Manager): @staticmethod def sync_for_team(team, api_members: list[dict]): @@ -128,6 +138,10 @@ class User(models.Model): role = models.PositiveSmallIntegerField(choices=Role.choices()) avatar_url = models.URLField() + # don't use "_timezone" directly, use the "timezone" property since it can be populated via slack user identity + _timezone = models.CharField(max_length=50, null=True, default=None) + working_hours = models.JSONField(null=True, default=default_working_hours) + notification = models.ManyToManyField("alerts.AlertGroup", through="alerts.UserHasNotification") unverified_phone_number = models.CharField(max_length=20, null=True, default=None) @@ -222,11 +236,17 @@ class User(models.Model): @property def timezone(self): - slack_user_identity = self.slack_user_identity - if slack_user_identity: - return slack_user_identity.timezone - else: - return None + if self._timezone: + return self._timezone + + if self.slack_user_identity: + return self.slack_user_identity.timezone + + return None + + @timezone.setter + def timezone(self, value): + self._timezone = value def short(self): return {"username": self.username, "pk": self.public_primary_key, "avatar": self.avatar_url}