-
-
Notifications
You must be signed in to change notification settings - Fork 959
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
Add request
argument to TemplateResponse
#2191
Changes from all commits
2af4d82
6847bc5
dda3bc0
527f4c3
1dc2743
c110b23
0e9f58d
1a2a387
ab6a689
b3418eb
f67e075
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -140,19 +140,87 @@ def url_for(context: dict, __name: str, **path_params: typing.Any) -> URL: | |
def get_template(self, name: str) -> "jinja2.Template": | ||
return self.env.get_template(name) | ||
|
||
@typing.overload | ||
def TemplateResponse( | ||
self, | ||
request: Request, | ||
name: str, | ||
context: dict, | ||
context: typing.Optional[dict] = None, | ||
status_code: int = 200, | ||
headers: typing.Optional[typing.Mapping[str, str]] = None, | ||
media_type: typing.Optional[str] = None, | ||
background: typing.Optional[BackgroundTask] = None, | ||
) -> _TemplateResponse: | ||
... | ||
|
||
@typing.overload | ||
def TemplateResponse( | ||
self, | ||
name: str, | ||
context: typing.Optional[dict] = None, | ||
status_code: int = 200, | ||
headers: typing.Optional[typing.Mapping[str, str]] = None, | ||
media_type: typing.Optional[str] = None, | ||
background: typing.Optional[BackgroundTask] = None, | ||
) -> _TemplateResponse: | ||
if "request" not in context: | ||
raise ValueError('context must include a "request" key') | ||
# Deprecated usage | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Unfortunately, we can't use Also, the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yes, so we have a comment here. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yeah yeah, just explaining for future reference 👀 |
||
... | ||
|
||
request = typing.cast(Request, context["request"]) | ||
def TemplateResponse( | ||
self, *args: typing.Any, **kwargs: typing.Any | ||
) -> _TemplateResponse: | ||
if args: | ||
if isinstance( | ||
args[0], str | ||
): # the first argument is template name (old style) | ||
warnings.warn( | ||
"The `name` is not the first parameter anymore. " | ||
"The first parameter should be the `Request` instance.\n" | ||
'Replace `TemplateResponse(name, {"request": request})` by `TemplateResponse(request, name)`.', # noqa: E501 | ||
DeprecationWarning, | ||
) | ||
|
||
name = args[0] | ||
context = args[1] if len(args) > 1 else kwargs.get("context", {}) | ||
status_code = ( | ||
args[2] if len(args) > 2 else kwargs.get("status_code", 200) | ||
) | ||
headers = args[2] if len(args) > 2 else kwargs.get("headers") | ||
media_type = args[3] if len(args) > 3 else kwargs.get("media_type") | ||
background = args[4] if len(args) > 4 else kwargs.get("background") | ||
|
||
if "request" not in context: | ||
raise ValueError('context must include a "request" key') | ||
request = context["request"] | ||
else: # the first argument is a request instance (new style) | ||
request = args[0] | ||
name = args[1] if len(args) > 1 else kwargs["name"] | ||
context = args[2] if len(args) > 2 else kwargs.get("context", {}) | ||
status_code = ( | ||
args[3] if len(args) > 3 else kwargs.get("status_code", 200) | ||
) | ||
headers = args[4] if len(args) > 4 else kwargs.get("headers") | ||
media_type = args[5] if len(args) > 5 else kwargs.get("media_type") | ||
background = args[6] if len(args) > 6 else kwargs.get("background") | ||
else: # all arguments are kwargs | ||
if "request" not in kwargs: | ||
warnings.warn( | ||
"The `TemplateResponse` now requires the `request` argument.\n" | ||
'Replace `TemplateResponse(name, {"context": context})` by `TemplateResponse(request, name)`.', # noqa: E501 | ||
DeprecationWarning, | ||
) | ||
if "request" not in kwargs.get("context", {}): | ||
raise ValueError('context must include a "request" key') | ||
Comment on lines
+212
to
+213
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This error is being raised with the warning above, can we make sure we only have the warning if this doesn't raise? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This preserves current behavior. |
||
|
||
context = kwargs.get("context", {}) | ||
request = kwargs.get("request", context.get("request")) | ||
name = typing.cast(str, kwargs["name"]) | ||
status_code = kwargs.get("status_code", 200) | ||
headers = kwargs.get("headers") | ||
media_type = kwargs.get("media_type") | ||
background = kwargs.get("background") | ||
|
||
context.setdefault("request", request) | ||
for context_processor in self.context_processors: | ||
context.update(context_processor(request)) | ||
|
||
|
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.
Is this the only thing in the documentation that uses
TemplateResponse
?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.
yes