From 3aa35799a78964477ac50b4d6f2f66c05c82ac17 Mon Sep 17 00:00:00 2001 From: vasileios Date: Tue, 26 Mar 2024 15:11:22 +0100 Subject: [PATCH] [#72] PR feedback --- src/openforms/formio/api/validators.py | 6 ++++ src/openforms/formio/components/vanilla.py | 22 ++++++++++--- .../tests/e2e/input_validation_base.py | 31 ++++++----------- .../tests/e2e/test_input_validation.py | 33 +++++++++++++++---- 4 files changed, 59 insertions(+), 33 deletions(-) diff --git a/src/openforms/formio/api/validators.py b/src/openforms/formio/api/validators.py index 3d214d80d5..6e0c45cad5 100644 --- a/src/openforms/formio/api/validators.py +++ b/src/openforms/formio/api/validators.py @@ -158,3 +158,9 @@ def __call__(self, uploaded_file: UploadedFile) -> None: raise serializers.ValidationError( _("The virus scan returned an unexpected status.") ) + + +class TruthyBooleanValueValidator: + def __call__(self, value): + if not value: + raise serializers.ValidationError(_("This field must be true.")) diff --git a/src/openforms/formio/components/vanilla.py b/src/openforms/formio/components/vanilla.py index 237e9d549b..975c89d138 100644 --- a/src/openforms/formio/components/vanilla.py +++ b/src/openforms/formio/components/vanilla.py @@ -22,6 +22,7 @@ from openforms.utils.urls import build_absolute_uri from openforms.validations.service import PluginValidator +from ..api.validators import TruthyBooleanValueValidator from ..dynamic_config.dynamic_options import add_options_to_config from ..formatters.formio import ( CheckboxFormatter, @@ -266,9 +267,9 @@ def build_serializer_field( required = validate.get("required", False) extra = {} - if max_value := validate.get("max"): + if (max_value := validate.get("max")) is not None: extra["max_value"] = max_value - if min_value := validate.get("min"): + if (min_value := validate.get("min")) is not None: extra["min_value"] = min_value validators = [] @@ -296,7 +297,18 @@ class Checkbox(BasePlugin[Component]): def build_serializer_field(self, component: Component) -> serializers.BooleanField: validate = component.get("validate", {}) required = validate.get("required", False) - return serializers.BooleanField(required=required) + + # dynamically add in more kwargs based on the component configuration + extra = {} + + validators = [TruthyBooleanValueValidator()] if required else [] + if plugin_ids := validate.get("plugins", []): + validators += [PluginValidator(plugin) for plugin in plugin_ids] + + if validators: + extra["validators"] = validators + + return serializers.BooleanField(**extra) @register("selectboxes") @@ -362,9 +374,9 @@ def build_serializer_field(self, component: Component) -> serializers.FloatField required = validate.get("required", False) extra = {} - if max_value := validate.get("max"): + if (max_value := validate.get("max")) is not None: extra["max_value"] = max_value - if min_value := validate.get("min"): + if (min_value := validate.get("min")) is not None: extra["min_value"] = min_value validators = [] diff --git a/src/openforms/tests/e2e/input_validation_base.py b/src/openforms/tests/e2e/input_validation_base.py index 159c0cbccf..0b9c7cd0a8 100644 --- a/src/openforms/tests/e2e/input_validation_base.py +++ b/src/openforms/tests/e2e/input_validation_base.py @@ -1,4 +1,4 @@ -from typing import Literal, TypedDict +from typing import TypedDict from django.test import override_settings, tag from django.urls import reverse @@ -58,7 +58,6 @@ def assertValidationIsAligned( component: Component, ui_input: str | int | float, expected_ui_error: str, - input_type: Literal["text", "other"] = "text", **kwargs: Unpack[ValidationKwargs], ): """ @@ -75,23 +74,24 @@ def assertValidationIsAligned( :arg ui_input: Input to enter in the field with playwright. :arg expected_ui_error: Content of the validation error to be expected in the playwright test. - :input_type: Depending on the literal we fill in the input or we leave the element - unmodified (e.g. checkbox, map, signature etc.). """ form = create_form(component) with self.subTest("frontend validation"): self._assertFrontendValidation( - form, component["label"], str(ui_input), expected_ui_error, input_type + form, component["label"], str(ui_input), expected_ui_error ) with self.subTest("backend validation"): api_value = kwargs.pop("api_value", ui_input) - self._assertBackendValidation(form, component["key"], api_value, input_type) + self._assertBackendValidation(form, component["key"], api_value) def _locate_input(self, page: Page, label: str): return page.get_by_label(label, exact=True) + async def apply_ui_input(self, page: Page, label: str, ui_input: str | int | float): + await self._locate_input(page, label).fill(ui_input) + @async_to_sync() async def _assertFrontendValidation( self, @@ -99,7 +99,6 @@ async def _assertFrontendValidation( label: str, ui_input: str, expected_ui_error: str, - input_type: str, ): frontend_path = reverse("forms:form-detail", kwargs={"slug": form.slug}) url = str(furl(self.live_server_url) / frontend_path) @@ -108,9 +107,7 @@ async def _assertFrontendValidation( await page.goto(url) await page.get_by_role("button", name="Formulier starten").click() - # fill in the test input in case of text otherwise continue - if input_type == "text": - await self._locate_input(page, label).fill(ui_input) + await self.apply_ui_input(page, label, ui_input) # try to submit the step which should be invalid, so we expect this to # render the error message. @@ -118,14 +115,7 @@ async def _assertFrontendValidation( await expect(page.get_by_text(expected_ui_error)).to_be_visible() - def _assertBackendValidation( - self, form: Form, key: str, value: JSONValue, input_type: str - ): - body_mappings = { - "text": {"data": {key: value}}, - "other": {"input": value}, - } - + def _assertBackendValidation(self, form: Form, key: str, value: JSONValue): submission = SubmissionFactory.create(form=form) step = form.formstep_set.get() self._add_submission_to_session(submission) @@ -137,9 +127,8 @@ def _assertBackendValidation( "step_uuid": step.uuid, }, ) - response = self.client.put( - step_endpoint, body_mappings[input_type], content_type="application/json" - ) + body = {"data": {key: value}} + response = self.client.put(step_endpoint, body, content_type="application/json") assert response.status_code in [201, 200] # try to complete the submission, this is expected to fail diff --git a/src/openforms/tests/e2e/test_input_validation.py b/src/openforms/tests/e2e/test_input_validation.py index f272af0935..b5c1dbf564 100644 --- a/src/openforms/tests/e2e/test_input_validation.py +++ b/src/openforms/tests/e2e/test_input_validation.py @@ -9,7 +9,7 @@ from pathlib import Path -from playwright.async_api import Page +from playwright.async_api import Page, expect from openforms.formio.typing import Component, DateComponent @@ -187,7 +187,7 @@ def test_required_field(self): expected_ui_error="Het verplichte veld Required bsn is niet ingevuld.", ) - def test_elfproef(self): + def test_elfproef_invalid_num_chars(self): component: Component = { "type": "bsn", "key": "requiredBSN", @@ -199,6 +199,13 @@ def test_elfproef(self): ui_input="1234", expected_ui_error="Ongeldig BSN", ) + + def test_elfproef_invalid_bsn(self): + component: Component = { + "type": "bsn", + "key": "requiredBSN", + "label": "Required bsn", + } self.assertValidationIsAligned( component, ui_input="123456781", @@ -208,12 +215,12 @@ def test_elfproef(self): class SingleDateTests(ValidationsTestCase): - def _locate_input(self, page: Page, label: str): + async def apply_ui_input(self, page: Page, label: str, ui_input: str = "") -> None: # fix the input resolution because the formio datepicker is not accessible label_node = page.get_by_text(label, exact=True) label_parent = label_node.locator("xpath=../..") input_node = label_parent.get_by_role("textbox", include_hidden=False) - return input_node + await input_node.fill(ui_input) def test_required_field(self): component: DateComponent = { @@ -313,6 +320,12 @@ def test_max_date_fixed_value(self): class SingleCheckboxTests(ValidationsTestCase): + + async def apply_ui_input(self, page: Page, label: str, ui_input: str | int | float): + await expect( + page.get_by_role("checkbox", name="Required checkbox") + ).not_to_be_checked() + def test_required_field(self): component: Component = { "type": "checkbox", @@ -324,7 +337,6 @@ def test_required_field(self): self.assertValidationIsAligned( component, ui_input="", - input_type="other", api_value=False, expected_ui_error="Het verplichte veld Required checkbox is niet ingevuld.", ) @@ -375,6 +387,11 @@ def test_max_value(self): class SingleMapTests(ValidationsTestCase): + async def apply_ui_input(self, page: Page, label: str, ui_input: str | int | float): + page.wait_for_selector( + ".openforms-leaflet-map, [aria-label='Required map']", state="visible" + ) + def test_required_field(self): component: Component = { "type": "map", @@ -386,7 +403,6 @@ def test_required_field(self): self.assertValidationIsAligned( component, ui_input="", - input_type="other", expected_ui_error="Het verplichte veld Required map is niet ingevuld.", ) @@ -408,6 +424,10 @@ def test_required_field(self): class SingleSignatureTests(ValidationsTestCase): + + async def apply_ui_input(self, page: Page, label: str, ui_input: str | int | float): + page.wait_for_selector("[aria-label='Required signature']", state="visible") + def test_required_field(self): component: Component = { "type": "signature", @@ -419,7 +439,6 @@ def test_required_field(self): self.assertValidationIsAligned( component, ui_input="", - input_type="other", expected_ui_error="Het verplichte veld Required signature is niet ingevuld.", )