Skip to content

Commit

Permalink
Merge pull request #4136 from open-formulieren/issue/4121-plugin-vali…
Browse files Browse the repository at this point in the history
…dators-must-be-or-instead-of-and

Fix plugin validators being AND-ed instead of OR-ed in the backend validation.
  • Loading branch information
sergei-maertens authored Apr 8, 2024
2 parents 046c340 + 107845c commit b44b3e6
Show file tree
Hide file tree
Showing 4 changed files with 53 additions and 15 deletions.
4 changes: 2 additions & 2 deletions src/openforms/formio/components/custom.py
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,7 @@ def build_serializer_field(
)

if plugin_ids := validate.get("plugins", []):
validators += [PluginValidator(plugin) for plugin in plugin_ids]
validators.append(PluginValidator(plugin_ids))

if validators:
extra["validators"] = validators
Expand Down Expand Up @@ -290,7 +290,7 @@ def build_serializer_field(

validators = [BSNValidator()]
if plugin_ids := validate.get("plugins", []):
validators += [PluginValidator(plugin) for plugin in plugin_ids]
validators.append(PluginValidator(plugin_ids))

extra["validators"] = validators

Expand Down
12 changes: 6 additions & 6 deletions src/openforms/formio/components/vanilla.py
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ def build_serializer_field(

# Run plugin validators at the end after all basic checks have been performed.
if plugin_ids := validate.get("plugins", []):
validators += [PluginValidator(plugin) for plugin in plugin_ids]
validators.append(PluginValidator(plugin_ids))

if validators:
extra["validators"] = validators
Expand Down Expand Up @@ -144,7 +144,7 @@ def build_serializer_field(

validators = []
if plugin_ids := validate.get("plugins", []):
validators += [PluginValidator(plugin) for plugin in plugin_ids]
validators.append(PluginValidator(plugin_ids))

if validators:
extra["validators"] = validators
Expand Down Expand Up @@ -271,7 +271,7 @@ def build_serializer_field(

# Run plugin validators at the end after all basic checks have been performed.
if plugin_ids := validate.get("plugins", []):
validators += [PluginValidator(plugin) for plugin in plugin_ids]
validators.append(PluginValidator(plugin_ids))

if validators:
extra["validators"] = validators
Expand Down Expand Up @@ -359,7 +359,7 @@ def build_serializer_field(

validators = []
if plugin_ids := validate.get("plugins", []):
validators += [PluginValidator(plugin) for plugin in plugin_ids]
validators.append(PluginValidator(plugin_ids))

if validators:
extra["validators"] = validators
Expand Down Expand Up @@ -400,7 +400,7 @@ def build_serializer_field(self, component: Component) -> serializers.BooleanFie
if required:
validators.append(validate_required_checkbox)
if plugin_ids := validate.get("plugins", []):
validators += [PluginValidator(plugin) for plugin in plugin_ids]
validators.append(PluginValidator(plugin_ids))

if validators:
extra["validators"] = validators
Expand Down Expand Up @@ -493,7 +493,7 @@ def build_serializer_field(self, component: Component) -> serializers.FloatField

validators = []
if plugin_ids := validate.get("plugins", []):
validators += [PluginValidator(plugin) for plugin in plugin_ids]
validators.append(PluginValidator(plugin_ids))

if validators:
extra["validators"] = validators
Expand Down
19 changes: 19 additions & 0 deletions src/openforms/formio/tests/validation/test_phonenumber.py
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,25 @@ def test_phonenumber_with_plugin_validator(self, validator: str):
error = extract_error(errors, "foo")
self.assertEqual(error.code, "invalid")

@tag("gh-4121")
def test_validators_are_or_rather_than_and(self):
component: Component = {
"type": "phoneNumber",
"key": "foo",
"label": "Phone",
"validate": {
"plugins": [
"phonenumber-international",
"phonenumber-nl",
]
},
}
data: JSONValue = {"foo": "0633975328"}

is_valid, _ = validate_formio_data(component, data)

self.assertTrue(is_valid)

@tag("gh-4068")
def test_multiple_with_form_builder_empty_defaults(self):
# Our own form builder does funky stuff here by setting the defaultValue to
Expand Down
33 changes: 26 additions & 7 deletions src/openforms/validations/drf_validators.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@
.. todo:: use type hints from drf-stubs when we add those to base dependencies.
"""

from collections.abc import Iterable

from rest_framework import serializers

from openforms.submissions.models import Submission
Expand All @@ -22,18 +24,35 @@ class PluginValidator:

requires_context = True

def __init__(self, plugin: str):
assert plugin in register, "Invalid plugin identifier specified"
self.plugin = plugin
def __init__(self, plugins: Iterable[str]):
assert all(
plugin in register for plugin in plugins
), "Invalid plugin identifier specified"
self.plugins = plugins

# FIXME: after deserializing the input data, this may actually be any python type,
# so the JSONValue type hint is not accurate.
def __call__(self, value: JSONValue, field: serializers.Field) -> None:
"""
Validate that the data conforms to at least one of the specified plugins.
Validation plugins on our Formio components operate on an OR-basis rather than
AND - the input is considered valid as soon as one of the validators treats
it as valid.
"""
submission = field.context.get("submission", None)
assert isinstance(
submission, Submission
), "You must pass the submission in the serializer context"
result = register.validate(self.plugin, value=value, submission=submission)
if result.is_valid:
return
raise serializers.ValidationError(result.messages, code="invalid")

# evaluate all plugins to collect the error messages in case none is valid
messages: list[str] = []
for plugin in self.plugins:
result = register.validate(plugin, value=value, submission=submission)
# as soon as one is valid -> abort, there are no validation messages to
# display
if result.is_valid:
return
messages += result.messages

raise serializers.ValidationError(messages, code="invalid")

0 comments on commit b44b3e6

Please sign in to comment.