#36182 closed Bug (fixed)
querystring templatetag should render empty querystring as "?" not "" when it has removed items from the querydict
Description ¶
Hello,
Consider 2 anchors that link to a filtered and unfiltered version of the current page via a GET param foo.
<a href="{% querystring foo=None %}">All Records</a> <a href="{% querystring foo=bar %}">Bar Records Only</a>
If the user is currently on the filtered page https://example.com/?foo=bar and clicks the first link, the querystring template tag, being empty, will render as "", which means href="" which the browser will interpret as reload the current (filtered) page. I believe instead the templatetag should render this case as "?"
https://github.com/dfeeley/django/compare/main..empty-querystring
thanks
Dave
Change History (9)
comment:1 by , 3 days ago
Cc: | added |
---|---|
Severity: | Normal → Release blocker |
Summary: | querystring templatetag should render empty querystring as "?" not "" → querystring templatetag should render empty querystring as "?" not "" when it has removed items from the querydict |
Triage Stage: | Unreviewed → Accepted |
comment:2 by , 3 days ago
Seems like we should do something here.
So for only when it's changed, because if nothing has changed, you would expect it to reload? I think I agree with that line of reasoning though I haven't spent any time thinking about possible use cases and edge cases.
follow-up: 4 comment:3 by , 3 days ago
{% querystring %}
Outputs the current query string verbatim. So if the query string is ?color=green, the output would be ?color=green.
I guess specifically, I did not expect the test template_tests.syntax_tests.test_querystring.QueryStringTagTests.test_querystring_empty_get_params
to now return "?"
instead of ""
. But perhaps there's no real advantage of keeping the url the same?
comment:4 by , 2 days ago
Replying to Sarah Boyce:
{% querystring %}
Outputs the current query string verbatim. So if the query string is ?color=green, the output would be ?color=green.
I guess specifically, I did not expect the test
template_tests.syntax_tests.test_querystring.QueryStringTagTests.test_querystring_empty_get_params
to now return"?"
instead of""
. But perhaps there's no real advantage of keeping the url the same?
I like your addition of the has_removed condition as I think rewriting an empty query string ("") to "?" more generally probably violates the principle of least surprise -- ie. why rewrite it when there is no need to.
comment:5 by , 29 hours ago
Has patch: | set |
---|---|
Owner: | set to |
Status: | new → assigned |
comment:6 by , 8 hours ago
Triage Stage: | Accepted → Ready for checkin |
---|
Hi David, thank you for the ticket!
I think I agree with a slight caveat that I feel this should only be the case when the query dict has changed
A test and a very rough solution just to illustrate what I mean:
TabularUnified django/template/defaulttags.py
"TabularUnified tests/template_tests/syntax_tests/test_querystring.py
I am cc-ing some other folks to see what they think as well