From 69224aef187eb19700a2d79c26daeec5e33adb76 Mon Sep 17 00:00:00 2001 From: Steven Bal Date: Mon, 6 Jan 2025 16:30:49 +0100 Subject: [PATCH] :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