From f743c6cf341687f789dfe3fcb48c00cbd49da925 Mon Sep 17 00:00:00 2001
From: Andreas Ellewsen <ae@uio.no>
Date: Tue, 23 Nov 2021 12:44:31 +0100
Subject: [PATCH] Simplify person endpoints
Serialization has been moved to serializer classes, sharing them when
sensible and Views have been converted to ViewSets to simplify code.
---
frontend/src/hooks/useGuests/index.tsx | 2 +-
frontend/src/routes/sponsor/index.tsx | 2 +-
.../src/routes/sponsor/register/frontPage.tsx | 10 +-
greg/api/serializers/identity.py | 6 +
greg/api/serializers/person.py | 63 ++++++-
greg/api/serializers/role.py | 38 +++-
gregui/api/urls.py | 11 +-
gregui/api/views/person.py | 164 ++++--------------
gregui/tests/api/views/test_person.py | 8 +-
gregui/urls.py | 2 -
gregui/validation.py | 5 +-
11 files changed, 160 insertions(+), 151 deletions(-)
diff --git a/frontend/src/hooks/useGuests/index.tsx b/frontend/src/hooks/useGuests/index.tsx
index 58a7b1c3..04e62653 100644
--- a/frontend/src/hooks/useGuests/index.tsx
+++ b/frontend/src/hooks/useGuests/index.tsx
@@ -10,7 +10,7 @@ const useGuests = () => {
const response = await fetch('/api/ui/v1/guests/?format=json')
const jsonResponse = await response.json()
if (response.ok) {
- const persons = await jsonResponse.persons
+ const persons = await jsonResponse
setGuests(
persons.map(
(person: FetchedGuest): Guest => ({
diff --git a/frontend/src/routes/sponsor/index.tsx b/frontend/src/routes/sponsor/index.tsx
index 0ae81216..e429db43 100644
--- a/frontend/src/routes/sponsor/index.tsx
+++ b/frontend/src/routes/sponsor/index.tsx
@@ -1,8 +1,8 @@
import { Route } from 'react-router-dom'
import FrontPage from 'routes/sponsor/frontpage'
+import useGuests from 'hooks/useGuests'
import GuestRoutes from './guest'
-import useGuests from '../../hooks/useGuests'
function Sponsor() {
const { guests, reloadGuests } = useGuests()
diff --git a/frontend/src/routes/sponsor/register/frontPage.tsx b/frontend/src/routes/sponsor/register/frontPage.tsx
index d53ef156..21b38e57 100644
--- a/frontend/src/routes/sponsor/register/frontPage.tsx
+++ b/frontend/src/routes/sponsor/register/frontPage.tsx
@@ -22,12 +22,12 @@ function FrontPage() {
if (event.target.value) {
console.log('searching')
const response = await fetch(
- `/api/ui/v1/person/search/${event.target.value}`
+ `/api/ui/v1/person/search/${event.target.value}?format=json`
)
const repjson = await response.json()
console.log(repjson)
if (response.ok) {
- setGuests(repjson.persons)
+ setGuests(repjson)
}
}
}
@@ -52,7 +52,11 @@ function FrontPage() {
guests.map((guest) => {
const guestTo = `/sponsor/guest/${guest.pid}`
return (
- <MenuItem component={Link} to={guestTo}>
+ <MenuItem
+ key={`${guest.pid}-${guest.value}`}
+ component={Link}
+ to={guestTo}
+ >
{guest.first} {guest.last}
<br />
{guest.value}
diff --git a/greg/api/serializers/identity.py b/greg/api/serializers/identity.py
index 8cfd2553..9caf8270 100644
--- a/greg/api/serializers/identity.py
+++ b/greg/api/serializers/identity.py
@@ -24,3 +24,9 @@ class IdentitySerializer(serializers.ModelSerializer):
if self.is_duplicate(attrs["type"], attrs["value"]):
raise ValidationError("Identity already exists")
return attrs
+
+
+class SpecialIdentitySerializer(serializers.ModelSerializer):
+ class Meta:
+ model = Identity
+ fields = ["id", "value", "type", "verified_at"]
diff --git a/greg/api/serializers/person.py b/greg/api/serializers/person.py
index f8fe4c9f..4327b3d9 100644
--- a/greg/api/serializers/person.py
+++ b/greg/api/serializers/person.py
@@ -1,9 +1,10 @@
from rest_framework import serializers
+from rest_framework.fields import BooleanField, CharField, SerializerMethodField
from greg.api.serializers.consent import ConsentSerializerBrief
-from greg.api.serializers.identity import IdentitySerializer
-from greg.api.serializers.role import RoleSerializer
-from greg.models import Person
+from greg.api.serializers.identity import IdentitySerializer, SpecialIdentitySerializer
+from greg.api.serializers.role import RoleSerializer, SpecialRoleSerializer
+from greg.models import Person, Identity
class PersonSerializer(serializers.ModelSerializer):
@@ -23,3 +24,59 @@ class PersonSerializer(serializers.ModelSerializer):
"roles",
"consents",
]
+
+
+class SpecialPersonSerializer(serializers.ModelSerializer):
+ """
+ Serializer for the person endpoint
+
+ Can be used to change or add an email to the person
+
+ """
+
+ pid = CharField(source="id", read_only=True)
+ first = CharField(source="first_name", read_only=True)
+ last = CharField(source="last_name", read_only=True)
+ email = SerializerMethodField(source="private_email")
+ mobile = SerializerMethodField(source="private_mobile", read_only=True)
+ fnr = SpecialIdentitySerializer(read_only=True)
+ passport = SpecialIdentitySerializer(read_only=True)
+ active = SerializerMethodField(source="active", read_only=True)
+ registered = BooleanField(source="is_registered", read_only=True)
+ verified = BooleanField(source="is_verified", read_only=True)
+ roles = SpecialRoleSerializer(many=True, read_only=True)
+
+ def get_email(self, obj):
+ return obj.private_email and obj.private_email.value
+
+ def get_mobile(self, obj):
+ return obj.private_mobile and obj.private_mobile.value
+
+ def get_active(self, obj):
+ return obj.is_registered and obj.is_verified
+
+ class Meta:
+ model = Person
+ fields = [
+ "pid",
+ "first",
+ "last",
+ "mobile",
+ "fnr",
+ "email",
+ "passport",
+ "active",
+ "registered",
+ "verified",
+ "roles",
+ ]
+
+
+class PersonSearchSerializer(serializers.ModelSerializer):
+ pid = CharField(source="person.id")
+ first = CharField(source="person.first_name")
+ last = CharField(source="person.last_name")
+
+ class Meta:
+ model = Identity
+ fields = ["pid", "first", "last", "value", "type"]
diff --git a/greg/api/serializers/role.py b/greg/api/serializers/role.py
index 31b2a43f..5b9c3b20 100644
--- a/greg/api/serializers/role.py
+++ b/greg/api/serializers/role.py
@@ -1,5 +1,5 @@
from rest_framework import serializers
-from rest_framework.fields import IntegerField
+from rest_framework.fields import IntegerField, SerializerMethodField
from greg.api.serializers.organizational_unit import OrganizationalUnitSerializer
from greg.models import Role, RoleType
@@ -33,3 +33,39 @@ class RoleWriteSerializer(RoleSerializer):
"""
orgunit = IntegerField(source="orgunit_id") # type: ignore
+
+
+class SpecialRoleSerializer(serializers.ModelSerializer):
+ name_nb = SerializerMethodField(source="type")
+ name_en = SerializerMethodField(source="type")
+ ou_nb = SerializerMethodField(source="orgunit")
+ ou_en = SerializerMethodField(source="orgunit")
+ max_days = SerializerMethodField(source="type")
+
+ def get_name_nb(self, obj):
+ return obj.type.name_nb
+
+ def get_name_en(self, obj):
+ return obj.type.name_en
+
+ def get_ou_nb(self, obj):
+ return obj.orgunit.name_nb
+
+ def get_ou_en(self, obj):
+ return obj.orgunit.name_en
+
+ def get_max_days(self, obj):
+ return obj.type.max_days
+
+ class Meta:
+ model = Role
+ fields = [
+ "id",
+ "name_nb",
+ "name_en",
+ "ou_nb",
+ "ou_en",
+ "start_date",
+ "end_date",
+ "max_days",
+ ]
diff --git a/gregui/api/urls.py b/gregui/api/urls.py
index b318d9ae..d7fa5f1f 100644
--- a/gregui/api/urls.py
+++ b/gregui/api/urls.py
@@ -9,7 +9,7 @@ from gregui.api.views.invitation import (
InvitedGuestView,
)
from gregui.api.views.ou import OusViewSet
-from gregui.api.views.person import PersonSearchView, PersonView
+from gregui.api.views.person import GuestInfoViewSet, PersonSearchViewSet, PersonViewSet
from gregui.api.views.role import RoleInfoViewSet
from gregui.api.views.roletypes import RoleTypeViewSet
from gregui.api.views.unit import UnitsViewSet
@@ -18,6 +18,8 @@ router = DefaultRouter(trailing_slash=False)
router.register(r"role", RoleInfoViewSet, basename="role")
router.register(r"identity", IdentityViewSet, basename="identity")
router.register(r"ous", OusViewSet, basename="ou")
+router.register(r"person", PersonViewSet, basename="person")
+router.register(r"guests/", GuestInfoViewSet, basename="guests")
urlpatterns = router.urls
urlpatterns += [
@@ -31,8 +33,9 @@ urlpatterns += [
name="invite-resend",
),
path("invite/", InvitationView.as_view(), name="invitation"),
- path("person/<int:id>", PersonView.as_view(), name="person-get"),
- path(
- "person/search/<searchstring>", PersonSearchView.as_view(), name="person-search"
+ re_path(
+ r"person/search/(?P<searchstring>\S+)", # search for sequence of any non-whitespace char
+ PersonSearchViewSet.as_view({"get": "list"}),
+ name="person-search",
),
]
diff --git a/gregui/api/views/person.py b/gregui/api/views/person.py
index fd8c8305..fec45bb3 100644
--- a/gregui/api/views/person.py
+++ b/gregui/api/views/person.py
@@ -1,18 +1,18 @@
-from django.http.response import JsonResponse
-from rest_framework import status
+from rest_framework import mixins
from rest_framework.authentication import SessionAuthentication, BasicAuthentication
from rest_framework.permissions import IsAuthenticated
-from rest_framework.response import Response
-from rest_framework.views import APIView
+from rest_framework.viewsets import GenericViewSet
+from greg.api.serializers.person import PersonSearchSerializer, SpecialPersonSerializer
from greg.models import Identity, Person
from greg.permissions import IsSponsor
+from gregui import validation
from gregui.api.serializers.guest import create_identity_or_update
from gregui.models import GregUserProfile
-from gregui.validation import validate_email
-class PersonView(APIView):
+class PersonViewSet(mixins.RetrieveModelMixin, mixins.UpdateModelMixin, GenericViewSet):
+
"""
Fetch person info for any guest as long as you are a sponsor
@@ -24,144 +24,48 @@ class PersonView(APIView):
authentication_classes = [SessionAuthentication, BasicAuthentication]
permission_classes = [IsAuthenticated, IsSponsor]
-
- def get(self, request, id):
- person = Person.objects.get(id=id)
- response = {
- "pid": person.id,
- "first": person.first_name,
- "last": person.last_name,
- "email": person.private_email and person.private_email.value,
- "mobile": person.private_mobile and person.private_mobile.value,
- "fnr": person.fnr
- and {
- "id": person.fnr.id,
- "value": person.fnr.value,
- "type": person.fnr.type,
- "verified_at": person.fnr.verified_at,
- },
- "passport": person.passport
- and {
- "id": person.passport.id,
- "value": person.passport.value,
- "type": person.passport.type,
- "verified_at": person.passport.verified_at,
- },
- "active": person.is_registered and person.is_verified,
- "registered": person.is_registered,
- "verified": person.is_verified,
- "roles": [
- {
- "id": role.id,
- "name_nb": role.type.name_nb,
- "name_en": role.type.name_en,
- "ou_nb": role.orgunit.name_nb,
- "ou_en": role.orgunit.name_en,
- "start_date": role.start_date,
- "end_date": role.end_date,
- "max_days": role.type.max_days,
- }
- for role in person.roles.all()
- ],
- }
- return JsonResponse(response)
-
- def patch(self, request, id):
- person = Person.objects.get(id=id)
- # For now only the e-mail is allowed to be updated
- email = request.data.get("email")
- if not email:
- return Response(status=status.HTTP_400_BAD_REQUEST)
- validate_email(email)
- # The following line will raise an exception if the e-mail is not valid
+ queryset = Person.objects.all()
+ http_methods = ["get", "patch"]
+ serializer_class = SpecialPersonSerializer
+
+ def perform_update(self, serializer):
+ """Update email when doing patch"""
+ email = self.request.data.get("email")
+ person = self.get_object()
+ validation.validate_email(email)
create_identity_or_update(Identity.IdentityType.PRIVATE_EMAIL, email, person)
+ return super().perform_update(serializer)
- return Response(status=status.HTTP_200_OK)
-
-class PersonSearchView(APIView):
+class PersonSearchViewSet(mixins.ListModelMixin, GenericViewSet):
"""Search for persons using email or phone number"""
authentication_classes = [SessionAuthentication, BasicAuthentication]
permission_classes = [IsAuthenticated, IsSponsor]
+ serializer_class = PersonSearchSerializer
- def get(self, requests, searchstring):
- search = Identity.objects.filter(
- value__icontains=searchstring, # icontains to include wrong case emails
+ def get_queryset(self):
+ search = self.kwargs["searchstring"]
+ return Identity.objects.filter(
+ value__icontains=search, # icontains to include wrong case emails
type__in=[
Identity.IdentityType.PRIVATE_EMAIL,
Identity.IdentityType.PRIVATE_MOBILE_NUMBER,
],
)[:10]
- response = {
- "persons": [
- {
- "pid": i.person.id,
- "first": i.person.first_name,
- "last": i.person.last_name,
- "value": i.value,
- "type": i.type,
- }
- for i in search
- ]
- }
- return JsonResponse(response)
-
-
-class GuestInfoView(APIView):
- """Fetch all the sponsors guests"""
+
+
+class GuestInfoViewSet(mixins.ListModelMixin, GenericViewSet):
+ """
+ Fetch all the sponsor's guests.
+
+ Lists all persons connected to the roles the logged in sponsor is connected to.
+ """
authentication_classes = [SessionAuthentication, BasicAuthentication]
permission_classes = [IsAuthenticated, IsSponsor]
+ serializer_class = SpecialPersonSerializer
- @staticmethod
- # pylint: disable=W0622
- def get(request, format=None):
- user = GregUserProfile.objects.get(user=request.user)
-
- return JsonResponse(
- {
- "persons": [
- {
- "pid": person.id,
- "first": person.first_name,
- "last": person.last_name,
- "email": person.private_email and person.private_email.value,
- "mobile": person.private_mobile and person.private_mobile.value,
- "fnr": person.fnr
- and {
- "id": person.fnr.id,
- "value": "".join((person.fnr.value[:-5], "*****")),
- "type": person.fnr.type,
- "verified_at": person.fnr.verified_at,
- },
- "passport": person.passport
- and {
- "id": person.passport.id,
- "value": person.passport.value,
- "type": person.passport.type,
- "verified_at": person.passport.verified_at,
- },
- "active": person.is_registered and person.is_verified,
- "registered": person.is_registered,
- "verified": person.is_verified,
- "roles": [
- {
- "id": role.id,
- "name_nb": role.type.name_nb,
- "name_en": role.type.name_en,
- "ou_nb": role.orgunit.name_nb,
- "ou_en": role.orgunit.name_en,
- "start_date": role.start_date,
- "end_date": role.end_date,
- "max_days": role.type.max_days,
- }
- for role in person.roles.all()
- ],
- }
- for person in Person.objects.filter(
- roles__sponsor=user.sponsor
- ).distinct()
- ]
- }
- )
+ def get_queryset(self):
+ user = GregUserProfile.objects.get(user=self.request.user)
+ return Person.objects.filter(roles__sponsor=user.sponsor).distinct()
diff --git a/gregui/tests/api/views/test_person.py b/gregui/tests/api/views/test_person.py
index 4afb1f77..e4a5578c 100644
--- a/gregui/tests/api/views/test_person.py
+++ b/gregui/tests/api/views/test_person.py
@@ -6,7 +6,7 @@ from rest_framework.reverse import reverse
@pytest.mark.django_db
def test_get_person_fail(client):
"""Anonymous user cannot get person info"""
- url = reverse("gregui-v1:person-get", kwargs={"id": 1})
+ url = reverse("gregui-v1:person-detail", kwargs={"pk": 1})
response = client.get(url)
assert response.status_code == status.HTTP_403_FORBIDDEN
@@ -15,7 +15,7 @@ def test_get_person_fail(client):
def test_get_person(client, log_in, user_sponsor, invited_person):
"""Logged in sponsor can get person info"""
person, _ = invited_person
- url = reverse("gregui-v1:person-get", kwargs={"id": person.id})
+ url = reverse("gregui-v1:person-detail", kwargs={"pk": person.id})
log_in(user_sponsor)
response = client.get(url)
assert response.status_code == status.HTTP_200_OK
@@ -25,7 +25,7 @@ def test_get_person(client, log_in, user_sponsor, invited_person):
def test_patch_person_no_data_fail(client, log_in, user_sponsor, invited_person):
"""No data in patch should fail"""
person, _ = invited_person
- url = reverse("gregui-v1:person-get", kwargs={"id": person.id})
+ url = reverse("gregui-v1:person-detail", kwargs={"pk": person.id})
log_in(user_sponsor)
response = client.patch(url)
assert response.status_code == status.HTTP_400_BAD_REQUEST
@@ -35,7 +35,7 @@ def test_patch_person_no_data_fail(client, log_in, user_sponsor, invited_person)
def test_patch_person_new_email_ok(client, log_in, user_sponsor, invited_person):
"""Logged in sponsor can update email address of person"""
person, _ = invited_person
- url = reverse("gregui-v1:person-get", kwargs={"id": person.id})
+ url = reverse("gregui-v1:person-detail", kwargs={"pk": person.id})
log_in(user_sponsor)
assert person.private_email.value == "foo@example.org"
response = client.patch(url, data={"email": "new@example.com"})
diff --git a/gregui/urls.py b/gregui/urls.py
index 0c2f085b..4292a73e 100644
--- a/gregui/urls.py
+++ b/gregui/urls.py
@@ -6,7 +6,6 @@ from django.urls.resolvers import URLResolver
from gregui.api import urls as api_urls
from gregui.api.views.userinfo import UserInfoView
-from gregui.api.views.person import GuestInfoView
from . import views
urlpatterns: List[URLResolver] = [
@@ -20,5 +19,4 @@ urlpatterns: List[URLResolver] = [
path("api/ui/v1/testmail/", views.send_test_email, name="api-testmail"),
path("api/ui/v1/whoami/", views.WhoAmIView.as_view(), name="api-whoami"),
path("api/ui/v1/userinfo/", UserInfoView.as_view(), name="api-userinfo"), # type: ignore
- path("api/ui/v1/guests/", GuestInfoView.as_view()),
]
diff --git a/gregui/validation.py b/gregui/validation.py
index 18ad553d..ecdfca28 100644
--- a/gregui/validation.py
+++ b/gregui/validation.py
@@ -1,4 +1,5 @@
import re
+from typing import Optional
import phonenumbers
from rest_framework import serializers
@@ -19,6 +20,6 @@ def validate_phone_number(value):
raise serializers.ValidationError("Invalid phone number")
-def validate_email(value):
- if not re.fullmatch(_valid_email_regex, value):
+def validate_email(value: Optional[str]):
+ if not value or not re.fullmatch(_valid_email_regex, value):
raise serializers.ValidationError("Invalid e-mail")
--
GitLab