diff --git a/.pylintrc b/.pylintrc index fc09c690635327ca663948431d88cbd6bfd6022d..41591c8b66351c04b268acfaf13d1fc6e638deb4 100644 --- a/.pylintrc +++ b/.pylintrc @@ -18,4 +18,5 @@ disable= redefined-outer-name, too-few-public-methods, too-many-ancestors, + too-many-arguments, unused-argument, diff --git a/greg/models.py b/greg/models.py index 1ef4d22f7aba66348deb29086d65caced200f536..430a80be47f856067bb56614e510abebd390c82c 100644 --- a/greg/models.py +++ b/greg/models.py @@ -1,6 +1,7 @@ import uuid from datetime import date +from typing import Optional from dirtyfields import DirtyFieldsMixin from django.conf import settings @@ -63,6 +64,18 @@ class Person(BaseModel): self.last_name, ) + @property + def private_email(self) -> Optional["Identity"]: + """The user provided private email address.""" + return self.identities.filter(type=Identity.IdentityType.PRIVATE_EMAIL).first() + + @property + def private_mobile(self) -> Optional["Identity"]: + """The user provided private mobile number.""" + return self.identities.filter( + type=Identity.IdentityType.PRIVATE_MOBILE_NUMBER + ).first() + @property def is_registered(self) -> bool: """ @@ -257,10 +270,19 @@ class Identity(BaseModel): ) verified_at = models.DateTimeField(null=True) + def __str__(self): + return "{}(id={!r}, type={!r}, value={!r})".format( + self.__class__.__name__, + self.pk, + self.type, + self.value, + ) + def __repr__(self): - return "{}(id={!r}, type={!r}, source={!r}, value={!r}, verified_by={!r}, verified_at={!r})".format( + return "{}(id={!r}, person_id={!r}, type={!r}, source={!r}, value={!r}, verified_by={!r}, verified_at={!r})".format( self.__class__.__name__, self.pk, + self.person_id, self.type, self.source, self.value, diff --git a/greg/tests/models/test_identity.py b/greg/tests/models/test_identity.py index f004d70af61cd1c92be8a47bfa98880c0aae56d1..3720642932d3e5ba3b78925e727b8c74c870427a 100644 --- a/greg/tests/models/test_identity.py +++ b/greg/tests/models/test_identity.py @@ -5,5 +5,5 @@ import pytest def test_identity_repr(person_foo_verified): assert ( repr(person_foo_verified) - == "Identity(id=3, type='passport_number', source='Test', value='12345', verified_by=Sponsor(id=1, feide_id='guy@example.org', first_name='Sponsor', last_name='Guy'), verified_at=datetime.datetime(2021, 6, 15, 12, 34, 56, tzinfo=<UTC>))" + == "Identity(id=3, person_id=1, type='passport_number', source='Test', value='12345', verified_by=Sponsor(id=1, feide_id='guy@example.org', first_name='Sponsor', last_name='Guy'), verified_at=datetime.datetime(2021, 6, 15, 12, 34, 56, tzinfo=<UTC>))" ) diff --git a/gregui/api/serializers/guest.py b/gregui/api/serializers/guest.py index 203aa441999692ad571fb96c288eb43f8f3b39d0..20a8759264756d36d231509342a3e864af683a46 100644 --- a/gregui/api/serializers/guest.py +++ b/gregui/api/serializers/guest.py @@ -1,29 +1,46 @@ -from django.db import transaction from rest_framework import serializers from greg.models import Identity, Person class GuestRegisterSerializer(serializers.ModelSerializer): + first_name = serializers.CharField(required=True) + last_name = serializers.CharField(required=True) email = serializers.CharField(required=True) + mobile_phone = serializers.CharField(required=True) - def create(self, validated_data): - # TODO: this serializer is untested + def update(self, instance, validated_data): email = validated_data.pop("email") - with transaction.atomic(): - person = super().create(**validated_data) + mobile_phone = validated_data.pop("mobile_phone") + + if not instance.private_email: Identity.objects.create( - person=person, + person=instance, type=Identity.IdentityType.PRIVATE_EMAIL, value=email, ) - return person + else: + instance.private_email.value = email + instance.private_email.save() + + if not instance.private_mobile: + Identity.objects.create( + person=instance, + type=Identity.IdentityType.PRIVATE_MOBILE_NUMBER, + value=mobile_phone, + ) + else: + instance.private_mobile.value = mobile_phone + instance.private_mobile.save() + + # TODO: we only want to allow changing the name if we don't have one + # from a reliable source (Feide/KORR) + instance.first_name = validated_data["first_name"] + instance.last_name = validated_data["last_name"] + + return instance class Meta: model = Person - fields = ("id", "first_name", "last_name", "email") + fields = ("id", "first_name", "last_name", "email", "mobile_phone") read_only_fields = ("id",) - extra_kwargs = { - "first_name": {"required": True}, - "last_name": {"required": True}, - } diff --git a/gregui/api/serializers/invitation.py b/gregui/api/serializers/invitation.py index 5a57e99edd15c7f3ab803973c8e896f6ba979a50..b1dcda9e88b24dee3cc8f1930fdc4c3a9f5d4c49 100644 --- a/gregui/api/serializers/invitation.py +++ b/gregui/api/serializers/invitation.py @@ -51,6 +51,3 @@ class InviteGuestSerializer(serializers.ModelSerializer): "uuid", ) read_only_field = ("uuid",) - - -foo = InviteGuestSerializer() diff --git a/gregui/api/views/invitation.py b/gregui/api/views/invitation.py index 18ac872356d9ba40d86c55a961351fd4458af473..176976302487930c0f415d87363b6f0002db152f 100644 --- a/gregui/api/views/invitation.py +++ b/gregui/api/views/invitation.py @@ -8,7 +8,7 @@ from django.http.response import JsonResponse from django.utils import timezone from rest_framework import serializers, status from rest_framework.authentication import SessionAuthentication, BasicAuthentication -from rest_framework.generics import CreateAPIView +from rest_framework.generics import CreateAPIView, GenericAPIView from rest_framework.parsers import JSONParser from rest_framework.permissions import AllowAny, IsAuthenticated from rest_framework.response import Response @@ -64,6 +64,8 @@ class CreateInvitationView(CreateAPIView): data=request.data, context={"request": request} ) serializer.is_valid(raise_exception=True) + + # TODO: check that sponsor has access to OU person = serializer.save() invitationlink = InvitationLink.objects.filter( @@ -99,7 +101,7 @@ class CheckInvitationView(APIView): return Response(status=status.HTTP_200_OK) -class InvitedGuestView(APIView): +class InvitedGuestView(GenericAPIView): authentication_classes = [SessionAuthentication, BasicAuthentication] permission_classes = [AllowAny] parser_classes = [JSONParser] @@ -140,8 +142,8 @@ class InvitedGuestView(APIView): "person": { "first_name": person.first_name, "last_name": person.last_name, - "email": person.email, - "mobile_phone": person.mobile_phone, + "email": person.private_email and person.private_email.value, + "mobile_phone": person.private_mobile and person.private_mobile.value, "fnr": fnr, "passport": passport, }, @@ -172,22 +174,20 @@ class InvitedGuestView(APIView): invite_id = request.session.get("invite_id") data = request.data + # Ensure the invitation link is valid and not expired + try: + invite_link = InvitationLink.objects.get(uuid=invite_id) + except (InvitationLink.DoesNotExist, exceptions.ValidationError): + return Response(status=status.HTTP_403_FORBIDDEN) + if invite_link.expire <= timezone.now(): + return Response(status=status.HTTP_403_FORBIDDEN) + + person = invite_link.invitation.role.person + with transaction.atomic(): - # Ensure the invitation link is valid and not expired - try: - invite_link = InvitationLink.objects.get(uuid=invite_id) - except (InvitationLink.DoesNotExist, exceptions.ValidationError): - return Response(status=status.HTTP_403_FORBIDDEN) - if invite_link.expire <= timezone.now(): - return Response(status=status.HTTP_403_FORBIDDEN) - - # Get objects to update - person = invite_link.invitation.role.person - - # Update with input from the guest - mobile = data.get("mobile_phone") - if mobile: - person.mobile_phone = data["mobile_phone"] + serializer = self.get_serializer(instance=person, data=request.data) + serializer.is_valid(raise_exception=True) + person = serializer.save() # Mark guest interaction done person.registration_completed_date = timezone.now().date() @@ -197,4 +197,4 @@ class InvitedGuestView(APIView): invite_link.expire = timezone.now() invite_link.save() # TODO: Send an email to the sponsor? - return Response(status=status.HTTP_201_CREATED) + return Response(status=status.HTTP_200_OK) diff --git a/gregui/tests/api/test_invitation.py b/gregui/tests/api/test_invitation.py index d2aa578d171189c87acb6c82d4f01f3f303d2724..5a0f3a95b9a8eb9b7ec504749b5dc32bacb4d50d 100644 --- a/gregui/tests/api/test_invitation.py +++ b/gregui/tests/api/test_invitation.py @@ -43,7 +43,9 @@ def test_get_invited_info_no_session(client, invitation_link): @pytest.mark.django_db -def test_get_invited_info_session_okay(client, invitation_link): +def test_get_invited_info_session_okay( + client, invitation_link, person_foo_data, sponsor_guy_data, role_type_foo, unit_foo +): # get a session client.get( reverse("gregui-v1:invite-verify", kwargs={"uuid": invitation_link.uuid}) @@ -52,20 +54,39 @@ def test_get_invited_info_session_okay(client, invitation_link): response = client.get(reverse("gregui-v1:invited-info")) assert response.status_code == status.HTTP_200_OK data = response.json() - assert data.get("person") - assert data.get("sponsor") - assert data.get("role") + assert data.get("person") == dict( + **person_foo_data, + email=None, + mobile_phone=None, + fnr=None, + passport=None, + ) + assert data.get("sponsor") == dict( + first_name=sponsor_guy_data["first_name"], + last_name=sponsor_guy_data["last_name"], + ) + assert data.get("role") == dict( + start=None, + end="2050-10-15", + comments="", + ou_name_en=unit_foo.name_en, + ou_name_nb=unit_foo.name_nb, + role_name_en=role_type_foo.name_en, + role_name_nb=role_type_foo.name_nb, + ) @pytest.mark.django_db -def test_get_invited_info_expired_link(client, invitation_link): +def test_get_invited_info_expired_link( + client, invitation_link, invitation_expired_date +): # Get a session while link is valid client.get( reverse("gregui-v1:invite-verify", kwargs={"uuid": invitation_link.uuid}) ) # Set expire link to expire long ago invlink = InvitationLink.objects.get(uuid=invitation_link.uuid) - invlink.expire = "1970-01-01" + invlink.expire = invitation_expired_date invlink.save() # Make a get request that should fail because invite expired after login, but # before get to userinfo @@ -74,45 +95,46 @@ def test_get_invited_info_expired_link(client, invitation_link): @pytest.mark.django_db -def test_post_invited_info_ok_mobile_update(client: APIClient, invitation_link): +def test_invited_guest_can_post_information( + client: APIClient, invitation_link, person_foo_data +): # get a session client.get( reverse("gregui-v1:invite-verify", kwargs={"uuid": invitation_link.uuid}) ) + + person = invitation_link.invitation.role.person + assert person.private_mobile is None + # post updated info to confirm from guest - new_phone = "12345678" - post_data = {"mobile_phone": new_phone} + new_email = "private@example.org" + new_phone = "+4712345678" + data = dict(email=new_email, mobile_phone=new_phone, **person_foo_data) response = client.post( reverse("gregui-v1:invited-info"), - post_data, + data, format="json", ) - assert response.status_code == status.HTTP_201_CREATED - # Check that the object was updated in the database - person = Person.objects.get(id=invitation_link.invitation.role.person.id) - assert person.mobile_phone == new_phone - + print(response.content) + assert response.status_code == status.HTTP_200_OK -@pytest.mark.django_db -def test_post_invited_info_ok(client, invitation_link): - # get a session - client.get( - reverse("gregui-v1:invite-verify", kwargs={"uuid": invitation_link.uuid}) - ) - # post updated info to confirm from guest - response = client.post(reverse("gregui-v1:invited-info")) - assert response.status_code == status.HTTP_201_CREATED + # Check that the object was updated in the database + assert Person.objects.count() == 1 + assert person.private_email.value == new_email + assert person.private_mobile.value == new_phone @pytest.mark.django_db -def test_post_invited_info_expired_session(client, invitation_link): +def test_post_invited_info_expired_session( + client, invitation_link, invitation_expired_date +): # get a session client.get( reverse("gregui-v1:invite-verify", kwargs={"uuid": invitation_link.uuid}) ) # Set expire link to expire long ago invlink = InvitationLink.objects.get(uuid=invitation_link.uuid) - invlink.expire = "1970-01-01" + invlink.expire = invitation_expired_date invlink.save() # post updated info to confirm from guest, should fail because of expired # invitation link diff --git a/gregui/tests/conftest.py b/gregui/tests/conftest.py index 40d3fab617fdbd9802247e84a7bf724ef0240e9b..2213168a9c3820fa8cf3138ac89d10b52d100070 100644 --- a/gregui/tests/conftest.py +++ b/gregui/tests/conftest.py @@ -1,10 +1,14 @@ +import datetime import logging +import pytest + + from django.contrib.auth import get_user_model +from django.utils.timezone import make_aware from rest_framework.authtoken.admin import User from rest_framework.test import APIClient -import pytest from greg.models import ( Invitation, @@ -30,21 +34,28 @@ def client() -> APIClient: @pytest.fixture def unit_foo() -> OrganizationalUnit: - ou = OrganizationalUnit.objects.create(orgreg_id="12345", name_en="foo_unit") + ou = OrganizationalUnit.objects.create( + orgreg_id="12345", name_en="Foo EN", name_nb="Foo NB" + ) return OrganizationalUnit.objects.get(id=ou.id) @pytest.fixture def role_type_foo() -> RoleType: - rt = RoleType.objects.create(identifier="role_foo", name_en="Role Foo") + rt = RoleType.objects.create( + identifier="role_foo", name_en="Role Foo EN", name_nb="Role Foo NB" + ) return RoleType.objects.get(id=rt.id) @pytest.fixture -def sponsor_guy(unit_foo: OrganizationalUnit) -> Sponsor: - sponsor = Sponsor.objects.create( - feide_id="guy@example.org", first_name="Sponsor", last_name="Guy" - ) +def sponsor_guy_data() -> dict: + return dict(feide_id="guy@example.org", first_name="Sponsor", last_name="Guy") + + +@pytest.fixture +def sponsor_guy(unit_foo: OrganizationalUnit, sponsor_guy_data) -> Sponsor: + sponsor = Sponsor.objects.create(**sponsor_guy_data) sponsor.units.add(unit_foo, through_defaults={"hierarchical_access": False}) return Sponsor.objects.get(id=sponsor.id) @@ -67,8 +78,16 @@ def user_sponsor(sponsor_guy: Sponsor) -> User: @pytest.fixture -def person() -> Person: - pe = Person.objects.create() +def person_foo_data() -> dict: + return dict( + first_name="Foo", + last_name="Bar", + ) + + +@pytest.fixture +def person(person_foo_data) -> Person: + pe = Person.objects.create(**person_foo_data) return Person.objects.get(id=pe.id) @@ -91,12 +110,26 @@ def invitation(role) -> Invitation: @pytest.fixture -def invitation_link(invitation) -> InvitationLink: - il = InvitationLink.objects.create(invitation=invitation, expire="2060-10-15") +def invitation_valid_date() -> datetime.datetime: + return make_aware(datetime.datetime(2060, 10, 15)) + + +@pytest.fixture +def invitation_expired_date() -> datetime.datetime: + return make_aware(datetime.datetime(1970, 1, 1)) + + +@pytest.fixture +def invitation_link(invitation, invitation_valid_date) -> InvitationLink: + il = InvitationLink.objects.create( + invitation=invitation, expire=invitation_valid_date + ) return InvitationLink.objects.get(id=il.id) @pytest.fixture -def invitation_link_expired(invitation) -> InvitationLink: - il = InvitationLink.objects.create(invitation=invitation, expire="1970-01-01") +def invitation_link_expired(invitation, invitation_expired_date) -> InvitationLink: + il = InvitationLink.objects.create( + invitation=invitation, expire=invitation_expired_date + ) return InvitationLink.objects.get(id=il.id)