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

@@ -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 => ({

frontend/src/routes/sponsor/index.tsx

 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()

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}

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"]

greg/api/serializers/person.py

 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"]

greg/api/serializers/role.py

 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",
+        ]

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",
     ),
 ]

gregui/api/views/person.py

-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()

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"})

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()),
 ]

gregui/validation.py

 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")