Opened 22 months ago

Closed 20 months ago

Last modified 20 months ago

#21590 closed Cleanup/optimization (invalid)

Don't require forms clean_* methods to return a value

Reported by: aaugustin Owned by: nobody
Component: Forms Version: master
Severity: Normal Keywords:
Cc: Triage Stage: Unreviewed
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no


Adding custom validation to forms isn't as DRY as it colud be:

    class MyForm(forms.ModelForm):

        class Meta:
            model = MyModel

        def clean_title(self):
            title = self.cleaned_data["title"]
            validate_title(title)  # custom validation
            return title

        def clean_slug(self):
            slug = self.cleaned_data["slug"]
            validate_slug(slug)   # custom validation
            return slug

The requirement that clean() return a cleaned_data dict was recently lifted; what about doing the same for clean_*()? Then the example above could be simplified to:

    class MyForm(forms.ModelForm):

        class Meta:
            model = MyModel

        def clean_title(self):
            validate_title(self.cleaned_data["title"])  # custom validation

        def clean_slug(self):
            validate_slug(self.cleaned_data["slug"])   # custom validation

Otherwise developers go for private APIs, eg.:

    class MyForm(forms.ModelForm):

        class Meta:
            model = MyModel

        def __init__(self, *args, **kwargs):
            super(MyForm, self).__init__(*args, **kwargs)

Change History (3)

comment:1 Changed 22 months ago by charettes

Maybe it was overlooked to allow clean_*() to return None?

comment:2 Changed 20 months ago by mjtamlyn

  • Resolution set to invalid
  • Status changed from new to closed

We can't do this, as None is potentially a valid value for the field. It is conceivable for a field to have a non-None value beforehand, and a clean method to modify it to become None. In the case of clean(), this is not a problem as None is not a valid value for self.cleaned_data.

comment:3 Changed 20 months ago by aaugustin

I has a sad, but so be it...

Note: See TracTickets for help on using tickets.
Back to Top