From f1282ffddc7d226f88cec5d3fb0cb8eacdb562de Mon Sep 17 00:00:00 2001 From: Paul Schilling Date: Fri, 23 Aug 2024 10:15:50 +0200 Subject: [PATCH 1/2] [#2700] Support multiple ZGW backends for zaak status notifications --- src/open_inwoner/openzaak/notifications.py | 61 ++-- .../openzaak/tests/test_notification_data.py | 260 +++++++++++++++++- .../openzaak/tests/test_notification_utils.py | 10 +- .../tests/test_notification_zaak_status.py | 121 ++++++-- 4 files changed, 376 insertions(+), 76 deletions(-) diff --git a/src/open_inwoner/openzaak/notifications.py b/src/open_inwoner/openzaak/notifications.py index 428281a81d..2da19d69ca 100644 --- a/src/open_inwoner/openzaak/notifications.py +++ b/src/open_inwoner/openzaak/notifications.py @@ -21,7 +21,6 @@ from open_inwoner.openzaak.clients import ( CatalogiClient, ZakenClient, - build_catalogi_client, build_zaken_client, ) from open_inwoner.openzaak.documents import fetch_single_information_object_url @@ -65,14 +64,6 @@ def handle_zaken_notification(notification: Notification): resources = ("status", "zaakinformatieobject") 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 - if notification.resource not in resources: log_system_action( f"ignored {r} notification: resource is not " @@ -81,9 +72,16 @@ def handle_zaken_notification(notification: Notification): ) return + try: + api_group = ZGWApiGroupConfig.objects.resolve_group_from_hints(url=case_url) + except ZGWApiGroupConfig.DoesNotExist: + logger.error("No API group defined for case %s", case_url) + return + + zaken_client = api_group.zaken_client + # check if we have users that need to be informed about this case - roles = client.fetch_case_roles(case_url) - if not roles: + if not (roles := zaken_client.fetch_case_roles(case_url)): log_system_action( f"ignored {r} notification: cannot retrieve rollen for case {case_url}", # NOTE this used to be logging.ERROR, but as this is also our first call @@ -101,17 +99,14 @@ def handle_zaken_notification(notification: Notification): return # check if this case is visible - case = client.fetch_case_by_url_no_cache(case_url) - if not case: + if not (case := zaken_client.fetch_case_by_url_no_cache(case_url)): log_system_action( f"ignored {r} notification: cannot retrieve case {case_url}", log_level=logging.ERROR, ) return - case_type = None - if catalogi_client := build_catalogi_client(): - case_type = catalogi_client.fetch_single_case_type(case.zaaktype) + case_type = api_group.catalogi_client.fetch_single_case_type(case.zaaktype) if not case_type: log_system_action( @@ -131,7 +126,7 @@ def handle_zaken_notification(notification: Notification): return if notification.resource == "status": - _handle_status_notification(notification, case, inform_users) + _handle_status_notification(notification, case, inform_users, api_group) elif notification.resource == "zaakinformatieobject": _handle_zaakinformatieobject_notification(notification, case, inform_users) else: @@ -142,10 +137,10 @@ def send_case_update_email( user: User, case: Zaak, template_name: str, + api_group: ZGWApiGroupConfig, status: Status | None = None, extra_context: dict = None, ): - group = ZGWApiGroupConfig.objects.resolve_group_from_hints(url=case.url) """ send the actual mail @@ -153,7 +148,7 @@ def send_case_update_email( case_detail_url = build_absolute_url( reverse( "cases:case_detail", - kwargs={"object_id": str(case.uuid), "api_group_id": group.id}, + kwargs={"object_id": str(case.uuid), "api_group_id": api_group.id}, ) ) @@ -511,27 +506,14 @@ def _handle_status_notification( notification: Notification, case: Zaak, inform_users: list[User], + api_group: ZGWApiGroupConfig, ): """ Check status notification settings of user and case-related objects/configs """ - oz_config = OpenZaakConfig.get_solo() - - catalogi_client = build_catalogi_client() - if not catalogi_client: - log_system_action( - f"ignored {notification.resource} notification for {case.url}: cannot create Catalogi API client", - log_level=logging.ERROR, - ) - return None - - zaken_client = build_zaken_client() - if not zaken_client: - log_system_action( - f"ignored {notification.resource} notification for {case.url}: cannot create Zaken API client", - log_level=logging.ERROR, - ) - return + oz_config = api_group.open_zaak_config + catalogi_client = api_group.catalogi_client + zaken_client = api_group.zaken_client if not (status_history := _check_status_history(notification, case, zaken_client)): return @@ -568,7 +550,7 @@ def _handle_status_notification( log_level=logging.INFO, ) # TODO: replace with notify_about_status_update(...args, method: Callable) - _handle_status_update(user, case, status, status_type_config) + _handle_status_update(user, case, status, status_type_config, api_group) def _handle_status_update( @@ -576,6 +558,7 @@ def _handle_status_update( case: Zaak, status: Status, status_type_config: ZaakTypeStatusTypeConfig, + api_group: ZGWApiGroupConfig, ): # choose template if status_type_config.action_required: @@ -611,7 +594,9 @@ def _handle_status_update( ) return - send_case_update_email(user, case, template_name, status=status) + send_case_update_email( + user, case, template_name, api_group=api_group, status=status + ) 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 a147f77dcc..32653623aa 100644 --- a/src/open_inwoner/openzaak/tests/test_notification_data.py +++ b/src/open_inwoner/openzaak/tests/test_notification_data.py @@ -14,9 +14,15 @@ ) from open_inwoner.utils.test import paginated_response -from ..models import OpenZaakConfig from .helpers import generate_oas_component_cached -from .shared import CATALOGI_ROOT, DOCUMENTEN_ROOT, ZAKEN_ROOT +from .shared import ( + ANOTHER_CATALOGI_ROOT, + ANOTHER_DOCUMENTEN_ROOT, + ANOTHER_ZAKEN_ROOT, + CATALOGI_ROOT, + DOCUMENTEN_ROOT, + ZAKEN_ROOT, +) class MockAPIData: @@ -218,7 +224,7 @@ def __init__(self): hoofd_object=self.zaak2["url"], ) - def install_mocks(self, m, *, res404: list[str] | None = None) -> "MockAPIData": + def install_mocks(self, m, *, res404: list[str] | None = None): if res404 is None: res404 = [] for attr in res404: @@ -270,13 +276,251 @@ def install_mocks(self, m, *, res404: list[str] | None = None) -> "MockAPIData": @classmethod def setUpServices(cls): - # openzaak config - config = OpenZaakConfig.get_solo() - ZGWApiGroupConfigFactory( + cls.api_group = ZGWApiGroupConfigFactory( ztc_service__api_root=CATALOGI_ROOT, zrc_service__api_root=ZAKEN_ROOT, drc_service__api_root=DOCUMENTEN_ROOT, form_service=None, ) - config.zaak_max_confidentiality = VertrouwelijkheidsAanduidingen.openbaar - config.save() + + +class MockAPIDataAlt: + """ + Additional API data for testing multiple ZGW backends. + + The set of mocks is a replica of `MockAPIData` but with different API roots + """ + + def __init__(self): + self.user_initiator_alt = DigidUserFactory( + bsn="200000002", + email="initiator_alt@example.com", + ) + self.eherkenning_user_initiator_alt = eHerkenningUserFactory( + kvk="87654321", + rsin="000000000", + email="initiator_alt_kvk@example.com", + ) + 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", + identificatie="My Zaaktype", + indicatieInternOfExtern="extern", + omschrijving="My Zaaktype omschrijving", + ) + self.status_type_initial_alt = generate_oas_component_cached( + "ztc", + "schemas/StatusType", + url=f"{ANOTHER_CATALOGI_ROOT}statustypen/aaaaaaaa-aaaa-aaaa-aaaa-111111111111", + zaaktype=self.zaak_type_alt["url"], + informeren=True, + volgnummer=1, + omschrijving="initial", + statustekst="", + isEindStatus=False, + ) + self.status_type_final_alt = generate_oas_component_cached( + "ztc", + "schemas/StatusType", + url=f"{ANOTHER_CATALOGI_ROOT}statustypen/aaaaaaaa-aaaa-aaaa-aaaa-222222222222", + zaaktype=self.zaak_type_alt["url"], + informeren=True, + volgnummer=2, + omschrijving="final", + statustekst="status_type_final_statustekst", + isEindStatus=True, + ) + self.zaak_alt = generate_oas_component_cached( + "zrc", + "schemas/Zaak", + url=f"{ANOTHER_ZAKEN_ROOT}zaken/xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxxx", + 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", + vertrouwelijkheidaanduiding=VertrouwelijkheidsAanduidingen.openbaar, + ) + self.zaak2_alt = generate_oas_component_cached( + "zrc", + "schemas/Zaak", + url=f"{ANOTHER_ZAKEN_ROOT}zaken/bbbbbbbb-bbbb-bbbb-bbbb-bbbbbbbbbbbb", + zaaktype=self.zaak_type_alt["url"], + status=f"{ANOTHER_ZAKEN_ROOT}statussen/aaaaaaaa-aaaa-aaaa-aaaa-222222222222", + resultaat=f"{ANOTHER_ZAKEN_ROOT}resultaten/aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa", + vertrouwelijkheidaanduiding=VertrouwelijkheidsAanduidingen.openbaar, + ) + self.status_initial_alt = generate_oas_component_cached( + "zrc", + "schemas/Status", + url=f"{ANOTHER_ZAKEN_ROOT}statussen/xxxxxxxx-xxxx-xxxx-xxxx-111111111111", + 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", + zaak=self.zaak_alt["url"], + statustype=self.status_type_final_alt["url"], + ) + + self.informatie_object_alt = generate_oas_component_cached( + "drc", + "schemas/EnkelvoudigInformatieObject", + url=f"{ANOTHER_DOCUMENTEN_ROOT}enkelvoudiginformatieobjecten/aaaaaaaa-0001-bbbb-aaaa-aaaaaaaaaaaa", + informatieobjecttype=f"{ANOTHER_CATALOGI_ROOT}informatieobjecttypen/aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa", + status="definitief", + vertrouwelijkheidaanduiding=VertrouwelijkheidsAanduidingen.openbaar, + ) + self.zaak_informatie_object_alt = generate_oas_component_cached( + "zrc", + "schemas/ZaakInformatieObject", + url=f"{ANOTHER_ZAKEN_ROOT}zaakinformatieobjecten/aaaaaaaa-0001-aaaa-aaaa-aaaaaaaaaaaa", + informatieobject=self.informatie_object_alt["url"], + zaak=self.zaak_alt["url"], + ) + self.zaak_informatie_object2_alt = generate_oas_component_cached( + "zrc", + "schemas/ZaakInformatieObject", + url=f"{ANOTHER_ZAKEN_ROOT}zaakinformatieobjecten/aaaaaaaa-0002-aaaa-aaaa-aaaaaaaaaaaa", + informatieobject=self.informatie_object_alt["url"], + zaak=self.zaak2_alt["url"], + ) + self.informatie_object_extra_alt = generate_oas_component_cached( + "drc", + "schemas/EnkelvoudigInformatieObject", + url=f"{ANOTHER_DOCUMENTEN_ROOT}enkelvoudiginformatieobjecten/aaaaaaaa-0002-bbbb-aaaa-aaaaaaaaaaaa", + informatieobjecttype=f"{ANOTHER_CATALOGI_ROOT}informatieobjecttypen/aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa", + status="definitief", + vertrouwelijkheidaanduiding=VertrouwelijkheidsAanduidingen.openbaar, + ) + self.zaak_informatie_object_extra_alt = generate_oas_component_cached( + "zrc", + "schemas/ZaakInformatieObject", + url=f"{ANOTHER_ZAKEN_ROOT}zaakinformatieobjecten/aaaaaaaa-0003-aaaa-aaaa-aaaaaaaaaaaa", + informatieobject=self.informatie_object_extra_alt["url"], + zaak=self.zaak_alt["url"], + ) + + self.role_initiator_alt = generate_oas_component_cached( + "zrc", + "schemas/Rol", + url=f"{ANOTHER_ZAKEN_ROOT}rollen/aaaaaaaa-0001-aaaa-aaaa-aaaaaaaaaaaa", + omschrijvingGeneriek=RolOmschrijving.initiator, + betrokkeneType=RolTypes.natuurlijk_persoon, + betrokkeneIdentificatie={ + "inpBsn": self.user_initiator_alt.bsn, + }, + ) + self.eherkenning_role_initiator_alt = generate_oas_component_cached( + "zrc", + "schemas/Rol", + url=f"{ANOTHER_ZAKEN_ROOT}rollen/aaaaaaaa-0002-aaaa-aaaa-aaaaaaaaaaaa", + omschrijvingGeneriek=RolOmschrijving.initiator, + betrokkeneType=RolTypes.niet_natuurlijk_persoon, + betrokkeneIdentificatie={ + "innNnpId": self.eherkenning_user_initiator_alt.kvk, + }, + ) + self.eherkenning_role_initiator2_alt = generate_oas_component_cached( + "zrc", + "schemas/Rol", + url=f"{ANOTHER_ZAKEN_ROOT}rollen/aaaaaaaa-0003-aaaa-aaaa-aaaaaaaaaaaa", + omschrijvingGeneriek=RolOmschrijving.initiator, + betrokkeneType=RolTypes.niet_natuurlijk_persoon, + betrokkeneIdentificatie={ + "innNnpId": self.eherkenning_user_initiator_alt.rsin, + }, + ) + + self.case_roles_alt = [self.role_initiator_alt] + self.eherkenning_case_roles_alt = [ + self.eherkenning_role_initiator_alt, + self.eherkenning_role_initiator2_alt, + ] + self.status_history_alt = [self.status_initial_alt, self.status_final_alt] + + self.status_notification_alt = NotificationFactory( + resource="status", + actie="update", + resource_url=self.status_final_alt["url"], + hoofd_object=self.zaak_alt["url"], + ) + self.zio_notification_alt = NotificationFactory( + resource="zaakinformatieobject", + actie="create", + resource_url=self.zaak_informatie_object_alt["url"], + hoofd_object=self.zaak_alt["url"], + ) + self.zio_notification2_alt = NotificationFactory( + resource="zaakinformatieobject", + actie="create", + resource_url=self.zaak_informatie_object2_alt["url"], + hoofd_object=self.zaak2_alt["url"], + ) + + def install_mocks(self, m, *, res404: list[str] | None = None) -> "MockAPIData": + if res404 is None: + res404 = [] + for attr in res404: + if not hasattr(self, attr): + raise Exception("configuration error") + + if "case_roles" in res404: + m.get( + f"{ANOTHER_ZAKEN_ROOT}rollen?zaak={self.zaak_alt['url']}", + status_code=404, + ) + else: + m.get( + f"{ANOTHER_ZAKEN_ROOT}rollen?zaak={self.zaak_alt['url']}", + json=paginated_response(self.case_roles_alt), + ) + m.get( + f"{ANOTHER_ZAKEN_ROOT}rollen?zaak={self.zaak2_alt['url']}", + json=paginated_response(self.eherkenning_case_roles_alt), + ) + + if "status_history" in res404: + m.get( + f"{ANOTHER_ZAKEN_ROOT}statussen?zaak={self.zaak_alt['url']}", + status_code=404, + ) + else: + m.get( + f"{ANOTHER_ZAKEN_ROOT}statussen?zaak={self.zaak_alt['url']}", + json=paginated_response(self.status_history_alt), + ) + + for resource_attr in [ + "zaak_alt", + "zaak2_alt", + "zaak_type_alt", + "status_initial_alt", + "status_final_alt", + "status_type_initial_alt", + "status_type_final_alt", + "informatie_object_alt", + "zaak_informatie_object_alt", + "zaak_informatie_object2_alt", + "informatie_object_extra_alt", + "zaak_informatie_object_extra_alt", + ]: + resource = getattr(self, resource_attr) + if resource_attr in res404: + m.get(resource["url"], status_code=404) + else: + m.get(resource["url"], json=resource) + + return self + + @classmethod + def setUpServices(cls): + cls.api_group_alt = ZGWApiGroupConfigFactory( + ztc_service__api_root=ANOTHER_CATALOGI_ROOT, + zrc_service__api_root=ANOTHER_ZAKEN_ROOT, + drc_service__api_root=ANOTHER_DOCUMENTEN_ROOT, + form_service=None, + ) diff --git a/src/open_inwoner/openzaak/tests/test_notification_utils.py b/src/open_inwoner/openzaak/tests/test_notification_utils.py index cb577d8fd1..403a467364 100644 --- a/src/open_inwoner/openzaak/tests/test_notification_utils.py +++ b/src/open_inwoner/openzaak/tests/test_notification_utils.py @@ -57,7 +57,11 @@ def test_send_case_update_email(self): ) as format_identificatie: format_identificatie.return_value = ret_val send_case_update_email( - user, case, "case_status_notification", status=status + user, + case, + "case_status_notification", + status=status, + api_group=self.api_group, ) format_identificatie.assert_called_once() @@ -88,7 +92,9 @@ def test_send_case_update_email__no_status(self): kwargs={"object_id": str(case.uuid), "api_group_id": self.api_group.id}, ) - send_case_update_email(user, case, "case_document_notification") + send_case_update_email( + user, case, "case_document_notification", api_group=self.api_group + ) self.assertEqual(len(mail.outbox), 1) email = mail.outbox[0] diff --git a/src/open_inwoner/openzaak/tests/test_notification_zaak_status.py b/src/open_inwoner/openzaak/tests/test_notification_zaak_status.py index f974ddc4ea..1b29005d22 100644 --- a/src/open_inwoner/openzaak/tests/test_notification_zaak_status.py +++ b/src/open_inwoner/openzaak/tests/test_notification_zaak_status.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 @@ -27,7 +26,7 @@ ZaakTypeStatusTypeConfigFactory, ) from .helpers import copy_with_new_uuid -from .test_notification_data import MockAPIData +from .test_notification_data import MockAPIData, MockAPIDataAlt @requests_mock.Mocker() @@ -36,19 +35,19 @@ class StatusNotificationHandlerTestCase( AssertTimelineLogMixin, ClearCachesMixin, TestCase ): maxDiff = None - config: OpenZaakConfig - note_config: NotificationsConfig @classmethod def setUpTestData(cls): super().setUpTestData() MockAPIData.setUpServices() + MockAPIDataAlt.setUpServices() - def test_status_handle_zaken_notification(self, m, mock_handle: Mock): + def test_handle_zaak_status_notifications(self, m, mock_handle: Mock): """ - happy-flow from valid data calls the (mocked) _handle_status_update() + Happy flow (with multiple ZGW backends) for notifications about zaak status updates """ data = MockAPIData().install_mocks(m) + data_alt = MockAPIDataAlt().install_mocks(m) # Added for https://taiga.maykinmedia.nl/project/open-inwoner/task/1904 # In eSuite it is possible to reuse a StatusType for multiple ZaakTypen, which @@ -68,6 +67,17 @@ def test_status_handle_zaken_notification(self, m, mock_handle: Mock): statustype_url=data.status_type_final["url"], ) + # additional configs for testing multiple backends + ztc2 = ZaakTypeConfigFactory.create( + catalogus__url=data_alt.zaak_type_alt["catalogus"], + identificatie=data_alt.zaak_type_alt["identificatie"], + ) + ZaakTypeStatusTypeConfigFactory.create( + zaaktype_config=ztc2, + omschrijving=data_alt.status_type_final_alt["omschrijving"], + statustype_url=data_alt.status_type_final_alt["url"], + ) + request = RequestFactory().get("/") webhook_view = ZakenNotificationsWebhookView() webhook_view.setup(request) @@ -76,21 +86,40 @@ def test_status_handle_zaken_notification(self, m, mock_handle: Mock): config.notifications_cases_enabled = True config.save() - webhook_view.handle_notification(data.status_notification) - - mock_handle.assert_called_once() - - # 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.status_final["url"]) - - self.assertTimelineLog( - "accepted status notification: attempt informing users ", - lookup=Lookups.startswith, - level=logging.INFO, - ) + for api_group, notification, zaak, status, user_initiator in [ + ( + data.api_group, + data.status_notification, + data.zaak, + data.status_final, + data.user_initiator, + ), + ( + data_alt.api_group_alt, + data_alt.status_notification_alt, + data_alt.zaak_alt, + data_alt.status_final_alt, + data_alt.user_initiator_alt, + ), + ]: + with self.subTest(f"api_group {api_group.id}"): + webhook_view.handle_notification(notification) + + mock_handle.assert_called() + + # 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, status["url"]) + + log_dump = self.getTimelineLogDump() + self.assertIn("total 2 timelinelogs", log_dump) + self.assertIn( + "accepted status 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_case_notifications_disabled(self, m, mock_handle: Mock): data = MockAPIData().install_mocks(m) @@ -127,6 +156,20 @@ def test_case_notifications_disabled(self, m, mock_handle: Mock): # start of generic checks + def test_no_api_group_found(self, m, mock_handle: Mock): + data = MockAPIData().install_mocks(m) + + request = RequestFactory().get("/") + webhook_view = ZakenNotificationsWebhookView() + webhook_view.setup(request) + + # API group is resolved from zaak url == hoofd_object + data.status_notification.hoofd_object = "http://www.bogus.com" + + webhook_view.handle_notification(data.status_notification) + + mock_handle.assert_not_called() + def test_status_bails_when_bad_notification_channel(self, m, mock_handle: Mock): notification = NotificationFactory(kanaal="not_zaken") with self.assertRaisesRegex( @@ -557,6 +600,11 @@ class NotificationHandlerUserMessageTestCase(AssertTimelineLogMixin, TestCase): note these tests match with a similar test from `test_notification_zaak_infoobject.py` """ + @classmethod + def setUpTestData(cls): + super().setUpTestData() + MockAPIData.setUpServices() + @patch( "open_inwoner.userfeed.hooks.case_status_notification_received", autospec=True ) @@ -594,9 +642,18 @@ def test_handle_status_update(self, mock_send: Mock, mock_feed_hook: Mock): ) # first call - _handle_status_update(user, case, status_initial, status_type_config_initial) + _handle_status_update( + user, case, status_initial, status_type_config_initial, data.api_group + ) mock_send.assert_called_once() + mock_send.assert_called_with( + user, + case, + template_name="case_status_notification", + api_group=data.api_group, + status=status_initial, + ) # check if userfeed hook was called mock_feed_hook.assert_called_once() @@ -622,14 +679,18 @@ def test_handle_status_update(self, mock_send: Mock, mock_feed_hook: Mock): ) # second call with same case/status - _handle_status_update(user, case, status_initial, status_type_config_initial) + _handle_status_update( + user, case, status_initial, status_type_config_initial, data.api_group + ) # no duplicate mail for this user/case/status mock_send.assert_not_called() with self.subTest("mails are throttled based on template_name"): # call with different status and different configuration with action_required=True - _handle_status_update(user, case, status_final, status_type_config_final) + _handle_status_update( + user, case, status_final, status_type_config_final, data.api_group + ) mock_send.assert_called_once() @@ -652,7 +713,7 @@ def test_handle_status_update(self, mock_send: Mock, mock_feed_hook: Mock): # other user is fine other_user = UserFactory.create() _handle_status_update( - other_user, case, status_initial, status_type_config_initial + other_user, case, status_initial, status_type_config_initial, data.api_group ) mock_send.assert_called_once() @@ -669,7 +730,9 @@ def test_handle_status_update(self, mock_send: Mock, mock_feed_hook: Mock): status.statustype = factory( StatusType, copy_with_new_uuid(data.status_type_final) ) - _handle_status_update(user, case, status, status_type_config_initial) + _handle_status_update( + user, case, status, status_type_config_initial, data.api_group + ) # not sent because we already send to this user within the frequency mock_send.assert_not_called() @@ -686,7 +749,9 @@ def test_handle_status_update(self, mock_send: Mock, mock_feed_hook: Mock): status.statustype = factory( StatusType, copy_with_new_uuid(data.status_type_final) ) - _handle_status_update(user, case, status, status_type_config_initial) + _handle_status_update( + user, case, status, status_type_config_initial, data.api_group + ) # this one succeeds mock_send.assert_called_once() @@ -718,7 +783,7 @@ def test_action_required_template(self, mock_send: Mock, mock_feed_hook: Mock): action_required=True, ) - _handle_status_update(user, case, status, status_type_config) + _handle_status_update(user, case, status, status_type_config, data.api_group) mock_send.assert_called_once() # check call arguments From a2d28f5953c44f31dfcb98ae55d8d6cf99b59535 Mon Sep 17 00:00:00 2001 From: Paul Schilling Date: Mon, 26 Aug 2024 14:36:44 +0200 Subject: [PATCH 2/2] [#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()