Opened 8 years ago

Closed 7 years ago

#13249 closed Bug (duplicate)

Feature request: ModelForm validation error handling after commit=False

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


For lack of a good explanation, I'll include an example first:

#First, the models and forms used in example:

class Account(models.Model):
    subdomain = SubdomainField(max_length=25, unique=True)

class Administrator(models.Model):
    account = models.ForeignKey(Account, related_name='administrators')
    username = LowerAlphanumericField(max_length=50)
    email = models.EmailField()
    class Meta:
        unique_together = (('account', 'username'), ('account', 'email'))

class AdministratorForm(ModelForm):
    class Meta:
        model = Administrator
        exclude = ('account',)

And, here's the example usage that breaks:

def create_administrator(request):
    from django.db.utils import IntegrityError
    from myproject.apps.account.forms import AdministratorForm
    if request.method == 'POST':
        form = AdministratorForm(request.POST)
        if form.is_valid():
            instance =
            instance.account = request.account  # ``account`` is attached to ``request`` in custom middleware.
            # What happens if there is an error here such as IntegrityError with one of the 2 unique_together fields ('username', and 'email').
            # You cannot tell which field failed the ``unique_together`` test. validate_unique skips over the excluded fields in the form.
            # You could run instance.full_clean() first and catch ValidationErrors and handle them manually, but this would not be very DRY after about 3 views.
            # I don't know what the new version would be like, but I can imagine a way to validate the instance and then add the errors into the form again or something.

I probably made this very confusing, but basically there is no way to use commit=False and still use ModelForm's built-in validation conveniences.

Change History (6)

comment:1 Changed 8 years ago by Oroku Saki

Triage Stage: UnreviewedDesign decision needed

Here is one case during the model validation talks in January where somebody talks about precisely what I'm dealing with, although he puts it better:

comment:2 Changed 8 years ago by Oroku Saki

One solution I could propose as an abstract idea is to allow for the inclusion of extra data during the creation of a ModelForm instance:

form = MySpamModelForm(request.POST, data_override={'account': request.account})
if form.is_valid():
    instance =

This doesn't allow for the use of, but it at lease allows you to add request related data to the instance before creation.

comment:3 Changed 8 years ago by Russell Keith-Magee

Resolution: wontfix
Status: newclosed
Triage Stage: Design decision neededUnreviewed

Please don't triage your own tickets. The DDN marker indicates that the idea has been accepted prima facie, but a final decision is required.

Firstly, the assertion in the problem statement ("there is no way to use commit=False and still use ModelForm?'s built-in validation conveniences") is false. When you call '', you're saying that you're taking control of making sure the instance is saved properly. If you're using model validation, this means taking control of the validation process, including calls to full_clean(), and the handling of the resultant errors. The docs added in r12206 pretty much say this explicitly, and give examples of how it should be handled.

As for potential fixes - the "data override" idea you suggest isn't required- it's already possible to do this by specifying an instance.

    if request.method == 'POST':
        new_obj = Administrator()
        new_obj.account = request.account
        form = AdministratorForm(data=request.POST, instance=new_obj)
        if form.is_valid():

comment:4 Changed 8 years ago by Oroku Saki


There's still one problem with doing what you did above.

Thanks for the code snippet BTW first of all. I ended up using form.instance.account = request.account before my call to form.is_valid() , but in the case of adding data to an instance or a form during all of this is usually due to the fact that the field is not included in the form because of editable=False on the model or exclude=('field',) in the form. When this is the case the fields that are excluded still don't go through validation with form.is_valid() . Is it reasonable to request the addition of either (A) If a field's value is set via form.instance.account = request.account , then it is included in validation again, or (B) A way to add the field back into validation again manually such as form.include_in_validation = 'account' .

If you'd like, I can create a new ticket, or is it relevant enough to this ticket, or do you think it's even worth the time for what I mentioned?

comment:5 Changed 7 years ago by legutierr

Easy pickings: unset
Resolution: wontfix
Severity: Normal
Status: closedreopened
Triage Stage: UnreviewedDesign decision needed
Type: Bug

I agree with orokusaki that although there is a better approach to that described in the original ticket (i.e. the use of, there is, nonetheless, a problem in the way that form.is_valid() and model validation validates unique_together. Currently, model validation ignores all unique_together tuples where *any* of the referenced fields are excluded; instead, model validation should only ignore those unique_together tuples where *all* of the referenced fields are excluded.

I am reopening this ticket with the purpose of marking it as a duplicate of #13091.

comment:6 Changed 7 years ago by legutierr

Resolution: duplicate
Status: reopenedclosed
Note: See TracTickets for help on using tickets.
Back to Top