Merge pull request #4952 from open-formulieren/fix/4821-do-not-report-brk-addressnl-problems-if-not-configured

[#4821] Fixed email digest for BRK and addressNL component

Author: vaszig

src/openforms/contrib/brk/service.py

@@ -1,16 +1,28 @@
+from glom import glom
+
 from openforms.forms.models.form import Form
 from openforms.plugins.exceptions import InvalidPluginConfiguration
 
 from .checks import BRKValidatorCheck
+from .models import BRKConfig
+from .validators import BRK_ZAKELIJK_GERECHTIGD_VALIDATOR_ID
 
 
 def check_brk_config_for_addressNL() -> str:
     live_forms = Form.objects.live()
 
-    if any(form.has_component("addressNL") for form in live_forms):
-        try:
-            BRKValidatorCheck.check_config()
-        except InvalidPluginConfiguration as e:
-            return e.args[0]
+    for form in live_forms:
+        components = form.iter_components()
+        for component in components:
+            if (
+                component["type"] == "addressNL"
+                and (plugins := glom(component, "validate.plugins", default=[]))
+                and BRK_ZAKELIJK_GERECHTIGD_VALIDATOR_ID in plugins
+                and BRKConfig.get_solo().service
+            ):
+                try:
+                    BRKValidatorCheck.check_config()
+                except InvalidPluginConfiguration as e:
+                    return e.args[0]
 
     return ""

src/openforms/contrib/brk/tests/base.py

@@ -12,13 +12,18 @@
 
 BRK_SERVICE = ServiceFactory.build(
     api_root="https://api.brk.kadaster.nl/esd-eto-apikey/bevragen/v2/",
-    oas="https://api.brk.kadaster.nl/esd-eto-apikey/bevragen/v2/",  # ignored/unused
     api_type=APITypes.orc,
     auth_type=AuthTypes.api_key,
     header_key="X-Api-Key",
     header_value=BRK_API_KEY,
 )
 
+INVALID_BRK_SERVICE = ServiceFactory.build(
+    api_root="https://api.brk.kadaster.nl/invalid",
+    api_type=APITypes.orc,
+    auth_type=AuthTypes.api_key,
+)
+
 
 class BRKTestMixin:
     api_root = BRK_SERVICE.api_root

src/openforms/contrib/brk/validators.py

@@ -21,6 +21,8 @@
 
 logger = logging.getLogger(__name__)
 
+BRK_ZAKELIJK_GERECHTIGD_VALIDATOR_ID = "brk-zakelijk-gerechtigd"
+
 
 @contextmanager
 def suppress_api_errors(error_message: str) -> Iterator[None]:

src/openforms/emails/tests/test_digest_functions.py

@@ -14,7 +14,7 @@
 from zgw_consumers.test.factories import ServiceFactory
 
 from openforms.contrib.brk.models import BRKConfig
-from openforms.contrib.brk.tests.base import BRK_SERVICE
+from openforms.contrib.brk.tests.base import BRK_SERVICE, INVALID_BRK_SERVICE
 from openforms.contrib.kadaster.models import KadasterApiConfig
 from openforms.forms.tests.factories import (
     FormFactory,
@@ -244,12 +244,119 @@ def test_timestamp_constraint_returns_no_results(self):
 @override_settings(LANGUAGE_CODE="en")
 class BrokenConfigurationTests(TestCase):
 
+    def test_no_addressNL_component_not_collected(self):
+        FormFactory.create(
+            generate_minimal_setup=True,
+            formstep__form_definition__configuration={
+                "components": [
+                    {
+                        "key": "textField",
+                        "type": "textfield",
+                        "label": "Text Field",
+                    }
+                ],
+            },
+        )
+
+        broken_configuration = collect_broken_configurations()
+
+        self.assertEqual(broken_configuration, [])
+
+    @patch(
+        "openforms.contrib.brk.client.BRKConfig.get_solo",
+        return_value=BRKConfig(service=None),
+    )
+    def test_no_brk_conf_for_addressnl_and_missing_validator_not_collected(self, m):
+        FormFactory.create(
+            generate_minimal_setup=True,
+            formstep__form_definition__configuration={
+                "components": [
+                    {
+                        "key": "addressNl",
+                        "type": "addressNL",
+                        "label": "AddressNL",
+                        "defaultValue": {
+                            "postcode": "",
+                            "houseLetter": "",
+                            "houseNumber": "",
+                            "houseNumberAddition": "",
+                        },
+                    }
+                ],
+            },
+        )
+
+        broken_configuration = collect_broken_configurations()
+
+        self.assertEqual(broken_configuration, [])
+
+    @patch(
+        "openforms.contrib.brk.client.BRKConfig.get_solo",
+        return_value=BRKConfig(service=None),
+    )
+    def test_no_brk_conf_for_addressnl_and_existing_validator_not_collected(self, m):
+        FormFactory.create(
+            generate_minimal_setup=True,
+            formstep__form_definition__configuration={
+                "components": [
+                    {
+                        "key": "addressNl",
+                        "type": "addressNL",
+                        "label": "AddressNL",
+                        "defaultValue": {
+                            "postcode": "",
+                            "houseLetter": "",
+                            "houseNumber": "",
+                            "houseNumberAddition": "",
+                        },
+                        "validate": {"plugins": ["brk-zakelijk-gerechtigd"]},
+                    }
+                ],
+            },
+        )
+
+        broken_configuration = collect_broken_configurations()
+
+        self.assertEqual(broken_configuration, [])
+
+    @patch(
+        "openforms.contrib.brk.client.BRKConfig.get_solo",
+        return_value=BRKConfig(service=BRK_SERVICE),
+    )
+    def test_valid_brk_conf_for_addressnl_and_missing_validator_not_collected(
+        self, brk_config
+    ):
+        FormFactory.create(
+            generate_minimal_setup=True,
+            formstep__form_definition__configuration={
+                "components": [
+                    {
+                        "key": "addressNl",
+                        "type": "addressNL",
+                        "label": "AddressNL",
+                        "defaultValue": {
+                            "postcode": "",
+                            "houseLetter": "",
+                            "houseNumber": "",
+                            "houseNumberAddition": "",
+                        },
+                    }
+                ],
+            },
+        )
+
+        broken_configuration = collect_broken_configurations()
+
+        self.assertEqual(broken_configuration, [])
+
     @patch(
         "openforms.contrib.brk.client.BRKConfig.get_solo",
         return_value=BRKConfig(service=BRK_SERVICE),
     )
     @requests_mock.Mocker()
-    def test_valid_brk_congiguration_for_addressNL_not_collected(self, brk_config, m):
+    def test_valid_brk_conf_for_addressnl_and_existing_validator_not_collected(
+        self, brk_config, m
+    ):
         FormFactory.create(
             generate_minimal_setup=True,
             formstep__form_definition__configuration={
@@ -264,6 +371,7 @@ def test_valid_brk_congiguration_for_addressNL_not_collected(self, brk_config, m
                             "houseNumber": "",
                             "houseNumberAddition": "",
                         },
+                        "validate": {"plugins": ["brk-zakelijk-gerechtigd"]},
                     }
                 ],
             },
@@ -280,9 +388,11 @@ def test_valid_brk_congiguration_for_addressNL_not_collected(self, brk_config, m
 
     @patch(
         "openforms.contrib.brk.client.BRKConfig.get_solo",
-        return_value=BRKConfig(service=None),
+        return_value=BRKConfig(service=INVALID_BRK_SERVICE),
     )
-    def test_invalid_brk_configuration_for_addressNL_is_collected(self, brk_config):
+    def test_invalid_brk_conf_for_addressnl_and_missing_validator_not_collected(
+        self, brk_config
+    ):
         FormFactory.create(
             generate_minimal_setup=True,
             formstep__form_definition__configuration={
@@ -304,30 +414,45 @@ def test_invalid_brk_configuration_for_addressNL_is_collected(self, brk_config):
 
         broken_configuration = collect_broken_configurations()
 
-        self.assertEqual(len(broken_configuration), 1)
-        self.assertEqual(broken_configuration[0].config_name, "BRK Client")
+        self.assertEqual(broken_configuration, [])
 
     @patch(
         "openforms.contrib.brk.client.BRKConfig.get_solo",
-        return_value=BRKConfig(service=None),
+        return_value=BRKConfig(service=INVALID_BRK_SERVICE),
     )
-    def test_invalid_brk_congiguration_but_unused_not_collected(self, brk_config):
+    @requests_mock.Mocker()
+    def test_invalid_brk_conf_for_addressnl_and_existing_validator_collected(
+        self, brk_config, m
+    ):
         FormFactory.create(
             generate_minimal_setup=True,
             formstep__form_definition__configuration={
                 "components": [
                     {
-                        "key": "textField",
-                        "type": "textfield",
-                        "label": "Text Field",
+                        "key": "addressNl",
+                        "type": "addressNL",
+                        "label": "AddressNL",
+                        "defaultValue": {
+                            "postcode": "",
+                            "houseLetter": "",
+                            "houseNumber": "",
+                            "houseNumberAddition": "",
+                        },
+                        "validate": {"plugins": ["brk-zakelijk-gerechtigd"]},
                     }
                 ],
             },
         )
 
+        m.get(
+            "https://api.brk.kadaster.nl/invalid/kadastraalonroerendezaken?postcode=1234AB&huisnummer=1",
+            status_code=400,
+        )
+
         broken_configuration = collect_broken_configurations()
 
-        self.assertEqual(broken_configuration, [])
+        self.assertEqual(len(broken_configuration), 1)
+        self.assertEqual(broken_configuration[0].config_name, "BRK Client")
 
     @patch(
         "openforms.contrib.kadaster.models.KadasterApiConfig.get_solo",
@@ -464,10 +589,8 @@ def test_invalid_bag_configuration_for_address_nl_component_is_collected(
 
         broken_configuration = collect_broken_configurations()
 
-        self.assertEqual(len(broken_configuration), 2)
-        self.assertIn(
-            "BAG Client", [config.config_name for config in broken_configuration]
-        )
+        self.assertEqual(len(broken_configuration), 1)
+        self.assertEqual(broken_configuration[0].config_name, "BAG Client")
 
     @patch("openforms.contrib.kadaster.models.KadasterApiConfig.get_solo")
     def test_invalid_bag_configuration_for_address_nl_component_not_collected_when_disabled(
@@ -491,10 +614,7 @@ def test_invalid_bag_configuration_for_address_nl_component_not_collected_when_d
 
         broken_configuration = collect_broken_configurations()
 
-        self.assertEqual(len(broken_configuration), 1)
-        self.assertNotIn(
-            "BAG Client", [config.config_name for config in broken_configuration]
-        )
+        self.assertEqual(len(broken_configuration), 0)
 
 
 @override_settings(LANGUAGE_CODE="en")

src/openforms/emails/tests/test_tasks_integration.py

@@ -7,6 +7,7 @@
 from django.test import TestCase, override_settings
 from django.urls import reverse
 
+import requests_mock
 from django_yubin.models import Message
 from freezegun import freeze_time
 from furl import furl
@@ -15,6 +16,7 @@
 
 from openforms.config.models import GlobalConfiguration
 from openforms.contrib.brk.models import BRKConfig
+from openforms.contrib.brk.tests.base import INVALID_BRK_SERVICE
 from openforms.forms.tests.factories import (
     FormFactory,
     FormLogicFactory,
@@ -118,11 +120,14 @@ def test_no_email_sent_if_no_recipients(self, mock_global_config):
 
     @patch(
         "openforms.contrib.brk.client.BRKConfig.get_solo",
-        return_value=BRKConfig(service=None),
+        return_value=BRKConfig(service=INVALID_BRK_SERVICE),
     )
     @freeze_time("2023-01-03T01:00:00+01:00")
     @override_settings(BASE_URL="http://testserver")
-    def test_email_sent_when_there_are_failures(self, mock_global_config, brk_config):
+    @requests_mock.Mocker()
+    def test_email_sent_when_there_are_failures(
+        self, mock_global_config, brk_config, m
+    ):
         """Integration test for all the possible failures
 
         - failed emails
@@ -147,6 +152,7 @@ def test_email_sent_when_there_are_failures(self, mock_global_config, brk_config
                             "houseNumber": "",
                             "houseNumberAddition": "",
                         },
+                        "validate": {"plugins": ["brk-zakelijk-gerechtigd"]},
                     }
                 ],
             },
@@ -200,6 +206,11 @@ def test_email_sent_when_there_are_failures(self, mock_global_config, brk_config
                 )
                 ServiceFactory.create(client_certificate=certificate)
 
+            m.get(
+                "https://api.brk.kadaster.nl/invalid/kadastraalonroerendezaken?postcode=1234AB&huisnummer=1",
+                status_code=400,
+            )
+
         # send the email digest
         send_email_digest()
         sent_email = mail.outbox[-1]
@@ -250,7 +261,7 @@ def test_email_sent_when_there_are_failures(self, mock_global_config, brk_config
 
         with self.subTest("broken configuration"):
             self.assertIn(
-                "The configuration for 'BRK Client' is invalid (KVK endpoint is not configured).",
+                "The configuration for 'BRK Client' is invalid",
                 sent_email.body,
             )
 

src/openforms/forms/models/form.py

@@ -675,11 +675,6 @@ def deactivate(self):
         logger.debug("Deactivating form %s", self.admin_name)
         self.save(update_fields=["active", "deactivate_on"])
 
-    def has_component(self, component_type: str) -> bool:
-        return any(
-            component["type"] == component_type for component in self.iter_components()
-        )
-
 
 class FormsExportQuerySet(DeleteFilesQuerySetMixin, models.QuerySet):
     pass