Skip to content

Commit

Permalink
🔊 [#4927] Add system check for missing allow_blank on non-required ch…
Browse files Browse the repository at this point in the history
…arfields
  • Loading branch information
stevenbal committed Jan 6, 2025
1 parent 7f8fd3b commit 417fb86
Showing 1 changed file with 58 additions and 0 deletions.
58 changes: 58 additions & 0 deletions src/openforms/utils/checks.py
Original file line number Diff line number Diff line change
@@ -1,9 +1,12 @@
import inspect
import os
from typing import Type

from django.conf import settings
from django.core.checks import Error, Warning, register
from django.forms import ModelForm

from rest_framework.serializers import CharField, Serializer, empty
from treebeard.forms import MoveNodeForm


Expand Down Expand Up @@ -81,3 +84,58 @@ def check_missing_init_files(app_configs, **kwargs):
)

return errors


def get_field_line_number(
serializer_class: Type[Serializer], field_name: str
) -> int | None:
# Get source lines and the starting line number of the serializer class
source_lines, start_line_number = inspect.getsourcelines(serializer_class)

# Find the line containing the field definition
for i, line in enumerate(source_lines):
if line.strip().startswith(f"{field_name} ="):
return start_line_number + i # Compute the exact line number

return None # Return None if field definition is not found


@register
def check_serializer_non_required_charfield_allow_blank_true(app_configs, **kwargs):
"""
https://github.com/open-formulieren/open-forms/issues/4927
Check for serializers.CharFields that have required=False, but not `allow_blank=True`
to avoid errors occurring when empty strings are passed from the frontend.
"""
errors = []
serializers = get_subclasses(Serializer)
for serializer in serializers:
# TODO not sure if this issue can also occur for fields defined in Meta.fields?
for field_name, field in serializer._declared_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)
line_number = get_field_line_number(serializer, field_name)

# No line number indicates that the field is inherited from another class
if not file_path or not line_number:
continue

errors.append(
Warning(
(
f"{serializer.__module__}.{serializer.__qualname__}.{field_name} does not have `allow_blank=True`\n"
f"{file_path}:{line_number}"
),
hint="Consider setting `allow_blank=True` to allow passing of empty strings",
id="utils.W002",
)
)
return errors

0 comments on commit 417fb86

Please sign in to comment.