Opened 4 years ago

Closed 3 years ago

#16744 closed New feature (fixed)

Class based view should have the view object in the context

Reported by: reinout Owned by: tobias
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 jezdez)

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 4 years ago.
patch reflecting the requested doc and code changes in the pull request
16744-2.diff (7.9 KB) - added by claudep 3 years ago.
Updated patch

Download all attachments as: .zip

Change History (23)

comment:1 Changed 4 years ago by reinout

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

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 Changed 4 years ago by jezdez

  • Description modified (diff)
  • Triage Stage changed from Unreviewed to Accepted

Corrected get_context_dict to get_context_data.

comment:3 Changed 4 years ago by jonash

  • Cc jonas-django@… added

comment:4 Changed 4 years ago by jacob

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 4 years ago by tobias

  • Owner changed from nobody to tobias
  • Status changed from new to assigned

comment:6 Changed 4 years ago by taavi223

  • Cc taavi@… added

comment:7 Changed 4 years ago by tobias

  • 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 4 years ago by mlavin

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

Changed 4 years ago by tobias

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

comment:9 Changed 4 years ago by tobias

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

comment:10 Changed 4 years ago by reinout

#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 4 years ago by reinout

  • Cc reinout@… added

comment:12 Changed 4 years ago by anonymous

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 4 years ago by reinout

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

comment:14 Changed 4 years ago by carljm

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 4 years ago by reinout

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 3 years ago by claudep

  • Patch needs improvement set

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

Changed 3 years ago by claudep

Updated patch

comment:17 Changed 3 years ago by claudep

  • Patch needs improvement unset

comment:18 Changed 3 years ago by reinout

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 3 years ago by dekkers

  • Cc jeroen@… added

comment:20 Changed 3 years ago by andrewgodwin

  • Patch needs improvement set

Patch no longer applies cleanly, especially the docs part.

comment:21 Changed 3 years ago by Andrew Godwin <andrew@…>

  • Resolution set to fixed
  • Status changed from assigned to closed

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