From 69224aef187eb19700a2d79c26daeec5e33adb76 Mon Sep 17 00:00:00 2001 From: Steven Bal Date: Mon, 6 Jan 2025 16:30:49 +0100 Subject: [PATCH 1/2] :loud_sound: [#4927] System check for missing allow_blank on non-required charfields because in most cases it is desired to allow empty strings as well --- src/openforms/utils/checks.py | 65 +++++++++++++++++++++++++++++++++++ 1 file changed, 65 insertions(+) diff --git a/src/openforms/utils/checks.py b/src/openforms/utils/checks.py index d50d2aab22..c09fecb9eb 100644 --- a/src/openforms/utils/checks.py +++ b/src/openforms/utils/checks.py @@ -1,11 +1,21 @@ +import inspect +import logging import os from django.conf import settings +from django.contrib.auth.models import AnonymousUser +from django.contrib.sessions.backends.cache import SessionStore from django.core.checks import Error, Warning, register +from django.db import ProgrammingError from django.forms import ModelForm +from psycopg.errors import DatabaseError +from rest_framework.serializers import CharField, Serializer, empty +from rest_framework.test import APIRequestFactory from treebeard.forms import MoveNodeForm +logger = logging.getLogger(__name__) + def get_subclasses(cls): for subclass in cls.__subclasses__(): @@ -81,3 +91,58 @@ def check_missing_init_files(app_configs, **kwargs): ) return errors + + +@register +def check_serializer_non_required_charfield_allow_blank_true( # pragma: no cover + app_configs, **kwargs +): + """ + Check for serializers.CharFields that have ``required=False``, but not ``allow_blank=True`` + to avoid bogus validation errors occurring when empty strings are provided by the frontend. + """ + request = APIRequestFactory().get("/") + request.user = AnonymousUser() + request.session = SessionStore() + + errors = [] + serializers = get_subclasses(Serializer) + for serializer_class in serializers: + serializer_defined_in = inspect.getfile(serializer_class) + if not serializer_defined_in.startswith(settings.DJANGO_PROJECT_DIR): + continue # ignore code not defined in our own codebase + + if hasattr(serializer_class, "Meta") and not hasattr( + serializer_class.Meta, "model" + ): + continue + + try: + serializer = serializer_class(context={"request": request}) + fields = serializer.fields + except (ProgrammingError, DatabaseError) as e: + logger.debug(f"Could not instantiate {serializer_class}: {e}") + continue + + for field_name, field in fields.items(): + if not isinstance(field, CharField) or field.read_only: + continue + + if ( + not field.required + and field.default in ("", None, empty) + and not field.allow_blank + ): + file_path = inspect.getfile(serializer_class) + + errors.append( + Warning( + ( + f"{serializer_class.__module__}.{serializer_class.__qualname__}.{field_name} does not have `allow_blank=True`\n" + f"{file_path}" + ), + hint="Consider setting `allow_blank=True` to allow providing empty string values", + id="utils.W002", + ) + ) + return errors From ecea9718f5dcc64055a9dd44fbd931d0fc9f3d52 Mon Sep 17 00:00:00 2001 From: Steven Bal Date: Mon, 6 Jan 2025 16:31:16 +0100 Subject: [PATCH 2/2] :rotating_light: [#4927] Add missing allow_blank=True for several fields --- src/openapi.yaml | 4 ---- src/openforms/config/api/serializers.py | 1 + src/openforms/contrib/kadaster/api/serializers.py | 1 + src/openforms/contrib/zgw/api/filters.py | 2 ++ src/openforms/contrib/zgw/api/serializers.py | 1 + src/openforms/contrib/zgw/serializers.py | 2 ++ src/openforms/formio/components/vanilla.py | 3 ++- src/openforms/forms/api/serializers/form_definition.py | 1 + src/openforms/forms/api/serializers/form_step.py | 4 +++- src/openforms/prefill/api/serializers.py | 1 + src/openforms/registrations/contrib/objects_api/config.py | 1 + src/openforms/registrations/contrib/zgw_apis/options.py | 6 ++++++ src/openforms/submissions/api/serializers.py | 2 ++ src/openforms/validations/api/serializers.py | 1 + 14 files changed, 24 insertions(+), 6 deletions(-) diff --git a/src/openapi.yaml b/src/openapi.yaml index 135bee6329..72d2fd328c 100644 --- a/src/openapi.yaml +++ b/src/openapi.yaml @@ -3238,7 +3238,6 @@ paths: schema: type: string default: '' - minLength: 1 description: Filter document types for a given case type. The identification is unique within the catalogue for a case type. Note that multiple versions of the same case type with the same identification exist. The filter returns @@ -3249,7 +3248,6 @@ paths: type: string format: uri default: '' - minLength: 1 description: Filter document types against this catalogue URL. - in: query name: objects_api_group @@ -3942,7 +3940,6 @@ paths: schema: type: string default: '' - minLength: 1 description: Filter document types for a given case type. The identification is unique within the catalogue for a case type. Note that multiple versions of the same case type with the same identification exist. The filter returns @@ -3953,7 +3950,6 @@ paths: type: string format: uri default: '' - minLength: 1 description: Filter document types against this catalogue URL. - in: query name: zgw_api_group diff --git a/src/openforms/config/api/serializers.py b/src/openforms/config/api/serializers.py index d643f86b86..e0dd0bae1d 100644 --- a/src/openforms/config/api/serializers.py +++ b/src/openforms/config/api/serializers.py @@ -26,6 +26,7 @@ class PrivacyPolicyInfoSerializer(serializers.Serializer): "The formatted label to use next to the checkbox when asking the user to agree to the privacy policy." ), required=False, + allow_blank=True, ) diff --git a/src/openforms/contrib/kadaster/api/serializers.py b/src/openforms/contrib/kadaster/api/serializers.py index fe6ff44f07..ac27325cf1 100644 --- a/src/openforms/contrib/kadaster/api/serializers.py +++ b/src/openforms/contrib/kadaster/api/serializers.py @@ -22,6 +22,7 @@ class GetStreetNameAndCityViewResultSerializer(serializers.Serializer): label=_("city and street name secret"), help_text=_("Secret for the combination of city and street name"), required=False, + allow_blank=True, ) diff --git a/src/openforms/contrib/zgw/api/filters.py b/src/openforms/contrib/zgw/api/filters.py index 6d2b5a8fa8..37e0ca8a99 100644 --- a/src/openforms/contrib/zgw/api/filters.py +++ b/src/openforms/contrib/zgw/api/filters.py @@ -17,6 +17,7 @@ class DocumentTypesFilter(serializers.Serializer): help_text=_("Filter document types against this catalogue URL."), required=False, default="", + allow_blank=True, ) case_type_identification = serializers.CharField( required=False, @@ -28,6 +29,7 @@ class DocumentTypesFilter(serializers.Serializer): "document type if it occurs within any version of the specified case type." ), default="", + allow_blank=True, ) def validate(self, attrs): diff --git a/src/openforms/contrib/zgw/api/serializers.py b/src/openforms/contrib/zgw/api/serializers.py index d5e5f3bacb..fe02a70f4b 100644 --- a/src/openforms/contrib/zgw/api/serializers.py +++ b/src/openforms/contrib/zgw/api/serializers.py @@ -107,4 +107,5 @@ class CaseTypeProductSerializer(serializers.Serializer): label=_("description"), help_text=_("The description of a product bound to a case type. "), required=False, + allow_blank=True, ) diff --git a/src/openforms/contrib/zgw/serializers.py b/src/openforms/contrib/zgw/serializers.py index 0c68d33404..cdf289285f 100644 --- a/src/openforms/contrib/zgw/serializers.py +++ b/src/openforms/contrib/zgw/serializers.py @@ -30,6 +30,7 @@ class CatalogueSerializer(serializers.Serializer): ), default="", validators=[validate_uppercase], + allow_blank=True, ) rsin = serializers.CharField( label=_("RSIN"), @@ -40,6 +41,7 @@ class CatalogueSerializer(serializers.Serializer): "The 'rsin' attribute for the Catalogus resource in the Catalogi API." ), default="", + allow_blank=True, ) class Meta: diff --git a/src/openforms/formio/components/vanilla.py b/src/openforms/formio/components/vanilla.py index 882705cff5..5b9823ab77 100644 --- a/src/openforms/formio/components/vanilla.py +++ b/src/openforms/formio/components/vanilla.py @@ -343,7 +343,8 @@ class FileSerializer(serializers.Serializer): data = FileDataSerializer() # type: ignore def __init__(self, *args, **kwargs) -> None: - self.mime_type_validator = MimeTypeValidator(kwargs.pop("allowed_mime_types")) + allowed_mime_types = kwargs.pop("allowed_mime_types", []) + self.mime_type_validator = MimeTypeValidator(allowed_mime_types) super().__init__(*args, **kwargs) def validate(self, attrs: dict[str, Any]) -> dict[str, Any]: diff --git a/src/openforms/forms/api/serializers/form_definition.py b/src/openforms/forms/api/serializers/form_definition.py index f6efe81eae..02e6dfd7b5 100644 --- a/src/openforms/forms/api/serializers/form_definition.py +++ b/src/openforms/forms/api/serializers/form_definition.py @@ -106,6 +106,7 @@ class Meta: "name": {"read_only": True}, # writing is done via the `translations` field "slug": { "required": False, + "allow_blank": True, }, } diff --git a/src/openforms/forms/api/serializers/form_step.py b/src/openforms/forms/api/serializers/form_step.py index dd615363a5..e0d14657c0 100644 --- a/src/openforms/forms/api/serializers/form_step.py +++ b/src/openforms/forms/api/serializers/form_step.py @@ -62,7 +62,8 @@ class Meta: extra_kwargs = { "uuid": { "read_only": True, - } + }, + "slug": {"allow_blank": True}, } @@ -137,6 +138,7 @@ class Meta: "uuid": { "read_only": True, }, + "slug": {"allow_blank": True}, } validators = [FormStepIsApplicableIfFirstValidator()] diff --git a/src/openforms/prefill/api/serializers.py b/src/openforms/prefill/api/serializers.py index 00b3d10edc..4c03b23f6f 100644 --- a/src/openforms/prefill/api/serializers.py +++ b/src/openforms/prefill/api/serializers.py @@ -33,6 +33,7 @@ class PrefillPluginQueryParameterSerializer(serializers.Serializer): required=False, label=_("Form.io component type"), help_text=_("Only return plugins applicable for the specified component type."), + allow_blank=True, ) def to_internal_value(self, data): diff --git a/src/openforms/registrations/contrib/objects_api/config.py b/src/openforms/registrations/contrib/objects_api/config.py index 409cbf42e7..6bf9c5c459 100644 --- a/src/openforms/registrations/contrib/objects_api/config.py +++ b/src/openforms/registrations/contrib/objects_api/config.py @@ -133,6 +133,7 @@ class ObjectsAPIOptionsSerializer(JsonSchemaSerializerMixin, serializers.Seriali validators=[validate_rsin], help_text=_("RSIN of organization, which creates the INFORMATIEOBJECT."), required=False, + allow_blank=True, ) iot_submission_report = serializers.CharField( diff --git a/src/openforms/registrations/contrib/zgw_apis/options.py b/src/openforms/registrations/contrib/zgw_apis/options.py index 11a150c62e..9f4501075a 100644 --- a/src/openforms/registrations/contrib/zgw_apis/options.py +++ b/src/openforms/registrations/contrib/zgw_apis/options.py @@ -72,6 +72,7 @@ class ZaakOptionsSerializer(JsonSchemaSerializerMixin, serializers.Serializer): "timestamp. When you specify this field, you MUST also specify a catalogue." ), default="", + allow_blank=True, ) document_type_description = serializers.CharField( required=False, # either htis field or informatieobjecttype (legacy) must be provided @@ -84,6 +85,7 @@ class ZaakOptionsSerializer(JsonSchemaSerializerMixin, serializers.Serializer): "are valid." ), default="", + allow_blank=True, ) product_url = serializers.URLField( label=_("Product url"), @@ -113,6 +115,7 @@ class ZaakOptionsSerializer(JsonSchemaSerializerMixin, serializers.Serializer): required=False, validators=[validate_rsin], help_text=_("RSIN of organization, which creates the ZAAK"), + allow_blank=True, ) zaak_vertrouwelijkheidaanduiding = serializers.ChoiceField( label=_("Vertrouwelijkheidaanduiding"), @@ -127,6 +130,7 @@ class ZaakOptionsSerializer(JsonSchemaSerializerMixin, serializers.Serializer): help_text=_( "Description (omschrijving) of the ROLTYPE to use for employees filling in a form for a citizen/company." ), + allow_blank=True, ) # Objects API @@ -149,6 +153,7 @@ class ZaakOptionsSerializer(JsonSchemaSerializerMixin, serializers.Serializer): "3) data (the submitted form data)" ), required=False, + allow_blank=True, ) objecttype_version = serializers.IntegerField( label=_("objects API - objecttype version"), @@ -166,6 +171,7 @@ class ZaakOptionsSerializer(JsonSchemaSerializerMixin, serializers.Serializer): ), ], required=False, + allow_blank=True, ) # Eigenschappen diff --git a/src/openforms/submissions/api/serializers.py b/src/openforms/submissions/api/serializers.py index 341532f99e..0e19267a0f 100644 --- a/src/openforms/submissions/api/serializers.py +++ b/src/openforms/submissions/api/serializers.py @@ -461,11 +461,13 @@ class SubmissionProcessingStatusSerializer(serializers.Serializer): label=_("Confirmation page title"), required=False, help_text=_("Title of the confirmation page."), + allow_blank=True, ) confirmation_page_content = CSPPostProcessedHTMLField( label=_("Confirmation page content"), required=False, help_text=_("Body text of the confirmation page. May contain HTML!"), + allow_blank=True, ) report_download_url = serializers.URLField( label=_("Report download URL"), diff --git a/src/openforms/validations/api/serializers.py b/src/openforms/validations/api/serializers.py index 418a32fc26..263a84a057 100644 --- a/src/openforms/validations/api/serializers.py +++ b/src/openforms/validations/api/serializers.py @@ -24,6 +24,7 @@ class ValidatorsFilterSerializer(serializers.Serializer): help_text=_( "Only return validators applicable for the specified component type." ), + allow_blank=True, ) def to_internal_value(self, data):