From 9630967e0e3de3469fd858d2cf6fd656be7c491e Mon Sep 17 00:00:00 2001 From: Andy Babic Date: Tue, 13 Apr 2021 16:10:45 +0100 Subject: [PATCH] Use resolve_url() instead of reverse() for pageurl fallbacks (#7002) --- CHANGELOG.txt | 1 + docs/releases/2.13.rst | 1 + docs/topics/writing_templates.rst | 2 +- wagtail/core/templatetags/wagtailcore_tags.py | 4 +- wagtail/core/tests/tests.py | 47 +++++++++++++++++-- 5 files changed, 47 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.txt b/CHANGELOG.txt index 92e0f1ec5b..e2484c88d4 100644 --- a/CHANGELOG.txt +++ b/CHANGELOG.txt @@ -18,6 +18,7 @@ Changelog * `get_settings` template tag now supports specifying the variable name with `{% get_settings as var %}` (Samir Shah) * Reinstate submitter's name on moderation notification email (Matt Westcott) * Add a new switch input widget as an alternative to checkboxes (Karl Hobley) + * Allow `{% pageurl %}` fallback to be a direct URL or an object with a `get_absolute_url` method (Andy Babic) * Fix: StreamField required status is now consistently handled by the `blank` keyword argument (Matt Westcott) * Fix: Show 'required' asterisks for blocks inside required StreamFields (Matt Westcott) * Fix: Make image chooser "Select format" fields translatable (Helen Chapman, Thibaud Colas) diff --git a/docs/releases/2.13.rst b/docs/releases/2.13.rst index fbed4c9530..f087094e61 100644 --- a/docs/releases/2.13.rst +++ b/docs/releases/2.13.rst @@ -35,6 +35,7 @@ Other features * ``get_settings`` template tag now supports specifying the variable name with ``{% get_settings as var %}`` (Samir Shah) * Reinstate submitter's name on moderation notification email (Matt Westcott) * Add a new switch input widget as an alternative to checkboxes (Karl Hobley) +* Allow ``{% pageurl %}`` fallback to be a direct URL or an object with a ``get_absolute_url`` method (Andy Babic) Bug fixes ~~~~~~~~~ diff --git a/docs/topics/writing_templates.rst b/docs/topics/writing_templates.rst index 556333954f..489649fdd5 100644 --- a/docs/topics/writing_templates.rst +++ b/docs/topics/writing_templates.rst @@ -174,7 +174,7 @@ Takes a Page object and returns a relative URL (``/foo/bar/``) if within the sam Back to index -A ``fallback`` keyword argument can be provided - this should be a URL route name that takes no parameters, and will be used as a substitute URL when the passed page is ``None``. +A ``fallback`` keyword argument can be provided - this can be a URL string, a named URL route that can be resolved with no parameters, or an object with a ``get_absolute_url`` method, and will be used as a substitute URL when the passed page is ``None``. .. code-block:: html+django diff --git a/wagtail/core/templatetags/wagtailcore_tags.py b/wagtail/core/templatetags/wagtailcore_tags.py index 7d6b63cae2..f2866b80f9 100644 --- a/wagtail/core/templatetags/wagtailcore_tags.py +++ b/wagtail/core/templatetags/wagtailcore_tags.py @@ -1,5 +1,5 @@ from django import template -from django.shortcuts import reverse +from django.shortcuts import resolve_url from django.template.defaulttags import token_kwargs from django.template.loader import render_to_string from django.utils.encoding import force_str @@ -21,7 +21,7 @@ def pageurl(context, page, fallback=None): If kwargs contains a fallback view name and page is None, the fallback view url will be returned. """ if page is None and fallback: - return reverse(fallback) + return resolve_url(fallback) if not hasattr(page, 'relative_url'): raise ValueError("pageurl tag expected a Page object, got %r" % page) diff --git a/wagtail/core/tests/tests.py b/wagtail/core/tests/tests.py index e6be2c676b..54ae4c380f 100644 --- a/wagtail/core/tests/tests.py +++ b/wagtail/core/tests/tests.py @@ -21,13 +21,50 @@ class TestPageUrlTags(TestCase): self.assertContains(response, 'Christmas') - def test_pageurl_fallback(self): - tpl = template.Template('''{% load wagtailcore_tags %}Fallback''') - result = tpl.render(template.Context({'page': None})) + def test_pageurl_with_named_url_fallback(self): + tpl = template.Template( + """{% load wagtailcore_tags %}Fallback""" + ) + result = tpl.render(template.Context({"page": None})) self.assertIn('Fallback', result) - def test_pageurl_fallback_without_valid_fallback(self): - tpl = template.Template('''{% load wagtailcore_tags %}Fallback''') + def test_pageurl_with_get_absolute_url_object_fallback(self): + class ObjectWithURLMethod: + def get_absolute_url(self): + return "/object-specific-url/" + + tpl = template.Template( + """{% load wagtailcore_tags %}Fallback""" + ) + result = tpl.render( + template.Context({"page": None, "object_with_url_method": ObjectWithURLMethod()}) + ) + self.assertIn('Fallback', result) + + def test_pageurl_with_valid_url_string_fallback(self): + """ + `django.shortcuts.resolve_url` accepts strings containing '.' or '/' as they are. + """ + tpl = template.Template( + """ + {% load wagtailcore_tags %} + Same page fallback + Homepage fallback + Up one step fallback + """ + ) + result = tpl.render(template.Context({"page": None})) + self.assertIn('Same page fallback', result) + self.assertIn('Homepage fallback', result) + self.assertIn('Up one step fallback', result) + + def test_pageurl_with_invalid_url_string_fallback(self): + """ + Strings not containing '.' or '/', and not matching a named URL will error. + """ + tpl = template.Template( + """{% load wagtailcore_tags %}Fallback""" + ) with self.assertRaises(NoReverseMatch): tpl.render(template.Context({'page': None}))