From adbde7780a20fc42c667bc208365e7ed32c4d3bb Mon Sep 17 00:00:00 2001 From: Tore Brede <Tore.Brede@uib.no> Date: Tue, 19 Oct 2021 16:36:28 +0200 Subject: [PATCH] GREG-85: Updates to how form determines which fields to show --- .../guest/register/authenticationMethod.ts | 2 +- .../routes/guest/register/guestDataForm.ts | 3 +- frontend/src/routes/guest/register/index.tsx | 12 ++--- .../guest/register/registerPage.test.tsx | 2 +- .../routes/guest/register/registerPage.tsx | 6 +-- gregui/api/serializers/guest.py | 27 +++++----- gregui/api/views/invitation.py | 51 ++++++++++--------- gregui/authentication/auth_backends.py | 4 +- gregui/tests/api/test_invitation.py | 20 ++++---- 9 files changed, 63 insertions(+), 64 deletions(-) diff --git a/frontend/src/routes/guest/register/authenticationMethod.ts b/frontend/src/routes/guest/register/authenticationMethod.ts index d25eecdb..f01a26db 100644 --- a/frontend/src/routes/guest/register/authenticationMethod.ts +++ b/frontend/src/routes/guest/register/authenticationMethod.ts @@ -3,7 +3,7 @@ */ enum AuthenticationMethod { Feide, - NationalIdNumberOrPassport, + Invite, } export default AuthenticationMethod diff --git a/frontend/src/routes/guest/register/guestDataForm.ts b/frontend/src/routes/guest/register/guestDataForm.ts index 75d0307f..44ab9823 100644 --- a/frontend/src/routes/guest/register/guestDataForm.ts +++ b/frontend/src/routes/guest/register/guestDataForm.ts @@ -22,6 +22,5 @@ export type ContactInformationBySponsor = { fnr?: string passport?: string - user_information_source: AuthenticationMethod - fnr_verified: boolean + authentication_method: AuthenticationMethod } diff --git a/frontend/src/routes/guest/register/index.tsx b/frontend/src/routes/guest/register/index.tsx index 8dc410be..23c4864a 100644 --- a/frontend/src/routes/guest/register/index.tsx +++ b/frontend/src/routes/guest/register/index.tsx @@ -51,8 +51,7 @@ export default function GuestRegister() { mobile_phone: '', fnr: '', passport: '', - user_information_source: AuthenticationMethod.Feide, - fnr_verified: false, + authentication_method: AuthenticationMethod.Invite, }) const guestContactInfo = async () => { @@ -64,12 +63,10 @@ export default function GuestRegister() { console.log(`Guest data from server: ${JSON.stringify(jsonResponse)}`) const authenticationMethod = - jsonResponse.meta.form_type === 'manual' - ? AuthenticationMethod.NationalIdNumberOrPassport + jsonResponse.meta.session_type === 'invite' + ? AuthenticationMethod.Invite : AuthenticationMethod.Feide - const fnrVerified = jsonResponse.meta.fnr_verified === true - setGuestFormData({ first_name: jsonResponse.person.first_name, last_name: jsonResponse.person.last_name, @@ -86,8 +83,7 @@ export default function GuestRegister() { fnr: jsonResponse.fnr, passport: jsonResponse.passport, - user_information_source: authenticationMethod, - fnr_verified: fnrVerified, + authentication_method: authenticationMethod, }) }) } diff --git a/frontend/src/routes/guest/register/registerPage.test.tsx b/frontend/src/routes/guest/register/registerPage.test.tsx index 7df85cc3..ccf4cfc8 100644 --- a/frontend/src/routes/guest/register/registerPage.test.tsx +++ b/frontend/src/routes/guest/register/registerPage.test.tsx @@ -24,7 +24,7 @@ test('Guest register page showing passport field on manual registration', async mobile_phone: '', fnr: '', passport: '', - user_information_source: AuthenticationMethod.NationalIdNumberOrPassport, + authentication_method: AuthenticationMethod.Invite, } render( diff --git a/frontend/src/routes/guest/register/registerPage.tsx b/frontend/src/routes/guest/register/registerPage.tsx index 8a41ed67..32180f46 100644 --- a/frontend/src/routes/guest/register/registerPage.tsx +++ b/frontend/src/routes/guest/register/registerPage.tsx @@ -161,8 +161,8 @@ const GuestRegisterStep = forwardRef( /> </Box> - {guestData.user_information_source === - AuthenticationMethod.NationalIdNumberOrPassport && ( + {guestData.authentication_method === + AuthenticationMethod.Invite && ( <> <TextField id="passport" @@ -187,7 +187,7 @@ const GuestRegisterStep = forwardRef( </> )} - {guestData.user_information_source === + {guestData.authentication_method === AuthenticationMethod.Feide && ( <TextField id="national_id_number" diff --git a/gregui/api/serializers/guest.py b/gregui/api/serializers/guest.py index 80f12230..ce702752 100644 --- a/gregui/api/serializers/guest.py +++ b/gregui/api/serializers/guest.py @@ -21,7 +21,9 @@ class GuestRegisterSerializer(serializers.ModelSerializer): # TODO first_name and last_name set as not required to throwing an exception if they are not included in what is sent back from the frontend. It is perhaps not required that they are in the reponse from the client if the guest should be allowed to change them first_name = serializers.CharField(required=False) last_name = serializers.CharField(required=False) - email = serializers.CharField(required=True) + # E-mail set to not required to avoid raising exception if it is not included in input. It is not given that + # the guest should be allowed to update it + email = serializers.CharField(required=False) mobile_phone = serializers.CharField( required=True, validators=[_validatePhoneNumber] ) @@ -30,19 +32,20 @@ class GuestRegisterSerializer(serializers.ModelSerializer): ) def update(self, instance, validated_data): - email = validated_data.pop("email") mobile_phone = validated_data.pop("mobile_phone") - if not instance.private_email: - Identity.objects.create( - person=instance, - type=Identity.IdentityType.PRIVATE_EMAIL, - value=email, - ) - else: - private_email = instance.private_email - private_email.value = email - private_email.save() + if "email" in validated_data: + email = validated_data.pop("email") + if not instance.private_email: + Identity.objects.create( + person=instance, + type=Identity.IdentityType.PRIVATE_EMAIL, + value=email, + ) + else: + private_email = instance.private_email + private_email.value = email + private_email.save() if not instance.private_mobile: Identity.objects.create( diff --git a/gregui/api/views/invitation.py b/gregui/api/views/invitation.py index 04cc76a5..2432ba0d 100644 --- a/gregui/api/views/invitation.py +++ b/gregui/api/views/invitation.py @@ -1,26 +1,21 @@ -import json -import datetime -from uuid import uuid4 +from enum import Enum -from django.contrib.auth.models import AnonymousUser from django.core import exceptions from django.db import transaction from django.http.response import JsonResponse - from django.utils import timezone -from rest_framework import serializers, status +from rest_framework import status from rest_framework.authentication import SessionAuthentication, BasicAuthentication from rest_framework.generics import CreateAPIView, GenericAPIView from rest_framework.parsers import JSONParser -from rest_framework.permissions import AllowAny, IsAuthenticated +from rest_framework.permissions import AllowAny from rest_framework.response import Response from rest_framework.views import APIView -from greg.models import Identity, Invitation, InvitationLink, Person, Role, Sponsor +from greg.models import Identity, InvitationLink from greg.permissions import IsSponsor from gregui.api.serializers.guest import GuestRegisterSerializer from gregui.api.serializers.invitation import InviteGuestSerializer - from gregui.models import GregUserProfile @@ -102,6 +97,11 @@ class CheckInvitationView(APIView): return Response(status=status.HTTP_200_OK) +class SessionType(Enum): + INVITE = "invite" + FEIDE = "feide" + + class InvitedGuestView(GenericAPIView): authentication_classes = [SessionAuthentication, BasicAuthentication] # The endpoint is only for invited guests, but the authorization happens in the actual method @@ -110,7 +110,7 @@ class InvitedGuestView(GenericAPIView): serializer_class = GuestRegisterSerializer # TODO Update to make dynamic based on where we get the information from. If we get some from Feide, then the user should not be allowed to change it - fields_allowed_to_update = ["email", "fnr", "mobile_phone"] + fields_allowed_to_update = ["email", "fnr", "mobile_phone", "passport"] def get(self, request, *args, **kwargs): """ @@ -135,17 +135,16 @@ class InvitedGuestView(GenericAPIView): sponsor = role.sponsor_id # If the user is not logged in then tell the client to take him through the manual registration process - is_not_logged_in = request.user.is_anonymous + session_type = ( + SessionType.INVITE.value + if request.user.is_anonymous + else SessionType.FEIDE.value + ) - fnr_verified = False try: - fnr_identity = person.identities.get( + fnr = person.identities.get( type=Identity.IdentityType.NORWEGIAN_NATIONAL_ID_NUMBER - ) - fnr = fnr_identity.value - # TODO Maybe other criteria should be specified here - if fnr.verified == Identity.Verified.AUTOMATIC: - fnr_verified = True + ).value except Identity.DoesNotExist: fnr = None @@ -178,10 +177,7 @@ class InvitedGuestView(GenericAPIView): "end": role.end_date, "comments": role.comments, }, - "meta": { - "form_type": "manual" if is_not_logged_in else "feide", - "fnr_verified": fnr_verified, - }, + "meta": {"session_type": session_type}, } return JsonResponse(data=data, status=status.HTTP_200_OK) @@ -215,7 +211,11 @@ class InvitedGuestView(GenericAPIView): return Response(status=status.HTTP_400_BAD_REQUEST) with transaction.atomic(): - serializer = self.get_serializer(instance=person, data=request.data) + # Note this only serializes data for the person, it does not look at other sections + # in the response that happen to be there + serializer = self.get_serializer( + instance=person, data=request.data["person"] + ) serializer.is_valid(raise_exception=True) person = serializer.save() @@ -241,8 +241,9 @@ class InvitedGuestView(GenericAPIView): def _only_allowed_fields_in_request(self, request_data) -> bool: # Check how many of the allowed fields are filled in + person_data = request_data["person"] number_of_fields_filled_in = sum( - map(lambda x: x in request_data.keys(), self.fields_allowed_to_update) + map(lambda x: x in person_data.keys(), self.fields_allowed_to_update) ) # Check that there are no other fields filled in - return number_of_fields_filled_in == len(request_data.keys()) + return number_of_fields_filled_in == len(person_data.keys()) diff --git a/gregui/authentication/auth_backends.py b/gregui/authentication/auth_backends.py index 2ea983e5..74b897c1 100644 --- a/gregui/authentication/auth_backends.py +++ b/gregui/authentication/auth_backends.py @@ -6,10 +6,10 @@ from django.conf import settings from django.contrib.auth import get_user_model from django.contrib.auth.backends import BaseBackend from django.core.exceptions import SuspiciousOperation -from mozilla_django_oidc.auth import OIDCAuthenticationBackend - from django.utils import timezone +from mozilla_django_oidc.auth import OIDCAuthenticationBackend + from greg.models import Identity, Person, Sponsor from gregui.models import GregUserProfile diff --git a/gregui/tests/api/test_invitation.py b/gregui/tests/api/test_invitation.py index d4ed39df..58c3275b 100644 --- a/gregui/tests/api/test_invitation.py +++ b/gregui/tests/api/test_invitation.py @@ -44,7 +44,7 @@ def test_get_invited_info_no_session(client, invitation_link): @pytest.mark.django_db def test_get_invited_info_session_okay( - client, invitation_link, person_foo_data, sponsor_guy_data, role_type_foo, unit_foo + client, invitation_link, person_foo_data, sponsor_guy_data, role_type_foo, unit_foo ): # get a session client.get( @@ -78,7 +78,7 @@ def test_get_invited_info_session_okay( @pytest.mark.django_db def test_get_invited_info_expired_link( - client, invitation_link, invitation_expired_date + client, invitation_link, invitation_expired_date ): # Get a session while link is valid client.get( @@ -96,7 +96,7 @@ def test_get_invited_info_expired_link( @pytest.mark.django_db def test_invited_guest_can_post_information( - client: APIClient, invitation_link, person_foo_data + client: APIClient, invitation_link, person_foo_data ): # get a session client.get( @@ -109,7 +109,7 @@ def test_invited_guest_can_post_information( # post updated info to confirm from guest new_email = "private@example.org" new_phone = "+4797543992" - data = dict(email=new_email, mobile_phone=new_phone) + data = dict(person=dict(email=new_email, mobile_phone=new_phone)) response = client.post( reverse("gregui-v1:invited-info"), data, @@ -125,7 +125,7 @@ def test_invited_guest_can_post_information( @pytest.mark.django_db def test_post_invited_info_expired_session( - client, invitation_link, invitation_expired_date + client, invitation_link, invitation_expired_date ): # get a session client.get( @@ -158,9 +158,9 @@ def test_post_invited_info_deleted_inv_link(client, invitation_link): @pytest.mark.django_db def test_post_invited_info_invalid_national_id_number( - client, invitation_link, person_foo_data, person + client, invitation_link, person_foo_data, person ): - data = {"mobile_phone": "+4707543001", "email": "test@example.com", "fnr": "123"} + data = {"person": {"mobile_phone": "+4707543001", "email": "test@example.com", "fnr": "123"}} url = reverse("gregui-v1:invited-info") assert person.fnr is None @@ -179,10 +179,10 @@ def test_post_invited_info_invalid_national_id_number( @pytest.mark.django_db def test_post_invited_info_valid_national_id_number( - client, invitation_link, person_foo_data, person + client, invitation_link, person_foo_data, person ): fnr = "11120618212" - data = {"mobile_phone": "+4797543992", "email": "test@example.com", "fnr": fnr} + data = {"person": {"mobile_phone": "+4797543992", "email": "test@example.com", "fnr": fnr}} url = reverse("gregui-v1:invited-info") assert person.fnr is None @@ -215,7 +215,7 @@ def test_email_update(client, invitation_link, person_foo_data, person): session["invite_id"] = str(invitation_link.uuid) session.save() - data = {"mobile_phone": "+4797543992", "email": "test2@example.com"} + data = {"person": {"mobile_phone": "+4797543992", "email": "test2@example.com"}} response = client.post(url, data, format="json") assert response.status_code == status.HTTP_200_OK -- GitLab