Skip to content

Commit

Permalink
[#3725] Added missing cases in invalid logic rules detection
Browse files Browse the repository at this point in the history
  • Loading branch information
vaszig committed May 21, 2024
1 parent 850c982 commit 8ef40fb
Show file tree
Hide file tree
Showing 6 changed files with 225 additions and 140 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
109 changes: 72 additions & 37 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 @@ -17,20 +18,23 @@

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 import logevent
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
class FailedEmail:
Expand Down Expand Up @@ -105,7 +109,7 @@ class InvalidRegistrationBackend:
config_name: StrOrPromise
exception_message: str
form_name: str
form_id: int | None = None
form_id: int

@property
def admin_link(self) -> str:
Expand All @@ -114,10 +118,6 @@ def admin_link(self) -> str:
)
return form_admin_url

@property
def form_level(self) -> bool:
return bool(self.form_id)


@dataclass
class InvalidLogicRule:
Expand Down Expand Up @@ -332,7 +332,7 @@ def collect_invalid_registration_backends() -> list[InvalidRegistrationBackend]:
form_id=form.id,
)
)
checked_plugins.add(plugin_cls)
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
Expand All @@ -359,7 +359,7 @@ def collect_invalid_registration_backends() -> list[InvalidRegistrationBackend]:


def collect_invalid_logic_rules() -> list[InvalidLogicRule]:
forms = Form.objects.live().prefetch_related("formvariable_set")
forms = Form.objects.live().iterator()
static_variables = {
var.key: {"source": var.source, "type": var.data_type}
for var in get_static_variables()
Expand All @@ -378,38 +378,73 @@ def collect_invalid_logic_rules() -> list[InvalidLogicRule]:
all_keys = list(static_variables) + list(form_variables)

form_logics = FormLogic.objects.filter(form=form)
form_logics_vars = [
var
for logic in form_logics
for var in introspect_json_logic(logic.json_logic_trigger).get_input_keys()
]

for var in form_logics_vars:
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(".")

if var.key not in all_keys:
if nested and (
(
outer in form_variables
and form_variables[outer]["type"]
in [FormVariableDataTypes.object, FormVariableDataTypes.array]
and form_variables[outer]["source"] == "user_defined"
)
or (
outer in static_variables
and static_variables[outer]["type"]
in [FormVariableDataTypes.object, FormVariableDataTypes.array]
)
):
logevent.invalid_variable_reference_in_logic(form, var.key)

else:
invalid_logic_rules.append(
InvalidLogicRule(
variable=var.key,
form_name=form.admin_name,
form_id=form.id,
)
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
2 changes: 1 addition & 1 deletion src/openforms/emails/templates/emails/admin_digest.html
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ <h5>{% trans "Registration backends problems" %}</h5>
</p>
<p>
{% blocktranslate with form_name=failed_backend.form_name trimmed %}
This affects form '{{ form_name }}'
This affects form '{{ form_name }}', possibly other forms are affected too.
{% endblocktranslate %}
</p>
<p><a href="{{ failed_backend.admin_link }}">{% trans "View form" %}</a></p>
Expand Down
Loading

0 comments on commit 8ef40fb

Please sign in to comment.