Saving forms without validation, and formsets

1,422 views
Skip to first unread message

Chris Wilson

unread,
Mar 18, 2014, 4:54:12 PM3/18/14
to Django Developers
Hi all,

(Apologies if this belongs on django-users instead. To clarify, I'm
interested in what appear to be potentially missing or useful features in
Django, and whether it's worth implementing them and submitting pull
requests, or creating separate reusable modules to implement them. I'm
also not asking for anyone to fix these issues for me.)

We're building an application which has some large forms including
repeating formsets and file upload fields. It's intended for use in
developing countries with slow and unreliable Internet connections. Thus
it's important for users to be able to save the form regularly, and to not
lose uploaded files even if there's a form validation error.

We encountered a number of issues which feel like they might either be:

* bugs in Django, or
* deliberate limitations of Django core, that could be useful as
a third-party module.

And I'd like your feedback about where you feel these issues lie. I'm
hoping that we're not the only ones to encounter them!

1. GCBV and Vanilla Views do a great job for simple forms, but they leave
out embedded formsets entirely. (For example our large form has repeating
sections for employment history, education, etc.) It feels like it would
be great to have formsets handled "more automatically" - instantiating,
validating and saving them just like we do for forms. A lot of our code is
shared between all our formsets, and potentially reusable.

2. Adding instances to a formset on the client side using Javascript.
There is a django-formset-js package on PyPI, but it requires special
attributes to be added to form elements that would have been difficult
with crispy forms (which we're also using) and it feels almost like this
functionality ought to be in Django core (maybe not today, but one day?)

3. We couldn't change the "extra" of a formset without redefining the
formset class. We wanted to do this because the required repeating
sections (employment history etc.) must not have a blank form in them if
they already have some valid forms, otherwise the blank forms prevent the
form being submitted because HTML5 client-side validation fails. So we had
to do all this:

def get_context_data(self, **kwargs):
...
for name, formset_class in self.formsets.iteritems():
# doesn't exist yet, so we can't call its queryset() method
queryset = formset_class.model._default_manager.filter(
**{formset_class.fk.name: application})
extra = 0 if queryset.exists() else 1

# need to reconstruct the formset class to change extra?
formset_class = inlineformset_factory(
Application,
formset_class.model,
formset_class.form,
formset_class,
extra=extra,
can_delete=formset_class.can_delete,
fields=formset_class.form.base_fields.keys(),
max_num=formset_class.max_num,
)

4. We needed to be able to save the form without validation. To do this,
we had to make all fields non-required on our model, and make them
required on the form instead. Since nearly all fields are required, we'd
have to redefine them all, making the ModelForm a bit pointless, but
instead we were naughty and looped over the fields setting their required
values to True, and also updating the widget (which gets a copy of this
attribute on constructions). Naughty but DRY. I feel that a shortcut to
list required fields of a ModelForm in its Meta class would be useful
(overriding the blank and null status of the underlying Model fields).

def _save_data_without_validation(self, form):
# construct a new form with the required value turned off on all
# fields, and use that to save the instance.
all_optional_form = self.get_form(
data=self.request.POST,
files=self.request.FILES,
instance=form.instance
)
for field in all_optional_form:
field.field.required = False
all_optional_form.full_clean()

5. We have to be able to save a new instance of the model even if form
validation fails. So both the form_valid and form_invalid methods of the
CreateView return redirects to the UpdateView for the newly saved
instance. But that makes the form load through GET instead of POST, and
thus it isn't validated. This is fine for saving the form, but not for
submitting it, so we have to validate it like this:

def get_form(self, data=None, files=None, **kwargs):
# If we've been redirected here because of a form error, then we
need
# to validate the form again. To do that, we need to generate a
# simulated "POST data dict" from the model data, and clean the
form,
# which we wouldn't normally do on a GET request.
# http://stackoverflow.com/a/8996585/648162

request_origin = self.request.GET.get('type')
if data is None and request_origin == 'submit':
data = model_to_dict(self.get_object())

form = super(ApplicationViewMixin, self).get_form(data, files,
**kwargs)

if request_origin == 'submit':
form.full_clean()

return form

And the same for all subforms (formsets). So this is a case where the
opposite would be useful - to disable the required status of all fields on
the form, without poking around in its internals.

6. Setting the HTML5 "required" attribute in the HTML of required
controls, *except* file fields that already have a file in them, because
you don't have to upload another file to make the underlying field valid
in this case. We've got it now, but it feels like Django ought to be doing
this. This is part of the request on
https://code.djangoproject.com/ticket/15924, which was closed as "too
broad", and the required attribute part of it never made it into a
separate ticket, but it seems like it might be useful.

7. Uploaded files appear to be lost if form validation fails, since the
model isn't saved.

Thanks in advance for your consideration.

Cheers, Chris.
--
Aptivate | http://www.aptivate.org | Phone: +44 1223 967 838
Citylife House, Sturton Street, Cambridge, CB1 2QF, UK

Aptivate is a not-for-profit company registered in England and Wales
with company number 04980791.

Carl Meyer

unread,
Mar 18, 2014, 5:35:29 PM3/18/14
to django-d...@googlegroups.com
Hi Chris,

I've definitely faced similar issues. A few scattered thoughts below:

On 03/18/2014 02:54 PM, Chris Wilson wrote:
[snip]
> 1. GCBV and Vanilla Views do a great job for simple forms, but they
> leave out embedded formsets entirely. (For example our large form has
> repeating sections for employment history, education, etc.) It feels
> like it would be great to have formsets handled "more automatically" -
> instantiating, validating and saving them just like we do for forms. A
> lot of our code is shared between all our formsets, and potentially
> reusable.

The approach I've used for this is encapsulating the creation and
handling of the inline formsets entirely within the "parent" form class,
so from the perspective of the view it acts like a normal simple form.
This requires overrides of a few methods on the parent form: __init__()
to create the formsets, is_valid() to ensure the formsets are valid too,
has_changed() to see if formsets have changed, and save() to save
formsets too.

If we were going to look at incorporating something into Django, I'd
like to consider this option as an alternative to adding more GCBVs with
inline-formset support. I think it's a nicer abstraction (because in a
real sense those inlines are "part of" the parent form), and I don't
think we want to add to the already-extensive list of GCBVs and mixin
classes.

I think either approach would be workable as a third-party project, too.

> 2. Adding instances to a formset on the client side using Javascript.
> There is a django-formset-js package on PyPI, but it requires special
> attributes to be added to form elements that would have been difficult
> with crispy forms (which we're also using) and it feels almost like this
> functionality ought to be in Django core (maybe not today, but one day?)

I've used this: http://plugins.jquery.com/django-superformset/

I am skeptical of adding things to Django core that can be implemented
mostly or entirely on the front-end. Django is a server-side framework.
I think changes to the forms library that make it easier to implement
such client-side libraries is a better place to start.

> 3. We couldn't change the "extra" of a formset without redefining the
> formset class. We wanted to do this because the required repeating
> sections (employment history etc.) must not have a blank form in them if
> they already have some valid forms, otherwise the blank forms prevent
> the form being submitted because HTML5 client-side validation fails. So
> we had to do all this:
>
> def get_context_data(self, **kwargs):
> ...
> for name, formset_class in self.formsets.iteritems():
> # doesn't exist yet, so we can't call its queryset() method
> queryset = formset_class.model._default_manager.filter(
> **{formset_class.fk.name: application})
> extra = 0 if queryset.exists() else 1
>
> # need to reconstruct the formset class to change extra?
> formset_class = inlineformset_factory(
> Application,
> formset_class.model,
> formset_class.form,
> formset_class,
> extra=extra,
> can_delete=formset_class.can_delete,
> fields=formset_class.form.base_fields.keys(),
> max_num=formset_class.max_num,
> )

I don't understand this. The 'extra' attribute of a formset class can be
tweaked dynamically as a normal attribute (although this may not be
documented); it doesn't require recreating the entire formset class.

I snipped your last three items, regarding saving invalid forms. I think
this is an unusual use case, and I'm not sure support for it belongs in
core. It would be interesting to experiment with something to make
filefields less painful when validation fails, but that can be a
third-party project first I think.

Carl

signature.asc

Chris Wilson

unread,
Mar 19, 2014, 8:58:21 AM3/19/14
to django-d...@googlegroups.com
Hi Carl,

On Tue, 18 Mar 2014, Carl Meyer wrote:
> On 03/18/2014 02:54 PM, Chris Wilson wrote:
>
>> 1. GCBV and Vanilla Views do a great job for simple forms, but they
>> leave out embedded formsets entirely. (For example our large form has
>> repeating sections for employment history, education, etc.) It feels
>> like it would be great to have formsets handled "more automatically" -
>> instantiating, validating and saving them just like we do for forms. A
>> lot of our code is shared between all our formsets, and potentially
>> reusable.
>
> The approach I've used for this is encapsulating the creation and
> handling of the inline formsets entirely within the "parent" form class,
> so from the perspective of the view it acts like a normal simple form.

That seems like an excellent idea, thanks! I'm reworking my forms to work
this way.

> This requires overrides of a few methods on the parent form: __init__()
> to create the formsets, is_valid() to ensure the formsets are valid too,
> has_changed() to see if formsets have changed, and save() to save
> formsets too.

There's a few more Form methods that I'm not sure whether I need to
override, especially if I want to be able to generalise this. I think just
_get_media and is_multipart should take sub-formsets into account?

> If we were going to look at incorporating something into Django, I'd
> like to consider this option as an alternative to adding more GCBVs with
> inline-formset support. I think it's a nicer abstraction (because in a
> real sense those inlines are "part of" the parent form), and I don't
> think we want to add to the already-extensive list of GCBVs and mixin
> classes.

I agree. Then it should also work for Vanilla Views with no changes.

> I think either approach would be workable as a third-party project, too.

I think it would be nice for forms to be able to treat embedded formsets
as a special (complex) type of 'field', so that as_table, as_p etc. would
render the formsets as well, and for ModelForm to be able to generate
these automatically for models with ForeignKeys that link back to the
ModelForm's Model.

>> 2. Adding instances to a formset on the client side using Javascript.
>> There is a django-formset-js package on PyPI, but it requires special
>> attributes to be added to form elements that would have been difficult
>> with crispy forms (which we're also using) and it feels almost like this
>> functionality ought to be in Django core (maybe not today, but one day?)
>
> I've used this: http://plugins.jquery.com/django-superformset/

Thanks for the tip, I didn't know about that one.

>> 3. We couldn't change the "extra" of a formset without redefining the
>> formset class. We wanted to do this because the required repeating
>> sections (employment history etc.) must not have a blank form in them if
>> they already have some valid forms, otherwise the blank forms prevent
>> the form being submitted because HTML5 client-side validation fails. So
>> we had to do all this:
>
> I don't understand this. The 'extra' attribute of a formset class can be
> tweaked dynamically as a normal attribute (although this may not be
> documented); it doesn't require recreating the entire formset class.

I'm not sure that's possible, because inline_formset defines a new class.
That class has a class attribute called 'extra' containing the value of
'extra', but changing it would change it for all instances of the class,
so it's not thread-safe. And when the class is instantiated, the value is
immediately used to create the initial empty forms, and not used after
that, so changing it after instantiating the form has no effect. Thus, it
seems that we have to subclass BaseFormSet anyway to get into the
constructor and change the value of extra before the initial forms are
created.

> I snipped your last three items, regarding saving invalid forms. I think
> this is an unusual use case, and I'm not sure support for it belongs in
> core. It would be interesting to experiment with something to make
> filefields less painful when validation fails, but that can be a
> third-party project first I think.

It may be unusual, but I'm not the only one to want this. At least two
people participated on this SO question:
http://stackoverflow.com/questions/8993749/transform-an-unbound-form-to-a-bound-one

From there I got my current approach, which is to use model_to_dict to
create a fake 'data' to make the unbound form bound, so that I can
validate it. However this is ugly, and it doesn't work with formsets
because it doesn't account for the management form, which complains about
having been tampered with because its data is missing. And there's no
form_to_dict equivalent of model_to_dict, so I'd have to hack something
up, all to pretend to Django that I've got an unbound form, just so I can
validate its contents.

Another approach to validating an unbound form appears to be to extract
"value" from the model instance instead of cleaned_data, and this bypasses
the management form issue, so I'm experimenting with that approach.

It seems to me that there would be value in being officially able to
validate unbound forms, and it might not be hard to implement.

Chris Wilson

unread,
Mar 19, 2014, 1:07:58 PM3/19/14
to django-d...@googlegroups.com
Hi all,

As part of our test process we have code that automatically fills in forms
with dummy values to make them valid. It fills in all required fields.

This works well except for formsets. BaseModelFormSet.add_fields() adds a
PK field to the form after it's created. It marks this field as not
required:

form.fields[self._pk_field.name] = ModelChoiceField(qs, initial=pk_value, required=False, widget=widget)
<https://github.com/django/django/blob/master/django/forms/models.py#L805>

However it will blow up if editing an existing instance and this field
isn't present, so in a sense it is required.

I know it's a very minor issue and I can easily work around it (however
ugly that is), but would it logically make sense for this field to be
required if pk is not None?

Carl Meyer

unread,
Mar 19, 2014, 1:44:07 PM3/19/14
to django-d...@googlegroups.com
On 03/19/2014 06:58 AM, Chris Wilson wrote:
>> This requires overrides of a few methods on the parent form: __init__()
>> to create the formsets, is_valid() to ensure the formsets are valid too,
>> has_changed() to see if formsets have changed, and save() to save
>> formsets too.
>
> There's a few more Form methods that I'm not sure whether I need to
> override, especially if I want to be able to generalise this. I think
> just _get_media and is_multipart should take sub-formsets into account?

Probably so, yeah - I haven't yet ever carried this through into a fully
reusable solution, just implemented it to meet my needs in a specific
case, which apparently never included form media or is-multipart. If
implementing a generic approach you'd want to do a thorough review of
the BaseForm API for places where encapsulated formsets might impact things.

> I think it would be nice for forms to be able to treat embedded formsets
> as a special (complex) type of 'field', so that as_table, as_p etc.
> would render the formsets as well, and for ModelForm to be able to
> generate these automatically for models with ForeignKeys that link back
> to the ModelForm's Model.

I don't use use the shortcut renderings (HTML generated in Python, ick),
so I've never looked into incorporating embedded formsets there.

Having an option to generate embedded formsets for linked models
automatically would be very cool.

I definitely think this idea should get proved out as a third-party app
before we consider it for core.

>>> 3. We couldn't change the "extra" of a formset without redefining the
>>> formset class. We wanted to do this because the required repeating
>>> sections (employment history etc.) must not have a blank form in them if
>>> they already have some valid forms, otherwise the blank forms prevent
>>> the form being submitted because HTML5 client-side validation fails. So
>>> we had to do all this:
>>
>> I don't understand this. The 'extra' attribute of a formset class can be
>> tweaked dynamically as a normal attribute (although this may not be
>> documented); it doesn't require recreating the entire formset class.
>
> I'm not sure that's possible, because inline_formset defines a new
> class. That class has a class attribute called 'extra' containing the
> value of 'extra', but changing it would change it for all instances of
> the class, so it's not thread-safe. And when the class is instantiated,
> the value is immediately used to create the initial empty forms, and not
> used after that, so changing it after instantiating the form has no
> effect. Thus, it seems that we have to subclass BaseFormSet anyway to
> get into the constructor and change the value of extra before the
> initial forms are created.

That's not quite accurate. The 'extra' attribute on a formset instance
isn't used immediately on instantiation, only lazily when the first
property on the formset is accessed that causes it to create all its
form instances. So there is a window immediately after instantiation
when it is safe and effective to set a different value for 'extra'
directly on the formset instance (not on the class, as you note). Or it
can be done in `__init__` of a BaseFormSet subclass (as you also note).
I think both of those approaches are easier/nicer than creating a whole
new formset class each time you need a different value for 'extra'.

To be clear, here's what I'm currently doing in a project:

class ConditionalExtraInlineFormset(BaseInlineFormSet):
"""Inline formset that adds one extra form if there are no initial
forms."""
def __init__(self, *a, **kw):
super(ConditionalExtraInlineFormset, self).__init__(*a, **kw)
self.extra = 0 if self.initial_form_count() else 1

But if you don't want to subclass for some reason, it would work just as
well to do the same thing from outside the instance, right after it is
instantiated.

>> I snipped your last three items, regarding saving invalid forms. I
>> think this is an unusual use case, and I'm not sure support for it
>> belongs in core. It would be interesting to experiment with something
>> to make filefields less painful when validation fails, but that can be
>> a third-party project first I think.
>
> It may be unusual, but I'm not the only one to want this. At least two
> people participated on this SO question:
> http://stackoverflow.com/questions/8993749/transform-an-unbound-form-to-a-bound-one
>
> From there I got my current approach, which is to use model_to_dict to
> create a fake 'data' to make the unbound form bound, so that I can
> validate it. However this is ugly, and it doesn't work with formsets
> because it doesn't account for the management form, which complains
> about having been tampered with because its data is missing. And there's
> no form_to_dict equivalent of model_to_dict, so I'd have to hack
> something up, all to pretend to Django that I've got an unbound form,
> just so I can validate its contents.
>
> Another approach to validating an unbound form appears to be to extract
> "value" from the model instance instead of cleaned_data, and this
> bypasses the management form issue, so I'm experimenting with that
> approach.
>
> It seems to me that there would be value in being officially able to
> validate unbound forms, and it might not be hard to implement.

Yes, I see the use case; not sure what the API would look like for it.

Carl

signature.asc
Reply all
Reply to author
Forward
0 new messages