Skip to content

Conversation

florianm
Copy link

@florianm florianm commented Oct 21, 2016

Fixes #20

This will make the data from the API view available as template variable {{ data }}.


This change is Reviewable

Fixes rest-framework-latex/issues/20
@florianm
Copy link
Author

tests fail with

File "/home/ubuntu/rest-framework-latex/tests/tests/testrenderers/tests/test_callback.py", line 1, in <module>
    from mock import MagicMock, patch
ImportError: No module named mock

Copy link
Contributor

@scott-w scott-w left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @florianm, thanks for this, it's much appreciated. I just have a couple of comments to make this a great addition to the code-base.

Could you include a test that triggers the bug along with the fix?

To resolve the mock issue, simply include a versioned mock in the requirements.txt file and Circle will install it.

# Get latex
tex = super(LatexRenderer, self).render(
data, accepted_media_type, renderer_context)
{'data': data}, accepted_media_type, renderer_context)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Personally, I'd be keen to not change the expected behaviour when a dictionary is passed - this change will wrap:

data = {
  'key': 'val',
}

into

data = {
  'data': {
    'key': 'value'
  }
}

which would break existing applications depending on the data dictionary being passed verbatim in the template.

@florianm
Copy link
Author

@scott-w thanks for the encouraging feedback, I'll aim to create a test.

Re wrapping data, this is the only way I was able to access the data (originally an OrderedDict) in the template. The OrderedDict consists of unnamed (key, val) tuples which I couldn't access in the template for the life of me.
Is there a way to get to the data otherwise? If so, maybe a working example in the docs would be a better fix (happy to PR) than changing existing behaviour.

Maybe the confusion is that from my end, data is an OrderedDict, while others may manage to provide data as a normal dict. I'll have to work through the request prior to drf-latex rendering the template - maybe that's a newer django or DRF feature or a config issue on my end.

@scott-w
Copy link
Contributor

scott-w commented Oct 22, 2016

Hi @florianm, my thinking was along the lines of checking whether data is a subclass of dict before deciding whether to wrap it in another dict. I hope this is clearer. Thanks again!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants