[#2811] Exclude empty email/phone number from being sent with contactform

src/open_inwoner/openklant/api_models.py

@@ -1,7 +1,7 @@
 import dataclasses
 from dataclasses import dataclass
 from datetime import datetime
-from typing import Optional, TypedDict, Union
+from typing import NotRequired, Optional, TypedDict, Union
 
 from zgw_consumers.api_models.base import ZGWModel
 
@@ -41,7 +41,7 @@ def get_name_display(self):
 
 class MedewerkerIdentificatie(TypedDict):
     identificatie: str
-    achternaam: str = ""
+    achternaam: NotRequired[str]
 
 
 class ContactMomentCreateData(TypedDict):
@@ -50,7 +50,8 @@ class ContactMomentCreateData(TypedDict):
     onderwerp: str
     type: str
     kanaal: str
-    medewerkerIdentificatie: MedewerkerIdentificatie
+    contactgegevens: NotRequired[dict[str, str]]
+    medewerkerIdentificatie: NotRequired[MedewerkerIdentificatie]
 
 
 @dataclass

src/open_inwoner/openklant/tests/data.py

@@ -459,14 +459,14 @@ def __init__(self):
 
         self.matchers = []
 
-    def install_mocks_anon_with_klant(self, m) -> "MockAPICreateData":
+    def install_mocks_anon(self, m) -> "MockAPICreateData":
         self.matchers = [
-            m.post(f"{KLANTEN_ROOT}klanten", json=self.klant_bsn, status_code=201),
             m.post(
                 f"{CONTACTMOMENTEN_ROOT}contactmomenten",
                 json=self.contactmoment,
                 status_code=201,
             ),
+            m.post(f"{KLANTEN_ROOT}klanten", json=self.klant_bsn, status_code=201),
             m.post(
                 f"{CONTACTMOMENTEN_ROOT}klantcontactmomenten",
                 json=self.klant_contactmoment,

src/open_inwoner/openklant/tests/test_contactform.py

@@ -1,4 +1,3 @@
-import inspect
 from unittest.mock import patch
 
 from django.contrib import messages
@@ -207,7 +206,7 @@ def test_expected_ordered_subjects_are_shown(
         )
         mock_send_confirm.assert_not_called()
 
-    def test_submit_and_register_via_email(self, m, mock_captcha, mock_send_confirm):
+    def test_register_contactmoment_via_email(self, m, mock_captcha, mock_send_confirm):
         config = OpenKlantConfig.get_solo()
         config.register_email = "example@example.com"
         config.has_form_configuration = True
@@ -248,7 +247,7 @@ def test_submit_and_register_via_email(self, m, mock_captcha, mock_send_confirm)
 
         mock_send_confirm.assert_called_once_with("foo@example.com", subject.subject)
 
-    def test_submit_and_register_anon_via_api_with_klant(
+    def test_register_contactmoment_for_anon_user_via_api(
         self, m, mock_captcha, mock_send_confirm
     ):
         MockAPICreateData.setUpServices()
@@ -261,7 +260,7 @@ def test_submit_and_register_anon_via_api_with_klant(
         config.save()
 
         data = MockAPICreateData()
-        data.install_mocks_anon_with_klant(m)
+        data.install_mocks_anon(m)
 
         subject = ContactFormSubjectFactory(
             config=config,
@@ -289,114 +288,18 @@ def test_submit_and_register_anon_via_api_with_klant(
 
         self.assertEqual(len(mail.outbox), 0)
 
-        for m in data.matchers:
-            self.assertTrue(m.called_once, str(m))
-
-        klant_create_data = data.matchers[0].request_history[0].json()
-        self.assertEqual(
-            klant_create_data,
-            {
-                "bronorganisatie": "123456789",
-                "voornaam": "Foo",
-                "voorvoegselAchternaam": "de",
-                "achternaam": "Bar",
-                "emailadres": "foo@example.com",
-                "telefoonnummer": "+31612345678",
-            },
-        )
-        contactmoment_create_data = data.matchers[1].request_history[0].json()
-        self.assertEqual(
-            contactmoment_create_data,
-            {
-                "medewerkerIdentificatie": {"identificatie": "FooVonBar"},
-                "bronorganisatie": "123456789",
-                "tekst": "hey!\n\nwaddup?",
-                "type": "Melding",
-                "kanaal": "contactformulier",
-                "onderwerp": "afdeling-xyz",
-                "contactgegevens": {
-                    "emailadres": "foo@example.com",
-                    "telefoonnummer": "+31612345678",
-                },
-            },
-        )
-        kcm_create_data = data.matchers[2].request_history[0].json()
-        self.assertEqual(
-            kcm_create_data,
-            {
-                "contactmoment": "https://contactmomenten.nl/api/v1/contactmoment/aaaaaaaa-aaaa-aaaa-aaaa-bbbbbbbbbbbb",
-                "klant": "https://klanten.nl/api/v1/klant/aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa",
-                "rol": KlantContactRol.BELANGHEBBENDE,
-            },
-        )
-        self.assertTimelineLog("created klant for anonymous user")
-        self.assertTimelineLog("registered contactmoment by API")
-
-        mock_send_confirm.assert_called_once_with("foo@example.com", subject.subject)
-
-    def test_submit_and_register_anon_via_api_without_klant(
-        self, m, mock_captcha, mock_send_confirm
-    ):
-        MockAPICreateData.setUpServices()
-
-        config = OpenKlantConfig.get_solo()
-        config.register_contact_moment = True
-        config.register_bronorganisatie_rsin = "123456789"
-        config.register_type = "Melding"
-        config.register_channel = "contactformulier"
-        config.register_employee_id = "FooVonBar"
-        config.save()
-
-        data = MockAPICreateData()
-        data.install_mocks_anon_without_klant(m)
-
-        subject = ContactFormSubjectFactory(
-            config=config,
-            subject="Aanvraag document",
-            subject_code="afdeling-xyz",
-        )
-
-        response = self.app.get(self.url)
-        form = response.forms["contactmoment-form"]
-        form["subject"].select(text=subject.subject)
-        form["first_name"] = "Foo"
-        form["infix"] = "de"
-        form["last_name"] = "Bar"
-        form["email"] = "foo@example.com"
-        form["phonenumber"] = "+31612345678"
-        form["question"] = "hey!\n\nwaddup?"
-        form["captcha"] = 42
-
-        response = form.submit().follow()
-
-        msgs = list(response.context["messages"])
-        self.assertEqual(len(msgs), 1)
-        self.assertEqual(str(msgs[0]), _("Vraag verstuurd!"))
-        self.assertEqual(msgs[0].level, messages.SUCCESS)
-
-        self.assertEqual(len(mail.outbox), 0)
-
-        for m in data.matchers:
-            self.assertTrue(m.called_once, str(m))
-
-        contactmoment_create_data = data.matchers[1].request_history[0].json()
-
-        text = inspect.cleandoc(
-            """
-        hey!
-
-        waddup?
-
-        Naam: Foo de Bar
-        """
-        )
+        # check that contactmomenten API but not klanten API is hit for anon user
+        self.assertTrue(data.matchers[0].called_once, str(m))
+        self.assertFalse(data.matchers[1].called_once, str(m))
+        self.assertFalse(data.matchers[2].called_once, str(m))
 
+        contactmoment_create_data = data.matchers[0].request_history[0].json()
         self.assertEqual(
             contactmoment_create_data,
             {
                 "medewerkerIdentificatie": {"identificatie": "FooVonBar"},
                 "bronorganisatie": "123456789",
-                "tekst": text,
+                "tekst": "hey!\n\nwaddup?\n\nNaam: Foo de Bar",
                 "type": "Melding",
                 "kanaal": "contactformulier",
                 "onderwerp": "afdeling-xyz",
@@ -406,14 +309,11 @@ def test_submit_and_register_anon_via_api_without_klant(
                 },
             },
         )
-        self.assertTimelineLog(
-            "could not retrieve or create klant for user, appended info to message"
-        )
         self.assertTimelineLog("registered contactmoment by API")
         mock_send_confirm.assert_called_once_with("foo@example.com", subject.subject)
 
     @patch("open_inwoner.openklant.forms.generate_question_answer_pair")
-    def test_submit_and_register_anon_via_api_without_klant_does_not_send_empty_email_or_telephone(
+    def test_register_contactmoment_for_anon_user_via_api_does_not_send_empty_email_or_telephone(
         self, m, mock_captcha2, mock_captcha, mock_send_confirm
     ):
         # we need to patch the captcha Q&A twice because they are re-generated by the form
@@ -474,7 +374,7 @@ def test_submit_and_register_anon_via_api_without_klant_does_not_send_empty_emai
                 else:
                     self.assertNotIn("telefoonnummer", contactgegevens.keys())
 
-    def test_register_bsn_user_via_api_without_id(
+    def test_register_contactmoment_for_bsn_user_via_api(
         self, m, mock_captcha, mock_send_confirm
     ):
         MockAPICreateData.setUpServices()
@@ -483,8 +383,7 @@ def test_register_bsn_user_via_api_without_id(
         config.register_contact_moment = True
         config.register_bronorganisatie_rsin = "123456789"
         config.register_type = "Melding"
-        # empty id should be excluded from contactmoment_create_data
-        config.register_employee_id = ""
+        config.register_employee_id = "FooVonBar"
         config.save()
 
         data = MockAPICreateData()
@@ -511,23 +410,47 @@ def test_register_bsn_user_via_api_without_id(
             ),
         )
         form["subject"].select(text=subject.subject)
-        form["question"] = "Lorem ipsum?"
+        form["question"] = "hey!\n\nwaddup?"
 
         response = form.submit().follow()
 
+        msgs = list(response.context["messages"])
+        self.assertEqual(len(msgs), 1)
+        self.assertEqual(str(msgs[0]), _("Vraag verstuurd!"))
+        self.assertEqual(msgs[0].level, messages.SUCCESS)
+
+        self.assertEqual(len(mail.outbox), 0)
+
+        for m in data.matchers:
+            self.assertTrue(m.called_once, str(m._url))
+
         contactmoment_create_data = data.matchers[1].request_history[0].json()
         self.assertEqual(
             contactmoment_create_data,
             {
+                "medewerkerIdentificatie": {"identificatie": "FooVonBar"},
                 "bronorganisatie": "123456789",
-                "tekst": "Lorem ipsum?",
+                "tekst": "hey!\n\nwaddup?",
                 "type": "Melding",
                 "kanaal": "contactformulier",
                 "onderwerp": "afdeling-xyz",
             },
         )
+        kcm_create_data = data.matchers[2].request_history[0].json()
+        self.assertEqual(
+            kcm_create_data,
+            {
+                "contactmoment": "https://contactmomenten.nl/api/v1/contactmoment/aaaaaaaa-aaaa-aaaa-aaaa-bbbbbbbbbbbb",
+                "klant": "https://klanten.nl/api/v1/klant/aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa",
+                "rol": KlantContactRol.BELANGHEBBENDE,
+            },
+        )
+
+        self.assertTimelineLog("retrieved klant for BSN or KVK user")
+        self.assertTimelineLog("registered contactmoment by API")
+        mock_send_confirm.assert_called_once_with("foo@example.com", subject.subject)
 
-    def test_submit_and_register_bsn_user_via_api(
+    def test_register_contactmoment_for_bsn_user_via_api_without_id(
         self, m, mock_captcha, mock_send_confirm
     ):
         MockAPICreateData.setUpServices()
@@ -536,7 +459,8 @@ def test_submit_and_register_bsn_user_via_api(
         config.register_contact_moment = True
         config.register_bronorganisatie_rsin = "123456789"
         config.register_type = "Melding"
-        config.register_employee_id = "FooVonBar"
+        # empty id should be excluded from contactmoment_create_data
+        config.register_employee_id = ""
         config.save()
 
         data = MockAPICreateData()
@@ -563,47 +487,23 @@ def test_submit_and_register_bsn_user_via_api(
             ),
         )
         form["subject"].select(text=subject.subject)
-        form["question"] = "hey!\n\nwaddup?"
+        form["question"] = "Lorem ipsum?"
 
         response = form.submit().follow()
 
-        msgs = list(response.context["messages"])
-        self.assertEqual(len(msgs), 1)
-        self.assertEqual(str(msgs[0]), _("Vraag verstuurd!"))
-        self.assertEqual(msgs[0].level, messages.SUCCESS)
-
-        self.assertEqual(len(mail.outbox), 0)
-
-        for m in data.matchers:
-            self.assertTrue(m.called_once, str(m._url))
-
         contactmoment_create_data = data.matchers[1].request_history[0].json()
         self.assertEqual(
             contactmoment_create_data,
             {
-                "medewerkerIdentificatie": {"identificatie": "FooVonBar"},
                 "bronorganisatie": "123456789",
-                "tekst": "hey!\n\nwaddup?",
+                "tekst": "Lorem ipsum?",
                 "type": "Melding",
                 "kanaal": "contactformulier",
                 "onderwerp": "afdeling-xyz",
             },
         )
-        kcm_create_data = data.matchers[2].request_history[0].json()
-        self.assertEqual(
-            kcm_create_data,
-            {
-                "contactmoment": "https://contactmomenten.nl/api/v1/contactmoment/aaaaaaaa-aaaa-aaaa-aaaa-bbbbbbbbbbbb",
-                "klant": "https://klanten.nl/api/v1/klant/aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa",
-                "rol": KlantContactRol.BELANGHEBBENDE,
-            },
-        )
-
-        self.assertTimelineLog("retrieved klant for BSN or KVK user")
-        self.assertTimelineLog("registered contactmoment by API")
-        mock_send_confirm.assert_called_once_with("foo@example.com", subject.subject)
 
-    def test_submit_and_register_kvk_or_rsin_user_via_api(
+    def test_register_contactmoment_for_kvk_or_rsin_user_via_api(
         self, _m, mock_captcha, mock_send_confirm
     ):
         MockAPICreateData.setUpServices()
@@ -708,7 +608,7 @@ def test_submit_and_register_kvk_or_rsin_user_via_api(
                     )
                     mock_send_confirm.reset_mock()
 
-    def test_submit_and_register_bsn_user_via_api_and_update_klant(
+    def test_register_contactmoment_for_bsn_user_via_api_and_update_klant(
         self, m, mock_captcha, mock_send_confirm
     ):
         MockAPICreateData.setUpServices()
@@ -793,7 +693,7 @@ def test_submit_and_register_bsn_user_via_api_and_update_klant(
         mock_send_confirm.reset_mock()
 
     @patch("open_inwoner.openklant.forms.generate_question_answer_pair")
-    def test_submit_and_register_kvk_or_rsin_user_via_api_and_update_klant(
+    def test_register_contactmoment_for_kvk_or_rsin_user_via_api_and_update_klant(
         self, m, mock_captcha2, mock_captcha, mock_send_confirm
     ):
         self.maxDiff = None
@@ -917,7 +817,7 @@ def test_send_email_confirmation_is_configurable(
         config.save()
 
         data = MockAPICreateData()
-        data.install_mocks_anon_with_klant(m)
+        data.install_mocks_anon(m)
 
         subject = ContactFormSubjectFactory(
             config=config,