From f30b527f170fd4ec9bf10b5012e1ef79776a8f8e Mon Sep 17 00:00:00 2001 From: Ben Cardy Date: Wed, 11 Dec 2024 19:40:28 +0000 Subject: [PATCH] Fixed #25582 -- Added support for query and fragment to django.urls.reverse(). --- AUTHORS | 1 + django/urls/base.py | 26 ++++++++-- docs/ref/urlresolvers.txt | 29 ++++++++++- docs/releases/5.2.txt | 4 +- tests/urlpatterns_reverse/tests.py | 82 +++++++++++++++++++++++++++++- 5 files changed, 136 insertions(+), 6 deletions(-) diff --git a/AUTHORS b/AUTHORS index 1fe38b5666..f853420f69 100644 --- a/AUTHORS +++ b/AUTHORS @@ -146,6 +146,7 @@ answer newbie questions, and generally made Django that much better: Batman Batuhan Taskaya Baurzhan Ismagulov + Ben Cardy Ben Dean Kawamura Ben Firshman Ben Godfrey diff --git a/django/urls/base.py b/django/urls/base.py index bb40ba2224..b7c1b83ea3 100644 --- a/django/urls/base.py +++ b/django/urls/base.py @@ -1,7 +1,8 @@ -from urllib.parse import unquote, urlsplit, urlunsplit +from urllib.parse import unquote, urlencode, urlsplit, urlunsplit from asgiref.local import Local +from django.http import QueryDict from django.utils.functional import lazy from django.utils.translation import override @@ -24,7 +25,16 @@ def resolve(path, urlconf=None): return get_resolver(urlconf).resolve(path) -def reverse(viewname, urlconf=None, args=None, kwargs=None, current_app=None): +def reverse( + viewname, + urlconf=None, + args=None, + kwargs=None, + current_app=None, + *, + query=None, + fragment=None, +): if urlconf is None: urlconf = get_urlconf() resolver = get_resolver(urlconf) @@ -85,7 +95,17 @@ def reverse(viewname, urlconf=None, args=None, kwargs=None, current_app=None): ns_pattern, resolver, tuple(ns_converters.items()) ) - return resolver._reverse_with_prefix(view, prefix, *args, **kwargs) + resolved_url = resolver._reverse_with_prefix(view, prefix, *args, **kwargs) + if query is not None: + if isinstance(query, QueryDict): + query_string = query.urlencode() + else: + query_string = urlencode(query, doseq=True) + if query_string: + resolved_url += "?" + query_string + if fragment is not None: + resolved_url += "#" + fragment + return resolved_url reverse_lazy = lazy(reverse, str) diff --git a/docs/ref/urlresolvers.txt b/docs/ref/urlresolvers.txt index eca00cf106..0c26f9578a 100644 --- a/docs/ref/urlresolvers.txt +++ b/docs/ref/urlresolvers.txt @@ -10,7 +10,7 @@ The ``reverse()`` function can be used to return an absolute path reference for a given view and optional parameters, similar to the :ttag:`url` tag: -.. function:: reverse(viewname, urlconf=None, args=None, kwargs=None, current_app=None) +.. function:: reverse(viewname, urlconf=None, args=None, kwargs=None, current_app=None, *, query=None, fragment=None) ``viewname`` can be a :ref:`URL pattern name ` or the callable view object used in the URLconf. For example, given the following @@ -67,6 +67,33 @@ namespaces into URLs on specific application instances, according to the The ``urlconf`` argument is the URLconf module containing the URL patterns to use for reversing. By default, the root URLconf for the current thread is used. +The ``query`` keyword argument specifies parameters to be added to the returned +URL. It can accept an instance of :class:`~django.http.QueryDict` (such as +``request.GET``) or any value compatible with :func:`urllib.parse.urlencode`. +The encoded query string is appended to the resolved URL, prefixed by a ``?``. + +The ``fragment`` keyword argument specifies a fragment identifier to be +appended to the returned URL (that is, after the path and query string, +preceded by a ``#``). + +For example: + +.. code-block:: pycon + + >>> from django.urls import reverse + >>> reverse("admin:index", query={"q": "biscuits", "page": 2}, fragment="results") + '/admin/?q=biscuits&page=2#results' + >>> reverse("admin:index", query=[("color", "blue"), ("color", 1), ("none", None)]) + '/admin/?color=blue&color=1&none=None' + >>> reverse("admin:index", query={"has empty spaces": "also has empty spaces!"}) + '/admin/?has+empty+spaces=also+has+empty+spaces%21' + >>> reverse("admin:index", fragment="no encoding is done") + '/admin/#no encoding is done' + +.. versionchanged:: 5.2 + + The ``query`` and ``fragment`` arguments were added. + .. note:: The string returned by ``reverse()`` is already diff --git a/docs/releases/5.2.txt b/docs/releases/5.2.txt index 5e692c0345..58d5727f32 100644 --- a/docs/releases/5.2.txt +++ b/docs/releases/5.2.txt @@ -370,7 +370,9 @@ Tests URLs ~~~~ -* ... +* :func:`~django.urls.reverse` now accepts ``query`` and ``fragment`` keyword + arguments, allowing the addition of a query string and/or fragment identifier + in the generated URL, respectively. Utilities ~~~~~~~~~ diff --git a/tests/urlpatterns_reverse/tests.py b/tests/urlpatterns_reverse/tests.py index 91d3f237ec..59a48c9987 100644 --- a/tests/urlpatterns_reverse/tests.py +++ b/tests/urlpatterns_reverse/tests.py @@ -11,7 +11,12 @@ from admin_scripts.tests import AdminScriptTestCase from django.conf import settings from django.contrib.auth.models import User from django.core.exceptions import ImproperlyConfigured, ViewDoesNotExist -from django.http import HttpRequest, HttpResponsePermanentRedirect, HttpResponseRedirect +from django.http import ( + HttpRequest, + HttpResponsePermanentRedirect, + HttpResponseRedirect, + QueryDict, +) from django.shortcuts import redirect from django.test import RequestFactory, SimpleTestCase, TestCase, override_settings from django.test.utils import override_script_prefix @@ -531,6 +536,81 @@ class URLPatternReverse(SimpleTestCase): with self.assertRaises(NoReverseMatch): reverse(views.view_func_from_cbv) + def test_reverse_with_query(self): + self.assertEqual( + reverse("test", query={"hello": "world", "foo": 123}), + "/test/1?hello=world&foo=123", + ) + + def test_reverse_with_query_sequences(self): + cases = [ + [("hello", "world"), ("foo", 123), ("foo", 456)], + (("hello", "world"), ("foo", 123), ("foo", 456)), + {"hello": "world", "foo": (123, 456)}, + ] + for query in cases: + with self.subTest(query=query): + self.assertEqual( + reverse("test", query=query), "/test/1?hello=world&foo=123&foo=456" + ) + + def test_reverse_with_fragment(self): + self.assertEqual(reverse("test", fragment="tab-1"), "/test/1#tab-1") + + def test_reverse_with_fragment_not_encoded(self): + self.assertEqual( + reverse("test", fragment="tab 1 is the best!"), "/test/1#tab 1 is the best!" + ) + + def test_reverse_with_query_and_fragment(self): + self.assertEqual( + reverse("test", query={"hello": "world", "foo": 123}, fragment="tab-1"), + "/test/1?hello=world&foo=123#tab-1", + ) + + def test_reverse_with_empty_fragment(self): + self.assertEqual(reverse("test", fragment=None), "/test/1") + self.assertEqual(reverse("test", fragment=""), "/test/1#") + + def test_reverse_with_invalid_fragment(self): + cases = [0, False, {}, [], set(), ()] + for fragment in cases: + with self.subTest(fragment=fragment): + with self.assertRaises(TypeError): + reverse("test", fragment=fragment) + + def test_reverse_with_empty_query(self): + cases = [None, "", {}, [], set(), (), QueryDict()] + for query in cases: + with self.subTest(query=query): + self.assertEqual(reverse("test", query=query), "/test/1") + + def test_reverse_with_invalid_query(self): + cases = [0, False, [1, 3, 5], {1, 2, 3}] + for query in cases: + with self.subTest(query=query): + with self.assertRaises(TypeError): + print(reverse("test", query=query)) + + def test_reverse_encodes_query_string(self): + self.assertEqual( + reverse( + "test", + query={ + "hello world": "django project", + "foo": [123, 456], + "@invalid": ["?", "!", "a b"], + }, + ), + "/test/1?hello+world=django+project&foo=123&foo=456" + "&%40invalid=%3F&%40invalid=%21&%40invalid=a+b", + ) + + def test_reverse_with_query_from_querydict(self): + query_string = "a=1&b=2&b=3&c=4" + query_dict = QueryDict(query_string) + self.assertEqual(reverse("test", query=query_dict), f"/test/1?{query_string}") + class ResolverTests(SimpleTestCase): def test_resolver_repr(self):