[#3725] PR feedback

src/openforms/emails/digest.py

@@ -10,17 +10,20 @@
 
 from django_yubin.models import Message
 from furl import furl
+from rest_framework import serializers
 
 from openforms.contrib.brk.service import check_brk_config_for_addressNL
+from openforms.forms.models import Form
 from openforms.forms.models.form_registration_backend import FormRegistrationBackend
 from openforms.forms.models.logic import FormLogic
-from openforms.forms.utils import is_form_config_valid
 from openforms.logging.models import TimelineLogProxy
-from openforms.plugins.checks import check_general_config
+from openforms.plugins.exceptions import InvalidPluginConfiguration
+from openforms.registrations.base import BasePlugin
 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.variables.service import get_static_variables
 
 
 @dataclass
@@ -75,7 +78,7 @@ class BrokenConfiguration:
 
 
 @dataclass
-class FailedRegistrationBackend:
+class InvalidRegistrationBackend:
     config_name: StrOrPromise
     exception_message: str
     form_name: str
@@ -91,17 +94,57 @@ def admin_link(self) -> str:
 
 
 @dataclass
-class FailedLogicRule:
+class InvalidLogicRule:
     variable: str
     form_name: str
     form_id: str
 
     @property
     def admin_link(self) -> str:
-        form_admin_link = reverse(
-            "admin:forms_form_change", kwargs={"object_id": self.form_id}
-        )
-        return form_admin_link
+        return reverse("admin:forms_form_change", kwargs={"object_id": self.form_id})
+
+
+def is_form_config_valid(plugin: BasePlugin, options: dict[str, str]) -> bool:
+    serializer = plugin.configuration_options(data=options)
+
+    try:
+        serializer.is_valid(raise_exception=True)
+    except serializers.ValidationError:
+        return False
+
+    return True
+
+
+def check_general_config(plugin: BasePlugin) -> str:
+    try:
+        plugin.check_config()
+
+    # we always raise an InvalidPluginConfiguration exception even when we have
+    # errors like HTTPError
+    except InvalidPluginConfiguration as e:
+        return e.args[0]
+
+    return ""
+
+
+def get_variables_from_logic(form_logic: FormLogic) -> list[str]:
+    data = form_logic.json_logic_trigger
+    var_values = []
+
+    def _recursive_find(data):
+        if isinstance(data, dict):
+            if "var" in data:
+                var_values.append(data["var"])
+            else:
+                for value in data.values():
+                    _recursive_find(value)
+        elif isinstance(data, list):
+            for item in data:
+                _recursive_find(item)
+
+    _recursive_find(data)
+
+    return var_values
 
 
 def collect_failed_emails(since: datetime) -> Iterable[FailedEmail]:
@@ -216,71 +259,78 @@ def collect_broken_configurations() -> list[BrokenConfiguration]:
     return broken_configurations
 
 
-def collect_failed_registration_backends() -> list[FailedRegistrationBackend]:
-    registration_backends = FormRegistrationBackend.objects.all()
-    form_backend_mappings = [
-        {registration_backend.form.admin_name: registration_backend}
-        for registration_backend in registration_backends
-        if registration_backend.form.is_available
-    ]
-
-    failed_registration_backends = []
-    for mapping in form_backend_mappings:
-        for form_name, registration_backend in mapping.items():
-            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
-            if broken_general_config := check_general_config(plugin):
-                failed_registration_backends.append(
-                    FailedRegistrationBackend(
-                        config_name=plugin.verbose_name,
-                        exception_message=broken_general_config,
-                        form_name=form_name,
-                    )
+def collect_invalid_registration_backends() -> list[InvalidRegistrationBackend]:
+    registration_backends = (
+        backend
+        for backend in FormRegistrationBackend.objects.select_related("form").iterator()
+        if backend.form.is_available
+    )
+
+    already_detected = []
+    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
+        if plugin.verbose_name not in already_detected and (
+            broken_general_config := check_general_config(plugin)
+        ):
+            already_detected.append(plugin.verbose_name)
+            invalid_registration_backends.append(
+                InvalidRegistrationBackend(
+                    config_name=plugin.verbose_name,
+                    exception_message=broken_general_config,
+                    form_name=form_name,
                 )
+            )
 
-            # 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
-            if not is_form_config_valid(plugin, backend_options):
-                failed_registration_backends.append(
-                    FailedRegistrationBackend(
-                        config_name=plugin.verbose_name,
-                        exception_message=_(
-                            "Invalid registration backend configuration has been detected in the form level"
-                        ),
-                        form_name=form_name,
-                        form_id=str(form.id),
-                        form_level=True,
-                    )
+        # 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
+        if not is_form_config_valid(plugin, backend_options):
+            invalid_registration_backends.append(
+                InvalidRegistrationBackend(
+                    config_name=plugin.verbose_name,
+                    exception_message=_(
+                        "Invalid registration backend configuration has been detected at the form level"
+                    ),
+                    form_name=form_name,
+                    form_id=str(form.id),
+                    form_level=True,
                 )
+            )
 
-    return failed_registration_backends
+    return invalid_registration_backends
 
 
-def collect_failed_logic_rules() -> list[FailedLogicRule]:
-    logic_for_active_forms = FormLogic.objects.filter(form__active=True)
-    variables_forms_mapping = [
-        {form_variable: logic.form}
-        for logic in logic_for_active_forms
-        for form_variable in logic.get_variables_from_logic()
-    ]
+def collect_invalid_logic_rules() -> list[InvalidLogicRule]:
+    forms = Form.objects.live()
+    static_variables = [var.key for var in get_static_variables()]
 
-    failed_logic_rules = []
-    for mapping in variables_forms_mapping:
-        for var, form in mapping.items():
-            form_variables_keys = [
-                form_variable.key for form_variable in form.formvariable_set.all()
-            ]
-
-            if var not in form_variables_keys:
-                failed_logic_rules.append(
-                    FailedLogicRule(
-                        variable=var, form_name=form.admin_name, form_id=str(form.id)
+    invalid_logic_rules = []
+    for form in forms:
+        form_variables_keys = [
+            form_variable.key for form_variable in form.formvariable_set.all()
+        ]
+
+        all_keys = static_variables + form_variables_keys
+        form_logics = FormLogic.objects.filter(form=form)
+        form_logics_vars = [
+            var for logic in form_logics for var in get_variables_from_logic(logic)
+        ]
+
+        for var in form_logics_vars:
+            if var not in all_keys:
+                invalid_logic_rules.append(
+                    InvalidLogicRule(
+                        variable=var,
+                        form_name=form.admin_name,
+                        form_id=str(form.id),
                     )
                 )
 
-    return failed_logic_rules
+    return invalid_logic_rules

src/openforms/emails/tasks.py

@@ -12,10 +12,10 @@
 from .digest import (
     collect_broken_configurations,
     collect_failed_emails,
-    collect_failed_logic_rules,
     collect_failed_prefill_plugins,
-    collect_failed_registration_backends,
     collect_failed_registrations,
+    collect_invalid_logic_rules,
+    collect_invalid_registration_backends,
 )
 from .utils import send_mail_html
 
@@ -29,16 +29,16 @@ def get_context_data(self) -> dict[str, Any]:
         failed_registrations = collect_failed_registrations(self.since)
         failed_prefill_plugins = collect_failed_prefill_plugins(self.since)
         broken_configurations = collect_broken_configurations()
-        failed_registration_backends = collect_failed_registration_backends()
-        failed_logic_rules = collect_failed_logic_rules()
+        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,
-            "failed_registration_backends": failed_registration_backends,
-            "failed_logic_rules": failed_logic_rules,
+            "invalid_registration_backends": invalid_registration_backends,
+            "invalid_logic_rules": invalid_logic_rules,
         }
 
     def render(self) -> str:

src/openforms/emails/templates/emails/admin_digest.html

@@ -78,11 +78,11 @@ <h5>{% trans "Configuration problems" %}</h5>
     </ul>
 {% endif %}
 
-{% if failed_registration_backends %}
+{% if invalid_registration_backends %}
     <h5>{% trans "Registration backends problems" %}</h5>
     <ul>
         <ul>
-        {% for failed_backend in failed_registration_backends %}
+        {% for failed_backend in invalid_registration_backends %}
             <li>
                 <p>
                     {% blocktranslate with name=failed_backend.config_name message=failed_backend.exception_message trimmed %}
@@ -103,11 +103,11 @@ <h5>{% trans "Registration backends problems" %}</h5>
     </ul>
 {% endif %}
 
-{% if failed_logic_rules %}
+{% if invalid_logic_rules %}
     <h5>{% trans "Logic rules problems" %}</h5>
     <ul>
         <ul>
-        {% for logic_rule in failed_logic_rules %}
+        {% for logic_rule in invalid_logic_rules %}
             <li>
                 <p>
                     {% blocktranslate with var=logic_rule.variable form_name=logic_rule.form_name trimmed %}