[Django] #28943: Unenforce manual get_context_data()
Django
------------------------------------------------+------------------------
Reporter: James Pic | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: Generic views | Version: 2.0
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 1
UI/UX: 0 |
------------------------------------------------+------------------------
Currently, TemplateView inherits render_to_response(context) from
TemplateResponseMixin which requires a context argument.
Instead, TemplateView should have its own render_to_response(context=None)
that would get a context by default from get_context_data().
TemplateView.render_to_response() should call its
ContextMixin.get_context_data() method automatically is because
TemplateView inherits from both TemplateResponseMixin, and ContextMixin
which provides the get_context_data() method.
--
Ticket URL: <https://code.djangoproject.com/ticket/28943>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
-------------------------------------+-------------------------------------
Reporter: James Pic | Owner: nobody
Cleanup/optimization |
Component: Generic views | Version: 2.0
Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
Description changed by James Pic:
Old description:
> Currently, TemplateView inherits render_to_response(context) from
> TemplateResponseMixin which requires a context argument.
>
> Instead, TemplateView should have its own
> render_to_response(context=None) that would get a context by default from
> get_context_data().
>
> TemplateView.render_to_response() should call its
> ContextMixin.get_context_data() method automatically is because
> TemplateView inherits from both TemplateResponseMixin, and ContextMixin
> which provides the get_context_data() method.
New description:
Currently, TemplateView inherits render_to_response(context) from
TemplateResponseMixin which requires a context argument.
Instead, TemplateView should have its own render_to_response(context=None)
that would get a context by default from get_context_data().
TemplateView.render_to_response() should call its
ContextMixin.get_context_data() method automatically is because
TemplateView inherits from both TemplateResponseMixin, and ContextMixin
which provides the get_context_data() method.
Currently, the workaround is to call TemplateResponse.get(request, *args,
**kwargs), but users should really be calling what they mean to do
instead: super().render_to_response()
--
--
Ticket URL: <https://code.djangoproject.com/ticket/28943#comment:1>
Django
-------------------------------------+-------------------------------------
Cleanup/optimization |
Component: Generic views | Version: 2.0
Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
Description changed by James Pic:
Old description:
> Currently, TemplateView inherits render_to_response(context) from
> TemplateResponseMixin which requires a context argument.
>
> Instead, TemplateView should have its own
> render_to_response(context=None) that would get a context by default from
> get_context_data().
>
> TemplateView.render_to_response() should call its
> ContextMixin.get_context_data() method automatically is because
> TemplateView inherits from both TemplateResponseMixin, and ContextMixin
> which provides the get_context_data() method.
>
> Currently, the workaround is to call TemplateResponse.get(request, *args,
> **kwargs), but users should really be calling what they mean to do
> instead: super().render_to_response()
New description:
Currently, TemplateView inherits render_to_response(context) from
TemplateResponseMixin which requires a context argument.
Instead, TemplateView should have its own render_to_response(context=None)
that would get a context by default from get_context_data().
TemplateView.render_to_response() should call its
ContextMixin.get_context_data() method automatically is because
TemplateView inherits from both TemplateResponseMixin, and ContextMixin
which provides the get_context_data() method.
Currently, the workaround is to call
{{{
# note usage of self in the method call
TemplateResponse.get(self, request, *args, **kwargs)
}}}
However, users should really be calling what they mean to do instead:
{{{
super().render_to_response()
}}}
Then, then can still resolve {{ view }} in the template to add more
variables.
--
--
Ticket URL: <https://code.djangoproject.com/ticket/28943#comment:2>
Django
-------------------------------------+-------------------------------------
Cleanup/optimization |
Component: Generic views | Version: 2.0
Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
-------------------------------------+-------------------------------------
Changes (by Tim Graham):
* easy: 1 => 0
Comment:
The ticket summary is cryptic. Please describe the use case in more
detail. I don't see an indication of where `TemplateResponse.get(self,
request, *args, **kwargs)` lives.
--
Ticket URL: <https://code.djangoproject.com/ticket/28943#comment:3>
Django
-------------------------------------+-------------------------------------
Cleanup/optimization |
Component: Generic views | Version: 2.0
Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
-------------------------------------+-------------------------------------
Description changed by James Pic:
Old description:
> Currently, TemplateView inherits render_to_response(context) from
> TemplateResponseMixin which requires a context argument.
>
> Instead, TemplateView should have its own
> render_to_response(context=None) that would get a context by default from
> get_context_data().
>
> TemplateView.render_to_response() should call its
> ContextMixin.get_context_data() method automatically is because
> TemplateView inherits from both TemplateResponseMixin, and ContextMixin
> which provides the get_context_data() method.
>
> Currently, the workaround is to call
> {{{
> # note usage of self in the method call
> TemplateResponse.get(self, request, *args, **kwargs)
> }}}
>
> However, users should really be calling what they mean to do instead:
>
> {{{
> super().render_to_response()
> }}}
>
> Then, then can still resolve {{ view }} in the template to add more
> variables.
New description:
Currently, TemplateView inherits render_to_response(context) from
TemplateResponseMixin which requires a context argument.
This means that when your TemplateView subclass wants to return the
TemplateResponse with the default context, you still have to create and
pass the default context:
{{{
class YourView(TemplateView):
def post(self, request, *a, **k):
if not self.dostuff():
return http.HttpResponseBadRequest()
context = self.get_context_data()
return self.render_to_response(context)
}}}
The reason for this is that ContentMixin defines get_context_data(), and
TemplateResponseMixin defines render_to_response(context, ...),
TemplateResponse mixes the two in get():
{{{
class TemplateView(TemplateResponseMixin, ContextMixin, View):
"""
Render a template. Pass keyword arguments from the URLconf to the
context.
"""
def get(self, request, *args, **kwargs):
context = self.get_context_data(**kwargs)
return self.render_to_response(context)
}}}
I think it would be more usable as such:
{{{
class TemplateView(TemplateResponseMixin, ContextMixin, View):
"""
Render a template. Pass keyword arguments from the URLconf to the
context.
"""
def get(self, request, *args, **kwargs):
return self.render_to_response()
def render_to_response(self, context=None):
context = context or self.get_context_data(**kwargs)
return self.render_to_response(context)
}}}
Then, users could call render_to_response() in their code, ie:
{{{
class YourView(TemplateView):
def get(self, request, *a, **k):
self.token = self.generatetoken()
return self.render_to_response()
def post(self, request, *a, **k):
if not self.dostuff():
return http.HttpResponseBadRequest()
return super().render_to_response()
}}}.
--
--
Ticket URL: <https://code.djangoproject.com/ticket/28943#comment:4>
Django
-------------------------------------+-------------------------------------
Cleanup/optimization |
Component: Generic views | Version: 2.0
Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
-------------------------------------+-------------------------------------
Comment (by James Pic):
Thanks for your feedback, I rewrote the issue, does it make any sense now
?
--
Ticket URL: <https://code.djangoproject.com/ticket/28943#comment:5>
Django
-------------------------------------+-------------------------------------
Cleanup/optimization |
Component: Generic views | Version: 2.0
Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
-------------------------------------+-------------------------------------
Description changed by James Pic:
Old description:
> Currently, TemplateView inherits render_to_response(context) from
> TemplateResponseMixin which requires a context argument.
>
New description:
Currently, TemplateView inherits render_to_response(context) from
TemplateResponseMixin which requires a context argument.
This means that when your TemplateView subclass wants to return the
{{{
}}}
{{{
return self.render_to_response()
}}}
--
--
Ticket URL: <https://code.djangoproject.com/ticket/28943#comment:6>
Django
-------------------------------------+-------------------------------------
Cleanup/optimization |
Component: Generic views | Version: 2.0
Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
-------------------------------------+-------------------------------------
Description changed by James Pic:
Old description:
> Currently, TemplateView inherits render_to_response(context) from
> TemplateResponseMixin which requires a context argument.
>
New description:
Currently, TemplateView inherits render_to_response(context) from
TemplateResponseMixin which requires a context argument.
This means that when your TemplateView subclass wants to return the
{{{
}}}
def render_to_response(self, context=None):
context = context or self.get_context_data(self.kwargs)
return self.render_to_response(context)
}}}
Then, users could call render_to_response() in their code, ie:
{{{
class YourView(TemplateView):
def get(self, request, *a, **k):
self.token = self.generatetoken()
return self.render_to_response()
def post(self, request, *a, **k):
if not self.dostuff():
return http.HttpResponseBadRequest()
return self.render_to_response()
}}}
--
--
Ticket URL: <https://code.djangoproject.com/ticket/28943#comment:7>
Django
-------------------------------------+-------------------------------------
Cleanup/optimization |
Component: Generic views | Version: 2.0
Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
-------------------------------------+-------------------------------------
Description changed by James Pic:
Old description:
> Currently, TemplateView inherits render_to_response(context) from
> TemplateResponseMixin which requires a context argument.
>
New description:
Currently, TemplateView inherits render_to_response(context) from
TemplateResponseMixin which requires a context argument.
This means that when your TemplateView subclass wants to return the
TemplateResponse with the default context, you still have to create and
pass the default context:
{{{
class YourView(TemplateView):
def post(self, request, *a, **k):
if not self.dostuff():
return http.HttpResponseBadRequest()
context = self.get_context_data()
return self.render_to_response(context)
}}}
The reason for this is that ContentMixin defines get_context_data(), and
TemplateResponseMixin defines render_to_response(context, ...),
TemplateResponse mixes the two in get():
{{{
class TemplateView(TemplateResponseMixin, ContextMixin, View):
"""
Render a template. Pass keyword arguments from the URLconf to the
context.
"""
def get(self, request, *args, **kwargs):
context = self.get_context_data(**kwargs)
return self.render_to_response(context)
}}}
I think it would be more usable as such:
{{{
class TemplateView(TemplateResponseMixin, ContextMixin, View):
"""
Render a template. Pass keyword arguments from the URLconf to the
context.
"""
def get(self, request, *args, **kwargs):
return self.render_to_response(**kwargs)
def render_to_response(self, context=None, **kwargs):
context = context or self.get_context_data(**kwargs)
return self.render_to_response(context)
}}}
Then, users could call render_to_response() in their code, ie:
{{{
class YourView(TemplateView):
def get(self, request, *a, **k):
self.token = self.generatetoken()
return self.render_to_response(**k)
def post(self, request, *a, **k):
if not self.dostuff():
return http.HttpResponseBadRequest()
return self.render_to_response(**k)
}}}
--
--
Ticket URL: <https://code.djangoproject.com/ticket/28943#comment:8>
Django
-------------------------------------+-------------------------------------
Cleanup/optimization |
Component: Generic views | Version: 2.0
Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
-------------------------------------+-------------------------------------
Description changed by James Pic:
Old description:
> Currently, TemplateView inherits render_to_response(context) from
> TemplateResponseMixin which requires a context argument.
>
New description:
Currently, TemplateView inherits render_to_response(context) from
TemplateResponseMixin which requires a context argument.
This means that when your TemplateView subclass wants to return the
TemplateResponse with the default context, you still have to create and
pass the default context:
{{{
class YourView(TemplateView):
def post(self, request, *a, **k):
if not self.dostuff():
return http.HttpResponseBadRequest()
context = self.get_context_data(*k)
return self.render_to_response(context)
}}}
{{{
--
--
Ticket URL: <https://code.djangoproject.com/ticket/28943#comment:9>
Django
-------------------------------------+-------------------------------------
Cleanup/optimization |
Component: Generic views | Version: 2.0
Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
-------------------------------------+-------------------------------------
Description changed by James Pic:
Old description:
> Currently, TemplateView inherits render_to_response(context) from
> TemplateResponseMixin which requires a context argument.
>
New description:
Currently, TemplateView inherits render_to_response(context) from
TemplateResponseMixin which requires a context argument.
This means that when your TemplateView subclass wants to return the
{{{
}}}
Then, users could call render_to_response() in their code without dealing
with a context they don't override because they are satisfied with the
default (which adds view=self in the context), ie:
{{{
class YourView(TemplateView):
def get(self, request, *a, **k):
self.token = self.generatetoken()
return self.render_to_response(**k)
def post(self, request, *a, **k):
if not self.dostuff():
return http.HttpResponseBadRequest()
return self.render_to_response(**k)
}}}
--
--
Ticket URL: <https://code.djangoproject.com/ticket/28943#comment:10>
Django
-------------------------------------+-------------------------------------
Reporter: James Pic | Owner: nobody
Cleanup/optimization |
Component: Generic views | Version: 2.0
Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
-------------------------------------+-------------------------------------
Comment (by Tim Graham):
Is there a reason to duplicate the `render_to_response()` call in
`post()`? I didn't test it, but I believe you could write both examples
as:
{{{
class YourView(TemplateView):
def post(self, request, *a, **k):
if not self.dostuff():
return http.HttpResponseBadRequest()
return super().get(request, *a, **k)
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/28943#comment:11>
Django
-------------------------------------+-------------------------------------
Cleanup/optimization |
Component: Generic views | Version: 2.0
| worksforme
Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
-------------------------------------+-------------------------------------
Changes (by Tim Graham):
* status: new => closed
* resolution: => worksforme
--
Ticket URL: <https://code.djangoproject.com/ticket/28943#comment:12>
Django
-------------------------------------+-------------------------------------
Cleanup/optimization |
Component: Generic views | Version: 2.0
| worksforme
Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
-------------------------------------+-------------------------------------
Comment (by James Pic):
Of course, that works, unless you've overridden get() to do things that
you don't want to happen on post.
In post, i would like to render to template again, not run get() ;)
--
Ticket URL: <https://code.djangoproject.com/ticket/28943#comment:13>
Django
-------------------------------------+-------------------------------------
Cleanup/optimization |
Component: Generic views | Version: 2.0
| worksforme
Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
-------------------------------------+-------------------------------------
Comment (by Tim Graham):
Unless I missed something, `super().get()` will call the `TemplateView`
implementation, even if you override `get()` in a subclass.
--
Ticket URL: <https://code.djangoproject.com/ticket/28943#comment:14>
Django
-------------------------------------+-------------------------------------
Cleanup/optimization |
Component: Generic views | Version: 2.0
| worksforme
Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
-------------------------------------+-------------------------------------
Comment (by James Pic):
Yes, returning super().get() allows to bypass the last level of get()
override. I try to demonstrate the issue with this script, perhaps it will
make more sense.
{{{
class TemplateView(object):
def get(self):
print('render to response()')
class YourBaseView(TemplateView):
def get(self):
print('generate token()')
return super().get()
class YourView(YourBaseView):
def post(self):
# if not dosomething(): return bad request
return super().get()
YourView().post()
}}}
This will print both 'generate token()' and 'render to response()'. If I
only want to print 'render to response()', then this works:
{{{
class YourView(YourBaseView):
def post(self):
# if not dosomething(): return bad request
return TemplateView.get(self)
}}}
But probably it would make sense to just move the render to response
outside get() as such:
{{{
class TemplateView(object):
def render_to_response(self):
print('render to response()')
def get(self):
return self.render_to_response()
class YourBaseView(TemplateView):
def get(self):
print('generate token()')
return super().get()
class YourView(YourBaseView):
def post(self):
# if not dosomething(): return bad request
return self.render_to_response()
}}}
For me it seems like the last solution is best OOP but perhaps i'm
mistaking again.
--
Ticket URL: <https://code.djangoproject.com/ticket/28943#comment:15>
Django
-------------------------------------+-------------------------------------
Cleanup/optimization |
Component: Generic views | Version: 2.0
| worksforme
Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
-------------------------------------+-------------------------------------
Comment (by James Pic):
> Unless I missed something, super().get() will call the TemplateView
implementation, even if you override get() in a subclass.
That's True unless YourFooDetailView inherits from YourProjectDetailView
which would inherit from django.views.generic.DetailView.
When you work as a Django user, you often want to add a project-specific
layer between your actual user facing views and django views, ie. to
refactor common features you have in all your
{List,Detail,Update,Create,Form,Object,Model}View classes accross the
project's app.
And anyway, I think get_context_data() deserves to be removed from
Django's public API, this is made to support legacy templates, new
templates just use {{ view.object }} than {{ object }} because then they
make @object a memoized property which they can always use in
{dispatch,get,post,delete,options} methods instead of thinking they're
paid by the quantity of lines of code and take pride in overriding and
decorating get_context_data() for no reason thanks to the visionary who
made view=self a default in CBV.
Maybe what I'm saying doesn't make any sense to anybody than myself lol
but at least you can point-godwin me because i don't consider the
resolution of this ticket worksforme, but don't take it personnaly, you
know i still admire you from the deepest of my heart Tim <3
With LOVE
∞
--
Ticket URL: <https://code.djangoproject.com/ticket/28943#comment:16>