From 1beceae1628e1b19c67b1922495d51a9a8cb2dd3 Mon Sep 17 00:00:00 2001
From: Tore Brede <Tore.Brede@uib.no>
Date: Fri, 12 Nov 2021 11:00:22 +0100
Subject: [PATCH] GREG-110: Allowing guest to change name in manual
registration
---
.../routes/guest/register/enteredGuestData.ts | 2 +
frontend/src/routes/guest/register/index.tsx | 7 +-
.../routes/guest/register/registerPage.tsx | 50 ++++++++---
gregui/api/serializers/guest.py | 31 +++----
gregui/api/views/invitation.py | 26 +++---
gregui/tests/api/test_invitation.py | 83 +++++++++++++++++--
6 files changed, 148 insertions(+), 51 deletions(-)
diff --git a/frontend/src/routes/guest/register/enteredGuestData.ts b/frontend/src/routes/guest/register/enteredGuestData.ts
index d7313603..cf5c8662 100644
--- a/frontend/src/routes/guest/register/enteredGuestData.ts
+++ b/frontend/src/routes/guest/register/enteredGuestData.ts
@@ -4,6 +4,8 @@
* most of the data there the guest cannot change.
*/
export type EnteredGuestData = {
+ firstName: string
+ lastName: string
mobilePhoneCountry: string
mobilePhone: string
nationalIdNumber?: string
diff --git a/frontend/src/routes/guest/register/index.tsx b/frontend/src/routes/guest/register/index.tsx
index 4fa17263..4c6cb9eb 100644
--- a/frontend/src/routes/guest/register/index.tsx
+++ b/frontend/src/routes/guest/register/index.tsx
@@ -146,7 +146,7 @@ export default function GuestRegister() {
const handleForwardFromRegister = (
updateFormData: EnteredGuestData
): void => {
- // TODO Should go to consent page here. Submit should after after consent page
+ // TODO Should go to consent page here, if there are consents defined in the database. Submit should happen after after consent page
const payload: any = {}
payload.person = {}
@@ -154,6 +154,11 @@ export default function GuestRegister() {
updateFormData.mobilePhoneCountry as CountryCode
)}${updateFormData.mobilePhone}`
+ if (guestFormData.authentication_method === AuthenticationMethod.Invite) {
+ payload.person.first_name = guestFormData.first_name
+ payload.person.last_name = guestFormData.last_name
+ }
+
if (updateFormData.passportNumber && updateFormData.passportNationality) {
// The user has entered some passport information, check that both nationality and number are present
if (
diff --git a/frontend/src/routes/guest/register/registerPage.tsx b/frontend/src/routes/guest/register/registerPage.tsx
index 8ce656e0..2651c922 100644
--- a/frontend/src/routes/guest/register/registerPage.tsx
+++ b/frontend/src/routes/guest/register/registerPage.tsx
@@ -138,18 +138,44 @@ const GuestRegisterStep = forwardRef(
<Box sx={{ maxWidth: '30rem' }}>
<form onSubmit={onSubmit}>
<Stack spacing={2}>
- <TextField
- id="firstName"
- label={t('input.firstName')}
- value={guestData.first_name}
- disabled
- />
- <TextField
- id="lastName"
- label={t('input.lastName')}
- value={guestData.last_name}
- disabled
- />
+ {/* The name is only editable if it is it is not coming from some trusted source */}
+ {guestData.authentication_method !==
+ AuthenticationMethod.Invite ? (
+ <>
+ <TextField
+ id="firstName"
+ label={t('input.firstName')}
+ value={guestData.first_name}
+ disabled
+ />
+ <TextField
+ id="lastName"
+ label={t('input.lastName')}
+ value={guestData.last_name}
+ disabled
+ />
+ </>
+ ) : (
+ <>
+ <TextField
+ id="firstName"
+ label={t('input.firstName')}
+ value={guestData.first_name}
+ {...register('firstName', {
+ required: true,
+ })}
+ />
+ <TextField
+ id="lastName"
+ label={t('input.lastName')}
+ value={guestData.last_name}
+ disabled
+ {...register('lastName', {
+ required: true,
+ })}
+ />
+ </>
+ )}
<TextField
id="email"
diff --git a/gregui/api/serializers/guest.py b/gregui/api/serializers/guest.py
index 18d18158..225ea44f 100644
--- a/gregui/api/serializers/guest.py
+++ b/gregui/api/serializers/guest.py
@@ -8,9 +8,8 @@ from gregui.validation import (
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)
+ first_name = serializers.CharField(required=False, min_length=1)
+ last_name = serializers.CharField(required=False, min_length=1)
# 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)
@@ -44,22 +43,16 @@ class GuestRegisterSerializer(serializers.ModelSerializer):
if "passport" in validated_data:
passport = validated_data.pop("passport")
- if not instance.passport:
- Identity.objects.create(
- person=instance,
- type=Identity.IdentityType.PASSPORT_NUMBER,
- value=passport,
- )
- else:
- passport_existing = instance.passport
- passport_existing.value = passport
- passport_existing.save()
+ create_identity_or_update(
+ Identity.IdentityType.PASSPORT_NUMBER, passport, instance
+ )
+
+ # If the name is not allowed to be updated, the request has already been denied at an earlier stage
+ if "first_name" in validated_data:
+ instance.first_name = validated_data["first_name"]
- # TODO: we only want to allow changing the name if we don't have one
- # from a reliable source (Feide/KORR)
- # TODO Comment back in after it is decided if name updates are allowed
- # instance.first_name = validated_data["first_name"]
- # instance.last_name = validated_data["last_name"]
+ if "last_name" in validated_data:
+ instance.last_name = validated_data["last_name"]
return instance
@@ -78,7 +71,7 @@ class GuestRegisterSerializer(serializers.ModelSerializer):
def create_identity_or_update(
- identity_type: Identity.IdentityType, value: str, person: Person
+ identity_type: Identity.IdentityType, value: str, person: Person
):
existing_identity = person.identities.filter(type=identity_type).first()
if not existing_identity:
diff --git a/gregui/api/views/invitation.py b/gregui/api/views/invitation.py
index bd8bc50a..db757d84 100644
--- a/gregui/api/views/invitation.py
+++ b/gregui/api/views/invitation.py
@@ -72,8 +72,8 @@ class InvitationView(CreateAPIView, DestroyAPIView):
person = serializer.save()
for invitationlink in InvitationLink.objects.filter(
- invitation__role__person_id=person.id,
- invitation__role__sponsor_id=sponsor_user.sponsor_id,
+ invitation__role__person_id=person.id,
+ invitation__role__sponsor_id=sponsor_user.sponsor_id,
):
send_invite_mail(invitationlink)
@@ -142,8 +142,9 @@ class InvitedGuestView(GenericAPIView):
parser_classes = [JSONParser]
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", "passport"]
+ # TODO Check these scenarios again. What should be allowed to be updated in the various coses
+ fields_allowed_to_update_if_invite = ["first_name", "last_name", "email", "fnr", "mobile_phone", "passport"]
+ fields_allowed_to_update_if_feide = ["mobile_phone"]
def get(self, request, *args, **kwargs):
"""
@@ -228,14 +229,16 @@ class InvitedGuestView(GenericAPIView):
return Response(status=status.HTTP_403_FORBIDDEN)
person = invite_link.invitation.role.person
-
data = request.data
- if self._verified_fnr_already_exists(person) and "fnr" in data:
- # The user should not be allowed to change a verified fnr
+ # If there is a Feide ID registered with the guest, assume that the name is also coming from there
+ feide_id = self._get_identity_or_none(person, Identity.IdentityType.FEIDE_ID)
+ if not self._only_allowed_fields_in_request(data,
+ self.fields_allowed_to_update_if_invite if feide_id is None else self.fields_allowed_to_update_if_feide):
return Response(status=status.HTTP_400_BAD_REQUEST)
- if not self._only_allowed_fields_in_request(data):
+ if self._verified_fnr_already_exists(person) and "fnr" in data:
+ # The user should not be allowed to change a verified fnr
return Response(status=status.HTTP_400_BAD_REQUEST)
with transaction.atomic():
@@ -268,18 +271,19 @@ class InvitedGuestView(GenericAPIView):
except Identity.DoesNotExist:
return False
- def _only_allowed_fields_in_request(self, request_data) -> bool:
+ @staticmethod
+ def _only_allowed_fields_in_request(request_data, fields_allowed_to_update) -> 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 person_data.keys(), self.fields_allowed_to_update)
+ map(lambda x: x in person_data.keys(), fields_allowed_to_update)
)
# Check that there are no other fields filled in
return number_of_fields_filled_in == len(person_data.keys())
@staticmethod
def _get_identity_or_none(
- person: Person, identity_type: Identity.IdentityType
+ person: Person, identity_type: Identity.IdentityType
) -> Optional[str]:
try:
return person.identities.get(type=identity_type).value
diff --git a/gregui/tests/api/test_invitation.py b/gregui/tests/api/test_invitation.py
index 44a792e8..88f987e7 100644
--- a/gregui/tests/api/test_invitation.py
+++ b/gregui/tests/api/test_invitation.py
@@ -40,9 +40,8 @@ def test_get_invited_info_no_session(client, invitation_link):
@pytest.mark.django_db
def test_get_invited_info_session_okay(
- client, invited_person, sponsor_foo_data, role_type_foo, unit_foo
+ client, invited_person, sponsor_foo_data, role_type_foo, unit_foo
):
-
person, invitation_link = invited_person
# get a session
client.post(reverse("gregui-v1:invite-verify"), data={"uuid": invitation_link.uuid})
@@ -76,7 +75,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(reverse("gregui-v1:invite-verify"), data={"uuid": invitation_link.uuid})
@@ -119,7 +118,7 @@ def test_invited_guest_can_post_information(client: APIClient, invited_person):
@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.post(reverse("gregui-v1:invite-verify"), data={"uuid": invitation_link.uuid})
@@ -231,10 +230,10 @@ def test_register_passport(client, invited_person):
session.save()
assert (
- Identity.objects.filter(
- person__id=person.id, type=Identity.IdentityType.PASSPORT_NUMBER
- ).count()
- == 0
+ Identity.objects.filter(
+ person__id=person.id, type=Identity.IdentityType.PASSPORT_NUMBER
+ ).count()
+ == 0
)
response = client.post(url, data, format="json")
@@ -247,3 +246,71 @@ def test_register_passport(client, invited_person):
).get()
assert registered_passport.value == passport_information
+
+
+@pytest.mark.django_db
+def test_name_update_not_allowed_if_feide_identity_is_present(client: APIClient, person_foo, sponsor_foo, unit_foo,
+ role_type_foo,
+ create_role, create_invitation, create_invitation_link,
+ invitation_valid_date):
+ # This person has a Feide ID, so the name is assumed to come from there as well
+ # and the guest should not be allowed to update it
+ role = create_role(
+ person=person_foo, sponsor=sponsor_foo, unit=unit_foo, role_type=role_type_foo
+ )
+ invitation = create_invitation(role)
+ invitation_link = create_invitation_link(invitation, invitation_valid_date)
+ client.post(reverse("gregui-v1:invite-verify"), data={"uuid": invitation_link.uuid})
+
+ person = invitation_link.invitation.role.person
+ data = {"person": {"first_name": "Someone", "last_name": "Test", "mobile_phone": "+4797543992"}}
+
+ # The update request should fail because it contains a name update
+ response = client.post(
+ reverse("gregui-v1:invited-info"),
+ data,
+ format="json",
+ )
+ assert response.status_code == status.HTTP_400_BAD_REQUEST
+
+ # Check that the name did not change in the database
+ assert Person.objects.count() == 1
+ assert person.first_name == person_foo.first_name
+ assert person.last_name == person_foo.last_name
+
+
+@pytest.mark.django_db
+def test_name_update_allowed_if_feide_identity_is_not_present(client: APIClient, create_person, sponsor_foo, unit_foo,
+ role_type_foo,
+ create_role, create_invitation, create_invitation_link,
+ invitation_valid_date):
+ # This person does not have a Feide ID, so he will be viewed as someone whose name was entered manually
+ # by the sponsor, and in it that case the guest can update it
+ person = create_person(
+ first_name="Foo",
+ last_name="Bar",
+ email="foo@bar.com")
+ role = create_role(
+ person=person, sponsor=sponsor_foo, unit=unit_foo, role_type=role_type_foo
+ )
+ invitation = create_invitation(role)
+ invitation_link = create_invitation_link(invitation, invitation_valid_date)
+ client.post(reverse("gregui-v1:invite-verify"), data={"uuid": invitation_link.uuid})
+
+ person = invitation_link.invitation.role.person
+ first_name = "Someone"
+ last_name = "Test"
+ data = {"person": {"first_name": first_name, "last_name": last_name, "mobile_phone": "+4797543992"}}
+
+ response = client.post(
+ reverse("gregui-v1:invited-info"),
+ data,
+ format="json",
+ )
+ assert response.status_code == status.HTTP_200_OK
+
+ # Check that the name has been updated in the database
+ assert Person.objects.count() == 1
+ person.refresh_from_db()
+ assert person.first_name == first_name
+ assert person.last_name == last_name
--
GitLab