Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support for RequestContext during block rendering #15

Merged
merged 9 commits into from
May 7, 2019
Merged

Support for RequestContext during block rendering #15

merged 9 commits into from
May 7, 2019

Conversation

vintage
Copy link
Contributor

@vintage vintage commented Jan 22, 2018

The current implementation hard-codes use of the Context instance, but lots of apps are using RequestContext instead - https://docs.djangoproject.com/en/2.0/ref/templates/api/#django.template.RequestContext

Refactor context creation to take into account, that both classes are supported by using Django built-in method called make_context.

@vintage vintage closed this Jan 22, 2018
@vintage vintage reopened this Jan 22, 2018
@clokep
Copy link
Owner

clokep commented Jan 22, 2018

@vintage I think what you're saying is that if you pass a RequestContext to render_block_to_string it gets rewritten to be a Context instances instead of a RequestContext. Is that accurate?

I've been unable to find any documentation for make_context, am I missing it somewhere?

Any thoughts on adding a test for this?

@vintage
Copy link
Contributor Author

vintage commented Jan 22, 2018

@clokep That's exactly the case, as right now it's not possible to pass RequestContext down to the block rendering (which I need to accomplish in current project). I've removed the use of make_context as indeed it's not documented anywhere in the official docs - replaced it with more understandable approach, as well with unit test to see how its working.

For some reason the newly added test is failing on Travis (works locally), any ideas?

@vintage
Copy link
Contributor Author

vintage commented Jan 23, 2018

I've managed to fix the breaking test, but there is still some problem with the Travis itself Error creating virtualenv. Note that some special characters - pytest-dev/pytest#2779

@clokep
Copy link
Owner

clokep commented Jan 23, 2018

I'll try to fix the issue with travis in a separate branch.

@vintage vintage changed the title Use make_context method to create proper context Support for RequestContext during block rendering Jan 25, 2018
@vintage
Copy link
Contributor Author

vintage commented Nov 23, 2018

Hey, got any news about this one?

@hugokernel
Copy link

Hi,
Thx @vintage for the PR.
@clokep, could you plan to merge this PR ?

render_block/django.py Outdated Show resolved Hide resolved
tests/tests.py Show resolved Hide resolved
@clokep
Copy link
Owner

clokep commented May 6, 2019

Sorry for the extreme delay on this. Overall it looks reasonable. I left a couple of comments, but can always make these changes myself if necessary. Do you know if Jinja has any similar concept to this?

@clokep
Copy link
Owner

clokep commented May 6, 2019

Also it would be great if we could update the README with some examples / API changes and the changelog too!

@vintage
Copy link
Contributor Author

vintage commented May 7, 2019

@clokep AFAIK Jinja do not provide any concept near the Django RequestContext. I've also updated the README about the new optional parameter.

@clokep
Copy link
Owner

clokep commented May 7, 2019

Thanks for this improvement! I'll probably try to do a couple minor things before releasing. 👍

@clokep clokep merged commit 8cf631f into clokep:master May 7, 2019
@clokep
Copy link
Owner

clokep commented May 8, 2019

@vintage FYI I just pushed version 0.6 with this change in it!

@vintage
Copy link
Contributor Author

vintage commented May 8, 2019

@clokep Awesome, great to hear that <3

@hugokernel
Copy link

Thx guys !

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.

3 participants