Opened 12 years ago
Closed 12 years ago
#19691 closed Cleanup/optimization (invalid)
Inconsistency in instantiating/validating ModelForms: CharFields vs. with ImageField only.
Reported by: | 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 |
Description
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) picture=models.ImageField(upload_to="pics")
And the following forms:
from django.forms import ModelForm from notebook.models import * class TextForm(ModelForm): class Meta: model=Text class PictureForm(ModelForm): class Meta: model=Picture
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 form_text.is_valid() # >>> FALSE form_pic.is_valid() # >>> 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() FALSE >>>form_text.initial {'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(): form.save() 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(): form.save() 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, })
This is intentional - imagine what would happen if users had to re-upload file every time they want to edit a model instance.