Opened 13 years ago

Closed 12 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 Jannis Leidel)

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)

16744.diff (8.8 KB ) - added by Tobias McNulty 12 years ago.
patch reflecting the requested doc and code changes in the pull request
16744-2.diff (7.9 KB ) - added by Claude Paroz 12 years ago.
Updated patch

Download all attachments as: .zip

Change History (23)

comment:1 by Reinout van Rees, 13 years ago

As background, here's what Russ Magee said on the mailinglist about this after I asked about it:

To the best of my knowledge, the only reason the view isn't included
in the template context is because over more than two years of design
discussion, I don't think the idea of including the view in the
context was ever proposed.

It seems like a reasonable idea to me, though, and it should be
possible to accommodate in a backwards compatible fashion. The trivial
fix would be to add a 'view' variable to the default context. It might
also be possible to replace the default get_context_data
implementation with something that reflects the attributes of the view
object itself -- however, this will obviously require a bit more
design work to make sure there aren't any backwards incompatible
implications.

comment:2 by Jannis Leidel, 13 years ago

Description: modified (diff)
Triage Stage: UnreviewedAccepted

Corrected get_context_dict to get_context_data.

comment:3 by Jonas H., 13 years ago

Cc: jonas-django@… added

comment:4 by Jacob, 13 years ago

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 by Tobias McNulty, 13 years ago

Owner: changed from nobody to Tobias McNulty
Status: newassigned

comment:6 by Taavi Taijala, 13 years ago

Cc: taavi@… added

comment:7 by Tobias McNulty, 12 years ago

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 by Mark Lavin, 12 years ago

This pull request looks good to me. Adds desired functionality with tests and docs.

by Tobias McNulty, 12 years ago

Attachment: 16744.diff added

patch reflecting the requested doc and code changes in the pull request

comment:9 by Tobias McNulty, 12 years ago

Per a discussion with kmtracey at the sprint today, #16074 should get merged before this patch goes in.

comment:10 by Reinout van Rees, 12 years ago

#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 by Reinout van Rees, 12 years ago

Cc: reinout@… added

comment:12 by anonymous, 12 years ago

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 by Reinout van Rees, 12 years ago

(Sorry, should have logged in. The "bah, 1.4 beta is out and this isn't in" comment is from me).

comment:14 by Carl Meyer, 12 years ago

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 by Reinout van Rees, 12 years ago

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 by Claude Paroz, 12 years ago

Patch needs improvement: set

Now that #16074 is in, the patch should be updated accordingly.

by Claude Paroz, 12 years ago

Attachment: 16744-2.diff added

Updated patch

comment:17 by Claude Paroz, 12 years ago

Patch needs improvement: unset

comment:18 by Reinout van Rees, 12 years ago

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 by Jeroen Dekkers, 12 years ago

Cc: jeroen@… added

comment:20 by Andrew Godwin, 12 years ago

Patch needs improvement: set

Patch no longer applies cleanly, especially the docs part.

comment:21 by Andrew Godwin <andrew@…>, 12 years ago

Resolution: fixed
Status: assignedclosed

In [58683e9c82d6e7c5fbb7acef79eef9408b776ab0]:

Fixed #16744 -- Class based view should have the view object in the context

Updated the most recent patch from @claudep to apply again and updated
the documentation location.

Note: See TracTickets for help on using tickets.
Back to Top