django-rest-framework
django-rest-framework copied to clipboard
TemplateHTMLRenderer - TypeError - context must be a dict rather than ReturnList.
Checklist
- [x] I have verified that that issue exists against the
master
branch of Django REST framework. - [x] I have searched for similar issues in both open and closed tickets and cannot find a duplicate.
- [x] This is not a usage question. (Those should be directed to the discussion group instead.)
- [x] This cannot be dealt with as a third party library. (We prefer new functionality to be in the form of third party libraries where possible.)
- [x] I have reduced the issue to the simplest possible case.
- [ ] I have included a failing test as a pull request. (If you are unable to do so we can still accept the issue.)
Steps to reproduce
In Django 1.8+, the template's render method takes a dictionary for the context parameter. Support for passing a Context instance is deprecated, and gives an error in Django 1.10+ (source1 / source2).
When adding a renderer to a generics.ListCreateAPIView
, a TypeError: context must be a dict rather than ReturnList.
pop.
To reproduce, create a simple Model+Serializer to fill data in a generics.ListCreateAPIView
. Add a renderer_classes = [TemplateHTMLRenderer]
to the ListView and try to access the page from a browser to render the template. Should crash (error 500 as an Exception is raised).
Expected behavior
The template should be rendered.
Actual behavior
A TypeError
is thrown because context must be a dict rather than ReturnList
Thanks for raising this.
Example stacktrace:
Traceback (most recent call last):
File "/PATH/python3.5/site-packages/django/core/handlers/exception.py", line 41, in inner
response = get_response(request)
File "/PATH/python3.5/site-packages/django/core/handlers/base.py", line 217, in _get_response
response = self.process_exception_by_middleware(e, request)
File "/PATH/python3.5/site-packages/django/core/handlers/base.py", line 215, in _get_response
response = response.render()
File "/PATH/python3.5/site-packages/django/template/response.py", line 107, in render
self.content = self.rendered_content
File "/PATH/python3.5/site-packages/rest_framework/response.py", line 72, in rendered_content
ret = renderer.render(self.data, accepted_media_type, context)
File "/PATH/python3.5/site-packages/rest_framework/renderers.py", line 176, in render
return template_render(template, context, request=request)
File "/PATH/python3.5/site-packages/rest_framework/compat.py", line 340, in template_render
return template.render(context, request=request)
File "/PATH/python3.5/site-packages/django/template/backends/django.py", line 64, in render
context = make_context(context, request, autoescape=self.backend.engine.autoescape)
File "/PATH/python3.5/site-packages/django/template/context.py", line 287, in make_context
raise TypeError('context must be a dict rather than %s.' % context.__class__.__name__)
TypeError: context must be a dict rather than ReturnList.
I think it may only be the case if PAGE_SIZE
is None, too, because changing that to != None wraps the results up nested within an OrderedDict
, which passes the check by the look of it.
I just ran into this. Is there a workaround? or a local fix I can apply for development?
@kezabelle @tomchristie As far as I understand this issue, irrespective of the change in Django 1.10+ mentioned originally by @Eskimon, the TemplateHTMLRenderer
has never been able to work with a ReturnList
, or anything other than a dict like object for that matter. When serializers are run in read mode using many=True
, the data structure coming out of them are lists. The problem occurs when your pagination_class
is set to None
. I have had to alter the behaviour of the ListSerializer
's to_representation
and ensure the returned object is a dict, in order to get my template renderer to work. If you are using pagination, then the list of results are anyways wrapped up in an dict and sent to the renderer, so it should work in that case.
@kezabelle You are right in saying that simply ensuring (somehow) that dicts are returned will make the renderer work.
As far as the Django 1.10+ issue is concerned, the change suggests using native dicts instead of Context
objects. From what I can see, this should not affect the working of TemplateHTMLRenderer
.
TL;DR: Does not look like a bug to me.
FWIW I expected a TemplateHTMLRenderer to behave like the API UI.
- On single pages, I expected the template to accept and render a single object.
- On list pages, I expected the template to accept a list of objects and be able to render a single page that contains a representation of each of those objects.
Apparently, this is not the expected behavior. It's also non-obvious why (or even how) paging magically changes an unacceptable list into a shorter but acceptable "list".
I stand by my "intuitive" argument, but dug through the code to figure out why paging "fixes" the issue.
It turns out that the default paginator nests the list of objects under a results
key. However, I believe this is a quirk of implementation as it would be acceptable (if not best practice) to return a simple list and place the paging metatdata in headers (like this) or exclude it entirely.
Obviously, I'm with the OP that this seems like a valid use case (albeit not a new issue). While the pagination class seems like the right place to handle this, I don't think it's practical:
- There's a proposal to remove the Default Pagination Class so no class would even be available.
- If the Default Pagination Class were retained, fixing this issue would spill over to all rendering engines which is presumably unwanted.
This militates for a fix that is limited to the TemplateHTMLRenderer. The two options I see are:
- If
response.data
is a list, wrap it in a key. I don't like this option because it hard codes a specific key for a particular edge case. If an actual paginator were used, the keys wouldn't necessarily match. - Instead of expanding
response.data
into the context, always next it under a key (e.g.data
). This key would be consistent regardless of the paginator or call type (instance vs. list). However, this would be backwards incompatible for current users of this Renderer.
Note that the documentation for TemplateHTMLRenderer states that that:
Unlike other renderers, the data passed to the Response does not need to be serialized
I imagine there are many unserialized objects that are not dict
-like. While backwards incompatible changes are undesirable, (2) would better align the implementation with the documentation.
... and for anyone who needs a workaround, add a line to your view like:
response.data = {'results': response.data}
or, mimicking my recommended fix (at data
instead of results
):
from rest_framework.renderers import TemplateHTMLRenderer
class MyTemplateHTMLRenderer(TemplateHTMLRenderer):
def get_template_context(self, data, renderer_context):
response = renderer_context['response']
if response.exception:
data['status_code'] = response.status_code
return {'data': data}
I seem to have come across this issue as well. From my point of view it's a bit baffling - with a ModelViewSet
, most of the renderers you'd expect to "just work" apart from TemplateHTMLRenderer
. This should at least be clarified in the documentation.
Thank you to @ambsw-technology for the workarounds.
PR to improve the documentation is welcomed :)
Okay. Would be useful to have someone who understands this issue better give some input on #6095.
I have been bitten by this this week. And agree with the above - it doesn't feel like expected behaviour - nor is it clear what is happening immediately.
I'm quite happy to submit a PR which would wrap the list in a dict with a key of items
or whatever is most appropriate (maybe results
to match the pagination class).
But I do want to know if this is even desirable for the maintainers - given this issue has been left open for 2 years.
FYI I've worked around this for now like so:
from rest_framework.renderers import TemplateHTMLRenderer
class MyHTMLRenderer(TemplateHTMLRenderer):
def get_template_context(self, *args, **kwargs):
context = super().get_template_context(*args, **kwargs)
if isinstance(context, list):
context = {"items": context}
return context
Which gives me the list of objects as an "items" object in my context.
@xordoquy, so I opened a PR two years ago to improve the documentation as you suggested. A review and merge would be welcomed. 😜
Hello there, just jumped into this problem and found an unanswered question about it in stackoverflow: https://stackoverflow.com/q/60816455/5750078
I did not find an answer so after some debugging the DRF found out a workaround similar to those proposed here. Also wrote it as my own answer in that SO page.
This is still opened right? wouldn't a slight change in TemplateHTMLRenderer's get_template_context method suffice to fix it?
Just got bitten by this. Interesting that the DRF docs which are usually very comprehensive do not cover this at all. If TemplateHTMLRenderer is not meant to work in certain scenarios by design, (at least not without tweaking) would be good to have some docs at a minimum around this.
Just got bitten by this. Interesting that the DRF docs which are usually very comprehensive do not cover this at all. If TemplateHTMLRenderer is not meant to work in certain scenarios by design, (at least not without tweaking) would be good to have some docs at a minimum around this.
From my limited understanding, I did try to clarify the docs myself: https://github.com/encode/django-rest-framework/pull/6095
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.
People were still complaining about this last year. It would be good to have input from the maintainers.
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.
There's a PR that seems to address this that's open.
Okay, so #8569 changes the behaviour more than I'd like. I'd suggest we aim to keep things absolutely as minimal as possible here, so...
- Add a test case that demonstrates the issue.
- Change
TemplateHTMLRenderer
so that whendata
is a list, not a dict/dict-like then it is returned under a{"results": data}
key. This has the advantage of being compatible with paginated results.
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.