Opened 9 months ago
Last modified 23 hours ago
#35529 assigned New feature
Have the template tag query_string support multiple arguments and arguments of type Mapping[str, Any]
Description (last modified by ) ¶
Based off this discussion: https://forum.djangoproject.com/t/adding-a-template-tag-to-generate-query-strings/24521/28
Currently query_string
only supports a single QueryDict
as an argument, consensus appears to want the following updates:
- support arguments of type
Mapping[str, Any]
- support multiple arguments
Tests which hopefully illustrate the requested updates
-
TabularUnified tests/template_tests/syntax_tests/test_query_string.py
a b class QueryStringTagTests(SimpleTestCase): 93 93 ) 94 94 self.assertEqual(output, "?a=2&b=2") 95 95 96 @setup({"query_string_dict": "{% query_string my_dict %}"}) 97 def test_query_string_with_explicit_dict_and_no_request(self): 98 context = {"my_dict": {"a": 1, "b": 2}} 99 output = self.engine.render_to_string("query_string_dict", context) 100 self.assertEqual(output, "?a=1&b=2") 101 102 @setup({"query_string_multiple_args": "{% query_string my_dict my_query_dict %}"}) 103 def test_query_string_with_multiple_args(self): 104 context = {"my_dict": {"a": 1, "b": 2}, "my_query_dict": QueryDict("c=1")} 105 output = self.engine.render_to_string("query_string_multiple_args", context) 106 self.assertEqual(output, "?a=1&b=2&c=1") 107 96 108 @setup({"query_string_no_request_no_query_dict": "{% query_string %}"}) 97 109 def test_query_string_without_request_or_explicit_query_dict(self):
According to the ticket's flags, the next step(s) to move this issue forward are:
- For anyone except the patch author to review the patch using the patch review checklist and either mark the ticket as "Ready for checkin" if everything looks good, or leave comments for improvement and mark the ticket as "Patch needs improvement".
Change History (18)
comment:1 by , 9 months ago
Description: | modified (diff) |
---|
comment:2 by , 9 months ago
Description: | modified (diff) |
---|
follow-up: 4 comment:3 by , 9 months ago
comment:4 by , 9 months ago
Replying to Carsten Fuchs:
Isn't this how it is already implemented?
No I don't think so. It must be a QueryDict
. The attached test currently fails with
... File "pathtodjango\django\template\defaulttags.py", line 1208, in query_string query_string = query_dict.urlencode() ^^^^^^^^^^^^^^^^^^^^ AttributeError: 'dict' object has no attribute 'urlencode'
follow-up: 6 comment:5 by , 9 months ago
No I don't think so. It must be a
QueryDict
.
Oh, right. I'm sorry.
These are two separate issues then: Supporting Mapping[str, Any]
and supporting more than one of them.
comment:6 by , 9 months ago
Replying to Carsten Fuchs:
Oh, right. I'm sorry.
These are two separate issues then: SupportingMapping[str, Any]
and supporting more than one of them.
Don't say sorry, they are very related as issues and go well together, thank you for pointing it out. Updated the ticket to capture both of these 👍
comment:7 by , 9 months ago
Description: | modified (diff) |
---|---|
Summary: | Have the template tag query_string support Mapping[str, Any] as an argument → Have the template tag query_string support multiple arguments and arguments of type Mapping[str, Any] |
comment:8 by , 9 months ago
Triage Stage: | Unreviewed → Accepted |
---|---|
Type: | New feature → Cleanup/optimization |
Accepting following the forum conversation.
comment:9 by , 9 months ago
Owner: | changed from | to
---|---|
Status: | new → assigned |
comment:11 by , 8 months ago
Patch needs improvement: | set |
---|
comment:12 by , 7 months ago
PR looks good but I found a few cases where it could be considered that we are breaking previous behavior. I have proposed PR 18487 to add extra tests for the existing tag as is. Leaving "patch needs improvement" set.
comment:13 by , 6 weeks ago
Type: | Cleanup/optimization → New feature |
---|---|
Version: | 5.1 → dev |
comment:14 by , 4 days ago
Cc: | added |
---|
comment:15 by , 4 days ago
I took another look since I had a reminder for this work, and the branch now has conflicts with main
and there are some comments from Tom to address. Leaving flags as they are.
comment:16 by , 2 days ago
Patch needs improvement: | unset |
---|
comment:17 by , 42 hours ago
Triage Stage: | Accepted → Ready for checkin |
---|
comment:18 by , 23 hours ago
Triage Stage: | Ready for checkin → Accepted |
---|
We need to solve #36268 before landing this PR.
Isn't this how it is already implemented?
My understanding of the discussion was that more than one dict could be supported.