Code

Opened 4 years ago

Closed 4 years ago

#13097 closed (duplicate)

Document full_clean() better (include cases where it isn't used)

Reported by: orokusaki Owned by: nobody
Component: Documentation Version: 1.2-beta
Severity: Keywords: models,validation
Cc: Triage Stage: Unreviewed
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

Description

If you read here: http://docs.djangoproject.com/en/dev/ref/models/instances/#id1

It says that full_clean() is called most of the time by ModelForm. When isn't it called, or better why isn't it called in some cases? Also, where is it called, if it's not called during Model.save()? In clicking through to ModelForm docs (recommended in the section linked above) there is no mention of full_clean(). It might be better to explain the relationship to ModelForm in this section and leave the link out. In my case, I added raise Exception("HELP ME") in a model's clean() method and when saving the model in the admin no exception was raised, which tells me that full_clean() indeed wasn't called. I don't have a suggestion as to how to better document this section because I still haven't solved my problem, but I believe it would really benefit anyone who is using the new model validation.

Attachments (0)

Change History (3)

comment:1 Changed 4 years ago by orokusaki

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

Update: http://docs.djangoproject.com/en/dev/topics/forms/modelforms/#the-is-valid-method-and-errors It mentions running "model validation". However, this doesn't answer why / if Admin doesn't do this.

comment:2 Changed 4 years ago by orokusaki

On further investigation, here's what I found (An important bug for those using the new model validation):

BaseForm.is_valid() calls BaseForm._get_errors() calls BaseForm.full_clean() calls BaseForm._post_clean() which calls the following 3 methods:

Model.clean_fields()
Model.clean()
BaseForm.validate_unique() which calls Model.validate_unique()

Model.full_clean() as defined on line 808 of django.db.models.base.py is never called anywhere in Django, period. This means that the docs are incorrect because ModelForms do not include a call to Model.full_clean(). This means that you cannot use transactions in Model methods (except maybe with hacking) because a full validation from one method isn't included. I'm trying to do this:

    @transaction.commit_on_success
    def full_clean(exclude=None):
        super(MyModel, self).full_clean(exclude)
    
    def clean(self):
        # Creating necessary related instance to attach to self.some_foreign_key (expecting this to be undone buy transaction mentioned above if self.save() never completes)

I'll be creating a bug ticket as well with this content, but I wanted to update my documentation ticket so that a decision on it can be made.

comment:3 Changed 4 years ago by jkocherhans

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

Essentially the same as #13100.

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.