Opened 13 years ago
Closed 13 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 , 13 years ago
comment:2 by , 13 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 , 13 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 , 13 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 , 13 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
ref #17242