Opened 5 years ago

Closed 5 years ago

#30376 closed Bug (invalid)

Saving ModelForm with missing, non-optional fields does not trigger a validation error

Reported by: Will Gordon Owned by: nobody
Component: Documentation Version: dev
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

I have my model

class Meeting(models.Model):
    title = models.CharField(max_length=255, help_text='A short header for your meeting')
    summary = models.TextField(help_text='An overview of the meeting. You\'ll have the opportunity to add specific notes later on')
    outcome = models.TextField(help_text='What was the final conclusion to the meeting')
    attended = models.DateField(help_text='When did you attend this meeting')
    attendees = models.ManyToManyField(Person, related_name='meetings')
    companies = models.ManyToManyField(Company, related_name='meetings')
    industries = models.ManyToManyField(Industry, related_name='meetings')
    author = models.ForeignKey(Person, on_delete=models.SET_NULL, related_name='my_meetings', null=True)

and my form

class MeetingForm(forms.ModelForm):
    class Meta:
        model = models.Meeting
        fields = ['author', 'attended', 'attendees', 'companies', 'industries', 'outcome']

Because title, and summary are not listed in my form's fields, I would expect this form to never be savable.

This was a result of my merging of two models, I updated the Model, but not the Form. I was surprised when I was then able to save the Form.

Based on documentation from https://docs.djangoproject.com/en/2.1/topics/forms/modelforms/#selecting-the-fields-to-use

Django will prevent any attempt to save an incomplete model, so if the model does not allow the missing fields to be empty, and does not provide a default value for the missing fields, any attempt to save() a ModelForm with missing fields will fail.

I would expect the validation to fail since there is no default value provided.

This is either a docs bug, with the form validation working as expected (not validating any fields explicitly excluded)...or a form validation bug, where the form should still validate all Model fields to ensure it's not saving blank fields.

Change History (3)

comment:1 by zeynel, 5 years ago

As it is written in documentation (https://docs.djangoproject.com/en/2.1/topics/forms/modelforms/#interaction-with-model-validation):

As part of the validation process, ModelForm will call the clean() method of each field on your model that has a corresponding field on your form. If you have excluded any model fields, validation will not be run on those fields.

It is not a bug, it is expected behaviour. ModelForm should not care about the validation of the fields that are not defined in fields attribute, since we are explicitly telling it which fields to use. Otherwise, it would cause too much confusion.

And here, it does not say that validation will fail:

Django will prevent any attempt to save an incomplete model, so if the model does not allow the missing fields to be empty, and does not provide a default value for the missing fields, any attempt to save() a ModelForm with missing fields will fail.

It says that any attempt to save() a ModelForm with missing fields will fail.

comment:2 by Will Gordon, 5 years ago

Ok, so maybe this is more of an issue with the save() as opposed to validation. As I read that quote

if the model does not allow the missing fields to be empty, and does not provide a default value for the missing fields, any attempt to save() a ModelForm with missing fields will fail

If I have a field with blank=False in my Model, then doing a save() on the ModelForm should fail...regardless of if I have specified that field in the ModelForm or not. Am I just taking a narrow view on that comment, or is it saying something other than what I'm reading?

comment:3 by Mariusz Felisiak, 5 years ago

Component: UncategorizedDocumentation
Resolution: invalid
Status: newclosed
Version: 2.1master

ModelForm doesn't take into account missing fields in both clean() and save() method, IMO it is expected behavior, see e.g.

Any fields not included in a form by the above logic will not be set by the form’s save() method. 

blank=True is not related with a DB constraint (like e.g. null=False) that's why IntegrityError is also not raised by a DB.

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