-
-
Notifications
You must be signed in to change notification settings - Fork 543
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
[3276] Fix incorrect template rendering #3279
base: main
Are you sure you want to change the base?
[3276] Fix incorrect template rendering #3279
Conversation
Thanks for adding the Here's a preview of the changelog: This release addresses an issue in Django's GraphQLView. Previously, variables were consumed too early when overriding the template, leading to a syntax error in the JavaScript code. Here's the tweet text:
|
CodSpeed Performance ReportMerging #3279 will not alter performanceComparing Summary
|
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## main #3279 +/- ##
===========================================
- Coverage 96.59% 40.50% -56.09%
===========================================
Files 482 480 -2
Lines 29957 29895 -62
Branches 3694 3685 -9
===========================================
- Hits 28936 12110 -16826
- Misses 835 17438 +16603
- Partials 186 347 +161 |
- Add a basic test to see if graphql template is rendering - Add test for the failed rendering of the setting when overriding the template
The `get_template()` call correctly fetches the template via the engine, but to get the same template type as the one used in the other branch, we need to access its template attribute, where the original template is stored. The engine template wrap the base template to allow direct rendering by TemplateResponse, however this conflicts with the "template via static files" approach taken by Strawberry. Since I'm unfamiliar with the reasoning for this approach, we'll leave it in tact.
Test is identical, but we need the decorator on one of them.
5c4686c
to
54bf902
Compare
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.
looks great! thanks 😊
@msopacua looks like tests on older versions of django are failing 😊 |
Ah forgot about that. Django < 4.2 doesn't rewrite headers into WSGI environment variables yet. |
Older Django versions, don't support requests-like headers for the test client.
Description
See commit messages.
Types of Changes
Issues Fixed or Closed by This PR
Checklist