From 31615c93efedcd3152b61a50e62ba2ce917f58cd Mon Sep 17 00:00:00 2001 From: Andy Babic Date: Fri, 7 Jan 2022 12:36:20 +0000 Subject: [PATCH] Update Redirect with fields to facilitate filtering and add support for redirecting to a sub-path (when automatically creating redirects for alternate routes, it makes sense for visitors to be redirected to the same route) --- wagtail/contrib/redirects/forms.py | 2 +- .../migrations/0007_add_autocreate_fields.py | 29 ++++++++++ wagtail/contrib/redirects/models.py | 57 ++++++++++++++++++- .../contrib/redirects/tests/test_redirects.py | 14 +++++ 4 files changed, 98 insertions(+), 4 deletions(-) create mode 100644 wagtail/contrib/redirects/migrations/0007_add_autocreate_fields.py diff --git a/wagtail/contrib/redirects/forms.py b/wagtail/contrib/redirects/forms.py index 9437d84ca9..5c2cdfa896 100644 --- a/wagtail/contrib/redirects/forms.py +++ b/wagtail/contrib/redirects/forms.py @@ -43,7 +43,7 @@ class RedirectForm(forms.ModelForm): class Meta: model = Redirect - fields = ('old_path', 'site', 'is_permanent', 'redirect_page', 'redirect_link') + fields = ('old_path', 'site', 'is_permanent', 'redirect_page', 'redirect_page_route_path', 'redirect_link') class ImportForm(forms.Form): diff --git a/wagtail/contrib/redirects/migrations/0007_add_autocreate_fields.py b/wagtail/contrib/redirects/migrations/0007_add_autocreate_fields.py new file mode 100644 index 0000000000..d4ac5fd167 --- /dev/null +++ b/wagtail/contrib/redirects/migrations/0007_add_autocreate_fields.py @@ -0,0 +1,29 @@ +# Generated by Django 3.0.8 on 2021-12-11 17:49 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('contenttypes', '0002_remove_content_type_name'), + ('wagtailredirects', '0006_redirect_increase_max_length'), + ] + + operations = [ + migrations.AddField( + model_name='redirect', + name='automatically_created', + field=models.BooleanField(default=False, editable=False, verbose_name='automatically created'), + ), + migrations.AddField( + model_name='redirect', + name='created_at', + field=models.DateTimeField(auto_now_add=True, null=True, verbose_name='created at'), + ), + migrations.AddField( + model_name='redirect', + name='redirect_page_route_path', + field=models.CharField(blank=True, max_length=255, verbose_name="target page route", help_text="Optionally specify a route on the target page to redirect to. Leave blank to redirect to the default page route.") + ) + ] diff --git a/wagtail/contrib/redirects/models.py b/wagtail/contrib/redirects/models.py index c242c09030..436ff1f695 100644 --- a/wagtail/contrib/redirects/models.py +++ b/wagtail/contrib/redirects/models.py @@ -1,6 +1,7 @@ from urllib.parse import urlparse from django.db import models +from django.urls import NoReverseMatch from django.utils.translation import gettext_lazy as _ from wagtail.core.models import Page @@ -26,7 +27,22 @@ class Redirect(models.Model): null=True, blank=True, on_delete=models.CASCADE ) + redirect_page_route_path = models.CharField( + verbose_name=_("target page route"), + help_text=_( + "Optionally specify a route on the target page to redirect to. " + "Leave blank to redirect to the default page route." + ), + blank=True, + max_length=255 + ) redirect_link = models.URLField(verbose_name=_("redirect to any URL"), blank=True, max_length=255) + automatically_created = models.BooleanField( + verbose_name=_("automatically created"), + default=False, + editable=False, + ) + created_at = models.DateTimeField(verbose_name=_("created at"), auto_now_add=True, null=True) @property def title(self): @@ -38,10 +54,17 @@ class Redirect(models.Model): @property def link(self): if self.redirect_page: - return self.redirect_page.url + page = self.redirect_page.specific + base_url = page.url + if not self.redirect_page_route_path: + return base_url + try: + page.resolve_subpage(self.redirect_page_route_path) + except NoReverseMatch: + return base_url + return base_url + self.redirect_page_route_path elif self.redirect_link: return self.redirect_link - return None def get_is_permanent_display(self): @@ -58,11 +81,12 @@ class Redirect(models.Model): return cls.objects.all() @staticmethod - def add_redirect(old_path, redirect_to=None, is_permanent=True): + def add_redirect(old_path, redirect_to=None, is_permanent=True, page_route_path=None, site=None, automatically_created=False): """ Create and save a Redirect instance with a single method. :param old_path: the path you wish to redirect + :param site: the Site (instance) the redirect is applicable to (if not all sites) :param redirect_to: a Page (instance) or path (string) where the redirect should point :param is_permanent: whether the redirect should be indicated as permanent (i.e. 301 redirect) :return: Redirect instance @@ -71,16 +95,21 @@ class Redirect(models.Model): # Set redirect properties from input parameters redirect.old_path = Redirect.normalise_path(old_path) + redirect.site = site # Check whether redirect to is string or Page if isinstance(redirect_to, Page): # Set redirect page redirect.redirect_page = redirect_to + # Set redirect page route + if isinstance(page_route_path, str): + redirect.redirect_page_route_path = Redirect.normalise_page_route(page_route_path) elif isinstance(redirect_to, str): # Set redirect link string redirect.redirect_link = redirect_to redirect.is_permanent = is_permanent + redirect.automatically_created = automatically_created redirect.save() @@ -121,9 +150,31 @@ class Redirect(models.Model): return path + def normalise_page_route_path(url): + # Strip whitespace + url = url.strip() + if not url: + return "" + + # Extract the path from the rest of the value + url_parsed = urlparse(url) + path = url_parsed[2] + + if path == "/": + return "" + elif not path.startswith("/"): + path = "/" + path + + return path + def clean(self): # Normalise old path self.old_path = Redirect.normalise_path(self.old_path) + # Normalise or clear page route path + if self.redirect_page: + self.redirect_page_route_path = Redirect.normalise_page_route_path(self.redirect_page_route_path) + else: + self.redirect_page_route_path = "" class Meta: verbose_name = _('redirect') diff --git a/wagtail/contrib/redirects/tests/test_redirects.py b/wagtail/contrib/redirects/tests/test_redirects.py index 92342b529a..a141daa10e 100644 --- a/wagtail/contrib/redirects/tests/test_redirects.py +++ b/wagtail/contrib/redirects/tests/test_redirects.py @@ -98,6 +98,20 @@ class TestRedirects(TestCase): normalise_path('/here/tésting-ünicode') ) + def test_route_path_normalisation(self): + normalise_path = models.Redirect.normalise_page_route_path + + # "/" should be normalized to a blank string + self.assertEqual("", normalise_path("/")) + + # leading slashes should always be added + self.assertEqual("/test/", normalise_path("test/")) + + # but trailing slashes are not enforced either way + # (that may cause regex matching for routes to fail) + self.assertEqual("/multiple/segment/test", normalise_path("/multiple/segment/test")) + self.assertEqual("/multiple/segment/test/", normalise_path("/multiple/segment/test/")) + def test_basic_redirect(self): # Create a redirect redirect = models.Redirect(old_path='/redirectme', redirect_link='/redirectto')