diff --git a/CHANGELOG.txt b/CHANGELOG.txt index fce06a1395..c57d0dceed 100644 --- a/CHANGELOG.txt +++ b/CHANGELOG.txt @@ -52,6 +52,7 @@ Changelog * Fix: Use the correct type scale for heading levels in rich text (Steven Steinwand) * Fix: Update alignment and reveal logic of fields’ comment buttons (Steven Steinwand) * Fix: Regression from Markdown conversion in documentation for API configuration - update to correctly use PEP-8 for example code (Storm Heg) + * Fix: Prevent 'Delete' link on page edit view from redirecting back to the deleted page (LB (Ben) Johnston) 4.0.1 (05.09.2022) diff --git a/docs/releases/4.0.2.md b/docs/releases/4.0.2.md index 41b45c0cca..27c06b3ebf 100644 --- a/docs/releases/4.0.2.md +++ b/docs/releases/4.0.2.md @@ -27,3 +27,4 @@ depth: 1 * Use the correct type scale for heading levels in rich text (Steven Steinwand) * Update alignment and reveal logic of fields’ comment buttons (Steven Steinwand) * Regression from Markdown conversion in documentation for API configuration - update to correctly use PEP-8 for example code (Storm Heg) + * Prevent 'Delete' link on page edit view from redirecting back to the deleted page (LB (Ben) Johnston) diff --git a/wagtail/admin/tests/test_buttons_hooks.py b/wagtail/admin/tests/test_buttons_hooks.py index 5ed25e12a2..fb8e6b0bce 100644 --- a/wagtail/admin/tests/test_buttons_hooks.py +++ b/wagtail/admin/tests/test_buttons_hooks.py @@ -4,7 +4,7 @@ from django.utils.http import urlencode from wagtail import hooks from wagtail.admin import widgets as wagtailadmin_widgets -from wagtail.admin.wagtail_hooks import page_listing_more_buttons +from wagtail.admin.wagtail_hooks import page_header_buttons, page_listing_more_buttons from wagtail.models import Page from wagtail.test.testapp.models import SimplePage from wagtail.test.utils import WagtailTestUtils @@ -17,6 +17,9 @@ class BasePagePerms: def can_copy(self): return False + def can_edit(self): + return False + def can_delete(self): return False @@ -29,12 +32,23 @@ class BasePagePerms: def can_reorder_children(self): return False + def can_add_subpage(self): + return False + class DeleteOnlyPagePerms(BasePagePerms): def can_delete(self): return True +class DeleteAndUnpublishPagePerms(BasePagePerms): + def can_delete(self): + return True + + def can_unpublish(self): + return True + + class ReorderOnlyPagePerms(BasePagePerms): def can_reorder_children(self): return True @@ -53,6 +67,8 @@ class TestButtonsHooks(TestCase, WagtailTestUtils): ) ) + +class TestPageListingButtonsHooks(TestButtonsHooks): def test_register_page_listing_buttons(self): def page_listing_buttons(page, page_perms, next_url=None): yield wagtailadmin_widgets.PageListingButton( @@ -74,6 +90,8 @@ class TestButtonsHooks(TestCase, WagtailTestUtils): self.assertContains(response, "Another useless page listing button") + +class TestPageListingMoreButtonsHooks(TestButtonsHooks): def test_register_page_listing_more_buttons(self): def page_listing_more_buttons(page, page_perms, next_url=None): yield wagtailadmin_widgets.Button( @@ -140,27 +158,12 @@ class TestButtonsHooks(TestCase, WagtailTestUtils): "Another useless dropdown button in "One more more button" dropdown", ) - def test_register_page_header_buttons(self): - def page_header_buttons(page, page_perms, next_url=None): - yield wagtailadmin_widgets.Button( - "Another useless header button", "/custom-url", priority=10 - ) + def test_delete_button_with_next_url(self): + """ + Ensure that the built in delete button supports a next_url provided. + """ - with hooks.register_temporarily( - "register_page_header_buttons", page_header_buttons - ): - response = self.client.get( - reverse("wagtailadmin_pages:edit", args=(self.root_page.id,)) - ) - - self.assertEqual(response.status_code, 200) - self.assertTemplateUsed( - response, "wagtailadmin/pages/listing/_modern_dropdown.html" - ) - - self.assertContains(response, "Another useless header button") - - def test_delete_button_next_url(self): + # page_listing_more_button generator yields only `Delete button` with this permission set page_perms = DeleteOnlyPagePerms() page = self.root_page base_url = reverse("wagtailadmin_pages:delete", args=[page.id]) @@ -168,20 +171,39 @@ class TestButtonsHooks(TestCase, WagtailTestUtils): next_url = "a/random/url/" full_url = base_url + "?" + urlencode({"next": next_url}) - # page_listing_more_button generator yields only `Delete button` delete_button = next( page_listing_more_buttons(page, page_perms, next_url=next_url) ) self.assertEqual(delete_button.url, full_url) - next_url = reverse("wagtailadmin_explore", args=[page.id]) - delete_button = next( - page_listing_more_buttons(page, page_perms, next_url=next_url) - ) + def test_delete_button_with_invalid_next_url(self): + """ + Ensure that the built in delete button on page listing will not use + the next_url provided if that URL is directing the user to edit the page. + As the page is now deleted and cannot be edited. + """ + # permissions should yield two buttons, delete and unpublish + page_perms = DeleteAndUnpublishPagePerms() + page = self.root_page + + base_url = reverse("wagtailadmin_pages:delete", args=[page.id]) + next_url = reverse("wagtailadmin_explore", args=[page.id]) + + buttons = page_listing_more_buttons(page, page_perms, next_url=next_url) + + delete_button = next(buttons) + + # check that the next_url is NOT included as it will not be available after deletion self.assertEqual(delete_button.url, base_url) + # check that any buttons after do correctly still include the next_url + unpublish_base_url = reverse("wagtailadmin_pages:unpublish", args=[page.id]) + unpublish_button = next(buttons) + full_url = unpublish_base_url + "?" + urlencode({"next": next_url}) + self.assertEqual(unpublish_button.url, full_url) + def test_reorder_button_visibility(self): page = self.root_page page_perms = BasePagePerms() @@ -195,3 +217,79 @@ class TestButtonsHooks(TestCase, WagtailTestUtils): reorder_button = next(page_listing_more_buttons(page, page_perms)) self.assertEqual(reorder_button.url, "?ordering=ord") + + +class TestPageHeaderButtonsHooks(TestButtonsHooks): + def test_register_page_header_buttons(self): + def custom_page_header_buttons(page, page_perms, next_url=None): + yield wagtailadmin_widgets.Button( + "Another useless header button", "/custom-url", priority=10 + ) + + with hooks.register_temporarily( + "register_page_header_buttons", custom_page_header_buttons + ): + response = self.client.get( + reverse("wagtailadmin_pages:edit", args=(self.root_page.id,)) + ) + + self.assertEqual(response.status_code, 200) + self.assertTemplateUsed( + response, "wagtailadmin/pages/listing/_modern_dropdown.html" + ) + + self.assertContains(response, "Another useless header button") + + def test_delete_button_with_next_url(self): + """ + Ensure that the built in delete button supports a next_url provided. + """ + + # page_listing_more_button generator yields only `Delete button` with this permission set + page_perms = DeleteOnlyPagePerms() + page = self.root_page + base_url = reverse("wagtailadmin_pages:delete", args=[page.id]) + + next_url = "a/random/url/" + full_url = base_url + "?" + urlencode({"next": next_url}) + + delete_button = next(page_header_buttons(page, page_perms, next_url=next_url)) + + self.assertEqual(delete_button.url, full_url) + + def test_delete_button_with_invalid_next_url(self): + """ + Ensure that the built in delete button on page edit/home (header button) will not use + the next_url provided if that URL is directing the user to edit the page. + As the page is now deleted and cannot be edited. + """ + + # permissions should yield two buttons, delete and unpublish + page_perms = DeleteAndUnpublishPagePerms() + page = self.root_page + + base_url = reverse("wagtailadmin_pages:delete", args=[page.id]) + next_url = reverse("wagtailadmin_explore", args=[page.id]) + + buttons = page_header_buttons(page, page_perms, next_url=next_url) + + delete_button = next(buttons) + + # check that the next_url is NOT included as it will not be available after deletion (page listing) + self.assertEqual(delete_button.url, base_url) + + base_url = reverse("wagtailadmin_pages:delete", args=[page.id]) + next_url = reverse("wagtailadmin_pages:edit", args=[page.id]) + + buttons = page_header_buttons(page, page_perms, next_url=next_url) + + delete_button = next(buttons) + + # check that the next_url is NOT included as it will not be available after deletion (edit page) + self.assertEqual(delete_button.url, base_url) + + # check that any buttons after do correctly still include the next_url + unpublish_base_url = reverse("wagtailadmin_pages:unpublish", args=[page.id]) + unpublish_button = next(buttons) + full_url = unpublish_base_url + "?" + urlencode({"next": next_url}) + self.assertEqual(unpublish_button.url, full_url) diff --git a/wagtail/admin/wagtail_hooks.py b/wagtail/admin/wagtail_hooks.py index 53d81ad481..5aaaf3ad0e 100644 --- a/wagtail/admin/wagtail_hooks.py +++ b/wagtail/admin/wagtail_hooks.py @@ -295,12 +295,13 @@ def page_listing_more_buttons(page, page_perms, next_url=None): ) if page_perms.can_delete(): url = reverse("wagtailadmin_pages:delete", args=[page.id]) + include_next_url = True # After deleting the page, it is impossible to redirect to it. if next_url == reverse("wagtailadmin_explore", args=[page.id]): - next_url = None + include_next_url = False - if next_url: + if next_url and include_next_url: url += "?" + urlencode({"next": next_url}) yield Button( @@ -402,11 +403,16 @@ def page_header_buttons(page, page_perms, next_url=None): if page_perms.can_delete(): url = reverse("wagtailadmin_pages:delete", args=[page.id]) + include_next_url = True + # After deleting the page, it is impossible to redirect to it. if next_url == reverse("wagtailadmin_explore", args=[page.id]): - next_url = None + include_next_url = False - if next_url: + if next_url == reverse("wagtailadmin_pages:edit", args=[page.id]): + include_next_url = False + + if next_url and include_next_url: url += "?" + urlencode({"next": next_url}) yield Button(