Skip to content

Commit

Permalink
[#72] PR feedback
Browse files Browse the repository at this point in the history
  • Loading branch information
vaszig committed Mar 26, 2024
1 parent 7030ec3 commit 3aa3579
Show file tree
Hide file tree
Showing 4 changed files with 59 additions and 33 deletions.
6 changes: 6 additions & 0 deletions src/openforms/formio/api/validators.py
Original file line number Diff line number Diff line change
Expand Up @@ -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."))

Check warning on line 166 in src/openforms/formio/api/validators.py

View check run for this annotation

Codecov / codecov/patch

src/openforms/formio/api/validators.py#L166

Added line #L166 was not covered by tests
22 changes: 17 additions & 5 deletions src/openforms/formio/components/vanilla.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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 = []
Expand Down Expand Up @@ -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")
Expand Down Expand Up @@ -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 = []
Expand Down
31 changes: 10 additions & 21 deletions src/openforms/tests/e2e/input_validation_base.py
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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],
):
"""
Expand All @@ -75,31 +74,31 @@ 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,
form: Form,
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)
Expand All @@ -108,24 +107,15 @@ 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.
await page.get_by_role("button", name="Volgende").click()

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)
Expand All @@ -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
Expand Down
33 changes: 26 additions & 7 deletions src/openforms/tests/e2e/test_input_validation.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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",
Expand All @@ -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",
Expand All @@ -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 = {
Expand Down Expand Up @@ -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",
Expand All @@ -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.",
)
Expand Down Expand Up @@ -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",
Expand All @@ -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.",
)

Expand All @@ -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",
Expand All @@ -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.",
)

Expand Down

0 comments on commit 3aa3579

Please sign in to comment.