Merge pull request #271 from maykinmedia/feature/234-email-validation

 ✨ [#234] Validate DigitaalAdres.adres if type is email

src/openklant/components/klantinteracties/admin/digitaal_adres.py

@@ -1,10 +1,24 @@
+from django import forms
 from django.contrib import admin
 
+from ..api.validators import OptionalEmailValidator
 from ..models.digitaal_adres import DigitaalAdres
 
 
+class DigitaalAdresAdminForm(forms.ModelForm):
+    class Meta:
+        model = DigitaalAdres
+        fields = "__all__"
+
+    def clean_adres(self):
+        data = self.cleaned_data
+        OptionalEmailValidator()(data["adres"], data.get("soort_digitaal_adres"))
+        return data["adres"]
+
+
 @admin.register(DigitaalAdres)
 class DigitaalAdresAdmin(admin.ModelAdmin):
     readonly_fields = ("uuid",)
     search_fields = ("adres",)
     autocomplete_fields = ("partij",)
+    form = DigitaalAdresAdminForm

src/openklant/components/klantinteracties/api/serializers/digitaal_adres.py

@@ -6,10 +6,14 @@
 from openklant.components.klantinteracties.api.serializers.constants import (
     SERIALIZER_PATH,
 )
-from openklant.components.klantinteracties.api.validators import digitaal_adres_exists
+from openklant.components.klantinteracties.api.validators import (
+    OptionalEmailValidator,
+    digitaal_adres_exists,
+)
 from openklant.components.klantinteracties.models.digitaal_adres import DigitaalAdres
 from openklant.components.klantinteracties.models.klantcontacten import Betrokkene
 from openklant.components.klantinteracties.models.partijen import Partij
+from openklant.utils.serializers import get_field_value
 
 
 class DigitaalAdresForeignKeySerializer(serializers.HyperlinkedModelSerializer):
@@ -86,6 +90,17 @@ class Meta:
             },
         }
 
+    def validate_adres(self, adres):
+        """
+        Define the validator here, to avoid DRF spectacular marking the format for
+        `adres` as `email`
+        """
+        soort_digitaal_adres = get_field_value(
+            self, self.initial_data, "soort_digitaal_adres"
+        )
+        OptionalEmailValidator()(adres, soort_digitaal_adres)
+        return adres
+
     @transaction.atomic
     def update(self, instance, validated_data):
         if "partij" in validated_data:

src/openklant/components/klantinteracties/api/tests/test_digitaal_adres.py

@@ -1,3 +1,6 @@
+from django.test import tag
+from django.utils.translation import gettext as _
+
 from rest_framework import status
 from vng_api_common.tests import reverse
 
@@ -90,6 +93,73 @@ def test_create_digitaal_adres(self):
             self.assertEqual(data["adres"], "[email protected]")
             self.assertEqual(data["omschrijving"], "omschrijving")
 
+    @tag("gh-234")
+    def test_create_digitaal_adres_email_validation(self):
+        list_url = reverse("klantinteracties:digitaaladres-list")
+        data = {
+            "verstrektDoorBetrokkene": None,
+            "verstrektDoorPartij": None,
+            "soortDigitaalAdres": SoortDigitaalAdres.email,
+            "adres": "invalid",
+            "omschrijving": "omschrijving",
+        }
+
+        response = self.client.post(list_url, data)
+
+        self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST)
+        data = response.json()
+        self.assertEqual(
+            data["invalidParams"],
+            [
+                {
+                    "name": "adres",
+                    "code": "invalid",
+                    "reason": _("Voer een geldig e-mailadres in."),
+                }
+            ],
+        )
+
+        digitaal_adres = DigitaalAdresFactory.create(
+            soort_digitaal_adres=SoortDigitaalAdres.email, adres="[email protected]"
+        )
+        detail_url = reverse(
+            "klantinteracties:digitaaladres-detail",
+            kwargs={"uuid": str(digitaal_adres.uuid)},
+        )
+
+        response = self.client.patch(detail_url, {"adres": "invalid"})
+
+        self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST)
+        data = response.json()
+        self.assertEqual(
+            data["invalidParams"],
+            [
+                {
+                    "name": "adres",
+                    "code": "invalid",
+                    "reason": _("Voer een geldig e-mailadres in."),
+                }
+            ],
+        )
+
+        with self.subTest("no validation applied if soort is not email"):
+            response = self.client.patch(
+                detail_url,
+                {
+                    "soortDigitaalAdres": SoortDigitaalAdres.telefoonnummer,
+                    "adres": "0612345678",
+                },
+            )
+
+            self.assertEqual(response.status_code, status.HTTP_200_OK)
+
+            digitaal_adres.refresh_from_db()
+
+            self.assertEqual(
+                digitaal_adres.soort_digitaal_adres, SoortDigitaalAdres.telefoonnummer
+            )
+            self.assertEqual(digitaal_adres.adres, "0612345678")
+
     def test_update_digitaal_adres(self):
         betrokkene, betrokkene2 = BetrokkeneFactory.create_batch(2)
         partij, partij2 = PartijFactory.create_batch(2)

src/openklant/components/klantinteracties/api/validators.py

@@ -1,9 +1,11 @@
+from django.core.validators import EmailValidator
 from django.db import models
 from django.utils.translation import gettext_lazy as _
 
 from rest_framework import serializers
 from rest_framework.validators import UniqueTogetherValidator, qs_filter
 
+from openklant.components.klantinteracties.constants import SoortDigitaalAdres
 from openklant.components.klantinteracties.models.actoren import Actor
 from openklant.components.klantinteracties.models.constants import SoortPartij
 from openklant.components.klantinteracties.models.digitaal_adres import DigitaalAdres
@@ -166,3 +168,14 @@ def Rekeningnummer_exists(value):
         Rekeningnummer.objects.get(uuid=str(value))
     except Rekeningnummer.DoesNotExist:
         raise serializers.ValidationError(_("Rekeningnummer object bestaat niet."))
+
+
+class OptionalEmailValidator(EmailValidator):
+    """
+    EmailValidator for SoortDigitaalAdres that only attempts to validate if
+    `SoortDigitaalAdres` is `email`
+    """
+
+    def __call__(self, value: str, soort_digitaal_adres: str):
+        if soort_digitaal_adres == SoortDigitaalAdres.email:
+            super().__call__(value)

src/openklant/components/klantinteracties/tests/test_admin.py

@@ -1,40 +1,36 @@
 from uuid import uuid4
 
-from django.conf import settings
-from django.test import override_settings
+from django.test import tag
 from django.urls import reverse
+from django.utils.translation import gettext as _
 
 from django_webtest import WebTest
 from maykin_2fa.test import disable_admin_mfa
-from webtest import Form, TestResponse
+from webtest import TestResponse
 
 from openklant.accounts.tests.factories import SuperUserFactory
 from openklant.components.klantinteracties.models import DigitaalAdres
 from openklant.components.klantinteracties.models.tests.factories.digitaal_adres import (
     DigitaalAdresFactory,
 )
+from openklant.components.klantinteracties.models.tests.factories.klantcontacten import (
+    BetrokkeneFactory,
+)
 from openklant.components.klantinteracties.models.tests.factories.partijen import (
     PartijFactory,
     PersoonFactory,
 )
 from openklant.utils.tests.webtest import add_dynamic_field
 
+from ..constants import SoortDigitaalAdres
 
-class PartijAdminTests(WebTest):
-    def _login_user(self, username: str, password: str) -> None:
-        request = self.app.get(reverse("admin:login"))
-
-        form: Form = request.forms["login-form"]
-        form["auth-username"] = username
-        form["auth-password"] = password
-        redirect = form.submit()
-
-        self.assertRedirects(redirect, reverse("admin:index"))
 
-    @override_settings(
-        MAYKIN_2FA_ALLOW_MFA_BYPASS_BACKENDS=settings.AUTHENTICATION_BACKENDS
-    )
+@disable_admin_mfa()
+class PartijAdminTests(WebTest):
     def test_search(self):
+        user = SuperUserFactory.create()
+        self.app.set_user(user)
+
         nummer_persoon = PersoonFactory(
             partij__nummer="123456789",
             contactnaam_voornaam="Willem",
@@ -56,10 +52,6 @@ def test_search(self):
             contactnaam_achternaam="Willemse",
         )
 
-        SuperUserFactory(username="admin", password="admin")
-
-        self._login_user(username="admin", password="admin")
-
         admin_url = reverse("admin:klantinteracties_partij_changelist")
 
         # Test a nummer search query
@@ -103,8 +95,8 @@ def test_digitaal_adres_inline(self):
         betrokkene should be optional
         """
 
-        SuperUserFactory(username="admin", password="admin")
-        self._login_user(username="admin", password="admin")
+        user = SuperUserFactory(username="admin", password="admin")
+        self.app.set_user(user)
 
         partij = PartijFactory(soort_partij="persoon", voorkeurs_digitaal_adres=None)
         PersoonFactory(partij=partij)
@@ -126,3 +118,50 @@ def test_digitaal_adres_inline(self):
         self.assertEqual(adres.omschrijving, "description")
         self.assertEqual(adres.adres, "[email protected]")
         self.assertIsNone(adres.betrokkene)
+
+
+@disable_admin_mfa()
+class DigitaalAdresAdminTests(WebTest):
+    @tag("gh-234")
+    def test_email_validation(self):
+        """
+        Check that the admin applies conditional email validation on `adres`, only if
+        `soort_digitaal_adres` is `email`
+        """
+        user = SuperUserFactory.create()
+        self.app.set_user(user)
+
+        betrokkene = BetrokkeneFactory.create()
+
+        admin_url = reverse("admin:klantinteracties_digitaaladres_add")
+
+        with self.subTest("apply validation for soort_digitaal_adres == email"):
+            response: TestResponse = self.app.get(admin_url)
+
+            form = response.form
+            form["betrokkene"] = betrokkene.pk
+            form["soort_digitaal_adres"] = SoortDigitaalAdres.email
+            form["adres"] = "invalid"
+            form["omschrijving"] = "foo"
+
+            response = form.submit()
+
+            self.assertEqual(response.status_code, 200)
+            self.assertFalse(DigitaalAdres.objects.exists())
+            self.assertEqual(
+                response.context["errors"], [[_("Voer een geldig e-mailadres in.")]]
+            )
+
+        with self.subTest("do not apply validation for soort_digitaal_adres != email"):
+            response: TestResponse = self.app.get(admin_url)
+
+            form = response.form
+            form["betrokkene"] = betrokkene.pk
+            form["soort_digitaal_adres"] = SoortDigitaalAdres.telefoonnummer
+            form["adres"] = "0612345678"
+            form["omschrijving"] = "foo"
+
+            response = form.submit()
+
+            self.assertEqual(response.status_code, 302)
+            self.assertTrue(DigitaalAdres.objects.exists())

src/openklant/utils/serializers.py

@@ -1,5 +1,8 @@
+from typing import Any
+
 from django.utils.module_loading import import_string
 
+from rest_framework.serializers import Serializer
 from rest_framework_nested.serializers import NestedHyperlinkedRelatedField
 
 
@@ -46,3 +49,23 @@ def to_representation(self, value):
             value = value.all()
 
         return serializer.to_representation(value)
+
+
+def get_field_value(
+    serializer: Serializer, attrs: dict[str, Any], field_name: str
+) -> Any:
+    """
+    Helper function to retrieve a field value from either the attrs (new data)
+    or the instance (existing data during updates).
+
+    :param serializer: The serializer instance
+    :param attrs: The attributes passed to `.validate`
+    :param field_name: The name of the field to retrieve
+    :return: The value of the field, or None if not present
+    """
+    if field_name in attrs:
+        return attrs.get(field_name)
+    # Fallback to instance value if it exists
+    if serializer.instance:
+        return getattr(serializer.instance, field_name, None)
+    return None