#26018 closed Bug (fixed)
FormMixin.get_context_data calls get_form improperly after form_invalid
Reported by: | Chris Cogdon | Owned by: | Chris Cogdon |
---|---|---|---|
Component: | Generic views | Version: | 1.9 |
Severity: | Normal | Keywords: | |
Cc: | Triage Stage: | Ready for checkin | |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
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.
Change History (6)
comment:1 by , 9 years ago
Owner: | changed from | to
---|---|
Status: | new → assigned |
comment:2 by , 9 years ago
Has patch: | set |
---|---|
Triage Stage: | Unreviewed → Accepted |
comment:3 by , 9 years ago
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.
comment:4 by , 9 years ago
Triage Stage: | Accepted → Ready for checkin |
---|
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.
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.