Code

Opened 4 months ago

Closed 2 months ago

Last modified 2 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

Description

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)
            self.fields["title"].validators.append(validate_title)
            self.fields["slug"].validators.append(validate_slug)

Attachments (0)

Change History (3)

comment:1 Changed 4 months ago by charettes

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

comment:2 Changed 2 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 2 months ago by aaugustin

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

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.