#25548: Can't Form.add_error() in FormView.form_valid()
-------------------------------------+-------------------------------------
Reporter: acatton | Owner: nobody
Type: Uncategorized | Status: new
Component: Generic views | Version: master
Severity: Normal | Resolution:
Keywords: performance | Triage Stage:
FormView form_invalid | Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by acatton):
* needs_better_patch: => 0
* needs_tests: => 0
* needs_docs: => 0
Old description:
> Let's consider this example:
>
> {{{#!python
> class MyView(FormView):
> def form_valid(self, form):
> value = form.cleaned_data['value']
> try:
> do_action(value)
> except ValueError as e:
> form.add_error(None, e.message)
> return self.form_invalid(form)
> return super().form_valid(form)
> }}}
>
> The error message is ignored. The main reason being that the form in
> discarded in `FormView.form_invalid()`. Here's the current implementation
> on master:
>
> {{{#!python
> def form_invalid(self, form):
> """
> If the form is invalid, re-render the context data with the
> data-filled form and errors.
> """
> return self.render_to_response(self.get_context_data())
> }}}
>
> I proposed to modify this implementation to:
>
> {{{#!python
> def form_invalid(self, form):
> return self.render_to_response(self.get_context_data(form=form))
> }}}
>
> since `Form.get_context_data()` is doing `kwargs.setdefault('form',
> self.get_form())`. I did that on my view, but it does feel this is
> something that belongs in Django.
>
> Also, from my little understanding of the code, this also mean that if I
> have a form like this:
>
> {{{#!python
> class MyForm(Form):
> def clean_value(self):
> value = self.cleaned_data['value']
> try:
> heavy_computation(value)
> except ValueError as e:
> raise ValidationError(e.message)
> return value
> }}}
>
> The `heavy_computation()` is going run twice.
New description:
Let's consider this example:
{{{#!python
class MyView(FormView):
def form_valid(self, form):
value = form.cleaned_data['value']
try:
do_action(value)
except ValueError as e:
form.add_error(None, e.message)
return self.form_invalid(form)
return super().form_valid(form)
}}}
The error message is ignored. The main reason being that the form in
discarded in `FormView.form_invalid()`. Here's the current implementation
on master:
{{{#!python
def form_invalid(self, form):
"""
If the form is invalid, re-render the context data with the
data-filled form and errors.
"""
return self.render_to_response(self.get_context_data())
}}}
I propose to modify this implementation to:
{{{#!python
def form_invalid(self, form):
return self.render_to_response(self.get_context_data(form=form))
}}}
since `Form.get_context_data()` is doing `kwargs.setdefault('form',
self.get_form())`. I did that on my view, but it does feel this is
something that belongs in Django.
Also, from my little understanding of the code, this also mean that if I
have a form like this:
{{{#!python
class MyForm(Form):
def clean_value(self):
value = self.cleaned_data['value']
try:
heavy_computation(value)
except ValueError as e:
raise ValidationError(e.message)
return value
}}}
The `heavy_computation()` is going run twice.
--
--
Ticket URL: <https://code.djangoproject.com/ticket/25548#comment:1>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
--
You received this message because you are subscribed to the Google Groups
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
To post to this group, send email to [email protected].
To view this discussion on the web visit
https://groups.google.com/d/msgid/django-updates/065.d4f26bf2e49e59cebcc712b1cef3f42d%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.
