From 9fbc62ab5267b0e08f27ee756e9a220edd411139 Mon Sep 17 00:00:00 2001 From: Sage Abdullah Date: Fri, 15 Sep 2023 10:56:19 +0100 Subject: [PATCH] Mark ModelViewSet and SnippetViewSet legacy URL patterns for removal --- .../tests/viewsets/test_model_viewset.py | 15 +++++++-- wagtail/admin/viewsets/model.py | 21 +++++++++++++ wagtail/snippets/tests/test_snippets.py | 31 +++++++++++++------ wagtail/snippets/views/snippets.py | 13 ++++++++ 4 files changed, 69 insertions(+), 11 deletions(-) diff --git a/wagtail/admin/tests/viewsets/test_model_viewset.py b/wagtail/admin/tests/viewsets/test_model_viewset.py index 66a64ae9e9..8ec69a7f5c 100644 --- a/wagtail/admin/tests/viewsets/test_model_viewset.py +++ b/wagtail/admin/tests/viewsets/test_model_viewset.py @@ -8,6 +8,7 @@ from openpyxl import load_workbook from wagtail.test.testapp.models import FeatureCompleteToy, JSONStreamModel from wagtail.test.utils.wagtail_tests import WagtailTestUtils +from wagtail.utils.deprecation import RemovedInWagtail60Warning class TestModelViewSetGroup(WagtailTestUtils, TestCase): @@ -721,6 +722,8 @@ class TestBreadcrumbs(WagtailTestUtils, TestCase): class TestLegacyPatterns(WagtailTestUtils, TestCase): + # RemovedInWagtail60Warning: legacy integer pk-based URLs will be removed + def setUp(self): self.user = self.login() @@ -733,13 +736,21 @@ class TestLegacyPatterns(WagtailTestUtils, TestCase): def test_legacy_edit(self): edit_url = reverse("streammodel:edit", args=(quote(self.object.pk),)) legacy_edit_url = "/admin/streammodel/1/" - response = self.client.get(legacy_edit_url) + with self.assertWarnsRegex( + RemovedInWagtail60Warning, + "`//` edit view URL pattern has been deprecated in favour of /edit//.", + ): + response = self.client.get(legacy_edit_url) self.assertEqual(edit_url, "/admin/streammodel/edit/1/") self.assertRedirects(response, edit_url, 301) def test_legacy_delete(self): delete_url = reverse("streammodel:delete", args=(quote(self.object.pk),)) legacy_delete_url = "/admin/streammodel/1/delete/" - response = self.client.get(legacy_delete_url) + with self.assertWarnsRegex( + RemovedInWagtail60Warning, + "`//delete/` delete view URL pattern has been deprecated in favour of /delete//.", + ): + response = self.client.get(legacy_delete_url) self.assertEqual(delete_url, "/admin/streammodel/delete/1/") self.assertRedirects(response, delete_url, 301) diff --git a/wagtail/admin/viewsets/model.py b/wagtail/admin/viewsets/model.py index 0bf18d004a..ff430b7949 100644 --- a/wagtail/admin/viewsets/model.py +++ b/wagtail/admin/viewsets/model.py @@ -1,3 +1,5 @@ +from warnings import warn + from django.core.exceptions import ImproperlyConfigured from django.forms.models import modelform_factory from django.shortcuts import redirect @@ -11,6 +13,7 @@ from wagtail.admin.admin_url_finder import ( from wagtail.admin.views import generic from wagtail.models import ReferenceIndex from wagtail.permissions import ModelPermissionPolicy +from wagtail.utils.deprecation import RemovedInWagtail60Warning from .base import ViewSet, ViewSetGroup @@ -155,6 +158,14 @@ class ModelViewSet(ViewSet): @property def redirect_to_edit_view(self): def redirect_to_edit(request, pk): + warn( + ( + "%s's `//` edit view URL pattern has been " + "deprecated in favour of /edit//." + ) + % (self.__class__.__name__), + category=RemovedInWagtail60Warning, + ) return redirect(self.get_url_name("edit"), pk, permanent=True) return redirect_to_edit @@ -162,6 +173,14 @@ class ModelViewSet(ViewSet): @property def redirect_to_delete_view(self): def redirect_to_delete(request, pk): + warn( + ( + "%s's `//delete/` delete view URL pattern has been " + "deprecated in favour of /delete//." + ) + % (self.__class__.__name__), + category=RemovedInWagtail60Warning, + ) return redirect(self.get_url_name("delete"), pk, permanent=True) return redirect_to_delete @@ -430,10 +449,12 @@ class ModelViewSet(ViewSet): path("new/", self.add_view, name="add"), path("edit//", self.edit_view, name="edit"), path("delete//", self.delete_view, name="delete"), + # RemovedInWagtail60Warning: Remove legacy URL patterns ] + self._legacy_urlpatterns @cached_property def _legacy_urlpatterns(self): + # RemovedInWagtail60Warning: Remove legacy URL patterns return [ path("/", self.redirect_to_edit_view), path("/delete/", self.redirect_to_delete_view), diff --git a/wagtail/snippets/tests/test_snippets.py b/wagtail/snippets/tests/test_snippets.py index 16a370ef26..3df6f82cab 100644 --- a/wagtail/snippets/tests/test_snippets.py +++ b/wagtail/snippets/tests/test_snippets.py @@ -69,6 +69,7 @@ from wagtail.test.testapp.models import ( ) from wagtail.test.utils import WagtailTestUtils from wagtail.test.utils.timestamps import submittable_timestamp +from wagtail.utils.deprecation import RemovedInWagtail60Warning from wagtail.utils.timestamps import render_timestamp @@ -5269,9 +5270,13 @@ class TestSnippetViewWithCustomPrimaryKey(WagtailTestUtils, TestCase): ) def test_redirect_to_edit(self): - response = self.client.get( - "/admin/snippets/snippetstests/standardsnippetwithcustomprimarykey/snippet_2F01/" - ) + with self.assertWarnsRegex( + RemovedInWagtail60Warning, + "`//` edit view URL pattern has been deprecated in favour of /edit//.", + ): + response = self.client.get( + "/admin/snippets/snippetstests/standardsnippetwithcustomprimarykey/snippet_2F01/" + ) self.assertRedirects( response, "/admin/snippets/snippetstests/standardsnippetwithcustomprimarykey/edit/snippet_2F01/", @@ -5279,9 +5284,13 @@ class TestSnippetViewWithCustomPrimaryKey(WagtailTestUtils, TestCase): ) def test_redirect_to_delete(self): - response = self.client.get( - "/admin/snippets/snippetstests/standardsnippetwithcustomprimarykey/snippet_2F01/delete/" - ) + with self.assertWarnsRegex( + RemovedInWagtail60Warning, + "`//delete/` delete view URL pattern has been deprecated in favour of /delete//.", + ): + response = self.client.get( + "/admin/snippets/snippetstests/standardsnippetwithcustomprimarykey/snippet_2F01/delete/" + ) self.assertRedirects( response, "/admin/snippets/snippetstests/standardsnippetwithcustomprimarykey/delete/snippet_2F01/", @@ -5289,9 +5298,13 @@ class TestSnippetViewWithCustomPrimaryKey(WagtailTestUtils, TestCase): ) def test_redirect_to_usage(self): - response = self.client.get( - "/admin/snippets/snippetstests/standardsnippetwithcustomprimarykey/snippet_2F01/usage/" - ) + with self.assertWarnsRegex( + RemovedInWagtail60Warning, + "`//usage/` usage view URL pattern has been deprecated in favour of /usage//.", + ): + response = self.client.get( + "/admin/snippets/snippetstests/standardsnippetwithcustomprimarykey/snippet_2F01/usage/" + ) self.assertRedirects( response, "/admin/snippets/snippetstests/standardsnippetwithcustomprimarykey/usage/snippet_2F01/", diff --git a/wagtail/snippets/views/snippets.py b/wagtail/snippets/views/snippets.py index df04b2c358..419eefcbac 100644 --- a/wagtail/snippets/views/snippets.py +++ b/wagtail/snippets/views/snippets.py @@ -1,3 +1,5 @@ +from warnings import warn + import django_filters from django.apps import apps from django.contrib.admin.utils import quote, unquote @@ -54,6 +56,7 @@ from wagtail.snippets.models import SnippetAdminURLFinder, get_snippet_models from wagtail.snippets.permissions import user_can_edit_snippet_type from wagtail.snippets.side_panels import SnippetStatusSidePanel from wagtail.snippets.views.chooser import SnippetChooserViewSet +from wagtail.utils.deprecation import RemovedInWagtail60Warning # == Helper functions == @@ -980,6 +983,14 @@ class SnippetViewSet(ModelViewSet): @property def redirect_to_usage_view(self): def redirect_to_usage(request, pk): + warn( + ( + "%s's `//usage/` usage view URL pattern has been " + "deprecated in favour of /usage//." + ) + % (self.__class__.__name__), + category=RemovedInWagtail60Warning, + ) return redirect(self.get_url_name("usage"), pk, permanent=True) return redirect_to_usage @@ -1343,11 +1354,13 @@ class SnippetViewSet(ModelViewSet): ), ] + # RemovedInWagtail60Warning: Remove legacy URL patterns return urlpatterns + self._legacy_urlpatterns @cached_property def _legacy_urlpatterns(self): return [ + # RemovedInWagtail60Warning: Remove legacy URL patterns # legacy URLs that could potentially collide if the pk matches one of the reserved names above # ('add', 'edit' etc) - redirect to the unambiguous version path("/", self.redirect_to_edit_view),