Opened 4 years ago

Closed 4 years ago

#19691 closed Cleanup/optimization (invalid)

Inconsistency in instantiating/validating ModelForms: CharFields vs. with ImageField only.

Reported by: pfeiffersimon@… Owned by: nobody
Component: Forms Version: 1.4
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


Hi I found a sligth inconsistency in the instantiation/validation of the different field types. Its kinda hard to describe so here some code:

Consider the following models:

class Text(models.Model):
    """Just a basic model to store some text"""
    create_date = models.DateTimeField(auto_now_add=True)
    content = models.TextField()

class Picture(models.Model):
    """Just a basic model to store a picture"""    
    create_date = models.DateTimeField(auto_now_add=True)

And the following forms:

from django.forms import ModelForm
from notebook.models import *

class TextForm(ModelForm):
    class Meta:
class PictureForm(ModelForm):
    class Meta:

When you now instantiate PictureForm and a TextForm with a bound instance, but empty request.POST / request.FILES likes this:

#get_or_none() returns a valid model instance in both cases below
#req.POST and req.FILES are both empty QueryDict instances

form_text = TextForm(req.POST,instance=get_or_none(Text,pk=primary_key))
form_pic = PictureForm(req.POST,req.FILES ,instance=get_or_none(Picture,pk=primary_key))

#The validation of form_text 
# >>> FALSE
# >>> True

So basically, the PictureForm ignores that it gets handed an empty req.FILES & req.POST and just uses the instance that is bound.

Whereas, the TextModel throws errors which get rendered although it has a bound a instance:

# found with pdb
>>> form_text.is_valid() 
{'content': u'Some content from the database', 'id': 15}

So as you can see its just a little inconsistency. I do not even know I should have reported it. If you are wondering how I came about doing something like that:

# Compact way of writing a create/edit form
def picture_edit(req,primary_keyk=None):
    form = PictureForm(req.POST,req.FILES ,instance=get_or_none(Picture,pk=primary_key))
    # Works for         ^^^^^^^^^^^^^^^^req.POST={} & req.FILES={}  and instance == Model()                             
    if req.method == 'POST': 
        if form.is_valid():
            return redirect("notebook_dashboard")
    return render(req, 'notebook/picture_edit.html', {'form': form, })

def text_edit(req,pk=None):
    form = TextForm(req.POST,instance=get_or_none(Text,pk=pk))
    # this does not   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^  when req.POST={} and instance == Model()
    if req.method == 'POST': # If the form has been submitted...
        if form.is_valid():
            return redirect("notebook_dashboard") #Redirect to dash
            #Optional else clause not explicitly used
    import pdb; pdb.set_trace()
    return render(req, 'notebook/text_edit.html', {'form': form, })

Change History (1)

comment:1 Changed 4 years ago by Rafal Stozek

Needs documentation: unset
Needs tests: unset
Patch needs improvement: unset
Resolution: invalid
Status: newclosed

This is intentional - imagine what would happen if users had to re-upload file every time they want to edit a model instance.

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