Opened 11 years ago
Closed 10 years ago
#16744 closed New feature (fixed)
Class based view should have the view object in the context
Reported by: | Reinout van Rees | Owned by: | Tobias McNulty |
---|---|---|---|
Component: | Generic views | Version: | 1.3 |
Severity: | Normal | Keywords: | |
Cc: | jonas-django@…, taavi@…, reinout@…, jeroen@… | Triage Stage: | Accepted |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | yes |
Easy pickings: | no | UI/UX: | no |
Description (last modified by )
Summary: your_cbv_object.get_context_data()
should include {'view': self}
.
Now that django has class based views, I was surprised that I don't get the view object in my template's context. And that I still have to hand-craft my context dictionary.
What would be handier than setting attributes or adding methods to the view class and accessing them with {{ view.my_attribute }}
and {{ view.my_method }}`? Without having to do the double work of adding them by hand to the context dictionary? An additional benefit: you encourage to do even more in python and even less in the template by making it easy to add helper methods in your view.
The solution could as simple as adding {'view': self}
in the three or four spots in django itself where a .get_context_data()
is defined.
Attachments (2)
Change History (23)
comment:1 Changed 11 years ago by
comment:2 Changed 11 years ago by
Description: | modified (diff) |
---|---|
Triage Stage: | Unreviewed → Accepted |
Corrected get_context_dict
to get_context_data
.
comment:3 Changed 11 years ago by
Cc: | jonas-django@… added |
---|
comment:4 Changed 11 years ago by
Sounds like a good idea to me. I'd think Russ's second suggestion (get_context_data
returning some sort of proxy object) is a bit too complex, but I do like having the view object be available as {{ view }}
in the template.
If anyone wants to work up a patch, one important part not to miss is a careful audit of all view methods for ones with side-effects; those should be marked as alters_data
(https://docs.djangoproject.com/en/dev/ref/templates/api/#s-rendering-a-context).
comment:5 Changed 11 years ago by
Owner: | changed from nobody to Tobias McNulty |
---|---|
Status: | new → assigned |
comment:6 Changed 11 years ago by
Cc: | taavi@… added |
---|
comment:7 Changed 11 years ago by
Has patch: | set |
---|
Patch request with tests and docs here: https://github.com/django/django/pull/80
From what I could tell, all destructive view methods require an argument, so they can't be called from the template anyways.
comment:8 Changed 11 years ago by
This pull request looks good to me. Adds desired functionality with tests and docs.
Changed 11 years ago by
Attachment: | 16744.diff added |
---|
patch reflecting the requested doc and code changes in the pull request
comment:9 Changed 11 years ago by
Per a discussion with kmtracey at the sprint today, #16074 should get merged before this patch goes in.
comment:10 Changed 11 years ago by
#16074 has a patch now, but it had its version set from 1.3 to "svn". #16074 apparently should be merged first, so my question is if this #16774 is still on track for 1.4?
I'm hoping it makes 1.4 as it makes explaining class based views much more attractive in that new django book that I'm writing :-)
comment:11 Changed 11 years ago by
Cc: | reinout@… added |
---|
comment:12 Changed 11 years ago by
Bah, 1.4 beta is out. And this is not in. Despite a patch. Was it such a good idea to wait for #16074?
I'm going to have to re-write part of my introductory views-and-templates chapter, as class based views look silly without this patch in. At least for the simpler usecases. I might just have to put it in a later chapter, but that takes away much positive PR from class based views (which I was looking forward to giving them...)
I guess the version keyword should be changed to SVN or so? It's not going to end up in 1.3, right?
comment:13 Changed 11 years ago by
(Sorry, should have logged in. The "bah, 1.4 beta is out and this isn't in" comment is from me).
comment:14 Changed 11 years ago by
Replying to anonymous:
I guess the version keyword should be changed to SVN or so? It's not going to end up in 1.3, right?
Version field represents the first version where the bug existed, not the target version for a fix. It should almost never be changed (and also usually doesn't matter much).
Clearly we need to find some ways to better advertise the ticket workflow. Neither this ticket nor #16074 are marked Ready for Checkin, which is the best way to bring them to the attention of a core developer for commit. Anyone other than the patch author can push a ticket to the RFC stage if they've tested it, it applies cleanly and fixes the problem, the code looks good, etc.
comment:15 Changed 11 years ago by
The ticket workflow is accessible through the "How to contribute to Django" link under the "getting involved" heading on https://code.djangoproject.com/ .
I'd say that the ticket workflow/triaging link would fit well as a separate link under that "getting involved"; that would make it more visible.
comment:16 Changed 11 years ago by
Patch needs improvement: | set |
---|
Now that #16074 is in, the patch should be updated accordingly.
comment:17 Changed 11 years ago by
Patch needs improvement: | unset |
---|
comment:18 Changed 11 years ago by
Is "variable that points to the View
instance" in the documentation clear enough? I understand what's happening, but I wonder if an "(the view's
self
)" addition would make it clearer?
comment:19 Changed 11 years ago by
Cc: | jeroen@… added |
---|
comment:20 Changed 10 years ago by
Patch needs improvement: | set |
---|
Patch no longer applies cleanly, especially the docs part.
comment:21 Changed 10 years ago by
Resolution: | → fixed |
---|---|
Status: | assigned → closed |
As background, here's what Russ Magee said on the mailinglist about this after I asked about it: