Opened 12 years ago

Closed 12 years ago

#17381 closed Bug (duplicate)

TemplateView get method passes kwargs into context

Reported by: Preston Holmes Owned by: nobody
Component: Generic views Version: dev
Severity: Normal Keywords:
Cc: Triage Stage: Unreviewed
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

TemplateView is unique among the CB Views in passing the kwargs passed to the get method (from the URLConf) directly into context

I believe this to be a bug - get_context_data does take kwargs, but they are intended as ways to override context values - context should not, by default, have all the kwargs being passed to the view from the URLConf.

The question is - can this fix be made as a backwards incompatible change - or does it need to go through deprecation?

Change History (6)

comment:1 by Preston Holmes, 12 years ago

ref #17242

comment:2 by anonymous, 12 years ago

When I started messing around with CBV I spent a significant amount of time trying to understand the logic behind that.

At first thought it was a weird API I didn't fully comprehend but after reading the development of #16074 I assumed it was simply a bug and worked around it in my own TemplateView implementation.

If that was indeed a design bug, is it really necessary to have a deprecation cycle to fix it?

comment:3 by Russell Keith-Magee, 12 years ago

The reason for this "feature" is historical; if you look at the old-style function based generic views, adding the kwargs to the template context was something that the direct_to_template generic view did. The old behavior was implemented to make it easy to port old views to the new structures.

On a broader project level -- If something is clearly a bug, we fix it. If this means breaking backwards compatibility, then we need to weigh the impact of the bug against the impact of making the change without a deprecation cycle. If there really is no way to fix a bug without introducing a backwards incompatibility, then we'll break backwards compatibility.

That said -- I'm not completely clear on what the 'bug' is considered to be here. Is it that get_context_data() is being passed the wrong arguments in a TemplateView, or that get_context_data() on a TemplateView is constructing the wrong context data? What exactly is the bug, and what is the consequences of that bug?

comment:4 by Preston Holmes, 12 years ago

Perhaps this is all backseat, after the fact, design bikeshedding, but to me a consistent behavior for get_context_data across all the class-based views makes it easier to understand what the behavior of a subclass would be when mixing together multiple parts of the class-based-view "toolkit"

This ticket is a near duplicate of #17228, in which I did recognize the historical reason, though I didn't initially here. I guess the reason this has come up for me multiple times is in working on #16074 and feeling like there should be an idea of a "get_context_data" contract, so that a view can call super across multiple classes and that there is a pattern that is followed throughout about how context is built.

This deviation from what is otherwise a relatively regular pattern can also throw people off about what should be the "default/standard" behavior for CBV see #17242

Where as direct-to-template was only usable in the simplest of situations, making a params context variable the primary method of customizing/tuning - TemplateView acts as a perfectly extendable base class, and it would be nice if it conformed a bit more to the pattern for get_context_data for the sake of predictability and comprehensibility.

Alternately documenting TemplateView as a "endpoint only" as a transition from direct-to-template, with a note to use the component mixins in a custom base for a more complex set of CBVs

comment:5 by anonymous, 12 years ago

ptone posted in #17228 what I believe should have been the proper way for TemplateView to mimic the direct_to_template behavior.

The current call to get_context_data in TemplateView breaks the pattern below which I believe is the only way to have multiple inheritance working properly especially now that the idea of having acontext_data dictionary have been dismissed in #16074.

def get_context_data(self, **kwargs):
    context = super(MyClass)(kwargs)
    context['foo'] = 'bar'
    return context

comment:6 by anonymous, 12 years ago

Resolution: duplicate
Status: newclosed

closing as dupe of #17228

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