diff --git a/src/open_inwoner/openklant/forms.py b/src/open_inwoner/openklant/forms.py index b643959513..b69e90d8fe 100644 --- a/src/open_inwoner/openklant/forms.py +++ b/src/open_inwoner/openklant/forms.py @@ -2,6 +2,7 @@ from django.forms import Form from django.utils.translation import gettext_lazy as _ +from open_inwoner.accounts.models import User from open_inwoner.openklant.models import ContactFormSubject, OpenKlantConfig from open_inwoner.utils.validators import validate_phone_number @@ -44,21 +45,47 @@ class ContactForm(Form): required=True, ) - def __init__(self, *args, **kwargs): + user: User + + def __init__(self, user, *args, **kwargs): super().__init__(*args, **kwargs) + self.user = user config = OpenKlantConfig.get_solo() self.fields["subject"].queryset = config.contactformsubject_set.order_by( "subject" ) + if self.user.is_authenticated: + del self.fields["first_name"] + del self.fields["last_name"] + del self.fields["infix"] + if self.user.email: + del self.fields["email"] + if self.user.phonenumber: + del self.fields["phonenumber"] + def clean(self, *args, **kwargs): cleaned_data = super().clean(*args, **kwargs) - email = cleaned_data["email"] - phonenumber = cleaned_data["phonenumber"] + email = cleaned_data.get("email", "") + phonenumber = cleaned_data.get("phonenumber", "") - if not email and not phonenumber: + if ("email" in self.fields and not email) and ( + "phonenumber" in self.fields and not phonenumber + ): msg = _("Vul een e-mailadres of telefoonnummer in.") self.add_error("email", msg) self.add_error("phonenumber", msg) + + if self.user.is_authenticated: + if not email and self.user.email: + cleaned_data["email"] = self.user.email + if not phonenumber and self.user.phonenumber: + cleaned_data["phonenumber"] = self.user.phonenumber + + cleaned_data["first_name"] = self.user.first_name + cleaned_data["infix"] = self.user.infix + cleaned_data["last_name"] = self.user.last_name + + return cleaned_data diff --git a/src/open_inwoner/openklant/models.py b/src/open_inwoner/openklant/models.py index 7961e9bfa0..391c539b5d 100644 --- a/src/open_inwoner/openklant/models.py +++ b/src/open_inwoner/openklant/models.py @@ -5,6 +5,12 @@ from zgw_consumers.constants import APITypes +class OpenKlantConfigManager(models.Manager): + def get_queryset(self): + qs = super().get_queryset() + return qs.select_related("klanten_service", "contactmomenten_service") + + class OpenKlantConfig(SingletonModel): """ Global configuration and defaults for Klant & Contactmomenten APIs @@ -67,6 +73,8 @@ class OpenKlantConfig(SingletonModel): "register_employee_id", ) + objects = OpenKlantConfigManager() + class Meta: verbose_name = _("Open Klant configuration") diff --git a/src/open_inwoner/openklant/tests/data.py b/src/open_inwoner/openklant/tests/data.py index 16792f733b..445159db11 100644 --- a/src/open_inwoner/openklant/tests/data.py +++ b/src/open_inwoner/openklant/tests/data.py @@ -100,6 +100,17 @@ def __init__(self): emailadres="foo@example.com", telefoonnummer="0612345678", ) + self.klant_no_contact_info = generate_oas_component( + "kc", + "schemas/Klant", + uuid="aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa", + url=f"{KLANTEN_ROOT}klant/aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa", + bronorganisatie="123456789", + voornaam="Foo", + achternaam="Bar", + emailadres="", + telefoonnummer="", + ) self.contactmoment = generate_oas_component( "cmc", "schemas/ContactMoment", @@ -123,7 +134,7 @@ def __init__(self): self.matchers = [] - def install_mocks_anon(self, m) -> "MockAPICreateData": + def install_mocks_anon_with_klant(self, m) -> "MockAPICreateData": self.setUpOASMocks(m) self.matchers = [ @@ -141,6 +152,19 @@ def install_mocks_anon(self, m) -> "MockAPICreateData": ] return self + def install_mocks_anon_without_klant(self, m) -> "MockAPICreateData": + self.setUpOASMocks(m) + + self.matchers = [ + m.post(f"{KLANTEN_ROOT}klanten", json=self.klant, status_code=500), + m.post( + f"{CONTACTMOMENTEN_ROOT}contactmomenten", + json=self.contactmoment, + status_code=201, + ), + ] + return self + def install_mocks_digid(self, m) -> "MockAPICreateData": self.setUpOASMocks(m) @@ -161,3 +185,28 @@ def install_mocks_digid(self, m) -> "MockAPICreateData": ), ] return self + + def install_mocks_digid_missing_contact_info(self, m) -> "MockAPICreateData": + self.setUpOASMocks(m) + self.matchers = [ + m.get( + f"{KLANTEN_ROOT}klanten?subjectNatuurlijkPersoon__inpBsn={self.user.bsn}", + json=paginated_response([self.klant_no_contact_info]), + ), + m.patch( + f"{KLANTEN_ROOT}klant/aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa", + json=self.klant, + status_code=200, + ), + m.post( + f"{CONTACTMOMENTEN_ROOT}contactmomenten", + json=self.contactmoment, + status_code=201, + ), + m.post( + f"{CONTACTMOMENTEN_ROOT}klantcontactmomenten", + json=self.klant_contactmoment, + status_code=201, + ), + ] + return self diff --git a/src/open_inwoner/openklant/tests/test_contactform.py b/src/open_inwoner/openklant/tests/test_contactform.py index cefb15d77e..4fb040d8e3 100644 --- a/src/open_inwoner/openklant/tests/test_contactform.py +++ b/src/open_inwoner/openklant/tests/test_contactform.py @@ -1,3 +1,5 @@ +import inspect + from django.contrib import messages from django.core import mail from django.urls import reverse @@ -6,16 +8,23 @@ import requests_mock from django_webtest import WebTest -from open_inwoner.accounts.tests.factories import DigidUserFactory -from open_inwoner.openklant.models import ContactFormSubject, OpenKlantConfig +from open_inwoner.accounts.tests.factories import UserFactory +from open_inwoner.openklant.models import OpenKlantConfig from open_inwoner.openklant.tests.data import MockAPICreateData from open_inwoner.openklant.tests.factories import ContactFormSubjectFactory from open_inwoner.openzaak.tests.factories import ServiceFactory from open_inwoner.utils.test import ClearCachesMixin, DisableRequestLogMixin +from open_inwoner.utils.tests.helpers import AssertFormMixin, AssertTimelineLogMixin @requests_mock.Mocker() -class ContactFormTestCase(ClearCachesMixin, DisableRequestLogMixin, WebTest): +class ContactFormTestCase( + ClearCachesMixin, + AssertTimelineLogMixin, + AssertFormMixin, + DisableRequestLogMixin, + WebTest, +): @classmethod def setUpTestData(cls): super().setUpTestData() @@ -68,7 +77,7 @@ def test_no_form_shown_if_not_has_configuration(self, m): self.assertContains(response, _("Contact formulier niet geconfigureerd.")) self.assertEqual(0, len(response.pyquery("#contactmoment-form"))) - def test_form_requires_either_email_or_phonenumber(self, m): + def test_anon_form_requires_either_email_or_phonenumber(self, m): config = OpenKlantConfig.get_solo() config.register_email = "example@example.com" config.save() @@ -76,6 +85,18 @@ def test_form_requires_either_email_or_phonenumber(self, m): response = self.app.get(self.url) form = response.forms["contactmoment-form"] + self.assertFormExactFields( + form, + ( + "subject", + "first_name", + "infix", + "last_name", + "email", + "phonenumber", + "question", + ), + ) form["subject"].select(text=subject.subject) form["first_name"] = "Foo" form["last_name"] = "Bar" @@ -88,6 +109,28 @@ def test_form_requires_either_email_or_phonenumber(self, m): response.context["errors"], [_("Vul een e-mailadres of telefoonnummer in.")] ) + def test_regular_auth_form_fills_email_and_phonenumber(self, m): + config = OpenKlantConfig.get_solo() + config.register_email = "example@example.com" + config.save() + subject = ContactFormSubjectFactory(config=config) + + user = UserFactory() + + response = self.app.get(self.url, user=user) + form = response.forms["contactmoment-form"] + self.assertFormExactFields( + form, + ( + "subject", + "question", + ), + ) + form["subject"].select(text=subject.subject) + form["question"] = "hey!\n\nwaddup?" + + response = form.submit(status=302) + def test_submit_and_register_via_email(self, m): config = OpenKlantConfig.get_solo() config.register_email = "example@example.com" @@ -123,7 +166,9 @@ def test_submit_and_register_via_email(self, m): self.assertIn("0612345678", email.body) self.assertIn("hey!\n\nwaddup?", email.body) - def test_submit_and_register_anon_via_api(self, m): + self.assertTimelineLog("registered contactmoment by email") + + def test_submit_and_register_anon_via_api_with_klant(self, m): MockAPICreateData.setUpServices() config = OpenKlantConfig.get_solo() @@ -134,9 +179,9 @@ def test_submit_and_register_anon_via_api(self, m): config.save() data = MockAPICreateData() - data.install_mocks_anon(m) + data.install_mocks_anon_with_klant(m) - subject = ContactFormSubjectFactory(config=config, subject="Vraag") + subject = ContactFormSubjectFactory(config=config, subject="Aanvraag document") response = self.app.get(self.url) form = response.forms["contactmoment-form"] @@ -178,7 +223,7 @@ def test_submit_and_register_anon_via_api(self, m): { "medewerkerIdentificatie": {"identificatie": "FooVonBar"}, "bronorganisatie": "123456789", - "tekst": f"Vraag\n\nhey!\n\nwaddup?", + "tekst": f"Aanvraag document\n\nhey!\n\nwaddup?", "type": "Melding", "kanaal": "Internet", }, @@ -191,8 +236,10 @@ def test_submit_and_register_anon_via_api(self, m): "klant": "https://klanten.nl/api/v1/klant/aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa", }, ) + self.assertTimelineLog("created klant for anonymous user") + self.assertTimelineLog("registered contactmoment by API") - def test_submit_and_register_bsn_user_via_api(self, m): + def test_submit_and_register_anon_via_api_without_klant(self, m): MockAPICreateData.setUpServices() config = OpenKlantConfig.get_solo() @@ -203,11 +250,11 @@ def test_submit_and_register_bsn_user_via_api(self, m): config.save() data = MockAPICreateData() - data.install_mocks_digid(m) + data.install_mocks_anon_without_klant(m) - subject = ContactFormSubjectFactory(config=config, subject="Vraag") + subject = ContactFormSubjectFactory(config=config, subject="Aanvraag document") - response = self.app.get(self.url, user=data.user) + response = self.app.get(self.url) form = response.forms["contactmoment-form"] form["subject"].select(text=subject.subject) form["first_name"] = "Foo" @@ -226,6 +273,76 @@ def test_submit_and_register_bsn_user_via_api(self, m): 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( + """ + Aanvraag document + + hey! + + waddup? + + Naam: Foo de Bar + Email: foo@example.com + Telefoonnummer: 0612345678 + """ + ) + + self.assertEqual( + contactmoment_create_data, + { + "medewerkerIdentificatie": {"identificatie": "FooVonBar"}, + "bronorganisatie": "123456789", + "tekst": text, + "type": "Melding", + "kanaal": "Internet", + }, + ) + self.assertTimelineLog( + "could not retrieve or create klant for user, appended info to message" + ) + self.assertTimelineLog("registered contactmoment by API") + + def test_submit_and_register_bsn_user_via_api(self, m): + MockAPICreateData.setUpServices() + + config = OpenKlantConfig.get_solo() + config.register_contact_moment = True + config.register_bronorganisatie_rsin = "123456789" + config.register_type = "Melding" + config.register_employee_id = "FooVonBar" + config.save() + + data = MockAPICreateData() + data.install_mocks_digid(m) + + subject = ContactFormSubjectFactory(config=config, subject="Aanvraag document") + + response = self.app.get(self.url, user=data.user) + form = response.forms["contactmoment-form"] + self.assertFormExactFields( + form, + ( + "subject", + "question", + ), + ) + form["subject"].select(text=subject.subject) + 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)) @@ -235,7 +352,7 @@ def test_submit_and_register_bsn_user_via_api(self, m): { "medewerkerIdentificatie": {"identificatie": "FooVonBar"}, "bronorganisatie": "123456789", - "tekst": f"Vraag\n\nhey!\n\nwaddup?", + "tekst": f"Aanvraag document\n\nhey!\n\nwaddup?", "type": "Melding", "kanaal": "Internet", }, @@ -248,3 +365,74 @@ def test_submit_and_register_bsn_user_via_api(self, m): "klant": "https://klanten.nl/api/v1/klant/aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa", }, ) + + self.assertTimelineLog("retrieved klant for BSN-user") + self.assertTimelineLog("registered contactmoment by API") + + def test_submit_and_register_bsn_user_via_api_and_update_klant(self, m): + MockAPICreateData.setUpServices() + + config = OpenKlantConfig.get_solo() + config.register_contact_moment = True + config.register_bronorganisatie_rsin = "123456789" + config.register_type = "Melding" + config.register_employee_id = "FooVonBar" + config.save() + + data = MockAPICreateData() + data.install_mocks_digid_missing_contact_info(m) + + subject = ContactFormSubjectFactory(config=config, subject="Aanvraag document") + + response = self.app.get(self.url, user=data.user) + form = response.forms["contactmoment-form"] + self.assertFormExactFields( + form, + ( + "subject", + "question", + ), + ) + form["subject"].select(text=subject.subject) + form["question"] = "hey!\n\nwaddup?" + + form.submit().follow() + # response tested in other cases + + for m in data.matchers: + self.assertTrue(m.called_once, str(m)) + + klant_patch_data = data.matchers[1].request_history[0].json() + self.assertEqual( + klant_patch_data, + { + "emailadres": data.user.email, + "telefoonnummer": data.user.phonenumber, + }, + ) + + contactmoment_create_data = data.matchers[2].request_history[0].json() + self.assertEqual( + contactmoment_create_data, + { + "medewerkerIdentificatie": {"identificatie": "FooVonBar"}, + "bronorganisatie": "123456789", + "tekst": f"Aanvraag document\n\nhey!\n\nwaddup?", + "type": "Melding", + "kanaal": "Internet", + }, + ) + kcm_create_data = data.matchers[3].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", + }, + ) + + self.assertTimelineLog("retrieved klant for BSN-user") + self.assertTimelineLog( + "patched klant from user with missing fields: emailadres, telefoonnummer" + ) + self.assertTimelineLog("registered contactmoment by API") diff --git a/src/open_inwoner/openklant/views/contactform.py b/src/open_inwoner/openklant/views/contactform.py index 44e95d8143..e4c080b20e 100644 --- a/src/open_inwoner/openklant/views/contactform.py +++ b/src/open_inwoner/openklant/views/contactform.py @@ -1,6 +1,3 @@ -import re -from typing import Tuple - from django.contrib import messages from django.utils.functional import cached_property from django.utils.translation import gettext_lazy as _ @@ -8,6 +5,7 @@ from mail_editor.helpers import find_template from view_breadcrumbs import BaseBreadcrumbMixin +from zds_client import ClientError from open_inwoner.openklant.forms import ContactForm from open_inwoner.openklant.models import OpenKlantConfig @@ -15,11 +13,12 @@ create_contactmoment, create_klant, fetch_klant_for_bsn, + patch_klant, ) -from open_inwoner.utils.views import CommonPageMixin +from open_inwoner.utils.views import CommonPageMixin, LogMixin -class ContactFormView(CommonPageMixin, BaseBreadcrumbMixin, FormView): +class ContactFormView(CommonPageMixin, LogMixin, BaseBreadcrumbMixin, FormView): form_class = ContactForm template_name = "pages/contactform/form.html" @@ -36,6 +35,11 @@ def page_title(self): def get_success_url(self): return self.request.path + def get_form_kwargs(self): + kwargs = super().get_form_kwargs() + kwargs["user"] = self.request.user + return kwargs + def get_initial(self): initial = super().get_initial() if self.request.user.is_authenticated: @@ -84,20 +88,35 @@ def register_by_email(self, form, recipient_email): if success: messages.add_message(self.request, messages.SUCCESS, self.message_success) + self.log_system_action(f"registered contactmoment by email") else: messages.add_message(self.request, messages.ERROR, self.message_failure) + self.log_system_action(f"error while registering contactmoment by email") def register_by_api(self, form, config: OpenKlantConfig): - assert config.has_form_configuration() + assert config.has_api_configuration() # fetch/update/create klant if self.request.user.is_authenticated and self.request.user.bsn: klant = fetch_klant_for_bsn(self.request.user.bsn) + if klant: + self.log_system_action("retrieved klant for BSN-user") + + # check if we have some data missing from the Klant + update_data = {} + if not klant.emailadres and form.cleaned_data["email"]: + update_data["emailadres"] = form.cleaned_data["email"] + if not klant.telefoonnummer and form.cleaned_data["phonenumber"]: + update_data["telefoonnummer"] = form.cleaned_data["phonenumber"] + if update_data: + patch_klant(klant, update_data) + self.log_system_action( + f"patched klant from user with missing fields: {', '.join(sorted(update_data.keys()))}" + ) + else: + self.log_system_action(f"could not retrieve klant for BSN-user") - # TODO update klant phone/email? (other ticket) else: - # TODO according to taiga #1437 we should disable Klanten/Contacten API if Digid is not enabled - # TODO registering klanten won't work in e-Suite as it always pulls from BRP data = { "bronorganisatie": config.register_bronorganisatie_rsin, "voornaam": form.cleaned_data["first_name"], @@ -106,15 +125,40 @@ def register_by_api(self, form, config: OpenKlantConfig): "emailadres": form.cleaned_data["email"], "telefoonnummer": form.cleaned_data["phonenumber"], } + # registering klanten won't work in e-Suite as it always pulls from BRP (but try anyway and fallback to appending details to tekst if fails) klant = create_klant(data) + if klant: + if self.request.user.is_authenticated: + self.log_system_action( + f"created klant for basic authenticated user" + ) + else: + self.log_system_action(f"created klant for anonymous user") # create contact moment subject = form.cleaned_data["subject"].subject - body = form.cleaned_data["question"] + question = form.cleaned_data["question"] + text = f"{subject}\n\n{question}" + + if not klant: + # if we don't have a BSN and can't create a Klant we'll add contact info to the tekst + parts = [form.cleaned_data[k] for k in ("first_name", "infix", "last_name")] + full_name = " ".join(p for p in parts if p) + text = f"{text}\n\nNaam: {full_name}" + + if form.cleaned_data["email"]: + text = f"{text}\nEmail: {form.cleaned_data['email']}" + + if form.cleaned_data["phonenumber"]: + text = f"{text}\nTelefoonnummer: {form.cleaned_data['phonenumber']}" + + self.log_system_action( + f"could not retrieve or create klant for user, appended info to message" + ) data = { "bronorganisatie": config.register_bronorganisatie_rsin, - "tekst": f"{subject}\n\n{body}", + "tekst": text, "type": config.register_type, "kanaal": "Internet", "medewerkerIdentificatie": { @@ -125,5 +169,7 @@ def register_by_api(self, form, config: OpenKlantConfig): if contactmoment: messages.add_message(self.request, messages.SUCCESS, self.message_success) + self.log_system_action(f"registered contactmoment by API") else: messages.add_message(self.request, messages.ERROR, self.message_failure) + self.log_system_action(f"error while registering contactmoment by API") diff --git a/src/open_inwoner/openklant/wrap.py b/src/open_inwoner/openklant/wrap.py index 066e3cd0b6..96bb01606e 100644 --- a/src/open_inwoner/openklant/wrap.py +++ b/src/open_inwoner/openklant/wrap.py @@ -145,7 +145,7 @@ def _fetch_contactmoment(url, *, client=None) -> Optional[ContactMoment]: return contact_moment -def create_klant(data: KlantCreateData): +def create_klant(data: KlantCreateData) -> Optional[Klant]: client = build_client("klanten") if client is None: return @@ -155,6 +155,26 @@ def create_klant(data: KlantCreateData): except (RequestException, ClientError) as e: logger.exception("exception while making request", exc_info=e) return + except ValueError as e: + # raised when 'Operation klant_create not found + # TODO make this optional? + return + + klant = factory(Klant, response) + + return klant + + +def patch_klant(klant: Klant, update_data) -> Optional[Klant]: + client = build_client("klanten") + if client is None: + return + + try: + response = client.partial_update("klant", url=klant.url, data=update_data) + except (RequestException, ClientError) as e: + logger.exception("exception while making request", exc_info=e) + return klant = factory(Klant, response) diff --git a/src/open_inwoner/utils/tests/helpers.py b/src/open_inwoner/utils/tests/helpers.py index 97bee1b8f9..957666b6d2 100644 --- a/src/open_inwoner/utils/tests/helpers.py +++ b/src/open_inwoner/utils/tests/helpers.py @@ -58,7 +58,11 @@ def getTimelineLogDump(self) -> str: ret = [] qs = TimelineLog.objects.all() c = qs.count() - ret.append(f"total {c} timelinelogs") + if c: + ret.append(f"total {c} timelinelogs:") + else: + ret.append(f"total {c} timelinelogs") + for log in qs: message = log.extra_data.get("message") log_level = log.extra_data.get("log_level") @@ -66,7 +70,7 @@ def getTimelineLogDump(self) -> str: log_level = logging.getLevelName(log_level) else: log_level = "NO_LEVEL" - msg = f"{log_level}: {message}" + msg = f" {log_level}: {message}" ret.append(msg) return "\n".join(ret) @@ -74,6 +78,31 @@ def dumpTimelineLog(self): print(self.getTimelineLogDump()) +class AssertFormMixin: + def assertFormExactFields( + self, form, field_names, *, with_csrf=True, drop_no_name=True + ): + """ + check if a form has expected fields, usually from WebTest + + - with_csrf: auto-adds adds the csrfmiddlewaretoken + - drop_no_name: ignore the empty field created by the submit button + """ + expect_names = set(field_names) + if with_csrf: + expect_names.add("csrfmiddlewaretoken") + + have_names = set(form.fields.keys()) + if drop_no_name: + have_names.discard("") + + self.assertEqual( + have_names, + expect_names, + "-> first set is actual in form, second set is expected", + ) + + class AssertRedirectsMixin: def assertRedirectsLogin( self, response, *, next: str = None, with_host: bool = False