Skip to content

Commit

Permalink
[#4821] Fixed email digest for BRK and addressNL component
Browse files Browse the repository at this point in the history 
We want the email digest to report problems (broken configuration) for
the addressNL component only when a valid BRK service and the validator
are defined/configured.
  • Loading branch information
@vaszig
vaszig committed Dec 19, 2024
1 parent 738be59 commit 28702dd
Show file tree
Hide file tree
Showing 6 changed files with 179 additions and 34 deletions.
22 changes: 17 additions & 5 deletions src/openforms/contrib/brk/service.py
View file
Original file line number Diff line number Diff line change
@@ -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 ""
7 changes: 6 additions & 1 deletion src/openforms/contrib/brk/tests/base.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 2 additions & 0 deletions src/openforms/contrib/brk/validators.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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]:
Expand Down
160 changes: 140 additions & 20 deletions src/openforms/emails/tests/test_digest_functions.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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={
Expand All @@ -264,6 +371,7 @@ def test_valid_brk_congiguration_for_addressNL_not_collected(self, brk_config, m
"houseNumber": "",
"houseNumberAddition": "",
},
"validate": {"plugins": ["brk-zakelijk-gerechtigd"]},
}
],
},
Expand All @@ -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={
Expand All @@ -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",
Expand Down Expand Up @@ -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(
Expand All @@ -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")
Expand Down
17 changes: 14 additions & 3 deletions src/openforms/emails/tests/test_tasks_integration.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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,
Expand Down Expand Up @@ -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
Expand All @@ -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"]},
}
],
},
Expand Down Expand Up @@ -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]
Expand Down Expand Up @@ -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,
)

Expand Down
5 changes: 0 additions & 5 deletions src/openforms/forms/models/form.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit 28702dd

Please sign in to comment.