From a39c28706aa7a8c3effd0980ae6d59ae67299d85 Mon Sep 17 00:00:00 2001 From: Giannis Terzopoulos Date: Wed, 19 Mar 2025 12:21:41 +0100 Subject: [PATCH] Fixed #35529 -- Added support for positional arguments in querystring template tag. Co-authored-by: Natalia <124304+nessita@users.noreply.github.com> --- AUTHORS | 1 + django/template/defaulttags.py | 54 +++++++---- docs/ref/templates/builtins.txt | 39 ++++++-- docs/releases/6.0.txt | 4 + .../syntax_tests/test_querystring.py | 96 ++++++++++++++++++- 5 files changed, 168 insertions(+), 26 deletions(-) diff --git a/AUTHORS b/AUTHORS index 98e06d9acc..637ca41ad7 100644 --- a/AUTHORS +++ b/AUTHORS @@ -391,6 +391,7 @@ answer newbie questions, and generally made Django that much better: Georg "Hugo" Bauer Georgi Stanojevski Gerardo Orozco + Giannis Terzopoulos Gil Gonçalves Girish Kumar Girish Sontakke diff --git a/django/template/defaulttags.py b/django/template/defaulttags.py index d26d54a826..a511f17310 100644 --- a/django/template/defaulttags.py +++ b/django/template/defaulttags.py @@ -4,12 +4,13 @@ import re import sys import warnings from collections import namedtuple -from collections.abc import Iterable +from collections.abc import Iterable, Mapping from datetime import datetime from itertools import cycle as itertools_cycle from itertools import groupby from django.conf import settings +from django.http import QueryDict from django.utils import timezone from django.utils.html import conditional_escape, escape, format_html from django.utils.lorem_ipsum import paragraphs, words @@ -1173,17 +1174,23 @@ def now(parser, token): @register.simple_tag(name="querystring", takes_context=True) -def querystring(context, query_dict=None, **kwargs): +def querystring(context, *args, **kwargs): """ - Build a query string using `query_dict` and `kwargs` arguments. + Build a query string using `args` and `kwargs` arguments. This tag constructs a new query string by adding, removing, or modifying - parameters, starting from the given `query_dict` (defaulting to - `request.GET`). Keyword arguments are processed sequentially, with later - arguments taking precedence. + parameters from the given positional and keyword arguments. Positional + arguments must be mappings (such as `QueryDict` or `dict`), and + `request.GET` is used as the starting point if `args` is empty. + + Keyword arguments are treated as an extra, final mapping. These mappings + are processed sequentially, with later arguments taking precedence. A query string prefixed with `?` is returned. + Raise TemplateSyntaxError if a positional argument is not a mapping or if + keys are not strings. + For example:: {# Set a parameter on top of `request.GET` #} @@ -1197,18 +1204,31 @@ def querystring(context, query_dict=None, **kwargs): {# Use a custom ``QueryDict`` #} {% querystring my_query_dict foo=3 %} + + {# Use multiple positional and keyword arguments #} + {% querystring my_query_dict my_dict foo=3 bar=None %} """ - if query_dict is None: - query_dict = context.request.GET - params = query_dict.copy() - for key, value in kwargs.items(): - if value is None: - if key in params: - del params[key] - elif isinstance(value, Iterable) and not isinstance(value, str): - params.setlist(key, value) - else: - params[key] = value + if not args: + args = [context.request.GET] + params = QueryDict(mutable=True) + for d in [*args, kwargs]: + if not isinstance(d, Mapping): + raise TemplateSyntaxError( + "querystring requires mappings for positional arguments (got " + "%r instead)." % d + ) + for key, value in d.items(): + if not isinstance(key, str): + raise TemplateSyntaxError( + "querystring requires strings for mapping keys (got %r " + "instead)." % key + ) + if value is None: + params.pop(key, None) + elif isinstance(value, Iterable) and not isinstance(value, str): + params.setlist(key, value) + else: + params[key] = value query_string = params.urlencode() if params else "" return f"?{query_string}" diff --git a/docs/ref/templates/builtins.txt b/docs/ref/templates/builtins.txt index 849faaa46f..f3d967b153 100644 --- a/docs/ref/templates/builtins.txt +++ b/docs/ref/templates/builtins.txt @@ -964,8 +964,14 @@ output (as a string) inside a variable. This is useful if you want to use Outputs a URL-encoded formatted query string based on the provided parameters. -This tag requires a :class:`~django.http.QueryDict` instance, which defaults to -:attr:`request.GET ` if none is provided. +This tag accepts positional arguments, which must be mappings (such as +:class:`~django.http.QueryDict` or :class:`dict`). If no positional arguments +are provided, :attr:`request.GET ` is used as the +default to construct the query string. + +Positional arguments are processed sequentially, while keyword arguments are +treated as key-value pairs, applied last. Later arguments take precedence over +earlier ones, ensuring the most recent pairs are reflected in the final result. The result always includes a leading ``"?"`` since this tag is mainly used for links, and an empty result could prevent the page from reloading as expected. @@ -1033,16 +1039,33 @@ Handling lists If ``my_list`` is ``["red", "blue"]``, the output will be ``?color=red&color=blue``, preserving the list structure in the query string. -Custom QueryDict -~~~~~~~~~~~~~~~~ +Customizing the base QueryDict +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +You can pass custom ``QueryDict`` or ``dict`` instances as positional arguments +to replace ``request.GET``. When multiple arguments are provided, key-value +pairs from later arguments take precedence over earlier ones. + +For example, if ``my_query_dict`` is ```` and ``my_dict`` is ``{'color': 'orange', 'fabric': 'silk', 'type': +'dress'}``, this outputs ``?color=orange&size=S&fabric=silk``. .. code-block:: html+django - {% querystring my_query_dict %} + {% querystring my_query_dict my_dict size="S" type=None %} -You can provide a custom ``QueryDict`` to be used instead of ``request.GET``. -So if ``my_query_dict`` is ````, this outputs -``?color=blue``. If ``my_query_dict`` is empty, the output will be ``?``. +If all keys are removed by setting them to ``None``, this outputs ``?``: + +.. code-block:: html+django + + {% querystring my_query_dict my_dict color=None size=None fabric=None type=None %} + +Similarly, if all positional arguments are empty and keyword arguments do not +contribute any new params, the output will also be ``?``. + +.. versionchanged:: 6.0 + + Support for multiple positional mapping arguments was added. Dynamic usage ~~~~~~~~~~~~~ diff --git a/docs/releases/6.0.txt b/docs/releases/6.0.txt index cd72185354..499163788f 100644 --- a/docs/releases/6.0.txt +++ b/docs/releases/6.0.txt @@ -236,6 +236,10 @@ Templates * The :ttag:`querystring` template tag now consistently prefixes the returned query string with a ``?``, ensuring reliable link generation behavior. +* The :ttag:`querystring` template tag now accepts multiple positional + arguments, which must be mappings, such as :class:`~django.http.QueryDict` + or :class:`dict`. + Tests ~~~~~ diff --git a/tests/template_tests/syntax_tests/test_querystring.py b/tests/template_tests/syntax_tests/test_querystring.py index 7b19bb11ad..af8dbde955 100644 --- a/tests/template_tests/syntax_tests/test_querystring.py +++ b/tests/template_tests/syntax_tests/test_querystring.py @@ -1,5 +1,6 @@ from django.http import QueryDict from django.template import RequestContext +from django.template.base import TemplateSyntaxError from django.test import RequestFactory, SimpleTestCase from ..utils import setup @@ -13,6 +14,10 @@ class QueryStringTagTests(SimpleTestCase): output = self.engine.render_to_string(template_name, context) self.assertEqual(output, expected) + def assertTemplateSyntaxError(self, template_name, context, expected): + with self.assertRaisesMessage(TemplateSyntaxError, expected): + self.engine.render_to_string(template_name, context) + @setup({"querystring_empty_get_params": "{% querystring %}"}) def test_querystring_empty_get_params(self): context = RequestContext(self.request_factory.get("/")) @@ -26,6 +31,19 @@ class QueryStringTagTests(SimpleTestCase): with self.subTest(context=context): self.assertRenderEqual("querystring_remove_all_params", context, "?") + @setup( + { + "querystring_remove_all_params_custom_querydict": ( + "{% querystring my_query_dict my_dict a=None %}" + ) + } + ) + def test_querystring_remove_all_params_custom_querydict(self): + context = {"my_query_dict": QueryDict("a=1&b=2"), "my_dict": {"b": None}} + self.assertRenderEqual( + "querystring_remove_all_params_custom_querydict", context, "?" + ) + @setup({"querystring_non_empty_get_params": "{% querystring %}"}) def test_querystring_non_empty_get_params(self): request = self.request_factory.get("/", {"a": "b"}) @@ -42,7 +60,7 @@ class QueryStringTagTests(SimpleTestCase): @setup({"querystring_empty_params": "{% querystring qd %}"}) def test_querystring_empty_params(self): - cases = [None, {}, QueryDict()] + cases = [{}, QueryDict()] request = self.request_factory.get("/") qs = "?a=b" request_with_qs = self.request_factory.get(f"/{qs}") @@ -87,6 +105,82 @@ class QueryStringTagTests(SimpleTestCase): "querystring_remove_nonexistent", context, expected="?x=y&a=1" ) + @setup({"querystring_remove_dict": "{% querystring my_dict a=1 %}"}) + def test_querystring_remove_from_dict(self): + request = self.request_factory.get("/", {"test": "value"}) + context = RequestContext(request, {"my_dict": {"test": None}}) + self.assertRenderEqual("querystring_remove_dict", context, expected="?a=1") + + @setup({"querystring_variable": "{% querystring a=a %}"}) + def test_querystring_variable(self): + request = self.request_factory.get("/") + context = RequestContext(request, {"a": 1}) + self.assertRenderEqual("querystring_variable", context, expected="?a=1") + + @setup({"querystring_dict": "{% querystring my_dict %}"}) + def test_querystring_dict(self): + context = {"my_dict": {"a": 1}} + self.assertRenderEqual("querystring_dict", context, expected="?a=1") + + @setup({"querystring_dict_list": "{% querystring my_dict %}"}) + def test_querystring_dict_list_values(self): + context = {"my_dict": {"a": [1, 2]}} + self.assertRenderEqual( + "querystring_dict_list", context, expected="?a=1&a=2" + ) + + @setup( + { + "querystring_multiple_args_override": ( + "{% querystring my_dict my_query_dict x=3 y=None %}" + ) + } + ) + def test_querystring_multiple_args_override(self): + context = {"my_dict": {"x": 0, "y": 42}, "my_query_dict": QueryDict("a=1&b=2")} + self.assertRenderEqual( + "querystring_multiple_args_override", + context, + expected="?x=3&a=1&b=2", + ) + + @setup({"querystring_request_get_ignored": "{% querystring my_mapping %}"}) + def test_querystring_request_get_ignored(self): + cases = [({"y": "x"}, "?y=x"), ({}, "?")] + request = self.request_factory.get("/", {"x": "y", "a": "b"}) + for param, expected in cases: + with self.subTest(param=param): + context = RequestContext(request, {"my_mapping": param}) + self.assertRenderEqual( + "querystring_request_get_ignored", context, expected=expected + ) + + @setup({"querystring_same_arg": "{% querystring a=1 a=2 %}"}) + def test_querystring_same_arg(self): + msg = "'querystring' received multiple values for keyword argument 'a'" + self.assertTemplateSyntaxError("querystring_same_arg", {}, msg) + + @setup({"querystring_non_mapping_args": "{% querystring somevar %}"}) + def test_querystring_non_mapping_args(self): + cases = [None, 0, "", []] + request = self.request_factory.get("/") + msg = ( + "querystring requires mappings for positional arguments (got %r " + "instead)." + ) + for param in cases: + with self.subTest(param=param): + context = RequestContext(request, {"somevar": param}) + self.assertTemplateSyntaxError( + "querystring_non_mapping_args", context, msg % param + ) + + @setup({"querystring_non_string_dict_keys": "{% querystring my_dict %}"}) + def test_querystring_non_string_dict_keys(self): + context = {"my_dict": {0: 1}} + msg = "querystring requires strings for mapping keys (got 0 instead)." + self.assertTemplateSyntaxError("querystring_non_string_dict_keys", context, msg) + @setup({"querystring_list": "{% querystring a=my_list %}"}) def test_querystring_add_list(self): request = self.request_factory.get("/")