Skip to content

Commit

Permalink
Merge pull request #4989 from open-formulieren/feature/4927-serialize…
Browse files Browse the repository at this point in the history
…r-allow-blank-system-check

🔊 [#4927] Add system check for missing allow_blank on non-required charfields
  • Loading branch information
sergei-maertens authored Jan 14, 2025
2 parents 41616e3 + ecea971 commit 95b628e
Show file tree
Hide file tree
Showing 15 changed files with 89 additions and 6 deletions.
4 changes: 0 additions & 4 deletions src/openapi.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down
1 change: 1 addition & 0 deletions src/openforms/config/api/serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
)


Expand Down
1 change: 1 addition & 0 deletions src/openforms/contrib/kadaster/api/serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
)


Expand Down
2 changes: 2 additions & 0 deletions src/openforms/contrib/zgw/api/filters.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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):
Expand Down
1 change: 1 addition & 0 deletions src/openforms/contrib/zgw/api/serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
)
2 changes: 2 additions & 0 deletions src/openforms/contrib/zgw/serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ class CatalogueSerializer(serializers.Serializer):
),
default="",
validators=[validate_uppercase],
allow_blank=True,
)
rsin = serializers.CharField(
label=_("RSIN"),
Expand All @@ -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:
Expand Down
3 changes: 2 additions & 1 deletion src/openforms/formio/components/vanilla.py
Original file line number Diff line number Diff line change
Expand Up @@ -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]:
Expand Down
1 change: 1 addition & 0 deletions src/openforms/forms/api/serializers/form_definition.py
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,7 @@ class Meta:
"name": {"read_only": True}, # writing is done via the `translations` field
"slug": {
"required": False,
"allow_blank": True,
},
}

Expand Down
4 changes: 3 additions & 1 deletion src/openforms/forms/api/serializers/form_step.py
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,8 @@ class Meta:
extra_kwargs = {
"uuid": {
"read_only": True,
}
},
"slug": {"allow_blank": True},
}


Expand Down Expand Up @@ -137,6 +138,7 @@ class Meta:
"uuid": {
"read_only": True,
},
"slug": {"allow_blank": True},
}
validators = [FormStepIsApplicableIfFirstValidator()]

Expand Down
1 change: 1 addition & 0 deletions src/openforms/prefill/api/serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,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):
Expand Down
1 change: 1 addition & 0 deletions src/openforms/registrations/contrib/objects_api/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
6 changes: 6 additions & 0 deletions src/openforms/registrations/contrib/zgw_apis/options.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -84,6 +85,7 @@ class ZaakOptionsSerializer(JsonSchemaSerializerMixin, serializers.Serializer):
"are valid."
),
default="",
allow_blank=True,
)
product_url = serializers.URLField(
label=_("Product url"),
Expand Down Expand Up @@ -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"),
Expand All @@ -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
Expand All @@ -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"),
Expand All @@ -166,6 +171,7 @@ class ZaakOptionsSerializer(JsonSchemaSerializerMixin, serializers.Serializer):
),
],
required=False,
allow_blank=True,
)

# Eigenschappen
Expand Down
2 changes: 2 additions & 0 deletions src/openforms/submissions/api/serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"),
Expand Down
65 changes: 65 additions & 0 deletions src/openforms/utils/checks.py
Original file line number Diff line number Diff line change
@@ -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__():
Expand Down Expand Up @@ -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
1 change: 1 addition & 0 deletions src/openforms/validations/api/serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down

0 comments on commit 95b628e

Please sign in to comment.