From a2d28f5953c44f31dfcb98ae55d8d6cf99b59535 Mon Sep 17 00:00:00 2001 From: Paul Schilling Date: Mon, 26 Aug 2024 14:36:44 +0200 Subject: [PATCH] [#2700] Support multiple ZGW backends for zaak info object notifications --- src/open_inwoner/cms/cases/views/status.py | 6 +- src/open_inwoner/openzaak/documents.py | 9 +- src/open_inwoner/openzaak/notifications.py | 42 ++++--- .../openzaak/tests/test_notification_data.py | 16 +-- .../test_notification_zaak_infoobject.py | 107 +++++++++++++----- 5 files changed, 115 insertions(+), 65 deletions(-) diff --git a/src/open_inwoner/cms/cases/views/status.py b/src/open_inwoner/cms/cases/views/status.py index e4fc771d8f..0af77c1feb 100644 --- a/src/open_inwoner/cms/cases/views/status.py +++ b/src/open_inwoner/cms/cases/views/status.py @@ -38,7 +38,7 @@ from open_inwoner.openzaak.api_models import Status, StatusType, Zaak from open_inwoner.openzaak.clients import CatalogiClient, ZakenClient from open_inwoner.openzaak.documents import ( - fetch_single_information_object_url, + fetch_single_information_object_from_url, fetch_single_information_object_uuid, ) from open_inwoner.openzaak.models import ( @@ -630,7 +630,9 @@ def get_case_document_files( # [case_info.informatieobject for case_info in case_info_objects], # ) info_objects = [ - fetch_single_information_object_url(case_info.informatieobject) + fetch_single_information_object_from_url( + case_info.informatieobject, api_group=api_group + ) for case_info in case_info_objects ] diff --git a/src/open_inwoner/openzaak/documents.py b/src/open_inwoner/openzaak/documents.py index b54b89a13b..191f08ebe5 100644 --- a/src/open_inwoner/openzaak/documents.py +++ b/src/open_inwoner/openzaak/documents.py @@ -3,7 +3,7 @@ from django.conf import settings from open_inwoner.openzaak.api_models import InformatieObject -from open_inwoner.openzaak.clients import DocumentenClient, build_documenten_client +from open_inwoner.openzaak.clients import DocumentenClient from ..utils.decorators import cache as cache_result @@ -11,9 +11,10 @@ @cache_result("information_object_url:{url}", timeout=settings.CACHE_ZGW_ZAKEN_TIMEOUT) -def fetch_single_information_object_url(url: str) -> InformatieObject | None: - if client := build_documenten_client(): - return client._fetch_single_information_object(url=url) +def fetch_single_information_object_from_url( + url: str, api_group +) -> InformatieObject | None: + return api_group.documenten_client._fetch_single_information_object(url=url) # not cached because currently only used in info-object download view diff --git a/src/open_inwoner/openzaak/notifications.py b/src/open_inwoner/openzaak/notifications.py index 2da19d69ca..2a4bcca152 100644 --- a/src/open_inwoner/openzaak/notifications.py +++ b/src/open_inwoner/openzaak/notifications.py @@ -18,12 +18,8 @@ ZaakType, ) from open_inwoner.openzaak.cases import resolve_status -from open_inwoner.openzaak.clients import ( - CatalogiClient, - ZakenClient, - build_zaken_client, -) -from open_inwoner.openzaak.documents import fetch_single_information_object_url +from open_inwoner.openzaak.clients import CatalogiClient, ZakenClient +from open_inwoner.openzaak.documents import fetch_single_information_object_from_url from open_inwoner.openzaak.models import ( OpenZaakConfig, UserCaseInfoObjectNotification, @@ -128,7 +124,9 @@ def handle_zaken_notification(notification: Notification): if notification.resource == "status": _handle_status_notification(notification, case, inform_users, api_group) elif notification.resource == "zaakinformatieobject": - _handle_zaakinformatieobject_notification(notification, case, inform_users) + _handle_zaakinformatieobject_notification( + notification, case, inform_users, api_group + ) else: raise NotImplementedError("programmer error in earlier resource filter") @@ -192,23 +190,18 @@ def _wrap_join(iter, glue="") -> str: # Helper functions for ZaakInformatieObject notifications # def _handle_zaakinformatieobject_notification( - notification: Notification, case: Zaak, inform_users + notification: Notification, + case: Zaak, + inform_users: list[User], + api_group: ZGWApiGroupConfig, ): - oz_config = OpenZaakConfig.get_solo() + oz_config = api_group.open_zaak_config r = notification.resource # short alias for logging - client = build_zaken_client() - if not client: - log_system_action( - f"ignored {r} notification: cannot build Zaken API client for case {case.url}", - log_level=logging.ERROR, - ) - return - # check if this is a zaakinformatieobject we want to inform on ziobj_url = notification.resource_url - ziobj = client.fetch_single_case_information_object(ziobj_url) + ziobj = api_group.zaken_client.fetch_single_case_information_object(ziobj_url) if not ziobj: log_system_action( @@ -217,7 +210,9 @@ def _handle_zaakinformatieobject_notification( ) return - info_object = fetch_single_information_object_url(ziobj.informatieobject) + info_object = fetch_single_information_object_from_url( + ziobj.informatieobject, api_group=api_group + ) if not info_object: log_system_action( f"ignored {r} notification: cannot retrieve informatieobject " @@ -262,11 +257,14 @@ def _handle_zaakinformatieobject_notification( log_level=logging.INFO, ) for user in inform_users: - _handle_zaakinformatieobject_update(user, case, ziobj) + _handle_zaakinformatieobject_update(user, case, ziobj, api_group) def _handle_zaakinformatieobject_update( - user: User, case: Zaak, zaak_info_object: ZaakInformatieObject + user: User, + case: Zaak, + zaak_info_object: ZaakInformatieObject, + api_group: ZGWApiGroupConfig, ): template_name = "case_document_notification" @@ -305,7 +303,7 @@ def _handle_zaakinformatieobject_update( ) return - send_case_update_email(user, case, template_name) + send_case_update_email(user, case, template_name, api_group=api_group) note.mark_sent() log_system_action( diff --git a/src/open_inwoner/openzaak/tests/test_notification_data.py b/src/open_inwoner/openzaak/tests/test_notification_data.py index 32653623aa..26ce3517bb 100644 --- a/src/open_inwoner/openzaak/tests/test_notification_data.py +++ b/src/open_inwoner/openzaak/tests/test_notification_data.py @@ -304,9 +304,9 @@ def __init__(self): self.zaak_type_alt = generate_oas_component_cached( "ztc", "schemas/ZaakType", - uuid="xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxxx", - url=f"{ANOTHER_CATALOGI_ROOT}zaaktype/xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxxx", - catalogus=f"{ANOTHER_CATALOGI_ROOT}catalogussen/xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxxx", + uuid="22fd7613-3c9d-4507-a0b1-32c503488988", + url=f"{ANOTHER_CATALOGI_ROOT}zaaktype/22fd7613-3c9d-4507-a0b1-32c503488988", + catalogus=f"{ANOTHER_CATALOGI_ROOT}catalogussen/22fd7613-3c9d-4507-a0b1-32c503488988", identificatie="My Zaaktype", indicatieInternOfExtern="extern", omschrijving="My Zaaktype omschrijving", @@ -336,10 +336,10 @@ def __init__(self): self.zaak_alt = generate_oas_component_cached( "zrc", "schemas/Zaak", - url=f"{ANOTHER_ZAKEN_ROOT}zaken/xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxxx", + url=f"{ANOTHER_ZAKEN_ROOT}zaken/22fd7613-3c9d-4507-a0b1-32c503488988", zaaktype=self.zaak_type_alt["url"], - status=f"{ANOTHER_ZAKEN_ROOT}statussen/xxxxxxxx-xxxx-xxxx-xxxx-222222222222", - resultaat=f"{ANOTHER_ZAKEN_ROOT}resultaten/xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxxx", + status=f"{ANOTHER_ZAKEN_ROOT}statussen/22fd7613-3c9d-4507-a0b1-32c503488988", + resultaat=f"{ANOTHER_ZAKEN_ROOT}resultaten/22fd7613-3c9d-4507-a0b1-32c503488988", vertrouwelijkheidaanduiding=VertrouwelijkheidsAanduidingen.openbaar, ) self.zaak2_alt = generate_oas_component_cached( @@ -354,14 +354,14 @@ def __init__(self): self.status_initial_alt = generate_oas_component_cached( "zrc", "schemas/Status", - url=f"{ANOTHER_ZAKEN_ROOT}statussen/xxxxxxxx-xxxx-xxxx-xxxx-111111111111", + url=f"{ANOTHER_ZAKEN_ROOT}statussen/22fd7613-3c9d-4507-a0b1-32c503488988", zaak=self.zaak_alt["url"], statustype=self.status_type_initial_alt["url"], ) self.status_final_alt = generate_oas_component_cached( "zrc", "schemas/Status", - url=f"{ANOTHER_ZAKEN_ROOT}statussen/xxxxxxxx-xxxx-xxxx-xxxx-222222222222", + url=f"{ANOTHER_ZAKEN_ROOT}statussen/22fd7613-3c9d-4507-a0b1-32c503488988", zaak=self.zaak_alt["url"], statustype=self.status_type_final_alt["url"], ) diff --git a/src/open_inwoner/openzaak/tests/test_notification_zaak_infoobject.py b/src/open_inwoner/openzaak/tests/test_notification_zaak_infoobject.py index beca52f907..70cad8ba4d 100644 --- a/src/open_inwoner/openzaak/tests/test_notification_zaak_infoobject.py +++ b/src/open_inwoner/openzaak/tests/test_notification_zaak_infoobject.py @@ -5,7 +5,6 @@ import requests_mock from freezegun import freeze_time -from notifications_api_common.models import NotificationsConfig from zgw_consumers.api_models.base import factory from zgw_consumers.api_models.constants import VertrouwelijkheidsAanduidingen @@ -24,7 +23,7 @@ from ..api_models import InformatieObject, Zaak, ZaakInformatieObject, ZaakType from ..models import OpenZaakConfig, UserCaseInfoObjectNotification from .helpers import copy_with_new_uuid -from .test_notification_data import MockAPIData +from .test_notification_data import MockAPIData, MockAPIDataAlt @requests_mock.Mocker() @@ -36,39 +35,66 @@ class ZaakInformatieObjectNotificationHandlerTestCase( AssertTimelineLogMixin, ClearCachesMixin, TestCase ): maxDiff = None - config: OpenZaakConfig - note_config: NotificationsConfig @classmethod def setUpTestData(cls): super().setUpTestData() MockAPIData.setUpServices() + MockAPIDataAlt.setUpServices() - def test_ziohandle_zaken_notification(self, m, mock_handle: Mock): + def test_handle_zaak_informatie_object_notifications(self, m, mock_handle: Mock): """ - happy-flow from valid data calls the (mocked) handle_zaakinformatieobject() + Happy flow (with multiple ZGW backends) for notifications about zaak informatie objecten """ data = MockAPIData().install_mocks(m) + data_alt = MockAPIDataAlt().install_mocks(m) ZaakTypeInformatieObjectTypeConfigFactory.from_case_type_info_object_dicts( - data.zaak_type, data.informatie_object, document_notification_enabled=True + data.zaak_type, + data.informatie_object, + document_notification_enabled=True, + ) + ZaakTypeInformatieObjectTypeConfigFactory.from_case_type_info_object_dicts( + data_alt.zaak_type_alt, + data_alt.informatie_object_alt, + document_notification_enabled=True, ) - handle_zaken_notification(data.zio_notification) - - mock_handle.assert_called_once() + for api_group, notification, zaak, zaak_info_obj, user_initiator in [ + ( + data.api_group, + data.zio_notification, + data.zaak, + data.zaak_informatie_object, + data.user_initiator, + ), + ( + data_alt.api_group_alt, + data_alt.zio_notification_alt, + data_alt.zaak_alt, + data_alt.zaak_informatie_object_alt, + data_alt.user_initiator_alt, + ), + ]: + with self.subTest(f"api_group {api_group.id}"): + handle_zaken_notification(notification) + + mock_handle.assert_called() - # check call arguments - args = mock_handle.call_args.args - self.assertEqual(args[0], data.user_initiator) - self.assertEqual(args[1].url, data.zaak["url"]) - self.assertEqual(args[2].url, data.zaak_informatie_object["url"]) - - self.assertTimelineLog( - "accepted zaakinformatieobject notification: attempt informing users ", - lookup=Lookups.startswith, - level=logging.INFO, + # check call arguments + args = mock_handle.call_args.args + self.assertEqual(args[0], user_initiator) + self.assertEqual(args[1].url, zaak["url"]) + self.assertEqual(args[2].url, zaak_info_obj["url"]) + + log_dump = self.getTimelineLogDump() + self.assertIn("total 2 timelinelogs", log_dump) + self.assertIn( + "accepted zaakinformatieobject notification: attempt informing users", + log_dump, ) + self.assertIn(data.user_initiator.email, log_dump) + self.assertIn(data_alt.user_initiator_alt.email, log_dump) def test_ziohandle_zaken_notification_niet_natuurlijk_persoon_initiator( self, m, mock_handle: Mock @@ -114,6 +140,16 @@ def test_ziohandle_zaken_notification_niet_natuurlijk_persoon_initiator( # start of generic checks + def test_no_api_group_found(self, m, mock_handle: Mock): + data = MockAPIData().install_mocks(m) + + # API group is resolved from zaak url == hoofd_object + data.zio_notification.hoofd_object = "http://www.bogus.com" + + handle_zaken_notification(data.zio_notification) + + mock_handle.assert_not_called() + def test_zio_bails_when_bad_notification_channel(self, m, mock_handle: Mock): notification = NotificationFactory(kanaal="not_zaken") with self.assertRaisesRegex( @@ -322,8 +358,6 @@ def test_zio_bails_when_zaak_type_info_object_type_config_is_found_not_marked_fo level=logging.INFO, ) - # TODO add some no-catalog variations - @override_settings(ZGW_LIMIT_NOTIFICATIONS_FREQUENCY=3600) @freeze_time("2023-01-01 01:00:00") @@ -332,6 +366,11 @@ class NotificationHandlerUserMessageTestCase(AssertTimelineLogMixin, TestCase): note these tests match with a similar test from `test_notification_zaak_status.py` """ + @classmethod + def setUpTestData(cls): + super().setUpTestData() + MockAPIData.setUpServices() + @patch( "open_inwoner.userfeed.hooks.case_document_added_notification_received", autospec=True, @@ -351,7 +390,7 @@ def test_handle_zaak_info_object_update_filters_disabled_notifications( zio = factory(ZaakInformatieObject, data.zaak_informatie_object) zio.informatieobject = factory(InformatieObject, data.informatie_object) - _handle_zaakinformatieobject_update(user, case, zio) + _handle_zaakinformatieobject_update(user, case, zio, api_group=data.api_group) # not called because disabled notifications mock_send.assert_not_called() @@ -384,7 +423,7 @@ def test_handle_zaak_info_object_update_filters_bad_email( zio = factory(ZaakInformatieObject, data.zaak_informatie_object) zio.informatieobject = factory(InformatieObject, data.informatie_object) - _handle_zaakinformatieobject_update(user, case, zio) + _handle_zaakinformatieobject_update(user, case, zio, api_group=data.api_group) # not called because bad email mock_send.assert_not_called() @@ -416,9 +455,15 @@ def test_handle_zaak_info_object_update( zio.informatieobject = factory(InformatieObject, data.informatie_object) # first call - _handle_zaakinformatieobject_update(user, case, zio) + _handle_zaakinformatieobject_update(user, case, zio, api_group=data.api_group) mock_send.assert_called_once() + mock_send.assert_called_with( + user, + case, + template_name="case_document_notification", + api_group=data.api_group, + ) # check if userfeed hook was called mock_feed_hook.assert_called_once() @@ -442,7 +487,7 @@ def test_handle_zaak_info_object_update( ) # second call with same case/status - _handle_zaakinformatieobject_update(user, case, zio) + _handle_zaakinformatieobject_update(user, case, zio, api_group=data.api_group) # no duplicate mail for this user/case/status mock_send.assert_not_called() @@ -455,7 +500,9 @@ def test_handle_zaak_info_object_update( # other user is fine other_user = UserFactory.create() - _handle_zaakinformatieobject_update(other_user, case, zio) + _handle_zaakinformatieobject_update( + other_user, case, zio, api_group=data.api_group + ) mock_send.assert_called_once() @@ -474,7 +521,7 @@ def test_handle_zaak_info_object_update( InformatieObject, copy_with_new_uuid(data.informatie_object) ) - _handle_zaakinformatieobject_update(user, case, zio) + _handle_zaakinformatieobject_update(user, case, zio, api_group=data.api_group) # not sent because we already send to this user within the frequency mock_send.assert_not_called() @@ -493,7 +540,9 @@ def test_handle_zaak_info_object_update( zio.informatieobject = factory( InformatieObject, copy_with_new_uuid(data.informatie_object) ) - _handle_zaakinformatieobject_update(user, case, zio) + _handle_zaakinformatieobject_update( + user, case, zio, api_group=data.api_group + ) # this one succeeds mock_send.assert_called_once()