Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

🔊 [#4927] Add system check for missing allow_blank on non-required charfields #4989

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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,
sergei-maertens marked this conversation as resolved.
Show resolved Hide resolved
)
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 @@ -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):
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)
stevenbal marked this conversation as resolved.
Show resolved Hide resolved

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
Loading