FormView.form_invalid() discards its form argument resulting in duplicated validation

[Antoine Catton]

Let's consider this example:

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:

    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:

    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:

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.

[Simon Charette]

This makes a lot of sense.

[Alex Morozov]

@dudepare, if you don't mind, I can make a PR.

[Andrew Artajos]

Sure go ahead, I'm curious to see the unittest for this.

[Alex Morozov]

​https://github.com/django/django/pull/5620
@dudepare, you mean there should be something tricky with a unittest? I feel like I've missed something big :).

[Simon Charette]

Provided patch LGTM pending a small docstring rephrasing.

[Alex Morozov]

@charettes, fixed.

[Simon Charette <charette.s@…>]

In e171a83:

Fixed #25548 -- Prevented FormView.form_invalid() from discarding its form argument.

[Ilya Baryshev]

Could this be backported to upcoming 1.9.1 please? This is a fix for regression in 1.9, so I hope this makes sense )

[Tim Graham]

Yes, I'll backport now. I think the only reason it wasn't is that there's no obvious indication that this is a regression (introduced in #24643).

[Tim Graham <timograham@…>]

In 0154702a:

[1.9.x] Fixed #25548 -- Prevented FormView.form_invalid() from discarding its form argument.

Backport of e171a83b1516a3a2a393a5300527af1aab21c213 from master

[Tim Graham <timograham@…>]

In 285b08ab:

Refs #25548 -- Forwardported 1.9.1 release note.