Allow bypassing validation in newforms (#5153)
Christopher Lenz
I've created a ticket which has been closed as wontfix, and was told
to bring it here. So here I am.
<http://code.djangoproject.com/ticket/5153>
So yeah, I want to be able to show previews of data that may at that
point not be valid. Imagine there's a form with 12 required fields,
and I want to enable users to see what their entry would look like
even before they've completely filled everything in. (Using some AJAX
tricks to do that without leaving the page.)
For that kind of preview I want to reuse the templates that are used
to present the final data. So I want to take the data from the form,
pack it into a temporary/unsaved model object, and pass that object
into the template.
contrib.formtools.FormPreview implements a different kind of
approach, and can't be used for this.
Now, that use case may sound crazy to you (I don't know why it would,
but hey). But the point is that there is currently NO WAY to do this
with newforms. Honestly, the design of newforms.Form.clean(), where
it deletes its own "cleaned_data" attribute as soon as there's a
single validation error, is totally backwards and unpythonic. There
should be *some* way to get at the cleaned_data values, even if some
of the fields failed to validate.
So, I created a fairly simple and clean patch to enable me to do what
I want to do here:
<http://code.djangoproject.com/attachment/ticket/5153/
ticket5153.diff>
I personally think it'd be a good idea to apply.
Thanks,
Chris
--
Christopher Lenz
cmlenz at gmx.de
http://www.cmlenz.net/
James Bennett
> contrib.formtools.FormPreview implements a different kind of
> approach, and can't be used for this.
I would personally think that, if there's a need for this, the
solution would be to extend or subclass FormPreview somehow to enable
it, but I'm still a little fuzzy on the idea of using something
designed for validation and explicitly disabling the validation ;)
--
"Bureaucrat Conrad, you are technically correct -- the best kind of correct."
Todd O'Bryan
> So yeah, I want to be able to show previews of data that may at that
> point not be valid. Imagine there's a form with 12 required fields,
> and I want to enable users to see what their entry would look like
> even before they've completely filled everything in. (Using some AJAX
> tricks to do that without leaving the page.)
Since you're not leaving the page, couldn't you provide default values
for the things that haven't been entered yet?
Suppose the user gets something that they like the look of and then hits
the submit button. Isn't it going to be a surprise that this preview
they've been seeing and are happy with is not legal and they have to
enter more stuff to get it?
You could also maybe write a function that takes a form and creates a
related form with much less rigid validation and then use that form
until it's time to actually validate.
Todd
Malcolm Tredinnick
>
> I've created a ticket which has been closed as wontfix, and was told
> to bring it here. So here I am.
>
> <http://code.djangoproject.com/ticket/5153>
I'm somewhere between -0 and -1 on this, for a couple of reasons. I
don't like the design and you can achieve the same functionality without
any changes to core. Explanations below...
[...]
> Now, that use case may sound crazy to you (I don't know why it would,
> but hey). But the point is that there is currently NO WAY to do this
> with newforms. Honestly, the design of newforms.Form.clean(), where
> it deletes its own "cleaned_data" attribute as soon as there's a
> single validation error, is totally backwards and unpythonic. There
> should be *some* way to get at the cleaned_data values, even if some
> of the fields failed to validate.
If that was how the code worked, you would have a point. However,
Form.full_clean() fills cleaned_data with the normalised forms of any
fields that validated and puts any errors into the internal error list.
So fields that are correct *do* appear in cleaned_data and it is not
erased when an error is encountered.
Reading your patch, it looks like you are actually going through
form_for_model or form_for_instance, since your real data flow changes
are in newforms/models.py. Since saving data into models absolutely
requires valid data, being able to disable data validation there would
be illogical.
I also don't like the change to disable validation in Form.full_clean(),
because that makes it kind of a no-op: cleaning is all about validation
and normalisation. Normalisation has to happen after or in parallel with
validation, otherwise the normalisation code would have to be prepared
to handle every possible input format under the sun, instead of just the
valid types. So once you disable validation, normalisation also doesn't
occur and there's nothing for cleaning to do. We'd be adding an extra
code path in a few places that has be maintained and kept correct in the
future and it's only purpose is to subvert the whole cleaning process.
Feels less neat than it should be to me.
However, here are two alternative approaches, neither of which require
changing any core code.
Firstly, the "whole form at once" approach, based on subclassing: you
already have a form with all the field definitions in it. Since the main
problem you seem to be working around is that not all data is necessary,
create a subclass of that form where the __init__ method alters the
fields and sets all the "required" statuses to False. So every field
becomes optional. Maybe you also want to override clean() in your
subclass to pull all data that may have been marked as invalid because
it was present but in the wrong format into cleaned_data and clean out
_errors. Then you give your partial data to this subclass, and it can be
validated as normal. Once you have *all* the data for the form, you can
reuse the original class for validation and it will complain about
missing fields as well. If you have further, looser validation
requirements for the intermediate stages of entry, you can add methods
to your subclass as needed.
You could also adopt something like this with your "fake model"
approach, by ensuring the dummy model you are setting up has all fields
being optional.
The second approach, that would seem more suitable to Ajax work, is to
only be validating the single fields that you are submitting. I'd do
that by calling the clean methods on those fields (plus any
clean_<fieldname> methods) directly, rather than re-validating the whole
form. Whether this is an easier approach depends on your particular
forms, obviously.
In both of these cases, nothing needs to be added to Django. Further, I
would personally be a bit reluctant at this point to even add utility
methods for this case, since it's not the only possible variation on
standard data entry that people might want and it's just not that much
code to write in the cases it's needed. So I'm not convinced that we
need to accommodate all that flexibility as helper functions when it's
already just a subclass away (and writing a routine to generate the
subclass for one's own particular circumstances isn't too onerous).
Regards,
Malcolm
--
Experience is something you don't get until just after you need it.
http://www.pointy-stick.com/blog/
Christopher Lenz
> On Mon, 2007-09-17 at 00:23 +0200, Christopher Lenz wrote:
>> Hey all,
>>
>> I've created a ticket which has been closed as wontfix, and was told
>> to bring it here. So here I am.
>>
>> <http://code.djangoproject.com/ticket/5153>
>
> I'm somewhere between -0 and -1 on this, for a couple of reasons. I
> don't like the design and you can achieve the same functionality
> without
> any changes to core. Explanations below...
>
> [...]
>> Now, that use case may sound crazy to you (I don't know why it would,
>> but hey). But the point is that there is currently NO WAY to do this
>> with newforms. Honestly, the design of newforms.Form.clean(), where
>> it deletes its own "cleaned_data" attribute as soon as there's a
>> single validation error, is totally backwards and unpythonic. There
>> should be *some* way to get at the cleaned_data values, even if some
>> of the fields failed to validate.
>
> If that was how the code worked, you would have a point. However,
> Form.full_clean() fills cleaned_data with the normalised forms of any
> fields that validated and puts any errors into the internal error
> list.
> So fields that are correct *do* appear in cleaned_data and it is not
> erased when an error is encountered.
Um, yes they are erased:
<http://code.djangoproject.com/browser/django/trunk/django/
newforms/forms.py#L205>
Am I missing something?
> Reading your patch, it looks like you are actually going through
> form_for_model or form_for_instance, since your real data flow changes
> are in newforms/models.py. Since saving data into models absolutely
> requires valid data, being able to disable data validation there would
> be illogical.
Saving data to the database should enforce valid data, simply
populating a model instance from forms data doesn't really need to.
It should by the default, sure.
> I also don't like the change to disable validation in
> Form.full_clean(),
> because that makes it kind of a no-op: cleaning is all about
> validation
> and normalisation. Normalisation has to happen after or in parallel
> with
> validation, otherwise the normalisation code would have to be prepared
> to handle every possible input format under the sun, instead of
> just the
> valid types. So once you disable validation, normalisation also
> doesn't
> occur and there's nothing for cleaning to do. We'd be adding an extra
> code path in a few places that has be maintained and kept correct
> in the
> future and it's only purpose is to subvert the whole cleaning process.
> Feels less neat than it should be to me.
"Subvert the whole cleaning process"?
No, the point is to be able to get the results from the cleaning
process even when some validation fails. That may mean that
cleaned_data is empty, or that fields are missing, etc. That doesn't
matter, I can check form.is_valid() or form.errors to check whether
the data is valid, I just want a way to still get at the data that's
available so far! Seriously, this shouldn't require any of the
workarounds you describe.
Cheers,
Malcolm Tredinnick
My mistake. Apologies.
That might be something to look into fixing.
I still not keen on the idea of being able to disable validation, since
it's the driving feature behind clean() (which is my problem with the
design, as opposed to the implementation issues). I want to think about
this some more.
Malcolm
--
What if there were no hypothetical questions?
http://www.pointy-stick.com/blog/
Malcolm Tredinnick
> On Mon, 2007-09-17 at 10:14 +0200, Christopher Lenz wrote:
> > Um, yes they are erased:
> >
> > <http://code.djangoproject.com/browser/django/trunk/django/
> > newforms/forms.py#L205>
> >
> > Am I missing something?
>
> My mistake. Apologies.
>
> That might be something to look into fixing.
>
> I still not keen on the idea of being able to disable validation, since
> it's the driving feature behind clean() (which is my problem with the
> design, as opposed to the implementation issues). I want to think about
> this some more.
So I thought this some more and I think I'm comfortable being +0 to +1
on removing the erasure of cleaned_data if there are any errors. So
cleaned_data would contain anything that survived validation.
I'm -1 on adding like a do_not_validate option to full_clean(). If you
want to clean a form, pass in a form with the correct constraints set up
in the first place and don't ask Django to ignore the constraints you
have set. It's not a workaround; it's consistent design.
Regards,
Malcolm
--
The only substitute for good manners is fast reflexes.
http://www.pointy-stick.com/blog/
Benjamin Slavin
> So I thought this some more and I think I'm comfortable being +0 to +1
> on removing the erasure of cleaned_data if there are any errors. So
> cleaned_data would contain anything that survived validation.
I've created a ticket for this, using a patch we've applied internally:
http://code.djangoproject.com/ticket/5524
- Ben