-
Notifications
You must be signed in to change notification settings - Fork 18
Add csrf_token template tag #118
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
base: main
Are you sure you want to change the base?
Conversation
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
This comment was marked as resolved.
This comment was marked as resolved.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for making a start on this!
I think the biggest thing we need is to integrate with context_processors
, so the csrf
processor can add csrf_token
to the context
. Some of this is implemented for Engine
, but it needs integration in Template::render
.
I'm also wondering if I've prematurely optimised Context
, so if the structure there is painfully different to how Django does it, we can refactor to be more similar and reconsider optimisation later when we have benchmarks.
context = {"csrf_token": "test123"} | ||
expected = '<input type="hidden" name="csrfmiddlewaretoken" value="test123">' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the main tests should add csrf_token
via the context processor.
This means these tests will need some refactoring to ensure the context processors are run for both Python and Rust engines.
We should also have extra tests that also set it explicitly like this.
68b64e0
to
ce1b984
Compare
Wasn't sure how to handle the
settings.DEBUG
here:https://github.com/django/django/blob/e183d6c26c8da4486c151f9ce973828e2404a796/django/template/defaulttags.py#L86-L95
Ended up importing
django.conf.settings
directly following the pattern of other Django imports in the codebase. Let me know if you want to take a different approach.