From 19a4baa4d858072cbbf49b729d6402dc6b28ca11 Mon Sep 17 00:00:00 2001 From: Sergei Maertens Date: Thu, 24 Nov 2022 16:21:25 +0100 Subject: [PATCH] :recycle: [#1068] -- refactor custom formio field types to use dynamic config approach Now it's just another flavour of mutating an existing component on the fly, with the added functionality of making some network calls to retrieve the relevant data. This essentially does away with the whole special treatment of custom field types. The use case of adding one or more components and rebuilding the component tree is removed, as there has not been any demand for this. We do keep in mind it may at some point be needed to inject components at other locations in the tree or do more advanced operations, but the current implementation does not sit in the way of that approach. Additionally, this fixes the bug where only top-level npFamilyMembers components could function correctly, which is now properly recursive for nested components. --- src/openforms/custom_field_types/__init__.py | 8 ++ src/openforms/custom_field_types/apps.py | 8 -- .../custom_field_types/family_members.py | 58 -------------- .../tests/test_family_members.py | 41 +++++----- src/openforms/formio/components/custom.py | 77 ++++++++++++++++++- .../formio/dynamic_config/__init__.py | 4 +- src/openforms/formio/registry.py | 29 ++----- src/openforms/formio/service.py | 8 +- src/openforms/forms/custom_field_types.py | 49 ------------ .../tests/test_get_submission_step.py | 50 ++++++++---- 10 files changed, 151 insertions(+), 181 deletions(-) delete mode 100644 src/openforms/custom_field_types/apps.py delete mode 100644 src/openforms/custom_field_types/family_members.py delete mode 100644 src/openforms/forms/custom_field_types.py diff --git a/src/openforms/custom_field_types/__init__.py b/src/openforms/custom_field_types/__init__.py index e69de29bb2..6485a7a552 100644 --- a/src/openforms/custom_field_types/__init__.py +++ b/src/openforms/custom_field_types/__init__.py @@ -0,0 +1,8 @@ +import warnings + +warnings.warn( + "The 'openforms.custom_field_types' is deprecated and will be removed when " + "the functionality to query 'arbitrary' data sources is finished. For more info, " + "see https://github.com/open-formulieren/open-forms/issues/2169.", + DeprecationWarning, +) diff --git a/src/openforms/custom_field_types/apps.py b/src/openforms/custom_field_types/apps.py deleted file mode 100644 index bd6db65fbb..0000000000 --- a/src/openforms/custom_field_types/apps.py +++ /dev/null @@ -1,8 +0,0 @@ -from django.apps import AppConfig - - -class CustomFieldTypesConfig(AppConfig): - name = "openforms.custom_field_types" - - def ready(self): - from . import family_members # noqa diff --git a/src/openforms/custom_field_types/family_members.py b/src/openforms/custom_field_types/family_members.py deleted file mode 100644 index 31c2b4f644..0000000000 --- a/src/openforms/custom_field_types/family_members.py +++ /dev/null @@ -1,58 +0,0 @@ -from typing import Any, Dict - -from openforms.authentication.constants import AuthAttribute -from openforms.formio.typing import Component -from openforms.forms.custom_field_types import register -from openforms.submissions.models import Submission - -from .constants import FamilyMembersDataAPIChoices -from .handlers.haal_centraal import get_np_children_haal_centraal -from .handlers.stuf_bg import get_np_children_stuf_bg -from .models import FamilyMembersTypeConfig - - -@register("npFamilyMembers") -def fill_out_family_members( - component: Component, submission: Submission -) -> Dict[str, Any]: - - # Check authentication details - if not submission.is_authenticated: - raise RuntimeError("No authenticated person!") - - if submission.auth_info.attribute != AuthAttribute.bsn: - raise RuntimeError("No BSN found in the authentication details.") - - bsn = submission.auth_info.value - - # Decide which API should be used to retrieve the children's data - config = FamilyMembersTypeConfig.get_solo() - handlers = { - FamilyMembersDataAPIChoices.haal_centraal: get_np_children_haal_centraal, - FamilyMembersDataAPIChoices.stuf_bg: get_np_children_stuf_bg, - } - - # Change the component configuration - component["type"] = "selectboxes" - component["fieldSet"] = False - component["inline"] = False - component["inputType"] = "checkbox" - - if not len(component["values"]) or component["values"][0] == { - "label": "", - "value": "", - }: - child_choices = handlers[config.data_api](bsn) - - component["values"] = [ - { - "label": label, - "value": value, - } - for value, label in child_choices - ] - - if "mask" in component: - del component["mask"] - - return component diff --git a/src/openforms/custom_field_types/tests/test_family_members.py b/src/openforms/custom_field_types/tests/test_family_members.py index 4d3cfa115a..4ee9110986 100644 --- a/src/openforms/custom_field_types/tests/test_family_members.py +++ b/src/openforms/custom_field_types/tests/test_family_members.py @@ -9,7 +9,7 @@ from openforms.authentication.constants import AuthAttribute from openforms.contrib.brp.models import BRPConfig -from openforms.forms.custom_field_types import handle_custom_types +from openforms.formio.service import get_dynamic_configuration from openforms.prefill.contrib.haalcentraal.tests.test_plugin import load_binary_mock from openforms.registrations.contrib.zgw_apis.tests.factories import ServiceFactory from openforms.submissions.tests.factories import SubmissionFactory @@ -24,14 +24,14 @@ class FamilyMembersCustomFieldTypeTest(TestCase): - @patch( - "openforms.custom_field_types.family_members.get_np_children_haal_centraal", - return_value=[("222333444", "Billy Doe"), ("333444555", "Jane Doe")], - ) - def test_get_values_for_custom_field(self, m): - configuration = { - "display": "form", - "components": [ + @patch("openforms.formio.components.custom.NPFamilyMembers._get_handler") + def test_get_values_for_custom_field(self, mock_get_handler): + mock_get_handler.return_value.return_value = [ + ("222333444", "Billy Doe"), + ("333444555", "Jane Doe"), + ] + submission = SubmissionFactory.from_components( + [ { "key": "npFamilyMembers", "type": "npFamilyMembers", @@ -39,32 +39,35 @@ def test_get_values_for_custom_field(self, m): "values": [{"label": "", "value": ""}], }, ], - } - submission = SubmissionFactory.create( auth_info__attribute=AuthAttribute.bsn, auth_info__value="111222333", - form__generate_minimal_setup=True, - form__formstep__form_definition__configuration=configuration, ) config = FamilyMembersTypeConfig.get_solo() config.data_api = FamilyMembersDataAPIChoices.haal_centraal config.save() + formio_wrapper = ( + submission.submissionstep_set.get().form_step.form_definition.configuration_wrapper + ) - rewritten_configuration = handle_custom_types(configuration, submission) + updated_config_wrapper = get_dynamic_configuration( + formio_wrapper, + request=None, + submission=submission, + ) - rewritten_component = rewritten_configuration["components"][0] + rewritten_component = updated_config_wrapper["npFamilyMembers"] self.assertEqual("selectboxes", rewritten_component["type"]) self.assertFalse(rewritten_component["fieldSet"]) self.assertFalse(rewritten_component["inline"]) self.assertEqual("checkbox", rewritten_component["inputType"]) self.assertEqual(2, len(rewritten_component["values"])) - self.assertDictEqual( - {"value": "222333444", "label": "Billy Doe"}, + self.assertEqual( rewritten_component["values"][0], + {"value": "222333444", "label": "Billy Doe"}, ) - self.assertDictEqual( - {"value": "333444555", "label": "Jane Doe"}, + self.assertEqual( rewritten_component["values"][1], + {"value": "333444555", "label": "Jane Doe"}, ) def test_get_children_haal_centraal(self): diff --git a/src/openforms/formio/components/custom.py b/src/openforms/formio/components/custom.py index ab660ecd6c..953b42e56c 100644 --- a/src/openforms/formio/components/custom.py +++ b/src/openforms/formio/components/custom.py @@ -1,11 +1,14 @@ import logging +from typing import Callable +from openforms.authentication.constants import AuthAttribute +from openforms.submissions.models import Submission from openforms.typing import DataMapping from openforms.utils.date import format_date_value from ..dynamic_config.date import FormioDateComponent, mutate as mutate_date from ..formatters.custom import DateFormatter, MapFormatter -from ..formatters.formio import TextFieldFormatter +from ..formatters.formio import DefaultFormatter, TextFieldFormatter from ..registry import BasePlugin, register from ..typing import Component from ..utils import conform_to_mask @@ -22,7 +25,7 @@ def normalizer(component: FormioDateComponent, value: str) -> str: return format_date_value(value) def mutate_config_dynamically( - self, component: FormioDateComponent, data: DataMapping + self, component: FormioDateComponent, submission: Submission, data: DataMapping ) -> None: """ Implement the behaviour for our custom date component options. @@ -58,3 +61,73 @@ def normalizer(component: Component, value: str) -> str: "Could not conform value '%s' to input mask '%s', returning original value." ) return value + + +@register("npFamilyMembers") +class NPFamilyMembers(BasePlugin): + # not actually relevant, as we transform the component into a different type + formatter = DefaultFormatter + + @staticmethod + def _get_handler() -> Callable[[str], list[tuple[str, str]]]: + # TODO: move these into a subpackage of openforms.formio + from openforms.custom_field_types.constants import FamilyMembersDataAPIChoices + from openforms.custom_field_types.handlers.haal_centraal import ( + get_np_children_haal_centraal, + ) + from openforms.custom_field_types.handlers.stuf_bg import ( + get_np_children_stuf_bg, + ) + from openforms.custom_field_types.models import FamilyMembersTypeConfig + + handlers = { + FamilyMembersDataAPIChoices.haal_centraal: get_np_children_haal_centraal, + FamilyMembersDataAPIChoices.stuf_bg: get_np_children_stuf_bg, + } + config = FamilyMembersTypeConfig.get_solo() + return handlers[config.data_api] + + @classmethod + def mutate_config_dynamically( + cls, component: Component, submission: Submission, data: DataMapping + ) -> None: + # Check authentication details/status before proceeding + if not submission.is_authenticated: + raise RuntimeError("No authenticated person!") + if submission.auth_info.attribute != AuthAttribute.bsn: + raise RuntimeError("No BSN found in the authentication details.") + + bsn = submission.auth_info.value + + component.update( + { + "type": "selectboxes", + "fieldSet": False, + "inline": False, + "inputType": "checkbox", + } + ) + + if "mask" in component: + del component["mask"] + + existing_values = component.get("values", []) + empty_option = { + "label": "", + "value": "", + } + if not existing_values or existing_values[0] == empty_option: + handler = cls._get_handler() + # make the API call + # TODO: this should eventually be replaced with logic rules/variables that + # retrieve data from an "arbitrary source", which will cause the data to + # become available in the ``data`` argument instead. + child_choices = handler(bsn) + + component["values"] = [ + { + "label": label, + "value": value, + } + for value, label in child_choices + ] diff --git a/src/openforms/formio/dynamic_config/__init__.py b/src/openforms/formio/dynamic_config/__init__.py index e405f01f84..e97604f826 100644 --- a/src/openforms/formio/dynamic_config/__init__.py +++ b/src/openforms/formio/dynamic_config/__init__.py @@ -7,6 +7,7 @@ from rest_framework.request import Request +from openforms.submissions.models import Submission from openforms.typing import DataMapping from ..datastructures import FormioConfigurationWrapper @@ -17,6 +18,7 @@ def rewrite_formio_components( configuration_wrapper: FormioConfigurationWrapper, + submission: Submission, data: Optional[DataMapping] = None, ) -> FormioConfigurationWrapper: """ @@ -33,7 +35,7 @@ def rewrite_formio_components( """ data = data or {} # normalize for component in configuration_wrapper: - register.update_config(component, data=data) + register.update_config(component, submission=submission, data=data) return configuration_wrapper diff --git a/src/openforms/formio/registry.py b/src/openforms/formio/registry.py index b0d8de444c..a2b84d6f06 100644 --- a/src/openforms/formio/registry.py +++ b/src/openforms/formio/registry.py @@ -21,7 +21,6 @@ from openforms.plugins.registry import BaseRegistry from openforms.typing import DataMapping -from .datastructures import FormioConfigurationWrapper from .typing import Component if TYPE_CHECKING: @@ -74,7 +73,7 @@ def verbose_name(self): return _("{type} component").format(type=self.identifier.capitalize()) def mutate_config_dynamically( - self, component: Component, data: DataMapping + self, component: Component, submission: "Submission", data: DataMapping ) -> None: # pragma: nocover ... @@ -109,7 +108,10 @@ def format(self, component: Component, value: Any, as_html=False): return formatter(component, value) def update_config( - self, component: Component, data: DataMapping | None = None + self, + component: Component, + submission: "Submission", + data: DataMapping | None = None, ) -> None: """ Mutate the component configuration in place. @@ -118,19 +120,19 @@ def update_config( for example) to work. """ # if there is no plugin registered for the component, return the input - if (component_type := component["type"]) not in register: + if (component_type := component["type"]) not in self: return # invoke plugin if exists plugin = self[component_type] - plugin.mutate_config_dynamically(component, data) + plugin.mutate_config_dynamically(component, submission, data) def update_config_for_request(self, component: Component, request: Request) -> None: """ Mutate the component in place for the given request context. """ # if there is no plugin registered for the component, return the input - if (component_type := component["type"]) not in register: + if (component_type := component["type"]) not in self: return # invoke plugin if exists @@ -140,21 +142,6 @@ def update_config_for_request(self, component: Component, request: Request) -> N rewriter(component, request) - def handle_custom_types( - self, - configuration: FormioConfigurationWrapper, - submission: "Submission", - ): - """ - Process custom backend-only formio types. - - Formio types can be transformed in the context of a given - :class:`openforms.submissions.models.Submission` and ultimately manifest as - modified or standard Formio types, essentially performing some sort of "rewrite" - of the Formio configuration object. - """ - raise NotImplementedError() - # Sentinel to provide the default registry. You can easily instantiate another # :class:`Registry` object to use as dependency injection in tests. diff --git a/src/openforms/formio/service.py b/src/openforms/formio/service.py index 28d91bf47e..faf101c2ae 100644 --- a/src/openforms/formio/service.py +++ b/src/openforms/formio/service.py @@ -13,7 +13,6 @@ import elasticapm from rest_framework.request import Request -from openforms.forms.custom_field_types import handle_custom_types from openforms.prefill import inject_prefill from openforms.submissions.models import Submission from openforms.typing import DataMapping @@ -62,12 +61,7 @@ def get_dynamic_configuration( The configuration is modified in the context of the provided :arg:`submission`. """ - # TODO: see if we can make the config wrapper smart enough to deal with this - config_wrapper.configuration = handle_custom_types( - config_wrapper.configuration, submission=submission - ) - - rewrite_formio_components(config_wrapper, data=data) + rewrite_formio_components(config_wrapper, submission=submission, data=data) # prefill is still 'special' even though it uses variables, as we specifically # set the `defaultValue` key to the resulting variable. diff --git a/src/openforms/forms/custom_field_types.py b/src/openforms/forms/custom_field_types.py deleted file mode 100644 index 37899deb47..0000000000 --- a/src/openforms/forms/custom_field_types.py +++ /dev/null @@ -1,49 +0,0 @@ -from typing import Any, Dict - -import elasticapm - -from openforms.submissions.models import Submission - -__all__ = ["register", "unregister", "handle_custom_types"] - -REGISTRY = {} - - -def register(custom_type: str): - def decorator(handler: callable): - if custom_type in REGISTRY: - raise ValueError(f"Custom type {custom_type} is already registered.") - - REGISTRY[custom_type] = handler - - return decorator - - -def unregister(custom_type: str): - if custom_type in REGISTRY: - del REGISTRY[custom_type] - - -@elasticapm.capture_span(span_type="app.formio") -def handle_custom_types( - configuration: Dict[str, Any], - submission: Submission, -) -> Dict[str, Any]: - - rewritten_components = [] - - for component in configuration["components"]: - type_key = component["type"] - - # no handler -> leave untouched - if type_key not in REGISTRY: - rewritten_components.append(component) - continue - - # if there is a handler, invoke it - handler = REGISTRY[type_key] - rewritten_components.append(handler(component, submission)) - - return { - "components": rewritten_components, - } diff --git a/src/openforms/submissions/tests/test_get_submission_step.py b/src/openforms/submissions/tests/test_get_submission_step.py index 201fb3fa2d..a12657405c 100644 --- a/src/openforms/submissions/tests/test_get_submission_step.py +++ b/src/openforms/submissions/tests/test_get_submission_step.py @@ -9,6 +9,7 @@ aware step definition. """ import uuid +from unittest.mock import patch from django.test import tag @@ -17,7 +18,9 @@ from rest_framework.test import APITestCase from openforms.accounts.tests.factories import StaffUserFactory, SuperUserFactory -from openforms.forms.custom_field_types import register, unregister +from openforms.formio.components.vanilla import SelectBoxes, TextField +from openforms.formio.formatters.formio import TextFieldFormatter +from openforms.formio.registry import BasePlugin, ComponentRegistry from openforms.forms.tests.factories import ( FormFactory, FormLogicFactory, @@ -112,15 +115,21 @@ def test_static_form_definition(self): self.assertEqual(response.json(), expected) def test_dynamic_form_definition(self): + register = ComponentRegistry() + register("selectboxes")(SelectBoxes) + @register("textfield") - def custom_handler(component: dict, request, submission): - component["label"] = "Rewritten label" - return component + class TextField(BasePlugin): + formatter = TextFieldFormatter + + @staticmethod + def mutate_config_dynamically(component, submission, data): + component["label"] = "Rewritten label" - self.addCleanup(lambda: unregister("textfield")) self._add_submission_to_session(self.submission) - response = self.client.get(self.step_url) + with patch("openforms.formio.dynamic_config.register", new=register): + response = self.client.get(self.step_url) self.assertEqual(response.status_code, status.HTTP_200_OK) expected = { @@ -443,18 +452,26 @@ def test_dynamic_date_component_config_based_on_variables(self): self.assertEqual(date2["datePicker"]["maxDate"], "2022-12-31T00:00:00+01:00") def test_custom_components_and_form_logic(self): - # set up custom field type for test only - self.addCleanup(lambda: unregister("testCustomType")) + register = ComponentRegistry() + register("textfield")(TextField) @register("testCustomType") - def custom_type(component, request, submission): - return { - "type": "textfield", - "key": component["key"], - "defaultValue": "testCustomType", - "hidden": False, - } + class CustomType(BasePlugin): + formatter = TextFieldFormatter + + @staticmethod + def mutate_config_dynamically(component, submission, data): + key = component["key"] + component.clear() + component.update( + { + "type": "textfield", + "key": key, + "defaultValue": "testCustomType", + "hidden": False, + } + ) form = FormFactory.create( generate_minimal_setup=True, @@ -504,7 +521,8 @@ def custom_type(component, request, submission): ) self._add_submission_to_session(submission) - response = self.client.get(endpoint) + with patch("openforms.formio.dynamic_config.register", new=register): + response = self.client.get(endpoint) self.assertEqual(response.status_code, status.HTTP_200_OK) components = response.data["form_step"]["configuration"]["components"]