Skip to content

Commit

Permalink
Merge pull request #4198 from open-formulieren/feature/3725-add-inval…
Browse files Browse the repository at this point in the history
…id-forms-to-email-digest

[#3725] Add invalid forms (configuration and logic rules) to the email digest
  • Loading branch information
sergei-maertens authored May 22, 2024
2 parents 4734c03 + 8ef40fb commit d2ba32a
Show file tree
Hide file tree
Showing 8 changed files with 634 additions and 61 deletions.
66 changes: 56 additions & 10 deletions src/openforms/conf/locale/nl/LC_MESSAGES/django.po
Original file line number Diff line number Diff line change
Expand Up @@ -3824,28 +3824,34 @@ msgstr "Bevestiging"
msgid "Co-sign confirmation"
msgstr "Mede-ondertekeningbevestiging"

#: openforms/emails/digest.py:199
#: openforms/emails/digest.py:239
msgid "BRK Client"
msgstr "BRK client"

#: openforms/emails/digest.py:206
#: openforms/emails/digest.py:246
#, fuzzy
#| msgid "BRK Client"
msgid "BAG Client"
msgstr "BRK client"

#: openforms/emails/digest.py:216
#: openforms/emails/digest.py:256
msgid "will expire soon"
msgstr ""

#: openforms/emails/digest.py:217
#: openforms/emails/digest.py:257
msgid "has invalid keypair"
msgstr ""

#: openforms/emails/digest.py:218
#: openforms/emails/digest.py:258
msgid "invalid keypair, will expire soon"
msgstr ""

#: openforms/emails/digest.py:351
#, fuzzy
#| msgid "Email registration configuration"
msgid "Invalid registration backend configuration detected"
msgstr "E-mailregistratieconfiguratie"

#: openforms/emails/models.py:26
msgid "Subject of the email message"
msgstr "Onderwerp van het e-mailbericht"
Expand Down Expand Up @@ -3887,7 +3893,7 @@ msgstr "(formulier onbekend)"
msgid "Confirmation email template - {form}"
msgstr "Bevestigingse-mail sjabloon - {form}"

#: openforms/emails/tasks.py:63
#: openforms/emails/tasks.py:69
msgid "[Open Forms] Daily summary of detected problems"
msgstr "[Open Formulieren] Dagelijks rapport van gedetecteerde problemen"

Expand Down Expand Up @@ -3929,10 +3935,13 @@ msgid "Registrations"
msgstr "Registraties"

#: openforms/emails/templates/emails/admin_digest.html:26
#, python-format
#, fuzzy, python-format
#| msgid ""
#| "Form '%(form_name)s' failed %(counter)s time(s) between "
#| "%(first_failure_at)s and %(last_failure_at)s.</br>"
msgid ""
"Form '%(form_name)s' failed %(counter)s time(s) between %(first_failure_at)s "
"and %(last_failure_at)s.</br>"
"and %(last_failure_at)s.<br>"
msgstr ""
"Formulier '%(form_name)s' faalde %(counter)s keer tussen "
"%(first_failure_at)s end %(last_failure_at)s.</br>"
Expand Down Expand Up @@ -4011,6 +4020,43 @@ msgstr ""
"\n"
"Toon gefaalde '%(form_name)s' inzendingen"

#: openforms/emails/templates/emails/admin_digest.html:111
#, fuzzy
#| msgid "registration backend result"
msgid "Registration backends problems"
msgstr "registratie backend resultaat"

#: openforms/emails/templates/emails/admin_digest.html:117
#, python-format
msgid ""
"The configuration for plugin '%(name)s' is invalid.<br> (%(message)s)<br>"
msgstr ""

#: openforms/emails/templates/emails/admin_digest.html:123
#, python-format
msgid ""
"This affects form '%(form_name)s', possibly other forms are affected too."
msgstr ""

#: openforms/emails/templates/emails/admin_digest.html:127
#: openforms/emails/templates/emails/admin_digest.html:144
#, fuzzy
#| msgid "Show form"
msgid "View form"
msgstr "Toon formulier"

#: openforms/emails/templates/emails/admin_digest.html:134
#, fuzzy
#| msgid "List logic rules"
msgid "Logic rules problems"
msgstr "Logicaregels weergeven"

#: openforms/emails/templates/emails/admin_digest.html:140
#, python-format
msgid ""
"Logic rule for variable '%(var)s' is invalid in form '%(form_name)s'.<br>"
msgstr ""

#: openforms/emails/templates/emails/co_sign/request.html:7
msgid ""
"Please visit the form page on our website and click on the 'co-sign' button."
Expand Down Expand Up @@ -8257,8 +8303,8 @@ msgid ""
"The co-sign component requires the '{field_label}' ({config_verbose_name}) "
"to be configured."
msgstr ""
"Het mede-ondertekencomponent vereist de configuratie van '{field_label}' "
"({config_verbose_name})."
"Het mede-ondertekencomponent vereist de configuratie van "
"'{field_label}' ({config_verbose_name})."

#: openforms/products/api/viewsets.py:15
msgid "Retrieve details of a single product"
Expand Down
191 changes: 191 additions & 0 deletions src/openforms/emails/digest.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import logging
import uuid
from dataclasses import dataclass
from datetime import datetime, timedelta
Expand All @@ -12,14 +13,27 @@

from django_yubin.models import Message
from furl import furl
from rest_framework import serializers
from simple_certmanager.models import Certificate

from openforms.contrib.brk.service import check_brk_config_for_addressNL
from openforms.contrib.kadaster.service import check_bag_config_for_address_fields
from openforms.forms.constants import LogicActionTypes
from openforms.forms.models import Form
from openforms.forms.models.form_registration_backend import FormRegistrationBackend
from openforms.forms.models.logic import FormLogic
from openforms.logging.models import TimelineLogProxy
from openforms.plugins.exceptions import InvalidPluginConfiguration
from openforms.registrations.registry import register
from openforms.submissions.models.submission import Submission
from openforms.submissions.utils import get_filtered_submission_admin_url
from openforms.typing import StrOrPromise
from openforms.utils.json_logic.datastructures import InputVar
from openforms.utils.json_logic.introspection import introspect_json_logic
from openforms.variables.constants import FormVariableDataTypes
from openforms.variables.service import get_static_variables

logger = logging.getLogger(__name__)


@dataclass
Expand Down Expand Up @@ -90,6 +104,32 @@ def admin_link(self) -> str:
return form_admin_url


@dataclass
class InvalidRegistrationBackend:
config_name: StrOrPromise
exception_message: str
form_name: str
form_id: int

@property
def admin_link(self) -> str:
form_admin_url = reverse(
"admin:forms_form_change", kwargs={"object_id": self.form_id}
)
return form_admin_url


@dataclass
class InvalidLogicRule:
variable: str
form_name: str
form_id: int

@property
def admin_link(self) -> str:
return reverse("admin:forms_form_change", kwargs={"object_id": self.form_id})


def collect_failed_emails(since: datetime) -> Iterable[FailedEmail]:
logs = TimelineLogProxy.objects.filter(
timestamp__gt=since,
Expand Down Expand Up @@ -257,3 +297,154 @@ def collect_invalid_certificates() -> list[InvalidCertificate]:
)

return invalid_certs


def collect_invalid_registration_backends() -> list[InvalidRegistrationBackend]:
registration_backends = (
backend
for backend in FormRegistrationBackend.objects.select_related("form").iterator()
if backend.form.is_available
)

checked_plugins = set()
invalid_registration_backends = []
for registration_backend in registration_backends:
form_name = registration_backend.form.admin_name
form = registration_backend.form
backend_options = registration_backend.options
backend_type = registration_backend.backend
plugin = register[backend_type]

# errors in general configuration are more straightforward for the user,
# so we show the exact ones
plugin_cls = type(plugin)
if plugin_cls not in checked_plugins:
try:
plugin.check_config()
# we always raise an InvalidPluginConfiguration exception even when we have
# errors like HTTPError
except InvalidPluginConfiguration as e:
invalid_registration_backends.append(
InvalidRegistrationBackend(
config_name=plugin.verbose_name,
exception_message=e.args[0],
form_name=form_name,
form_id=form.id,
)
)
checked_plugins.add(plugin_cls)

# errors in the form level can be more detailed so here we want to show
# a more general error and the link to the broken form to investigate
serializer = plugin.configuration_options(
data=backend_options,
context={"validate_business_logic": True},
)

try:
serializer.is_valid(raise_exception=True)
except serializers.ValidationError:
invalid_registration_backends.append(
InvalidRegistrationBackend(
config_name=plugin.verbose_name,
exception_message=_(
"Invalid registration backend configuration detected"
),
form_name=form_name,
form_id=form.id,
)
)

return invalid_registration_backends


def collect_invalid_logic_rules() -> list[InvalidLogicRule]:
forms = Form.objects.live().iterator()
static_variables = {
var.key: {"source": var.source, "type": var.data_type}
for var in get_static_variables()
}

invalid_logic_rules = []
for form in forms:
form_variables = {
form_variable.key: {
"source": form_variable.source,
"type": form_variable.data_type,
}
for form_variable in form.formvariable_set.all()
}

all_keys = list(static_variables) + list(form_variables)

form_logics = FormLogic.objects.filter(form=form)

form_logics_vars = []
for logic in form_logics:
for var in introspect_json_logic(logic.json_logic_trigger).get_input_keys():
form_logics_vars.append(var)
for action in logic.actions:
if action["action"]["type"] == LogicActionTypes.variable:
form_logics_vars.append(InputVar(key=action["variable"]))
expression = action["action"]["value"]
form_logics_vars += [
var
for var in introspect_json_logic(expression).get_input_keys()
]

for var in set(form_logics_vars):
# there is a variable with this exact key, it is a valid reference
if var.key in all_keys:
continue

outer, *nested = var.key.split(".")

def _report():
invalid_logic_rules.append(
InvalidLogicRule(
variable=var.key,
form_name=form.admin_name,
form_id=form.id,
)
)

# Before checking the type of the parent/outer bit, check if it exists in
# the first place. It could also be that it's not a parent at all (nested is
# empty), so that's a guaranteed broken reference too.
if outer not in all_keys:
_report()
continue

# Nested cannot be empty now - if it were, either the key exists as a var,
# which is the first thing we checked above, or it doesn't exist as a var,
# which is the second thing we checked above and errored out.
assert nested

# We can only check the outer bit of the variable, which must be a suitable
# container type to be valid. Container types are objects and arrays, but
# we only consider arrays as a valid type if the first bit of the nested
# key is numeric, as this must refer to an array index.
# E.g. foo.bar -> only objects are possible, but foo.3 -> objects and arrays
# are possible (you could have a dict with the string "3" as key").
expected_container_types = {FormVariableDataTypes.object}
if nested[0].isnumeric():
expected_container_types.add(FormVariableDataTypes.array)

# Now, check that the type of the container variable is as expected. If the
# variable does *not* have any of the container types, then it's a
# guaranteed error since lookups inside primitives are not possible.
parent_var = form_variables.get(outer) or static_variables.get(outer)
if parent_var["type"] not in expected_container_types:
_report()
continue

# all the other situations - we cannot (yet) conclude anything meaningful,
# so instead, log information for our own insight into (typical) usage/
# constructions.
logger.info(
"possible invalid variable reference (%s) in logic of form %s",
var.key,
form.admin_name,
)

return invalid_logic_rules
6 changes: 6 additions & 0 deletions src/openforms/emails/tasks.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@
collect_failed_prefill_plugins,
collect_failed_registrations,
collect_invalid_certificates,
collect_invalid_logic_rules,
collect_invalid_registration_backends,
)
from .utils import send_mail_html

Expand All @@ -29,13 +31,17 @@ def get_context_data(self) -> dict[str, Any]:
failed_prefill_plugins = collect_failed_prefill_plugins(self.since)
broken_configurations = collect_broken_configurations()
invalid_certificates = collect_invalid_certificates()
invalid_registration_backends = collect_invalid_registration_backends()
invalid_logic_rules = collect_invalid_logic_rules()

return {
"failed_emails": failed_emails,
"failed_registrations": failed_registrations,
"failed_prefill_plugins": failed_prefill_plugins,
"broken_configurations": broken_configurations,
"invalid_certificates": invalid_certificates,
"invalid_registration_backends": invalid_registration_backends,
"invalid_logic_rules": invalid_logic_rules,
}

def render(self) -> str:
Expand Down
Loading

0 comments on commit d2ba32a

Please sign in to comment.