[Django] #26018: FormMixin.get_context_data calls get_form improperly after form_invalid
Django
-------------------------------+--------------------
Reporter: chmarr | Owner: nobody
Type: Bug | Status: new
Component: Generic views | Version: 1.9
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------+--------------------
In the fix for ticket #25548, the validated (and failed) form is now
passed in as a parameter to get_context_data. In that code,
self.get_form() is called unnecessarily, which will likely cause a
performance and side-effect penalty equivalent or worse to validating the
form twice.
Solution is to use the "if x not in d" pattern instead of setdefault.
I'll create a pull request.
--
Ticket URL: <https://code.djangoproject.com/ticket/26018>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: chmarr | Owner: chmarr
Type: Bug | Status: assigned
Component: Generic views | Version: 1.9
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by chmarr):
* owner: nobody => chmarr
* needs_docs: => 0
* status: new => assigned
* needs_tests: => 0
* needs_better_patch: => 0
--
Ticket URL: <https://code.djangoproject.com/ticket/26018#comment:1>
Django
Reporter: chmarr | Owner: chmarr
Type: Bug | Status: assigned
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by chmarr):
* has_patch: 0 => 1
* stage: Unreviewed => Accepted
Comment:
Pull request: https://github.com/django/django/pull/5900
Release notes for 1.9.1 updated. Docs compile without error. Test case
added. All tests pass under Sqlite.
--
Ticket URL: <https://code.djangoproject.com/ticket/26018#comment:2>
Django
Reporter: chmarr | Owner: chmarr
Type: Bug | Status: assigned
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by chmarr):
Note: I added a call-counter stub to the AuthorUpdate view. Ideally I'd
use the "mock" library to do this kind of thing, as that would also let me
check the call-count after form_valid, rather than only being able to test
if res.context['view'] was present.
--
Ticket URL: <https://code.djangoproject.com/ticket/26018#comment:3>
Django
Reporter: chmarr | Owner: chmarr
Type: Bug | Status: assigned
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timgraham):
* stage: Accepted => Ready for checkin
Comment:
Thanks, I'll merge the release note with the one for #25548 since it seems
this issue isn't present in 1.9 but rather introduced in the fix for that
ticket as you mentioned.
By the way, we have automated checks on the pull request for docs building
without error and tests passing, so there's no need to mention all that
here.
--
Ticket URL: <https://code.djangoproject.com/ticket/26018#comment:4>
Django
Reporter: chmarr | Owner: chmarr
Component: Generic views | Version: 1.9
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Tim Graham <timograham@…>):
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"e429c5186ceed81c4627165518e0c70c58e69595" e429c51]:
{{{
#!CommitTicketReference repository=""
revision="e429c5186ceed81c4627165518e0c70c58e69595"
Fixed #26018 -- Prevented unecessary get_form() call in
FormMixin.get_context_data().
Changed "dict.setdefault" to "if x in dict" pattern so that get_form()
would not
be called unnecessarily, specifically in the case where
FormMixin.form_invalid()
calls get_context_data() with the current form.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/26018#comment:5>
Django
Reporter: chmarr | Owner: chmarr
Type: Bug | Status: closed
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Tim Graham <timograham@…>):
In [changeset:"8202ce45a5ae5dac2c6fdf94bf3554180bbdd7b7" 8202ce45]:
{{{
#!CommitTicketReference repository=""
revision="8202ce45a5ae5dac2c6fdf94bf3554180bbdd7b7"
[1.9.x] Fixed #26018 -- Prevented unecessary get_form() call in
FormMixin.get_context_data().
Changed "dict.setdefault" to "if x in dict" pattern so that get_form()
would not
be called unnecessarily, specifically in the case where
FormMixin.form_invalid()
calls get_context_data() with the current form.
Backport of e429c5186ceed81c4627165518e0c70c58e69595 from master
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/26018#comment:6>