Opened 11 years ago

Closed 4 years ago

Last modified 4 years ago

#19878 closed Cleanup/optimization (wontfix)

Stop TemplateView automatically passing kwargs into the context

Reported by: Alexey Boriskin Owned by: Adam Johnson
Component: Generic views Version: dev
Severity: Normal Keywords: django-sprint
Cc: marc.tamlyn@…, mszamot@…, chris.jerdonek@…, Adam Johnson Triage Stage: Unreviewed
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

Only TemplateView pushes self.kwargs to the context. ListView does not, I yet have to check others.
This is inconsistency and, I think, it should be fixed.

Change History (25)

comment:1 by Carl Meyer, 11 years ago

Triage Stage: UnreviewedDesign decision needed
Type: BugNew feature

Setting this DDN pending comment from someone who uses class-based views. Seems like a new feature rather than a bug, in any case.

comment:2 by Jonas Svensson <jonas.s.svensson@…>, 11 years ago

Has patch: set
Keywords: django-sprint added
Resolution: fixed
Status: newclosed

https://github.com/django/django/pull/753

The CBV:

  • BaseDetailView
  • ProcessFormView
  • BaseListView

Did not push the kwargs into the context

comment:3 by Alexey Boriskin, 11 years ago

Resolution: fixed
Status: closednew

The ticket is not fixed until patch in merged into the django. Please don't set status to closed until it's not closed.

comment:4 by Aymeric Augustin, 11 years ago

This can be backwards incompatible in keys in kwargs clash with keys in the output of get_context_data().

The current behavior might be for backwards compatibility with the now-removed function-based generic views. Maybe this was discussed on django-developers?

This would need docs (release notes + check if the CBV docs need to be updated), and tests.

comment:5 by Aymeric Augustin, 11 years ago

Needs documentation: set
Needs tests: set

comment:6 by Marc Tamlyn, 11 years ago

Cc: marc.tamlyn@… added

I am strongly the other way on this issue - I think the feature is wrong in TemplateView and should be deprecated.

For example, it means that it's hard to have context_object_name and slug_url_kwarg the same - whether you get the object or the slug would be rather arbitrary.

comment:7 by Jacob, 11 years ago

Resolution: wontfix
Status: newclosed

Agreed with mjtamlyn, so marking this wontfix.

comment:8 by Marc Tamlyn, 11 years ago

Jacob - what's your opinion on taking the deprecation the other way - this one-off behaviour of TemplateView has bitten me before. Personally I'd like the views to be consistently *without* this feature.

comment:9 by Aymeric Augustin, 11 years ago

Resolution: wontfix
Status: closednew

comment:10 by Aymeric Augustin, 11 years ago

Has patch: unset
Needs documentation: unset
Needs tests: unset
Triage Stage: Design decision neededAccepted

IIRC TemplateView works like this for backwards-compatibility with the (now defunct) function-based generic views.

It would make sense to normalize it now, assuming we can figure out a deprecation path.

comment:11 by Marc Tamlyn, 11 years ago

Summary: Not all class based views push self.kwargs to contextStop TemplateView automatically passing kwargs into the context

comment:12 by Marcin Szamotulski <mszamot@…>, 11 years ago

Cc: mszamot@… added

comment:13 by Marc Tamlyn, 11 years ago

Hmm, I know I'd done something at some point using these views which had bitten me, but I can't remember what it was. This is near impossible to deprecate nicely and probably shouldn't be that big a deal.

That said, it didn't exist before 1.5 in its current form - the way these arguments are used was (backwardsly incompatibly) changed then anyway. Personally I'd like to just remove it as it "feels wrong", but whether that is a good idea is another question.

History lesson

Django 1.2 had direct_to_template which would pass through URL kwargs as an object called params into the context
Django 1.3 & 1.4 continued this pattern in the new TemplateView
Django 1.5 changed TemplateView to place the arguments directly into the context (and deprecated direct_to_template)

My personal feeling is that using URLconf kwargs directly in a view is a bad pattern, but it is a historical Django pattern. This, and the ability to customise CBVs using the as_view() call, are the remaining parts of it.

comment:14 by loic84, 11 years ago

I ran into the same problem as @mjtamlyn with TemplateView; and I also think that having the URL kwargs directly in the context_data is an anti pattern.

The anti pattern added to the discrepancy between the different views is IMO a problem big enough to warrant a documented backward incompatible change.

I tried to think of a transparent deprecation cycle, but what I found was at best fragile and hackish.

One option would be to ask people to set a kwargs_to_context = False argument on their TemplatView to get the "new" behavior and therefore opt out from the DeprecationWarning.

Thoughts?

comment:15 by Tim Graham, 11 years ago

I'm not much of a user of CBVs but that deprecation plan sounds like it would work.

comment:16 by Chris Jerdonek, 10 years ago

Note that there is a distinction between passing **kwargs to get_context_data() and having get_context_data() return **kwargs.

Personally, I would favor TemplateView continuing to pass **kwargs to get_context_data() even if its return value is changed by default not to include kwargs. This would keep overriding get_context_data() simple because you can simply access **kwargs in the method without having to decide between **kwargs and self.kwargs.

For consistency, I would like it if all generic views passed the full view's keyword arguments to get_context_data(). Some classes like ProcessFormView restrict what is passed to get_context_data(), which was surprising to me because then **kwargs for the method differs from self.kwargs. I just opened #21964 which is about this issue.

comment:17 by Chris Jerdonek, 10 years ago

Cc: chris.jerdonek@… added

comment:18 by Adam Johnson, 5 years ago

Has patch: set
Needs documentation: set
Owner: changed from nobody to Adam Johnson
Patch needs improvement: set
Status: newassigned

comment:19 by Adam Johnson, 5 years ago

Cc: Adam Johnson added

comment:20 by Adam Johnson, 4 years ago

Needs documentation: unset
Patch needs improvement: unset

comment:21 by Mariusz Felisiak, 4 years ago

Triage Stage: AcceptedReady for checkin
Type: New featureCleanup/optimization

comment:22 by Mariusz Felisiak <felisiak.mariusz@…>, 4 years ago

Resolution: fixed
Status: assignedclosed

In 4ed53475:

Fixed #19878 -- Deprecated TemplateView passing URL kwargs into context.

comment:23 by Mariusz Felisiak <felisiak.mariusz@…>, 4 years ago

In bb8f6693:

Fixed #31877 -- Reverted "Fixed #19878 -- Deprecated TemplateView passing URL kwargs into context."

This reverts commit 4ed534758cb6a11df9f49baddecca5a6cdda9311.

comment:24 by Mariusz Felisiak <felisiak.mariusz@…>, 4 years ago

In e81aa7a:

[3.1.x] Fixed #31877 -- Reverted "Fixed #19878 -- Deprecated TemplateView passing URL kwargs into context."

This reverts commit 4ed534758cb6a11df9f49baddecca5a6cdda9311.

Backport of bb8f66934d93faf80cd1a2dda65aaedce21a6fc5 from master

comment:25 by Mariusz Felisiak, 4 years ago

Has patch: unset
Resolution: fixedwontfix
Triage Stage: Ready for checkinUnreviewed

Marked as wontfix because we don't have a clear deprecation path (see #31877).

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