From 09dc738c9e0e69aa692d7914f0bbb5b5a57b8c36 Mon Sep 17 00:00:00 2001 From: Paul Schilling Date: Mon, 16 Dec 2024 16:09:16 +0100 Subject: [PATCH] [#2940] Add OpenKlant2 configuration model --- src/open_inwoner/accounts/signals.py | 9 +- .../accounts/tests/test_profile_views.py | 4 - src/open_inwoner/accounts/views/signals.py | 47 +++++----- .../cms/cases/tests/test_contactform.py | 2 +- src/open_inwoner/cms/cases/views/status.py | 14 +-- .../migrations/0015_openklant2config.py | 75 ++++++++++++++++ src/open_inwoner/openklant/models.py | 85 ++++++++++--------- src/open_inwoner/openklant/services.py | 17 ++-- src/open_inwoner/openklant/tests/data.py | 1 + src/open_inwoner/openklant/tests/factories.py | 18 ++++ .../tests/test_openklant2_service.py | 37 ++------ .../openklant/tests/test_views.py | 2 - src/openklant2/_resources/actor.py | 2 +- src/openklant2/_resources/betrokkene.py | 2 +- src/openklant2/_resources/digitaal_adres.py | 2 +- src/openklant2/_resources/interne_taak.py | 2 +- src/openklant2/_resources/klant_contact.py | 2 +- src/openklant2/_resources/onderwerp_object.py | 2 +- src/openklant2/_resources/partij.py | 2 +- .../_resources/partij_identificator.py | 2 +- 20 files changed, 191 insertions(+), 136 deletions(-) create mode 100644 src/open_inwoner/openklant/migrations/0015_openklant2config.py diff --git a/src/open_inwoner/accounts/signals.py b/src/open_inwoner/accounts/signals.py index ad82f577a9..19579fb100 100644 --- a/src/open_inwoner/accounts/signals.py +++ b/src/open_inwoner/accounts/signals.py @@ -1,7 +1,6 @@ import logging from django.contrib.auth.signals import user_logged_in, user_logged_out -from django.core.exceptions import ImproperlyConfigured from django.dispatch import receiver from django.urls import reverse from django.utils.translation import gettext as _ @@ -39,17 +38,15 @@ def update_user_from_klant_on_login(sender, user, request, *args, **kwargs): # OpenKlant2 try: service = OpenKlant2Service() - except ImproperlyConfigured: - logger.error("OpenKlant2 configuration missing") + except Exception: + logger.error("OpenKlant2 service failed to build") else: _update_user_from_openklant2(user=user, service=service, request=request) # eSuite try: service = eSuiteKlantenService() - except ImproperlyConfigured: - logger.error("eSuiteKlantenService missing configuration") - except RuntimeError: + except Exception: logger.error("eSuiteKlantenService failed to build") else: _update_user_from_esuite(user=user, service=service, request=request) diff --git a/src/open_inwoner/accounts/tests/test_profile_views.py b/src/open_inwoner/accounts/tests/test_profile_views.py index a82f85ed87..cc80ac8fae 100644 --- a/src/open_inwoner/accounts/tests/test_profile_views.py +++ b/src/open_inwoner/accounts/tests/test_profile_views.py @@ -4,7 +4,6 @@ from django import forms from django.conf import settings -from django.contrib.auth import signals from django.template.defaultfilters import date as django_date from django.test import TestCase, override_settings, tag from django.urls import reverse, reverse_lazy @@ -16,7 +15,6 @@ from webtest import Upload from open_inwoner.accounts.choices import NotificationChannelChoice, StatusChoices -from open_inwoner.accounts.signals import update_user_from_klant_on_login from open_inwoner.cms.profile.cms_appconfig import ProfileConfig from open_inwoner.configurations.models import SiteConfiguration from open_inwoner.haalcentraal.tests.mixins import HaalCentraalMixin @@ -318,8 +316,6 @@ def test_messages_enabled_disabled(self): ) class EditProfileTests(AssertTimelineLogMixin, WebTest): def setUp(self): - signals.user_logged_in.disconnect(receiver=update_user_from_klant_on_login) - self.url = reverse("profile:edit") self.return_url = reverse("profile:detail") self.user = UserFactory() diff --git a/src/open_inwoner/accounts/views/signals.py b/src/open_inwoner/accounts/views/signals.py index 0381d09d8b..d225358c45 100644 --- a/src/open_inwoner/accounts/views/signals.py +++ b/src/open_inwoner/accounts/views/signals.py @@ -1,11 +1,9 @@ import logging -from django.conf import settings from django.db.models.signals import post_save from django.dispatch import receiver from open_inwoner.accounts.models import User -from open_inwoner.openklant.models import OpenKlant2Config from open_inwoner.openklant.services import OpenKlant2Service, eSuiteKlantenService logger = logging.getLogger(__name__) @@ -23,37 +21,34 @@ def get_or_create_klant_for_new_user( user = instance # OpenKlant2 - # TODO: replace with proper config and refactor branching - use_ok2 = getattr(settings, "OPENKLANT2_CONFIG", None) - if use_ok2 and (openklant2_config := OpenKlant2Config.from_django_settings()): - try: - service = OpenKlant2Service(config=openklant2_config) - except RuntimeError: - logger.error("OpenKlant2 service failed to build") - return - - if not ( - fetch_params := service.get_fetch_parameters( - user=user, use_vestigingsnummer=True - ) - ): - return + try: + service = OpenKlant2Service() + except Exception: + logger.error("OpenKlant2 service failed to build") + return - partij, created = service.get_or_create_partij_for_user( - fetch_params=fetch_params, user=user + if not ( + fetch_params := service.get_fetch_parameters( + user=user, use_vestigingsnummer=True ) - if not partij: - logger.error("Failed to create partij for new user %s", user) - return - elif not created: - service.update_user_from_partij(partij_uuid=partij["uuid"], user=user) + ): + return + + partij, created = service.get_or_create_partij_for_user( + fetch_params=fetch_params, user=user + ) + if not partij: + logger.error("Failed to create partij for new user %s", user) + return + elif not created: + service.update_user_from_partij(partij_uuid=partij["uuid"], user=user) - logger.info("Created partij %s for new user %s", partij, user) + logger.info("Created partij %s for new user %s", partij, user) # eSuite try: service = eSuiteKlantenService() - except RuntimeError: + except Exception: logger.error("eSuiteKlantenService failed to build") return diff --git a/src/open_inwoner/cms/cases/tests/test_contactform.py b/src/open_inwoner/cms/cases/tests/test_contactform.py index ef045769a5..70fa83394f 100644 --- a/src/open_inwoner/cms/cases/tests/test_contactform.py +++ b/src/open_inwoner/cms/cases/tests/test_contactform.py @@ -62,7 +62,7 @@ class CasesContactFormTestCase(AssertMockMatchersMixin, ClearCachesMixin, WebTes def setUp(self): super().setUp() - self.user = DigidUserFactory(bsn="900222086") + self.user = DigidUserFactory(bsn="900222086", email="foo@example.com") # services self.api_group = ZGWApiGroupConfigFactory( diff --git a/src/open_inwoner/cms/cases/views/status.py b/src/open_inwoner/cms/cases/views/status.py index 40963c6034..62748ae345 100644 --- a/src/open_inwoner/cms/cases/views/status.py +++ b/src/open_inwoner/cms/cases/views/status.py @@ -7,11 +7,7 @@ from django.conf import settings from django.contrib import messages -from django.core.exceptions import ( - ImproperlyConfigured, - ObjectDoesNotExist, - PermissionDenied, -) +from django.core.exceptions import ObjectDoesNotExist, PermissionDenied from django.http import ( Http404, HttpRequest, @@ -130,14 +126,12 @@ def get_service(self, service_type: KlantenServiceType) -> VragenService | None: if service_type == KlantenServiceType.OPENKLANT2: try: return OpenKlant2Service() - except ImproperlyConfigured: - logger.error("OpenKlant2 configuration missing") + except Exception: + logger.error("Failed to build OpenKlant2 service") if service_type == KlantenServiceType.ESUITE: try: return eSuiteVragenService() - except ImproperlyConfigured: - logger.error("eSuiteVragenService configuration missing") - except RuntimeError: + except Exception: logger.error("Failed to build eSuiteVragenService") def store_statustype_mapping(self, zaaktype_identificatie): diff --git a/src/open_inwoner/openklant/migrations/0015_openklant2config.py b/src/open_inwoner/openklant/migrations/0015_openklant2config.py new file mode 100644 index 0000000000..7c83d120d9 --- /dev/null +++ b/src/open_inwoner/openklant/migrations/0015_openklant2config.py @@ -0,0 +1,75 @@ +# Generated by Django 4.2.16 on 2024-12-18 13:21 + +import django.db.models.deletion +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ("zgw_consumers", "0022_set_default_service_slug"), + ("openklant", "0014_contactformconfig"), + ] + + operations = [ + migrations.CreateModel( + name="OpenKlant2Config", + fields=[ + ( + "id", + models.AutoField( + auto_created=True, + primary_key=True, + serialize=False, + verbose_name="ID", + ), + ), + ( + "mijn_vragen_kanaal", + models.CharField( + blank=True, default="", verbose_name="Mijn vragen kanaal" + ), + ), + ( + "mijn_vragen_organisatie_naam", + models.CharField( + blank=True, + default="", + verbose_name="Mijn vragen organisatie naam", + ), + ), + ( + "mijn_vragen_actor", + models.CharField( + blank=True, default="", verbose_name="Mijn vragen actor" + ), + ), + ( + "interne_taak_gevraagde_handeling", + models.CharField( + blank=True, + default="", + verbose_name="Interne taak gevraagde handeling", + ), + ), + ( + "interne_taak_toelichting", + models.CharField( + blank=True, default="", verbose_name="Interne taak toelichting" + ), + ), + ( + "service", + models.OneToOneField( + on_delete=django.db.models.deletion.PROTECT, + related_name="+", + to="zgw_consumers.service", + verbose_name="Klanten API", + ), + ), + ], + options={ + "verbose_name": "OpenKlant2 configuration", + }, + ), + ] diff --git a/src/open_inwoner/openklant/models.py b/src/open_inwoner/openklant/models.py index c1d0029910..2f20e55502 100644 --- a/src/open_inwoner/openklant/models.py +++ b/src/open_inwoner/openklant/models.py @@ -1,9 +1,3 @@ -import uuid -from dataclasses import dataclass -from typing import Self -from urllib.parse import urljoin, urlparse, urlunparse - -from django.core.exceptions import ImproperlyConfigured from django.db import models from django.utils.translation import gettext_lazy as _ @@ -138,6 +132,51 @@ def has_api_configuration(self): return all(getattr(self, f, "") for f in self.register_api_required_fields) +class OpenKlant2ConfigManager(models.Manager): + def get_queryset(self): + qs = super().get_queryset() + return qs.select_related("service") + + +class OpenKlant2Config(models.Model): + service = models.OneToOneField( + "zgw_consumers.Service", + verbose_name=_("Klanten API"), + on_delete=models.PROTECT, + related_name="+", + ) + + # Vragen + mijn_vragen_kanaal = models.CharField( + verbose_name=_("Mijn vragen kanaal"), + default="", + blank=True, + ) + mijn_vragen_organisatie_naam = models.CharField( + verbose_name=_("Mijn vragen organisatie naam"), + default="", + blank=True, + ) + mijn_vragen_actor = models.CharField( + verbose_name=_("Mijn vragen actor"), + default="", + blank=True, + ) + interne_taak_gevraagde_handeling = models.CharField( + verbose_name=_("Interne taak gevraagde handeling"), + default="", + blank=True, + ) + interne_taak_toelichting = models.CharField( + verbose_name=_("Interne taak toelichting"), + default="", + blank=True, + ) + + class Meta: + verbose_name = _("OpenKlant2 configuration") + + class ContactFormSubject(OrderedModel): subject = models.CharField( verbose_name=_("Onderwerp"), @@ -190,37 +229,3 @@ class Meta: verbose_name = _("KlantContactMoment") verbose_name_plural = _("KlantContactMomenten") unique_together = [["user", "contactmoment_url"]] - - -@dataclass -class OpenKlant2Config: - api_root: str - api_path: str - api_token: str - - # Question/Answer settings - mijn_vragen_kanaal: str - mijn_vragen_organisatie_naam: str - mijn_vragen_actor: str | uuid.UUID | None - interne_taak_gevraagde_handeling: str - interne_taak_toelichting: str - - @property - def api_url(self): - joined = urljoin(self.api_root, self.api_path) - scheme, netloc, path, params, query, fragment = urlparse(joined) - path = path.replace("//", "/") - return urlunparse((scheme, netloc, path, params, query, fragment)) - - @classmethod - def from_django_settings(cls) -> Self: - from django.conf import settings - - if not (config := getattr(settings, "OPENKLANT2_CONFIG", None)): - raise ImproperlyConfigured( - "Please set OPENKLANT2_CONFIG in your settings to configure OpenKlant2" - ) - - return cls(**config) - - # TODO: add from_openklant_config_model or similar diff --git a/src/open_inwoner/openklant/services.py b/src/open_inwoner/openklant/services.py index d2dae1caa8..bdd198b1c7 100644 --- a/src/open_inwoner/openklant/services.py +++ b/src/open_inwoner/openklant/services.py @@ -823,16 +823,17 @@ class OpenKlant2Service(KlantenService): client: OpenKlant2Client def __init__(self, config: OpenKlant2Config | None = None): - self.config = config or OpenKlant2Config.from_django_settings() - if not self.config: - raise ImproperlyConfigured( - "Please set OPENKLANT2_CONFIG in your settings to configure OpenKlant2" - ) + try: + self.config = config or OpenKlant2Config.objects.get() + except OpenKlant2Config.DoesNotExist: + raise ImproperlyConfigured("OpenKlant2Config not configured") + except OpenKlant2Config.MultipleObjectsReturned: + raise ImproperlyConfigured("Found multiple instances of OpenKlant2Config") self.client = OpenKlant2Client( - base_url=self.config.api_url, + base_url=self.config.service.api_root, request_kwargs={ - "headers": {"Authorization": f"Token {self.config.api_token}"} + "headers": {"Authorization": f"Token {self.config.service.secret}"} }, ) @@ -1133,7 +1134,7 @@ def create_question( if len(question.rstrip()) == 0: raise ValueError("You must provide a question") - if self.config.mijn_vragen_actor is None: + if not self.config.mijn_vragen_actor: raise RuntimeError( "You must define an actor to whom the question will be assigned. " "Initialize the service with a value for `mijn_vragen_actor`." diff --git a/src/open_inwoner/openklant/tests/data.py b/src/open_inwoner/openklant/tests/data.py index e010786559..6fad7f6308 100644 --- a/src/open_inwoner/openklant/tests/data.py +++ b/src/open_inwoner/openklant/tests/data.py @@ -16,6 +16,7 @@ from open_inwoner.utils.test import paginated_response KLANTEN_ROOT = "https://klanten.nl/api/v1/" +OPENKLANT2_ROOT = "http://localhost:8338/klantinteracties/api/v1" CONTACTMOMENTEN_ROOT = "https://contactmomenten.nl/api/v1/" diff --git a/src/open_inwoner/openklant/tests/factories.py b/src/open_inwoner/openklant/tests/factories.py index ba28e76782..e17d8f2380 100644 --- a/src/open_inwoner/openklant/tests/factories.py +++ b/src/open_inwoner/openklant/tests/factories.py @@ -1,12 +1,16 @@ import datetime import factory +from zgw_consumers.constants import APITypes from open_inwoner.accounts.tests.factories import UserFactory from open_inwoner.openklant.constants import KlantenServiceType from open_inwoner.openklant.services import Question, QuestionValidator +from open_inwoner.openzaak.tests.factories import ServiceFactory from open_inwoner.utils.url import uuid_from_url +from .data import OPENKLANT2_ROOT + class ContactFormSubjectFactory(factory.django.DjangoModelFactory): class Meta: @@ -24,6 +28,20 @@ class Meta: user = factory.SubFactory(UserFactory) +class OpenKlant2ConfigFactory(factory.django.DjangoModelFactory): + service = factory.SubFactory( + ServiceFactory, api_root=OPENKLANT2_ROOT, api_type=APITypes.kc + ) + mijn_vragen_kanaal = "oip_mijn_vragen" + mijn_vragen_organisatie_naam = "Open Inwoner Platform" + mijn_vragen_actor = "ca0783a1-1d74-4e07-b3e0-185b1d2fccd4" + interne_taak_gevraagde_handeling = "Beantwoorden vraag Mijn Omgeving" + interne_taak_toelichting = "Beantwoorden vraag" + + class Meta: + model = "openklant.OpenKlant2Config" + + def make_question_from_contactmoment( contact_moment_data: dict, new_answer_available: bool = False, diff --git a/src/open_inwoner/openklant/tests/test_openklant2_service.py b/src/open_inwoner/openklant/tests/test_openklant2_service.py index f9b8177a1e..8e5a4dbbaa 100644 --- a/src/open_inwoner/openklant/tests/test_openklant2_service.py +++ b/src/open_inwoner/openklant/tests/test_openklant2_service.py @@ -6,8 +6,8 @@ from open_inwoner.accounts.models import User from open_inwoner.accounts.tests.factories import UserFactory -from open_inwoner.openklant.models import OpenKlant2Config from open_inwoner.openklant.services import OpenKlant2Question, OpenKlant2Service +from open_inwoner.openklant.tests.factories import OpenKlant2ConfigFactory from open_inwoner.openklant.tests.helpers import Openklant2ServiceTestCase from openklant2.factories.partij import CreatePartijPersoonDataFactory from openklant2.types.resources.partij import PartijValidator @@ -17,16 +17,8 @@ class PartijGetOrCreateTestCase(Openklant2ServiceTestCase): def setUp(self): super().setUp() - self.openklant2_config = OpenKlant2Config( - api_root=self.openklant2_api_root, - api_path=self.openklant2_api_path, - api_token=self.openklant2_api_token, - mijn_vragen_kanaal="oip_mijn_vragen", - mijn_vragen_organisatie_naam="Open Inwoner Platform", - mijn_vragen_actor="ca0783a1-1d74-4e07-b3e0-185b1d2fccd4", - interne_taak_gevraagde_handeling="Beantwoorden vraag Mijn Omgeving", - interne_taak_toelichting="Beantwoorden vraag", - ) + + self.openklant2_config = OpenKlant2ConfigFactory() self.service = OpenKlant2Service(config=self.openklant2_config) self.user = UserFactory(first_name="John", last_name="Doe") @@ -220,16 +212,8 @@ def test_get_or_create_organisatie_with_vestiging(self): class Openklant2ServiceTest(Openklant2ServiceTestCase): def setUp(self): super().setUp() - self.openklant2_config = OpenKlant2Config( - api_root=self.openklant2_api_root, - api_path=self.openklant2_api_path, - api_token=self.openklant2_api_token, - mijn_vragen_kanaal="oip_mijn_vragen", - mijn_vragen_organisatie_naam="Open Inwoner Platform", - mijn_vragen_actor="ca0783a1-1d74-4e07-b3e0-185b1d2fccd4", - interne_taak_gevraagde_handeling="Beantwoorden vraag Mijn Omgeving", - interne_taak_toelichting="Beantwoorden vraag", - ) + + self.openklant2_config = OpenKlant2ConfigFactory() self.service = OpenKlant2Service(config=self.openklant2_config) self.persoon = self.openklant_client.partij.create_persoon( @@ -360,16 +344,7 @@ def setUp(self): } ) - self.openklant2_config = OpenKlant2Config( - api_root=self.openklant2_api_root, - api_path=self.openklant2_api_path, - api_token=self.openklant2_api_token, - mijn_vragen_kanaal="oip_mijn_vragen", - mijn_vragen_organisatie_naam="Open Inwoner Platform", - mijn_vragen_actor=self.designated_actor["uuid"], - interne_taak_gevraagde_handeling="Beantwoorden vraag Mijn Omgeving", - interne_taak_toelichting="Beantwoorden vraag", - ) + self.openklant2_config = OpenKlant2ConfigFactory() self.service = OpenKlant2Service(config=self.openklant2_config) def test_designated_actor_is_required_to_create_question(self): diff --git a/src/open_inwoner/openklant/tests/test_views.py b/src/open_inwoner/openklant/tests/test_views.py index 0094deb7cf..fac5534f64 100644 --- a/src/open_inwoner/openklant/tests/test_views.py +++ b/src/open_inwoner/openklant/tests/test_views.py @@ -54,8 +54,6 @@ class ContactMomentViewsTestCase( maxDiff = None def setUp(self): - # signals.user_logged_in.disconnect(receiver=update_user_from_klant_on_login) - self.user = DigidUserFactory( email="test@example.com", phonenumber="0100000000", diff --git a/src/openklant2/_resources/actor.py b/src/openklant2/_resources/actor.py index f48de99bfa..33b897ca94 100644 --- a/src/openklant2/_resources/actor.py +++ b/src/openklant2/_resources/actor.py @@ -10,7 +10,7 @@ class ActorResource(ResourceMixin): http_client: APIClient - base_path: str = "/actoren" + base_path: str = "actoren" def create( self, diff --git a/src/openklant2/_resources/betrokkene.py b/src/openklant2/_resources/betrokkene.py index bb1f7f066d..0dc97feea3 100644 --- a/src/openklant2/_resources/betrokkene.py +++ b/src/openklant2/_resources/betrokkene.py @@ -10,7 +10,7 @@ class BetrokkeneResource(ResourceMixin): http_client: APIClient - base_path: str = "/betrokkenen" + base_path: str = "betrokkenen" def __init__(self, *args, **kwargs): super().__init__(*args, **kwargs) diff --git a/src/openklant2/_resources/digitaal_adres.py b/src/openklant2/_resources/digitaal_adres.py index 5bd78d0d71..ba87ce4f59 100644 --- a/src/openklant2/_resources/digitaal_adres.py +++ b/src/openklant2/_resources/digitaal_adres.py @@ -14,7 +14,7 @@ class DigitaalAdresResource(ResourceMixin): http_client: APIClient - base_path: str = "/digitaleadressen" + base_path: str = "digitaleadressen" def list( self, *, params: ListDigitaalAdresParams | None = None diff --git a/src/openklant2/_resources/interne_taak.py b/src/openklant2/_resources/interne_taak.py index efa92a274a..5c605a617c 100644 --- a/src/openklant2/_resources/interne_taak.py +++ b/src/openklant2/_resources/interne_taak.py @@ -10,7 +10,7 @@ class InterneTaakResource(ResourceMixin): http_client: APIClient - base_path: str = "/internetaken" + base_path: str = "internetaken" def __init__(self, *args, **kwargs): super().__init__(*args, **kwargs) diff --git a/src/openklant2/_resources/klant_contact.py b/src/openklant2/_resources/klant_contact.py index cc99e93ffa..cb2a61fe12 100644 --- a/src/openklant2/_resources/klant_contact.py +++ b/src/openklant2/_resources/klant_contact.py @@ -15,7 +15,7 @@ class KlantContactResource(ResourceMixin): http_client: APIClient - base_path: str = "/klantcontacten" + base_path: str = "klantcontacten" def __init__(self, *args, **kwargs): super().__init__(*args, **kwargs) diff --git a/src/openklant2/_resources/onderwerp_object.py b/src/openklant2/_resources/onderwerp_object.py index 4299db360b..e7ae95296d 100644 --- a/src/openklant2/_resources/onderwerp_object.py +++ b/src/openklant2/_resources/onderwerp_object.py @@ -14,7 +14,7 @@ class OnderwerpObjectResource(ResourceMixin): http_client: APIClient - base_path: str = "/onderwerpobjecten" + base_path: str = "onderwerpobjecten" def create( self, diff --git a/src/openklant2/_resources/partij.py b/src/openklant2/_resources/partij.py index c97cbe74e8..4016635211 100644 --- a/src/openklant2/_resources/partij.py +++ b/src/openklant2/_resources/partij.py @@ -17,7 +17,7 @@ class PartijResource(ResourceMixin): http_client: APIClient - base_path: str = "/partijen" + base_path: str = "partijen" def list( self, *, params: PartijListParams | None = None diff --git a/src/openklant2/_resources/partij_identificator.py b/src/openklant2/_resources/partij_identificator.py index 39f095b9b6..58d77e3755 100644 --- a/src/openklant2/_resources/partij_identificator.py +++ b/src/openklant2/_resources/partij_identificator.py @@ -14,7 +14,7 @@ class PartijIdentificatorResource(ResourceMixin): http_client: APIClient - base_path: str = "/partij-identificatoren" + base_path: str = "partij-identificatoren" def list( self, *, params: ListPartijIdentificatorenParams | None = None